[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [XEN][RFC PATCH v4 11/16] common/device_tree: Add rwlock for dt_host


  • To: Julien Grall <julien@xxxxxxx>, xen-devel@xxxxxxxxxxxxxxxxxxxx
  • From: Vikram Garhwal <vikram.garhwal@xxxxxxx>
  • Date: Wed, 1 Feb 2023 09:05:24 -0800
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=amd.com; dmarc=pass action=none header.from=amd.com; dkim=pass header.d=amd.com; arc=none
  • Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=GA94RVLce3o5GlWAMhI7az39HzBXE50ceQe87PcPDdE=; b=d/gvSInXtqhQCTuA9jNvbuw2hIbqQUsaMCM1P0VIW93QIbEPrTZzCRDWUNvHnGGQHkW02LCOTKPJczlgICG9yOo3bbme6reWyydWEmkwU6rWVv/dIYrNmMIpBnm0IpSZJ7I3C3I6uKQDP79zNwDD232Jn8w3OC91MYliocfkY0Y/bUOmihHlFAaE6q4PY3js16oOaUG6rbSFGv9hfceYOOPdXhelgEIj8q8f2HPVRngjWDbeqzloej6N+rDiVDghbOrGBO6NbXuzvxd/vBc//WKaumYWbdno1AfZv1I+SNBX7o52ID8wv+90pE5TIirOk4XGdkVKmntqnsNHCFre/w==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=keDqcSU0e2Tbb5l1nzlpF3mVhZ7hDzNJnMiuvQxCbU9VG+eIWPZFq0xlyEUvU/sYSLbMJ9xIPalSnOZvJlGKk7AkDtC5EmBVeZeSdBCzxJo1a6N6gDt7+vmrXm7RZhYW8tkYu1DcMkvceNh2YfcOKxipBSrfKDVQC7d2f8CMO/5UvWPcqBZS+u4cwPkbVfQSsXFSivNFcZsw0yItuXzqgd0B7itVe/xhC9WkdKFhs9SVf2NKyLcLLh/0SIhYg3dEUE2wgf8jZxD2Zy4Fd7BygAptc9OKI5X0MDnwHST5WA5VBTwDCOeqlhmkUPQYTnT0+Jw9mZ60fFLG7VV9Inh7TQ==
  • Authentication-results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=amd.com;
  • Cc: sstabellini@xxxxxxxxxx, Luca.Fancellu@xxxxxxx
  • Delivery-date: Wed, 01 Feb 2023 17:05:42 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

Hi Julien,

On 12/7/22 8:31 AM, Julien Grall wrote:
Hi Vikram,

On 07/12/2022 06:18, Vikram Garhwal wrote:
  Dynamic programming ops will modify the dt_host and there might be other   function which are browsing the dt_host at the same time. To avoid the race
  conditions, adding rwlock for browsing the dt_host.

Looking at the user below, it is not entirely clear what the lock is actually protecting. For instance...

Purpose of the lock was to protect the read/scanning of dt_host while we remove the add/nodes. This lock is also used when nodes are added/removed in "[XEN][RFC PATCH v4 12/16]: static int remove_nodes(const struct overlay_track *tracker)".




Signed-off-by: Vikram Garhwal <vikram.garhwal@xxxxxxx>
---
  xen/common/device_tree.c      | 27 +++++++++++++++++++++++++++
  xen/include/xen/device_tree.h |  6 ++++++
  2 files changed, 33 insertions(+)

diff --git a/xen/common/device_tree.c b/xen/common/device_tree.c
index acf26a411d..51ee2a5edf 100644
--- a/xen/common/device_tree.c
+++ b/xen/common/device_tree.c
@@ -140,6 +140,8 @@ const struct dt_property *dt_find_property(const struct dt_device_node *np,
      if ( !np )
          return NULL;
  +    read_lock(&dt_host->lock);
+
      for ( pp = np->properties; pp; pp = pp->next )
      {
          if ( dt_prop_cmp(pp->name, name) == 0 )
@@ -150,6 +152,7 @@ const struct dt_property *dt_find_property(const struct dt_device_node *np,
          }
      }
  +    read_unlock(&dt_host->lock);
      return pp;
  }
  @@ -336,11 +339,14 @@ struct dt_device_node *dt_find_node_by_name(struct dt_device_node *from,
      struct dt_device_node *np;
      struct dt_device_node *dt;
  +    read_lock(&dt_host->lock);
+
      dt = from ? from->allnext : dt_host;
      dt_for_each_device_node(dt, np)
          if ( np->name && (dt_node_cmp(np->name, name) == 0) )
              break;
  +    read_unlock(&dt_host->lock);
      return np;

... I was expecting the read lock to also protect the value returned from being freed. But this is not the case.

Okay. Shall I remove the lock from here and perhaps add it when dt_find_node_by_name() and other related functions are called?
Cheers,




 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.