Re: [PATCH] dcache d_drop() bug fix / __d_drop() use fix

[Jeff Moyer]

==> Regarding Re: [PATCH] dcache d_drop() bug fix / __d_drop() use fix; raven@xxxxxxxxxx adds:

raven> On Wed, 16 Feb 2005, Jan Blunck wrote:
>> This is a re-submission of the patch I sent about a month ago.
>> 
>> While working on my code I realized that d_drop() might race against 
>> __d_lookup(). __d_drop() (which is called by d_drop() after acquiring the 
>> dcache_lock) is accessing dentry->d_flags to set the DCACHE_UNHASHED flag. 
>> This shouldn't be done without holding dentry->d_lock, like stated in 
>> dcache.h:
>> 
>> struct dentry {
>> ...
>> unsigned int d_flags;		/* protected by d_lock */
>> ...
>> };
>> 
>> Therefore d_drop() must acquire the dentry->d_lock. Likewise every use of 
>> __d_drop() must acquire that lock.
>> 
>> This patch fixes d_drop() and every grep'able __d_drop() use. This patch is 
>> against today's http://linux.bkbits.net/linux-2.5.
>> 

raven> For my part, in autofs4, I would prefer:

> --- linux-2.6.9/fs/autofs4/root.c.d_lock	2005-02-16 23:15:18.000000000 +0800
> +++ linux-2.6.9/fs/autofs4/root.c	2005-02-16 23:15:35.000000000 +0800
> @@ -621,7 +621,7 @@
>   		spin_unlock(&dcache_lock);
>   		return -ENOTEMPTY;
>   	}
> -	__d_drop(dentry);
> +	d_drop(dentry);
>   	spin_unlock(&dcache_lock);

>   	dput(ino->dentry);

Ian, this would deadlock.  You already hold the dcache lock here, and
d_drop takes it:

static inline void d_drop(struct dentry *dentry)
{
        spin_lock(&dcache_lock);
        __d_drop(dentry);
        spin_unlock(&dcache_lock);
}

The proposed patch was to take the dentry->d_lock.

-Jeff
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/