[<prev] [next>] [thread-next>] [day] [month] [year] [list]
Message-ID: <20240510221901.520546-1-stephen.s.brennan@oracle.com>
Date: Fri, 10 May 2024 15:18:59 -0700
From: Stephen Brennan <stephen.s.brennan@...cle.com>
To: Jan Kara <jack@...e.cz>
Cc: Amir Goldstein <amir73il@...il.com>, linux-fsdevel@...r.kernel.org,
linux-kernel@...r.kernel.org,
Stephen Brennan <stephen.s.brennan@...cle.com>
Subject: [PATCH 0/1] fsnotify: clear PARENT_WATCHED flags lazily
Hi Amir, Jan, et al,
It's been a while since I worked with you on the patch series[1] that aimed to
make __fsnotify_update_child_dentry_flags() a sleepable function. That work got
to a point that it was close to ready, but there were some locking issues which
Jan found, and the kernel test robot reported, and I didn't find myself able to
tackle them in the amount of time I had.
But looking back on that series, I think I threw out the baby with the
bathwater. While I may not have resolved the locking issues associated with the
larger change, there was one patch which Amir shared, that probably resolves
more than 90% of the issues that people may see. I'm sending that here, since it
still applies to the latest master branch, and I think it's a very good idea.
To refresh you, the underlying issue I was trying to resolve was when
directories have many dentries (frequently, a ton of negative dentries), the
__fsnotify_update_child_dentry_flags() operation can take a while, and it
happens under spinlock.
Case #1 - if the directory has tens of millions of dentries, then you could get
a soft lockup from a single call to this function. I have seen some cases where
a single directory had this many dentries, but it's pretty rare.
Case #2 - suppose you have a system with many CPUs and a busy directory. Suppose
the directory watch is removed. The caller will begin executing
__fsnotify_update_child_dentry_flags() to clear the PARENT_WATCHED flag, but in
parallel, many other CPUs could wind up in __fsnotify_parent() and decide that
they, too, must call __fsnotify_update_child_dentry_flags() to clear the flags.
These CPUs will all spin waiting their turn, at which point they'll re-do the
long (and likely, useless) call. Even if the original call only took a second or
two, if you have a dozen or so CPUs that end up in that call, some CPUs will
spin a long time.
Amir's patch to clear PARENT_WATCHED flags lazily resolves that easily. In
__fsnotify_parent(), if callers notice that the parent is no longer watching,
they merely update the flags for the current dentry (not all the other
children). The __fsnotify_recalc_mask() function further avoids excess calls by
only updating children if the parent started watching. This easily handles case
#2 above. Perhaps case #1 could still cause issues, for the cases of truly huge
dentry counts, but we shouldn't let "perfect" get in the way of "good enough" :)
Thanks,
Stephen
[1]: https://lore.kernel.org/all/20221013222719.277923-1-stephen.s.brennan@oracle.com/
Amir Goldstein (1):
fsnotify: clear PARENT_WATCHED flags lazily
fs/notify/fsnotify.c | 26 ++++++++++++++++++++------
fs/notify/fsnotify.h | 3 ++-
fs/notify/mark.c | 32 +++++++++++++++++++++++++++++---
include/linux/fsnotify_backend.h | 8 +++++---
4 files changed, 56 insertions(+), 13 deletions(-)
--
2.43.0
Powered by blists - more mailing lists