From mboxrd@z Thu Jan 1 00:00:00 1970 From: Eric Paris Subject: Re: [PATCH 10/10] fsnotify: use dget_parent Date: Mon, 11 Oct 2010 09:59:22 -0400 Message-ID: <1286805562.2795.10.camel@localhost.localdomain> References: <20101010093620.416498597@canuck.infradead.org> <20101010093722.745269700@canuck.infradead.org> Mime-Version: 1.0 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: 7bit Cc: viro@zeniv.linux.org.uk, linux-fsdevel@vger.kernel.org To: Christoph Hellwig Return-path: Received: from mx1.redhat.com ([209.132.183.28]:23948 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754021Ab0JKN73 (ORCPT ); Mon, 11 Oct 2010 09:59:29 -0400 In-Reply-To: <20101010093722.745269700@canuck.infradead.org> Sender: linux-fsdevel-owner@vger.kernel.org List-ID: On Sun, 2010-10-10 at 05:36 -0400, Christoph Hellwig wrote: > plain text document attachment (fsnotify-use-dget_parent) > Use dget_parent instead of opencoding it. This simplifies the code, but > more importanly prepares for the more complicated locking for a parent > dget in the dcache scale patch series. > > It means we do grab a reference to the parent now if need to be watched, > but not with the specified mask. If this turns out to be a problem > we'll have to revisit it, but for now let's keep as much as possible > dcache internals inside dcache.[ch]. > > Signed-off-by: Christoph Hellwig Nope this is fine, we were doing lots of extra work for little benefit before. Acked-by: Eric Paris > Index: linux-2.6/fs/notify/fsnotify.c > =================================================================== > --- linux-2.6.orig/fs/notify/fsnotify.c 2010-10-10 09:35:58.488004195 +0200 > +++ linux-2.6/fs/notify/fsnotify.c 2010-10-10 09:53:53.103010272 +0200 > @@ -88,8 +88,6 @@ void __fsnotify_parent(struct path *path > { > struct dentry *parent; > struct inode *p_inode; > - bool send = false; > - bool should_update_children = false; > > if (!dentry) > dentry = path->dentry; > @@ -97,29 +95,12 @@ void __fsnotify_parent(struct path *path > if (!(dentry->d_flags & DCACHE_FSNOTIFY_PARENT_WATCHED)) > return; > > - spin_lock(&dentry->d_lock); > - parent = dentry->d_parent; > + parent = dget_parent(dentry); > p_inode = parent->d_inode; > > - if (fsnotify_inode_watches_children(p_inode)) { > - if (p_inode->i_fsnotify_mask & mask) { > - dget(parent); > - send = true; > - } > - } else { > - /* > - * The parent doesn't care about events on it's children but > - * at least one child thought it did. We need to run all the > - * children and update their d_flags to let them know p_inode > - * doesn't care about them any more. > - */ > - dget(parent); > - should_update_children = true; > - } > - > - spin_unlock(&dentry->d_lock); > - > - if (send) { > + if (unlikely(!fsnotify_inode_watches_children(p_inode))) > + __fsnotify_update_child_dentry_flags(p_inode); > + else if (p_inode->i_fsnotify_mask & mask) { > /* we are notifying a parent so come up with the new mask which > * specifies these are events which came from a child. */ > mask |= FS_EVENT_ON_CHILD; > @@ -130,13 +111,9 @@ void __fsnotify_parent(struct path *path > else > fsnotify(p_inode, mask, dentry->d_inode, FSNOTIFY_EVENT_INODE, > dentry->d_name.name, 0); > - dput(parent); > } > > - if (unlikely(should_update_children)) { > - __fsnotify_update_child_dentry_flags(p_inode); > - dput(parent); > - } > + dput(parent); > } > EXPORT_SYMBOL_GPL(__fsnotify_parent); > >