Re: [PATCH] vfs: make real_lookup do dentry revalidation with i_mutex held

[Sage Weil <sage@xxxxxxxxxxxx>]

On Tue, 17 Mar 2009, Christoph Hellwig wrote:
> Keeping i_mutes over do_revalidate seem fine from a first glance, but
> can you please do it without rearranging the whole code?

Yeah, but not without an extra goto.  Holding i_mutex over revalidate is 
only half of it... we also want to go ahead with the ->lookup if the 
revalidate fails (instead of returning -ENOENT).  I make the patch easier 
to read (with a goto), but I assumed we'd want the resulting code to be 
more clear?

FWIW, here's the patched result:

	result = d_lookup(parent, name);
	if (result) {
		/*
		 * The cache was re-populated while we waited on the
		 * mutex.  We need to revalidate, this time while
		 * holding i_mutex (to avoid another race).
		 */
		if (result->d_op && result->d_op->d_revalidate) {
			result = do_revalidate(result, nd);
			if (result)
				goto out_unlock;
			/*
			 * The dentry was left behind invalid.  Just
			 * do the lookup.
			 */
		} else {
			goto out_unlock;
		}
	}

	/* Don't create child dentry for a dead directory. */
	result = ERR_PTR(-ENOENT);
	if (IS_DEADDIR(dir))
		goto out_unlock;

	dentry = d_alloc(parent, name);
	result = ERR_PTR(-ENOMEM);
	if (dentry) {
		result = dir->i_op->lookup(dir, dentry, nd);
		if (result)
			dput(dentry);
		else
			result = dentry;
	}
out_unlock:

Let me know!
sage


> Something like the tiny untested patch below should archive the same
> thing:
> 
> 
> Index: linux-2.6/fs/namei.c
> ===================================================================
> --- linux-2.6.orig/fs/namei.c	2009-03-17 09:15:53.430978739 +0100
> +++ linux-2.6/fs/namei.c	2009-03-17 09:16:19.553981306 +0100
> @@ -512,12 +512,12 @@ out_unlock:
>  	 * Uhhuh! Nasty case: the cache was re-populated while
>  	 * we waited on the semaphore. Need to revalidate.
>  	 */
> -	mutex_unlock(&dir->i_mutex);
>  	if (result->d_op && result->d_op->d_revalidate) {
>  		result = do_revalidate(result, nd);
>  		if (!result)
>  			result = ERR_PTR(-ENOENT);
>  	}
> +	mutex_unlock(&dir->i_mutex);
>  	return result;
>  }
>  
> 
> 
--
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