On Sat, Jun 29, 2019 at 08:06:24PM +0100, Al Viro wrote: > On Sat, Jun 29, 2019 at 05:38:03AM +0100, Al Viro wrote: > > > PS: the problem is not gone in the next iteration of the patchset in > > question. The patch I'm proposing (including dput_to_list() and _ONLY_ > > compile-tested) follows. Comments? > > FWIW, there's another unpleasantness in the whole thing. Suppose we have > picked a page full of dentries, all with refcount 0. We decide to > evict all of them. As it turns out, they are from two filesystems. > Filesystem 1 is NFS on a server, with currently downed hub on the way > to it. Filesystem 2 is local. We attempt to evict an NFS dentry and > get stuck - tons of dirty data with no way to flush them on server. > In the meanwhile, admin tries to unmount the local filesystem. And > gets stuck as well, since umount can't do anything to its dentries > that happen to sit in our shrink list. > > I wonder if the root of problem here isn't in shrink_dcache_for_umount(); > all it really needs is to have everything on that fs with refcount 0 > dragged through __dentry_kill(). If something had been on a shrink > list, __dentry_kill() will just leave behind a struct dentry completely > devoid of any connection to superblock, other dentries, filesystem > type, etc. - it's just a piece of memory that won't be freed until > the owner of shrink list finally gets around to it. Which can happen > at any point - all they'll do to it is dentry_free(), and that doesn't > need any fs-related data structures. > > The logics in shrink_dcache_parent() is > collect everything evictable into a shrink list > if anything found - kick it out and repeat the scan > otherwise, if something had been on other's shrink list > repeat the scan > > I wonder if after the "no evictable candidates, but something > on other's shrink lists" we ought to do something along the > lines of > rcu_read_lock > walk it, doing > if dentry has zero refcount > if it's not on a shrink list, > move it to ours > else > store its address in 'victim' > end the walk > if no victim found > rcu_read_unlock > else > lock victim for __dentry_kill > rcu_read_unlock > if it's still alive > if it's not IS_ROOT > if parent is not on shrink list > decrement parent's refcount > put it on our list > else > decrement parent's refcount > __dentry_kill(victim) > else > unlock > if our list is non-empty > shrink_dentry_list on it > in there... Thanks for still thinking about this Al. I don't have a lot of idea about what to do with your comments until I can grok them fully but I wanted to acknowledge having read them. Thanks, Tobin.
