Linux filesystem development
 help / color / mirror / Atom feed
From: Tejun Heo <tj@kernel.org>
To: Shakeel Butt <shakeel.butt@linux.dev>
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 10:33:20 -1000	[thread overview]
Message-ID: <06b62af1faefaddcb9d5d09cbaf7a2ad@kernel.org> (raw)
In-Reply-To: <20261016164345.31618-1-shakeel.butt@linux.dev>

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.

The helper would then just run under kernfs_rwsem. Maybe
kernfs_get_inode_locked() would be a better name for it?

kernfs_fill_super() can preallocate the same way, which gets rid of the
last inode allocation under kernfs_rwsem. cgroup_may_write() allocates
the inode under cgroup_mutex and could allocate before grabbing the
mutex too.

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

Thanks.

-- 
tejun

  reply	other threads:[~2026-10-16 20:33 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 [this message]
2026-10-16 21:02   ` 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=06b62af1faefaddcb9d5d09cbaf7a2ad@kernel.org \
    --to=tj@kernel.org \
    --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=shakeel.butt@linux.dev \
    /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