Re: [PATCH v2 5/6] fs/dcache: Avoid a try_lock loop in shrink_dentry_list()

From: Al Viro
Date: Thu Feb 22 2018 - 22:48:35 EST


On Fri, Feb 23, 2018 at 12:50:24AM +0100, John Ogness wrote:
> - while (dentry && !lockref_put_or_lock(&dentry->d_lockref)) {
> - parent = lock_parent(dentry);
> - if (dentry->d_lockref.count != 1) {
> - dentry->d_lockref.count--;
> - spin_unlock(&dentry->d_lock);
> - if (parent)
> - spin_unlock(&parent->d_lock);
> - break;
> - }
> - inode = dentry->d_inode; /* can't be NULL */
> - if (unlikely(!spin_trylock(&inode->i_lock))) {
> - spin_unlock(&dentry->d_lock);
> - if (parent)
> - spin_unlock(&parent->d_lock);
> - cpu_relax();
> - continue;
> - }
> - __dentry_kill(dentry);
> - dentry = parent;
> - }
> + while (dentry && !lockref_put_or_lock(&dentry->d_lockref))
> + dentry = dentry_kill(dentry);

Hmm... OK, that's interesting. I agree that it looks similar to dentry_kill()
loop, with one exception - here we are aggressively pruning the branch. None
of the "do we want to retain that sucker" stuff here. It doesn't matter for
most of the callers, with one exception: prune_dcache_sb(). OTOH, there it
just might be the right thing to do anyway - after all, it matters only if
somebody has grabbed and dropped the sucker while we'd been trying to do
lock_parent(). Had we lost the race with their dput(), we would've left
the damn thing alone, and we are called from a memory shrinker, so we'll get
called again if needed.