Re: [PATCH] Busy inodes after unmount, be more verbose in generic_shutdown_super

[Jan Blunck <jblunck@suse.de>]

On Thu, Mar 02, Neil Brown wrote:

>   This requires:
>   - Breaking dentry_iput into 2 pieces, one that happens while the
>     dcache locks are held, and one that happens unlocked.
>   - Also, dput needs a variant which can be called with the spinlocks
>     held.
>   - This also requires a suitable comment in the code.
> 
>     It is possible that the dentry_iput call in dput might need to be
>     split into the locked/unlocked portions as well.  That would
>     require collecting a list of inodes and dentries to be freed once
>     the lock is dropped, which would be ugly.
>     An alternative might be to skip the tail recursion when
>     dput_locked was called as I *think* it is just an optimisation.
> 
> 
>  The following patch addressed the first three points.
> 
> Comments?  Please :-?
> 

This looks very much like a fixed version of my patch from
http://lkml.org/lkml/2006/1/20/303. Therfore, in general I'm fine with it ;)

Comments below !

> +void dput_locked(struct dentry *dentry)
> +{
> +	if (!dentry)
> +		return;
> +	if (!atomic_dec_and_test(&dentry->d_count)) {
> + 		spin_unlock(&dentry->d_lock);
> + 		spin_unlock(&dcache_lock);
> +		return;
> +	}
> +	__dput_locked(dentry);
> +}
> +

A comment like in dentry_iput() would be fine here:
 * Called with dcache_lock and per dentry lock held, drops both.

>  static inline void prune_one_dentry(struct dentry * dentry)
>  {
>  	struct dentry * parent;
> +	struct inode * ino;
>  
>  	__d_drop(dentry);
>  	list_del(&dentry->d_u.d_child);
>  	dentry_stat.nr_dentry--;	/* For d_free, below */
> -	dentry_iput(dentry);
> +	ino = dentry_iput_locked(dentry);
>  	parent = dentry->d_parent;
> -	d_free(dentry);
>  	if (parent != dentry)
> -		dput(parent);
> +		dput_locked(parent);
> +	else {
> +		spin_unlock(&dentry->d_lock);
> +		spin_unlock(&dcache_lock);
> +	}
> +	dentry_iput_unlocked(dentry, ino);
> +	d_free(dentry);
> +
>  	spin_lock(&dcache_lock);
>  }
>  

You missed getting the parent dentry's lock before calling dput_locked().

Regards,
	Jan

-- 
Jan Blunck                                               jblunck@suse.de
SuSE LINUX AG - A Novell company
Maxfeldstr. 5                                          +49-911-74053-608
D-90409 Nürnberg                                      http://www.suse.de
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/