Re: [PATCH 6/6] Optimise d_find_alias() [try #6]

[Nick Piggin <nickpiggin@yahoo.com.au>]

David Howells wrote:
> The attached patch optimises d_find_alias() to only take the spinlock if
> there's anything in the the inode's alias list. If there isn't, it returns NULL
> immediately.
> 
> With respect to the superblock sharing patch, this should reduce by one the
> number of times the dcache_lock is taken by nfs_lookup() for ordinary
> directory lookups.
> 
> Only in the case where there's already a dentry for particular directory inode
> (such as might happen when another mountpoint is rooted at that dentry) will
> the lock then be taken the extra time.
> 
> Signed-Off-By: David Howells <dhowells@redhat.com>
> ---
> 
>  fs/dcache.c |   11 +++++++----
>  1 files changed, 7 insertions(+), 4 deletions(-)
> 
> diff --git a/fs/dcache.c b/fs/dcache.c
> index 97e1e44..32051ba 100644
> --- a/fs/dcache.c
> +++ b/fs/dcache.c
> @@ -325,10 +325,13 @@ static struct dentry * __d_find_alias(st
>  
>  struct dentry * d_find_alias(struct inode *inode)
>  {
> -	struct dentry *de;
> -	spin_lock(&dcache_lock);
> -	de = __d_find_alias(inode, 0);
> -	spin_unlock(&dcache_lock);
> +	struct dentry *de = NULL;
> +	smp_rmb();

Having the smp_rmb() here implies there is some sort of memory barrier
based synchronisation protocol at a higher level (than this function),
because you don't actually do anything before them smp_rmb() here.

So can you comment what that is?

> +	if (!list_empty(&inode->i_dentry)) {
> +		spin_lock(&dcache_lock);
> +		de = __d_find_alias(inode, 0);
> +		spin_unlock(&dcache_lock);
> +	}
>  	return de;
>  }
>  

-- 
SUSE Labs, Novell Inc.
Send instant messages to your online friends http://au.messenger.yahoo.com