mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To: Tejun Heo <tj@kernel.org>
Cc: Shakeel Butt <shakeel.butt@linux.dev>,
	Christian Brauner <christian@brauner.io>,
	Meta kernel team <kernel-team@meta.com>,
	linux-fsdevel@vger.kernel.org, driver-core@lists.linux.dev,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH v3] kernfs: allocate inodes and dentry outside kernfs_rwsem
Date: Sat, 17 Oct 2026 11:14:13 +0200	[thread overview]
Message-ID: <2026101745-brisket-outspoken-ef6c@gregkh> (raw)
In-Reply-To: <236f5fe73f01f03d75dddca9362f39a7@kernel.org>

On Fri, Oct 16, 2026 at 12:42:29PM -1000, Tejun Heo wrote:
> On Fri, Oct 16, 2026 at 03:00:29PM -0700, Shakeel Butt wrote:
> > In the Meta fleet, on 6.16 with the kernfs changes queued for 7.4
> > backported, we still see long cgroupfs stalls:
> >
> > - oomd and below waited 72 seconds in kernfs_fop_readdir() while an out
> >   of memory container created cgroups.  They resumed when the kernel OOM
> >   killer killed the container's workload.
> >
> > - oomd waited 15 minutes in kernfs_iop_lookup().  PID 1 waited for
> >   kernfs_rwsem in cgroup_rmdir() while holding cgroup_mutex.
> >
> > - A lock profiler sampling part of the fleet saw kernfs_iop_lookup()
> >   hold kernfs_rwsem across memcg reclaim on 26 hosts in 29 days, for up
> >   to 625 ms.
> >
> > The profiler shows where these stalls come from.  kernfs_iop_lookup()
> > and the export ops allocate a new inode while holding kernfs_rwsem for
> > read.  The inode is charged to the caller's memcg.  If that memcg is at
> > memory.max, the allocation can stay in reclaim for a long time,
> > sometimes until an OOM kill.  A writer then queues on the lock and every
> > new reader queues behind it.  On cgroup2 this blocks every cgroupfs user
> > on the machine.
> >
> > Such lookups are common in a memcg that is out of memory.  Its reclaim
> > drops its kernfs dentries fast, and the files of a new cgroup have no
> > inodes yet.
> >
> > Allocate the inode before taking kernfs_rwsem, and free it if it is not
> > used, as commit 8b64f32fee95 ("kernfs: allocate the new name outside
> > kernfs_rwsem") and commit 5ad03af18434 ("kernfs: allocate the open node
> > outside the open file mutex") do.  Everything else still runs under the
> > lock, so a new inode is added only for an active node, and
> > __kernfs_remove() still finds every inode it must clear i_nlink on, as
> > commit dc1a0cc95737 ("kernfs: don't lose IN_DELETE_SELF when decoding a
> > file handle") needs.
> >
> > The inode goes unused when the name does not exist, the node is
> > inactive, or it already has an inode.  In a VM, a lookup of a missing
> > name takes about 0.35 us longer, 1.65 us instead of 1.3 us on
> > cgroupfs.  Cold lookups of existing sysfs and cgroupfs files are not
> > slower.
> >
> > kernfs_get_parent_dentry() also held the lock across d_obtain_alias(),
> > which allocates a dentry.  Hold it only around the inode lookup, as
> > __kernfs_fh_to_dentry() already does.  This also gives ->get_parent the
> > active test that the IN_DELETE_SELF fix left out: a parent that is
> > being removed and has no inode now gets -ESTALE instead of a new inode
> > with i_nlink 1.
> >
> > kernfs_fill_super() preallocates the root inode the same way, so no
> > inode is allocated under kernfs_rwsem any more.
> >
> > Tested on a 316-CPU machine.  A task in a full 256 MB memcg with slow
> > swap looks up the files of new cgroups.  Worst waits in 60 seconds:
> >
> >                                   before    after
> >   oomd, for kernfs_rwsem          236 ms   0.1 ms
> >   below, for kernfs_rwsem         356 ms   0.8 ms
> >   systemd, for kernfs_rwsem       160 ms    37 us
> >   readdir of another directory    530 ms   3.9 ms
> >
> > The lookup itself is still slow.  The VFS still holds its directory's
> > lock, so other work in that one directory still waits.
> >
> > Signed-off-by: Shakeel Butt <shakeel.butt@linux.dev>
> > Suggested-by: Tejun Heo <tj@kernel.org>
> 
> Acked-by: Tejun Heo <tj@kernel.org>

This looks sane, but it's really late in the cycle, can we wait until
after -rc1 to add it to the tree for lots of testing in -next?

thanks,

greg k-h

  reply	other threads:[~2026-10-17  9:14 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-10-16 22:00 Shakeel Butt
2026-10-16 22:42 ` Tejun Heo
2026-10-17  9:14   ` Greg Kroah-Hartman [this message]
2026-10-17 17:21     ` Shakeel Butt

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=2026101745-brisket-outspoken-ef6c@gregkh \
    --to=gregkh@linuxfoundation.org \
    --cc=christian@brauner.io \
    --cc=driver-core@lists.linux.dev \
    --cc=kernel-team@meta.com \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=shakeel.butt@linux.dev \
    --cc=tj@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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®