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...
