Re: [REGRESSION] kernfs: empty cgroup rmdir latency after delete-notification changes
From: Chengfeng Lin
Date: Thu Sep 17 2026 - 06:14:38 EST
Hi T.J.,
Thanks for explaining the locking issues. I tried a variation on your
first idea, using a separate inode_ever_requested field. In one four-boot
run, rmdir() was 22.36-22.72% faster and the continuous
mkdir() -> stat() -> rmdir() sequence was 7.35-9.01% shorter.
The tradeoff is an extra 8 bytes per kernfs_node in this build.
The watcher shortcut raises a separate concern: it also skips
clear_nlink(). An open FD can keep an inode alive without any watchers,
so that inode would still need its link count cleared.
The attached patch sets inode_ever_requested to true before calling
iget_locked().
It never clears the field, even after allocation failure or eviction.
A false value allows removal to skip the lookup; a true value retains
the existing lookups and link-count handling for every superblock.
It does not modify kn->flags or acquire a new lock during initialization.
Creation and removal still need the existing lifetime locking;
READ_ONCE()/WRITE_ONCE() do not provide that protection. I checked the
five kernfs_get_inode() call sites. Four use kernfs_rwsem with Shakeel's
fix applied; cgroup_may_write() instead relies on cgroup_mutex.
Those lifetime assumptions still need review.
I tested on 2f0c1cf72f46 (7.3-rc2):
A: baseline + Shakeel's file-handle decoding read-lock fix [1]
B: A + the attached prototype, without the old INODE_INITED changes
Four fresh boots ran in A1 -> B1 -> B2 -> A2 order:
A1 B1 B2 A2
original rmdir (us) 9.758 7.541 7.563 9.741
mkdir -> first stat -> rmdir (us) 30.023 27.317 27.356 29.526
The workload still removes empty cgroups, with no tasks or watches.
The stat() in the sequence is the first directory stat().
I used the same binaries on the bare-metal i7-12700KF, CPU0, with
sched_ext disabled, full preemption, the performance governor and Turbo
disabled. Both kernels used GCC 15.2 and the same config except LOCALVERSION.
Each value is the median of nine sample means, with 16 warmups per sample.
The original test measured 128 removals per sample; the sequence test
measured 32 sequences, without pauses between operations. For these two
metrics, CV stayed below 1.96% and boot drift stayed below 1.66%.
The improvement ranges compare each B boot against each A boot.
Both improvements held after dropping the first sample from each boot.
Separate traces showed ilookup() calls falling from 26 to 1 per removal,
or from 26 to 2 when one file FD was held open. Required clear_nlink()
calls and deletion lock counts were unchanged. Held-FD link-count checks,
notification positive controls and bounded open/remove checks passed.
I also tested first opens racing removal on fresh, unwatched files:
176 cases without tracing and 176 with tracing on each kernel,
including ordered controls.
Tracing confirmed 96 new inodes on each kernel. In B, the marker was false
on entry and true before iget_locked(). All successfully opened FDs
reported nlink=0 after removal.
I did not capture a remover waiting on the first lookup's read lock.
Eviction/recreation, distinct superblocks, other kernfs users and
weak-memory architectures remain untested.
kernfs_node grew from 136 to 144 bytes, and objects per 4 KiB slab fell
from 30 to 28. Simply reordering the fields does not avoid the size
increase in this build. I have not found a safe way to avoid this extra
memory cost. First-file stat() timing was too variable to quantify its
overhead. This is a prototype for review, not a finished fix.
Results, test sources and the patch are available at [2].
These results are from a microbenchmark. I have not measured the impact
on a real application. Given the extra memory cost and remaining
correctness questions, do you think this direction is worth pursuing?
[1] [PATCH v2 3/4] kernfs: don't lose IN_DELETE_SELF
when decoding a file handle
https://lists.openwall.net/linux-kernel/2026/09/05/828
[2] Prototype results and reproduction
https://github.com/lcf0399/linux-regression-evidence/tree/b9ce1d1b34aa1fe5b5bf6b49dc334f36a7aa8bfd/kernfs-empty-cgroup-removal/bare-metal/inode-requested
Thanks,
Chengfeng
T.J. Mercier <tjmercier@xxxxxxxxxx> 于2026年9月16日周三 06:01写道：
>
> On Mon, Sep 14, 2026 at 11:08 AM Chengfeng Lin <lin2530632123@xxxxxxxxx> wrote:
> >
> > Hi T.J.,
> >
> > Thanks for the patch. Your changes reduced rmdir() latency by
> > 21.87-22.86% in the same empty-cgroup benchmark, with sched_ext disabled.
> >
> > I tested these two states on 2f0c1cf72f46 (7.3-rc2):
> >
> > A: baseline + Shakeel's file-handle decoding read-lock fix [1]
> > B: A + your INODE_INITED changes, without modifying your code
> >
> > I used the original binary on the bare-metal i7-12700KF, pinned to CPU0,
> > with full preemption, the performance governor and Turbo disabled.
> > Both kernels used GCC 15.2 and the same config except LOCALVERSION.
> >
> > The clean timing sequence used four fresh boots: A1 -> B1 -> B2 -> A2.
> >
> > A1 B1 B2 A2
> > rmdir (us) 9.727 7.600 7.548 9.784
> >
> > Each value is the median of nine sample means. Each sample contains
> > 128 measured removals after 16 warmups. All four B/A comparisons showed
> > the improvement above. Removal CVs were below 2.03%. The drift between
> > boots was +0.59% for A and -0.68% for B. The improvement held after
> > dropping the first sample from each boot.
> >
> > Separate, untimed traces showed the same 26 kernfs nodes per removal,
> > but ilookup() calls fell from 26 to 1. Deletion lock counts were unchanged.
> > Basic checks of file/directory deletion notifications and link counts
> > with FDs held open passed on both kernels.
> >
> > I also timed the full mkdir() -> stat() -> rmdir() sequence using a
> > separate binary on the same kernels. The latest continuous follow-up used
> > four fresh boots, with nine samples per boot. Each sample had 16 warmups
> > and 32 measured sequences, with no pauses between them. The full sequence
> > fell from 30.07-30.65 us to 27.97-28.34 us, a 5.77-8.76% reduction.
> > The maximum CV was 1.76%, and boot drift stayed below 2% in both states.
> > In the earlier continuous follow-up with 128 operations per sample, the
> > full sequence was 5.15-6.52% shorter, but CVs of 4.52-6.46% were too high
> > to establish a stable overall improvement percentage.
> >
> > Untimed tracing placed the added write lock in first stat(), during inode
> > initialization. The patch skips lookups for nodes whose inodes were never
> > initialized; it does not move those lookups into stat(). The individual
> > mkdir() and stat() timings remained too variable to quantify their changes.
> >
> > I also checked whether the benefit persisted with less frequent operations.
> > For this diagnostic, I added 50 and 200 ms pauses between sequences,
> > outside the timers. I kept 16 warmups and 32 measured sequences per sample.
> > rmdir() still saved 2.10-2.58 us, but its baseline rose from about 10 us
> > to 18 us, reducing the percentage gain. The full sequence was 0.66-2.85%
> > shorter, with a maximum CV of 1.53%.
> >
> > I included [1] because the file-handle path on this base lacks
> > kernfs_rwsem protection. Source review suggests a possible wait cycle
> > when that path recreates an evicted inode. Initialization waits for
> > kernfs_iattr_rwsem, while removal holds it and waits for I_NEW to clear.
> > I have not reproduced this possible deadlock in the kernel.
> > Does the base for your diff already include [1] or equivalent locking?
> >
> > [1] [PATCH v2 3/4] kernfs: don't lose IN_DELETE_SELF
> > when decoding a file handle
> > https://lists.openwall.net/linux-kernel/2026/09/05/828
> >
> > Results and reproducers:
> > https://github.com/lcf0399/linux-regression-evidence/tree/f101b1b9c0d3dfce07f81b11ca2da2c252150e4c/kernfs-empty-cgroup-removal/bare-metal/inode-inited
> >
> > Thanks,
> > Chengfeng
>
> Thanks for testing. I was also not aware of Shakeel's fix [1] so it
> was not in my base; I used 2d3090a8aeb5 ("Merge tag 'v7.1-p5' of
> git://git.kernel.org/pub/scm/linux/kernel/git/herbert/crypto-2.6").
>
> I think you are correct about the deadlock. The synchronization
> requirements here are more difficult than I initially realized.
> kernfs_iattr_rwsem can't be used because of the deadlock you
> mentioned. Additionally, kn->flags is non-atomic and only modified
> under down_write(&root->kernfs_rwsem). Inode allocation runs under a
> read lock (or locklessly before Shakeel's fix), so setting a flag
> there races with writers and risks lost updates on other flags.
> Grabbing a write lock in kernfs_init_inode() would add contention to
> normal lookup and stat paths. So I'm not seeing a simple way forward
> with the KERNFS_INODE_INITED idea.
>
> This is a more straightforward but even less exact way:
>
> @@ -1526,8 +1527,12 @@ static void kernfs_clear_inode_nlink(struct
> kernfs_node *kn)
> lockdep_assert_held_read(&root->kernfs_supers_rwsem);
>
> list_for_each_entry(info, &root->supers, node) {
> - struct inode *inode = ilookup(info->sb, kernfs_ino(kn));
> + struct inode *inode;
>
> + if (!fsnotify_sb_has_watchers(info->sb))
> + continue;
> +
> + inode = ilookup(info->sb, kernfs_ino(kn));
>
> I'm not sure that's really worth a few microseconds though.
>
> > T.J. Mercier <tjmercier@xxxxxxxxxx> 于2026年9月14日周一 00:17写道：
> > >
> > > On Sun, Sep 13, 2026 at 8:47 AM Chengfeng Lin <lin2530632123@xxxxxxxxx> wrote:
> > > >
> > > > Hi Tejun,
> > > >
> > > > Thanks. I found this during research into kernel performance regressions,
> > > > using a microbenchmark of cgroup creation and removal. This was not
> > > > prompted by a production issue.
> > > >
> > > > I reported it to document the added removal cost and ask whether some
> > > > of it could be reduced while preserving the fixes. I agree that the
> > > > roughly 3 us increase per removal does not, by itself, demonstrate a
> > > > significant impact on real workloads.
> > > >
> > > > Thanks,
> > > > Chengfeng
> > > >
> > > > Tejun Heo <tj@xxxxxxxxxx> 于2026年9月13日周日 23:03写道：
> > > > >
> > > > > Hello,
> > > > >
> > > > > On Sun, Sep 13, 2026 at 10:43:04PM +0800, Chengfeng Lin wrote:
> > > > > > I found an increase in empty cgroup removal latency across 507d8ce13f5b
> > > > > > ("kernfs: Don't set_nlink for directories being removed") and eea5d2bb34ba
> > > > > > ("kernfs: Send IN_DELETE_SELF and IN_IGNORED"). With sched_ext disabled,
> > > > > > rmdir() went from about 6.5 us to 9.6 us, an increase of 46-47%.
> > > > > > The result held in two independent runs.
> > > > >
> > > > > I don't want to make cgroup removal unnecessarily expensive but at the same
> > > > > time it's not an operation that I consider to be a hot path, so as long as
> > > > > the operaiton can finish in a reasonable amount of time and single digit us
> > > > > definitely is, performance of rmdir usually isn't something which is high in
> > > > > priority. Can you please detail why this matters for you?
> > > > >
> > > > > Thanks.
> > > > >
> > > > > --
> > > > > tejun
> > >
> > > Hi Chengfeng and Tejun,
> > >
> > > We use this kernfs IN_DELETE_SELF feature on Android where multiple
> > > cgroups are created and removed frequently on a per-application basis,
> > > but we hadn't noticed a significant delay due to the inotify
> > > functionality during cgroup removal. It's probably masked by much
> > > larger delays (milliseconds) we regularly see due to unreleated issues
> > > with cgroup locks like priority inversion with cgroup_mutex, and
> > > contention for cgroup_threadgroup_rwsem.
> > >
> > > As far as what we can do, I think avoiding inode lookups for kernfs
> > > nodes that never had an inode created should improve the situation. It
> > > doesn't eliminate the locking overhead, which I don't think can easily
> > > be eliminated. The code below doesn't deal with inode eviction either,
> > > but at least files which are never accessed won't introduce the inode
> > > lookup overhead.
> > >
> > > diff --git a/fs/kernfs/dir.c b/fs/kernfs/dir.c
> > > index 4f9ade82b08a..3ec9f79b1aa2 100644
> > > --- a/fs/kernfs/dir.c
> > > +++ b/fs/kernfs/dir.c
> > > @@ -1523,6 +1523,9 @@ static void kernfs_clear_inode_nlink(struct
> > > kernfs_node *kn)
> > > struct kernfs_root *root = kernfs_root(kn);
> > > struct kernfs_super_info *info;
> > >
> > > + if (!(kn->flags & KERNFS_INODE_INITED))
> > > + return;
> > > +
> > > lockdep_assert_held_read(&root->kernfs_supers_rwsem);
> > >
> > > list_for_each_entry(info, &root->supers, node) {
> > > diff --git a/fs/kernfs/inode.c b/fs/kernfs/inode.c
> > > index 38b28aa7cd02..f8b4a8b8ade4 100644
> > > --- a/fs/kernfs/inode.c
> > > +++ b/fs/kernfs/inode.c
> > > @@ -208,6 +208,10 @@ static void kernfs_init_inode(struct kernfs_node
> > > *kn, struct inode *inode)
> > > set_default_inode_attr(inode, kn->mode);
> > > kernfs_refresh_inode(kn, inode);
> > >
> > > + down_write(&kernfs_root(kn)->kernfs_iattr_rwsem);
> > > + kn->flags |= KERNFS_INODE_INITED;
> > > + up_write(&kernfs_root(kn)->kernfs_iattr_rwsem);
> > > +
> > > /* initialize inode according to type */
> > > switch (kernfs_type(kn)) {
> > > case KERNFS_DIR:
> > > diff --git a/include/linux/kernfs.h b/include/linux/kernfs.h
> > > index e21b2f7f4159..87f074ce19d3 100644
> > > --- a/include/linux/kernfs.h
> > > +++ b/include/linux/kernfs.h
> > > @@ -113,6 +113,7 @@ enum kernfs_node_flag {
> > > KERNFS_EMPTY_DIR = 0x1000,
> > > KERNFS_HAS_RELEASE = 0x2000,
> > > KERNFS_REMOVING = 0x4000,
> > > + KERNFS_INODE_INITED = 0x8000,
> > > };
> > >
> > > -T.J.
--- a/fs/kernfs/dir.c
+++ b/fs/kernfs/dir.c
@@ -1524,6 +1524,10 @@
lockdep_assert_held_read(&root->kernfs_supers_rwsem);
+ /* No inode can exist if creation has never been requested. */
+ if (!READ_ONCE(kn->inode_ever_requested))
+ return;
+
list_for_each_entry(info, &root->supers, node) {
struct inode *inode = ilookup(info->sb, kernfs_ino(kn));
--- a/fs/kernfs/inode.c
+++ b/fs/kernfs/inode.c
@@ -247,6 +247,16 @@
{
struct inode *inode;
+ /*
+ * Callers must serialize inode creation against node removal,
+ * using kernfs_rwsem or the owning subsystem's lifetime lock.
+ * Concurrent readers may set this independent hint to true.
+ * Set it before iget_locked(), and keep it set on failure or
+ * eviction: a false positive only causes an extra lookup.
+ */
+ if (!READ_ONCE(kn->inode_ever_requested))
+ WRITE_ONCE(kn->inode_ever_requested, true);
+
inode = iget_locked(sb, kernfs_ino(kn));
if (inode && (inode_state_read_once(inode) & I_NEW))
kernfs_init_inode(kn, inode);
--- a/include/linux/kernfs.h
+++ b/include/linux/kernfs.h
@@ -217,6 +217,9 @@
unsigned int hash; /* ns + name hash */
unsigned short flags;
umode_t mode;
+
+ /* Conservative hint; never reset during this node's lifetime. */
+ bool inode_ever_requested;
union {
struct kernfs_elem_dir dir;