Re: [PATCH 10/10] fsnotify: use dget_parent

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

 



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 <hch@xxxxxx>

Nope this is fine, we were doing lots of extra work for little benefit
before.

Acked-by: Eric Paris <eparis@xxxxxxxxxx>

> 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);
>  
> 


--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Ext4 Filesystem]     [Union Filesystem]     [Filesystem Testing]     [Ceph Users]     [Ecryptfs]     [AutoFS]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux Cachefs]     [Reiser Filesystem]     [Linux RAID]     [Samba]     [Device Mapper]     [CEPH Development]
  Powered by Linux