On 07/20/2017 03:20 AM, Miklos Szeredi wrote: > On Wed, Jul 19, 2017 at 10:42 PM, Waiman Long <longman@xxxxxxxxxx> wrote: >> >>>> @@ -603,7 +698,13 @@ static struct dentry *dentry_kill(struct dentry *dentry) >>>> >>>> if (!IS_ROOT(dentry)) { >>>> parent = dentry->d_parent; >>>> - if (unlikely(!spin_trylock(&parent->d_lock))) { >>>> + /* >>>> + * Force the killing of this negative dentry when >>>> + * DCACHE_KILL_NEGATIVE flag is set. >>>> + */ >>>> + if (unlikely(dentry->d_flags & DCACHE_KILL_NEGATIVE)) { >>>> + spin_lock(&parent->d_lock); >>> This looks like d_lock ordering problem (should be parent first, child >>> second). Why is this needed, anyway? >>> >> Yes, that is a bug. I should have used lock_parent() instead. > lock_parent() can release dentry->d_lock, which means it's perfectly > useless for this. As the reference count is kept at 1 in dentry_kill(), the dentry won't go away even if the dentry lock is temporarily released. > I still feel forcing free is wrong here. Why not just block until > the number of negatives goes below the limit (start reclaim if not > already doing so, etc...)? Force freeing is the simplest. Any other ways will require adding more code and increasing code complexity. One reason why I prefer this is to avoid adding unpredictable latency to the regular directory lookup and other dentry related operations. We can always change the code later on if there is a better way of doing it. Cheers, Longman -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html