mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [REGRESSION] kernfs: empty cgroup rmdir latency after delete-notification changes
@ 2026-09-13 14:43 Chengfeng Lin
  2026-09-13 15:03 ` Tejun Heo
  0 siblings, 1 reply; 7+ messages in thread
From: Chengfeng Lin @ 2026-09-13 14:43 UTC (permalink / raw)
  To: T.J. Mercier
  Cc: Greg Kroah-Hartman, Tejun Heo, Jan Kara, driver-core, cgroups,
	linux-fsdevel, linux-kernel, regressions

Hi T.J.,

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.

The workload creates an empty cgroup v2 leaf under a private parent with
the CPU controller enabled, checks it with stat(), and removes it. It
places no tasks in the leaf and does not register any inotify watches.
I time mkdir() and rmdir() separately; stat() is outside both timed regions.

These are consecutive source states, not different release kernels:

  base:   f917dc56060a, before both changes
  guard:  507d8ce13f5b, the nlink race protection
  notify: eea5d2bb34ba, plus the deletion notifications

For the second run with sched_ext disabled, the medians of the nine
sample means per boot were:

                         boot A       boot B
  base                    6.504        6.528 us/rmdir
  guard                   6.956        6.922 us/rmdir
  notify                  9.553        9.558 us/rmdir

The first change added 6.05-6.95%; the second added a further 37.33-38.08%.
Each run used fresh boots in base-A/guard-A/notify-A/notify-B/guard-B/base-B
order. Each point had nine samples of 128 removals, after 16 warmups.
I used an i7-12700KF, CPU0, full preemption, the performance governor and
Turbo disabled. These three kernels used the same toolchain and configuration,
apart from LOCALVERSION. I used the same control binary throughout.
Removal sample CV stayed below 3%, and endpoint drift stayed below 2%.
The result also held after dropping the first sample.
A similar slowdown was also observed with sched_ext enabled, both with
and without cgroup callbacks.

Separate, untimed traces showed 26 kernfs nodes removed per leaf. The
guard change doubled the attribute write-lock entries from 26 to 52.
The notify change added 26 inode lookups. A return-value probe found one
directory inode and 25 NULL results. These counts do not show how much
time each step costs, or that a lookup returning NULL can safely be skipped.

I understand that the notifications and race protection are needed.
A local root-reuse prototype saved about 4-5%, but did not recover most
of the increase; it is not a complete fix.

A separate v7.2-A / 2f0c1cf72f46 (7.3-rc2) / v7.2-B check gave
9.919 / 9.719 / 9.953 us/rmdir with the same control binary. Mainline was
2.02-2.36% faster, with sample CV below 0.75% and old-end drift of 0.35%.
Both versions still removed 26 nodes and made 26 inode lookups per leaf.
The later name-hash change was on the measured path, but this comparison
does not isolate its effect or repeat the exact-commit test above.

Results, configuration identities and the tested workload:
https://github.com/lcf0399/linux-regression-evidence/tree/b1eef2a0fb18babbf8e0d5832b63a655bfd28926/kernfs-empty-cgroup-removal

Do you see a safe way to reduce this per-node removal work while keeping
the notification and race fixes?

Thanks,
Chengfeng

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [REGRESSION] kernfs: empty cgroup rmdir latency after delete-notification changes
  2026-09-13 14:43 [REGRESSION] kernfs: empty cgroup rmdir latency after delete-notification changes Chengfeng Lin
@ 2026-09-13 15:03 ` Tejun Heo
  2026-09-13 15:46   ` Chengfeng Lin
  0 siblings, 1 reply; 7+ messages in thread
From: Tejun Heo @ 2026-09-13 15:03 UTC (permalink / raw)
  To: Chengfeng Lin
  Cc: T.J. Mercier, Greg Kroah-Hartman, Jan Kara, driver-core, cgroups,
	linux-fsdevel, linux-kernel, regressions

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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [REGRESSION] kernfs: empty cgroup rmdir latency after delete-notification changes
  2026-09-13 15:03 ` Tejun Heo
@ 2026-09-13 15:46   ` Chengfeng Lin
  2026-09-13 16:17     ` T.J. Mercier
  0 siblings, 1 reply; 7+ messages in thread
From: Chengfeng Lin @ 2026-09-13 15:46 UTC (permalink / raw)
  To: Tejun Heo
  Cc: T.J. Mercier, Greg Kroah-Hartman, Jan Kara, driver-core, cgroups,
	linux-fsdevel, linux-kernel, regressions

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@kernel.org> 于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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [REGRESSION] kernfs: empty cgroup rmdir latency after delete-notification changes
  2026-09-13 15:46   ` Chengfeng Lin
@ 2026-09-13 16:17     ` T.J. Mercier
  2026-09-14 18:08       ` Chengfeng Lin
  0 siblings, 1 reply; 7+ messages in thread
From: T.J. Mercier @ 2026-09-13 16:17 UTC (permalink / raw)
  To: Chengfeng Lin
  Cc: Tejun Heo, Greg Kroah-Hartman, Jan Kara, driver-core, cgroups,
	linux-fsdevel, linux-kernel, regressions

On Sun, Sep 13, 2026 at 8:47 AM Chengfeng Lin <lin2530632123@gmail.com> 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@kernel.org> 于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.

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [REGRESSION] kernfs: empty cgroup rmdir latency after delete-notification changes
  2026-09-13 16:17     ` T.J. Mercier
@ 2026-09-14 18:08       ` Chengfeng Lin
  2026-09-15 22:01         ` T.J. Mercier
  0 siblings, 1 reply; 7+ messages in thread
From: Chengfeng Lin @ 2026-09-14 18:08 UTC (permalink / raw)
  To: T.J. Mercier
  Cc: Tejun Heo, Greg Kroah-Hartman, Jan Kara, driver-core, cgroups,
	linux-fsdevel, linux-kernel, regressions

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

T.J. Mercier <tjmercier@google.com> 于2026年9月14日周一 00:17写道:
>
> On Sun, Sep 13, 2026 at 8:47 AM Chengfeng Lin <lin2530632123@gmail.com> 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@kernel.org> 于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.

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [REGRESSION] kernfs: empty cgroup rmdir latency after delete-notification changes
  2026-09-14 18:08       ` Chengfeng Lin
@ 2026-09-15 22:01         ` T.J. Mercier
  2026-09-17 10:06           ` Chengfeng Lin
  0 siblings, 1 reply; 7+ messages in thread
From: T.J. Mercier @ 2026-09-15 22:01 UTC (permalink / raw)
  To: Chengfeng Lin
  Cc: Tejun Heo, Greg Kroah-Hartman, Jan Kara, driver-core, cgroups,
	linux-fsdevel, linux-kernel, regressions

On Mon, Sep 14, 2026 at 11:08 AM Chengfeng Lin <lin2530632123@gmail.com> 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@google.com> 于2026年9月14日周一 00:17写道:
> >
> > On Sun, Sep 13, 2026 at 8:47 AM Chengfeng Lin <lin2530632123@gmail.com> 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@kernel.org> 于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.

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [REGRESSION] kernfs: empty cgroup rmdir latency after delete-notification changes
  2026-09-15 22:01         ` T.J. Mercier
@ 2026-09-17 10:06           ` Chengfeng Lin
  0 siblings, 0 replies; 7+ messages in thread
From: Chengfeng Lin @ 2026-09-17 10:06 UTC (permalink / raw)
  To: T.J. Mercier
  Cc: Tejun Heo, Greg Kroah-Hartman, Jan Kara, driver-core, cgroups,
	linux-fsdevel, linux-kernel, regressions

[-- Attachment #1: Type: text/plain, Size: 14217 bytes --]

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@google.com> 于2026年9月16日周三 06:01写道:

>
> On Mon, Sep 14, 2026 at 11:08 AM Chengfeng Lin <lin2530632123@gmail.com> 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@google.com> 于2026年9月14日周一 00:17写道:
> > >
> > > On Sun, Sep 13, 2026 at 8:47 AM Chengfeng Lin <lin2530632123@gmail.com> 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@kernel.org> 于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.

[-- Attachment #2: 0004-inode-requested.patch --]
[-- Type: text/x-patch, Size: 1321 bytes --]

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

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2026-09-17 10:07 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-09-13 14:43 [REGRESSION] kernfs: empty cgroup rmdir latency after delete-notification changes Chengfeng Lin
2026-09-13 15:03 ` Tejun Heo
2026-09-13 15:46   ` Chengfeng Lin
2026-09-13 16:17     ` T.J. Mercier
2026-09-14 18:08       ` Chengfeng Lin
2026-09-15 22:01         ` T.J. Mercier
2026-09-17 10:06           ` Chengfeng Lin

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®