Re: [PATCH v3 20/21] __dentry_kill(): new locking scheme

[Al Viro <viro@zeniv.linux.org.uk>]

On Mon, Jul 07, 2025 at 11:06:06PM +0200, Max Kellermann wrote:
> On Mon, Jul 7, 2025 at 10:59 PM Al Viro <viro@zeniv.linux.org.uk> wrote:
> > Umm...  Note that further in that loop we'll be actively stealing the stuff from that
> > shrink list that hasn't gotten to __dentry_kill().  Does your busy loop go into
> > if (data.victim) after the second d_walk()?  IOW, does it manage to pull anything out
> > of that shrink list?
> 
> No, I traced this, there is never a "data.victim" because none of the
> dentries has DCACHE_SHRINK_LIST. "data.found" is only ever incremented
> once (per loop iteration) because a "dead" lockref was found. The
> second d_walk() doesn't find anything because it doesn't look for dead
> (dying) dentries. You added that check only to the first call (only to
> select_collect(), but not to select_collect2()).
> 
> I think we're getting closer to the point I was trying to make :-)

The second d_walk() does not have the if (!data.found) break; after it.
So if your point is that we should ignore these and bail out as soon as we
reach that state, we are not getting any closer to it.

The second d_walk() is specifically about the stuff already in some other
thread's shrink list.  If it finds more than that, all the better, but the
primary goal is to make some progress in case if there's something in
another thread's shrink list they are yet to get around to evicting.

Again, what would you have it do?  The requirement is to take out everything
that has no busy descendents.

BTW, is that the same dentry all along in your reproducer?  Or does it switch
to a different dentry after a while?