From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:56174) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1T0zfc-0007Px-Rs for qemu-devel@nongnu.org; Mon, 13 Aug 2012 14:47:22 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1T0zfb-0002Gy-Cl for qemu-devel@nongnu.org; Mon, 13 Aug 2012 14:47:20 -0400 Received: from e28smtp04.in.ibm.com ([122.248.162.4]:38047) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1T0zfa-0002Ga-PY for qemu-devel@nongnu.org; Mon, 13 Aug 2012 14:47:19 -0400 Received: from /spool/local by e28smtp04.in.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Tue, 14 Aug 2012 00:17:11 +0530 Received: from d28av03.in.ibm.com (d28av03.in.ibm.com [9.184.220.65]) by d28relay04.in.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id q7DIl7t816974024 for ; Tue, 14 Aug 2012 00:17:08 +0530 Received: from d28av03.in.ibm.com (loopback [127.0.0.1]) by d28av03.in.ibm.com (8.14.4/8.13.1/NCO v10.0 AVout) with ESMTP id q7DIl7Op005743 for ; Tue, 14 Aug 2012 04:47:07 +1000 Message-ID: <50294BA9.6070107@linux.vnet.ibm.com> Date: Tue, 14 Aug 2012 00:17:05 +0530 From: Harsh Bora MIME-Version: 1.0 References: <1344426944-7638-1-git-send-email-pbonzini@redhat.com> <1344426944-7638-2-git-send-email-pbonzini@redhat.com> In-Reply-To: <1344426944-7638-2-git-send-email-pbonzini@redhat.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 1/2] 9p-synth: fix read-side critical sections List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Paolo Bonzini Cc: qemu-devel@nongnu.org, aneesh.kumar@linux.vnet.ibm.com On 08/08/2012 05:25 PM, Paolo Bonzini wrote: > The read-side critical sections in 9p-synth currently only include the > navigation of the list. This is incorrect; it works for two reasons, > first obviously because rcu_read_lock/unlock are still no-ops; second, > because elements of the list are never deleted from the list (only added). > In fact, only adding items is the reason why rcu_read_lock/unlock can > be left as no-ops. > > If items were deleted, they could be reclaimed as soon as the read-side > critical section ends. So, the read-side critical section must include > all _usage_ of the node we got from the list too. Acked-by: Harsh Prateek Bora > > Signed-off-by: Paolo Bonzini > --- > hw/9pfs/virtio-9p-synth.c | 35 ++++++++++++++++++++--------------- > 1 file modificato, 20 inserzioni(+), 15 rimozioni(-) > > diff --git a/hw/9pfs/virtio-9p-synth.c b/hw/9pfs/virtio-9p-synth.c > index 92e0b09..a91ebe1 100644 > --- a/hw/9pfs/virtio-9p-synth.c > +++ b/hw/9pfs/virtio-9p-synth.c > @@ -237,14 +237,15 @@ static int v9fs_synth_get_dentry(V9fsSynthNode *dir, struct dirent *entry, > } > i++; > } > - rcu_read_unlock(); > if (!node) { > /* end of directory */ > *result = NULL; > - return 0; > + goto out; > } > v9fs_synth_direntry(node, entry, off); > *result = entry; > +out: > + rcu_read_unlock(); > return 0; > } > > @@ -466,6 +467,7 @@ static int v9fs_synth_name_to_path(FsContext *ctx, V9fsPath *dir_path, > { > V9fsSynthNode *node; > V9fsSynthNode *dir_node; > + int ret = 0; > > /* "." and ".." are not allowed */ > if (!strcmp(name, ".") || !strcmp(name, "..")) { > @@ -473,34 +475,37 @@ static int v9fs_synth_name_to_path(FsContext *ctx, V9fsPath *dir_path, > return -1; > > } > + > + rcu_read_lock(); > if (!dir_path) { > dir_node = &v9fs_synth_root; > } else { > dir_node = *(V9fsSynthNode **)dir_path->data; > } > - if (!strcmp(name, "/")) { > - node = dir_node; > - goto out; > - } > - /* search for the name in the childern */ > - rcu_read_lock(); > - QLIST_FOREACH(node, &dir_node->child, sibling) { > - if (!strcmp(node->name, name)) { > - break; > + > + node = dir_node; > + if (strcmp(name, "/") != 0) { > + /* search for the name in the childern */ > + QLIST_FOREACH(node, &dir_node->child, sibling) { > + if (!strcmp(node->name, name)) { > + break; > + } > } > } > - rcu_read_unlock(); > > if (!node) { > errno = ENOENT; > - return -1; > + ret = -1; > + goto err_out; > } > -out: > + > /* Copy the node pointer to fid */ > target->data = g_malloc(sizeof(void *)); > memcpy(target->data, &node, sizeof(void *)); > target->size = sizeof(void *); > - return 0; > +err_out: > + rcu_read_unlock(); > + return ret; > } > > static int v9fs_synth_renameat(FsContext *ctx, V9fsPath *olddir, >