lkml.org 
[lkml]   [2006]   [Mar]   [2]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH] Busy inodes after unmount, be more verbose in generic_shutdown_super
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/

\
 
 \ /
  Last update: 2006-03-02 11:50    [W:0.111 / U:0.928 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site