From: Shakeel Butt <shakeel.butt@linux.dev>
To: Tejun Heo <tj@kernel.org>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
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 v2] kernfs: allocate inodes and dentry outside kernfs_rwsem
Date: Fri, 16 Oct 2026 14:02:56 -0700 [thread overview]
Message-ID: <atKQcFFeKG76LkLQ@linux.dev> (raw)
In-Reply-To: <06b62af1faefaddcb9d5d09cbaf7a2ad@kernel.org>
On Fri, Oct 16, 2026 at 10:33:20AM -1000, Tejun Heo wrote:
> Hello, Shakeel.
>
> On Fri, Oct 16, 2026 at 09:43:45AM -0700, Shakeel Butt wrote:
> > Allocate the inode with the lock dropped. kernfs_iget() first looks for
> > an existing inode. If there is none, it pins the node, drops the lock,
> > allocates, takes the lock again, and adds the inode only if the node is
> > still active. So __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.
>
> Would it be simpler to allocate the inode before taking kernfs_rwsem and
> free it if it ends up unused, as 8b64f32fee95 ("kernfs: allocate the new
> name outside kernfs_rwsem") and 5ad03af18434 ("kernfs: allocate the open
> node outside the open file mutex") do? Then the lock doesn't have to be
> dropped, and there's no need for the pin and the active recheck. The
> unused cases (negative lookups, inactive nodes, nodes which already have
> an inode) are exceptions, and an inode alloc and free isn't expensive.
Thanks for taking a look and yes, it is much better and simpler.
>
> The helper would then just run under kernfs_rwsem. Maybe
> kernfs_get_inode_locked() would be a better name for it?
Ack
>
> kernfs_fill_super() can preallocate the same way, which gets rid of the
> last inode allocation under kernfs_rwsem.
Ack
> cgroup_may_write() allocates
> the inode under cgroup_mutex and could allocate before grabbing the
> mutex too.
Let me send this one as a separate patch.
>
> > kernfs_get_parent_dentry() also held the lock across d_obtain_alias(),
> > which allocates a dentry. Hold it only around kernfs_iget(), as
> > __kernfs_fh_to_dentry() already does.
>
> This also adds the active test to ->get_parent. A parent which is being
> removed and has no inode now gets -ESTALE instead of a new inode with
> i_nlink 1, which closes the window dc1a0cc95737 ("kernfs: don't lose
> IN_DELETE_SELF when decoding a file handle") left open. Can you mention
> it in the description?
Yes, I will do.
Thanks again for the review.
prev parent reply other threads:[~2026-10-16 21:02 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-10-16 16:43 [PATCH v2] kernfs: allocate inodes and dentry outside kernfs_rwsem Shakeel Butt
2026-10-16 20:33 ` Tejun Heo
2026-10-16 21:02 ` Shakeel Butt [this message]
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=atKQcFFeKG76LkLQ@linux.dev \
--to=shakeel.butt@linux.dev \
--cc=christian@brauner.io \
--cc=driver-core@lists.linux.dev \
--cc=gregkh@linuxfoundation.org \
--cc=kernel-team@meta.com \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.