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 [email protected]
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 [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[Index of Archives]
[Kernel Newbies]
[Netfilter]
[Bugtraq]
[Photo]
[Stuff]
[Gimp]
[Yosemite News]
[MIPS Linux]
[ARM Linux]
[Linux Security]
[Linux RAID]
[Video 4 Linux]
[Linux for the blind]
[Linux Resources]
