Hi Al,
On Mon, Jul 7, 2025 at 7:04 PM Al Viro <viro@xxxxxxxxxxxxxxxxxx> wrote:
> @@ -1478,6 +1444,8 @@ static enum d_walk_ret select_collect(void *_data, struct dentry *dentry)
> } else if (!dentry->d_lockref.count) {
> to_shrink_list(dentry, &data->dispose);
> data->found++;
> + } else if (dentry->d_lockref.count < 0) {
> + data->found++;
> }
> /*
> * We can return to the caller if we have found some (this
I have doubts about this part of your patch. (Warning: I don't really
know what your patch is about.)
Why does this new check exist?
(It checks for "dead" or "killed" entries, but why aren't you using
__lockref_is_dead() here?)
Actual problem why I found this: while debugging (yet another) Ceph
bug, I found that a kworker is busy-looping inside
shrink_dcache_parent(). Each iteration finds a dead/killed dentry,
thus "found=true" and the loop keeps on looping forever, yet nothing
ever gets done.
It does this because a userspace process is trying to write to Ceph
file, that write() system call invokes the shrinker (via
filmap_add_folio() / memcg). The shrinker calls shrink_dentry_list(),
__dentry_kill() - now that dentry is dead/killed, but it remains
listed in the parent because the thread is stuck in ceph_evict_inode()
/ netfs_wait_for_outstanding_io().
I am seeing this because Ceph doesn't finish I/O on the inode, which
causes the kworker to busy-loop forever without making any progress.
But even if Ceph weren't as buggy as it is, there may still be some
time waiting for the Ceph server, and that will always cause brief
periods of busy-looping in the kworker, won't it?
I don't know how to solve this (I have no idea about the dcache,
having opened its source for the first time today), but I wonder why
select_collect() ever cares about dead/killed dentries. Removing that
check seems like the obvious solution, but there must be a reason why
you added it.
Max
