Linux filesystem development
 help / color / mirror / Atom feed
* [PATCH] kernfs: allocate the open node outside the open file mutex
@ 2026-09-10  4:54 Shakeel Butt
  2026-09-10 21:12 ` Tejun Heo
  0 siblings, 1 reply; 2+ messages in thread
From: Shakeel Butt @ 2026-09-10  4:54 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Tejun Heo, Christian Brauner
  Cc: Sebastian Andrzej Siewior, Meta kernel team, linux-fsdevel,
	driver-core, linux-kernel

kernfs_get_open_node() allocates the kernfs_open_node while holding one
of the hashed kernfs_open_file mutexes, so opening a file nobody has open
yet can enter reclaim with that mutex held.  The mutex is shared by every
node that hashes to it, so unrelated opens, closes and xattr updates in
the same bucket wait.

It is the most contended lock in kernfs on our fleet: 2.19M waiters over
29 days, more than two hundred times the waiters on kernfs_rwsem, and the
highest median hold of any kernfs lock at 77ms.  Most of it is monitoring
daemons opening cgroup control files, and the open node is freed once the
last descriptor closes, so an open-read-close loop allocates every time.

Look at kn->attr.open before taking the mutex, and allocate then if
nothing has the file open.  Should the peek be wrong, which needs the
last descriptor to close inside the window, fall back to allocating under
the mutex as before.

Eight tasks opening and closing four cgroup files 20000 times each, with
lock_stat on the hashed mutex:

                     contentions    hold total    hold avg
   before                    198        4.19 s     8.67 us
   after                     119        2.31 s     4.78 us

and the same number of acquisitions either way.

This does not empty the bucket.  kernfs_fop_release() and
kernfs_xattr_set() still sleep under the same mutex.  It stops opens
doing it.

Assisted-by: LLM
Signed-off-by: Shakeel Butt <shakeel.butt@linux.dev>
---
 fs/kernfs/file.c | 22 ++++++++++++++++++----
 1 file changed, 18 insertions(+), 4 deletions(-)

diff --git a/fs/kernfs/file.c b/fs/kernfs/file.c
index 7e3800526b1e..cca9f83fc9b5 100644
--- a/fs/kernfs/file.c
+++ b/fs/kernfs/file.c
@@ -525,18 +525,31 @@ static int kernfs_fop_mmap(struct file *file, struct vm_area_struct *vma)
 static int kernfs_get_open_node(struct kernfs_node *kn,
 				struct kernfs_open_file *of)
 {
-	struct kernfs_open_node *on;
+	struct kernfs_open_node *on, *new_on = NULL;
 	struct mutex *mutex;
 
+	/*
+	 * Peek without the mutex: if nothing has this open, we will need a
+	 * node and can allocate before taking a mutex shared by every node
+	 * hashing to it.
+	 */
+	if (!rcu_access_pointer(kn->attr.open))
+		new_on = kzalloc_obj(*new_on);
+
 	mutex = kernfs_open_file_mutex_lock(kn);
 	on = kernfs_deref_open_node_locked(kn);
 
 	if (!on) {
 		/* not there, initialize a new one */
-		on = kzalloc_obj(*on);
+		on = new_on;
+		new_on = NULL;
 		if (!on) {
-			mutex_unlock(mutex);
-			return -ENOMEM;
+			/* the peek raced; rare, so allocate here */
+			on = kzalloc_obj(*on);
+			if (!on) {
+				mutex_unlock(mutex);
+				return -ENOMEM;
+			}
 		}
 		atomic_set(&on->event, 1);
 		init_waitqueue_head(&on->poll);
@@ -549,6 +562,7 @@ static int kernfs_get_open_node(struct kernfs_node *kn,
 		on->nr_to_release++;
 
 	mutex_unlock(mutex);
+	kfree(new_on);
 	return 0;
 }
 
-- 
2.53.0-Meta


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

* Re: [PATCH] kernfs: allocate the open node outside the open file mutex
  2026-09-10  4:54 [PATCH] kernfs: allocate the open node outside the open file mutex Shakeel Butt
@ 2026-09-10 21:12 ` Tejun Heo
  0 siblings, 0 replies; 2+ messages in thread
From: Tejun Heo @ 2026-09-10 21:12 UTC (permalink / raw)
  To: Shakeel Butt
  Cc: Greg Kroah-Hartman, Christian Brauner, Sebastian Andrzej Siewior,
	Meta kernel team, linux-fsdevel, driver-core, linux-kernel

On Wed, Sep 09, 2026 at 09:54:33PM -0700, Shakeel Butt wrote:
> kernfs_get_open_node() allocates the kernfs_open_node while holding one
> of the hashed kernfs_open_file mutexes, so opening a file nobody has open
> yet can enter reclaim with that mutex held.  The mutex is shared by every
> node that hashes to it, so unrelated opens, closes and xattr updates in
> the same bucket wait.
> 
> It is the most contended lock in kernfs on our fleet: 2.19M waiters over
> 29 days, more than two hundred times the waiters on kernfs_rwsem, and the
> highest median hold of any kernfs lock at 77ms.  Most of it is monitoring
> daemons opening cgroup control files, and the open node is freed once the
> last descriptor closes, so an open-read-close loop allocates every time.
> 
> Look at kn->attr.open before taking the mutex, and allocate then if
> nothing has the file open.  Should the peek be wrong, which needs the
> last descriptor to close inside the window, fall back to allocating under
> the mutex as before.
> 
> Eight tasks opening and closing four cgroup files 20000 times each, with
> lock_stat on the hashed mutex:
> 
>                      contentions    hold total    hold avg
>    before                    198        4.19 s     8.67 us
>    after                     119        2.31 s     4.78 us
> 
> and the same number of acquisitions either way.
> 
> This does not empty the bucket.  kernfs_fop_release() and
> kernfs_xattr_set() still sleep under the same mutex.  It stops opens
> doing it.
> 
> Assisted-by: LLM
> Signed-off-by: Shakeel Butt <shakeel.butt@linux.dev>

Acked-by: Tejun Heo <tj@kernel.org>

Thanks.

-- 
tejun

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

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

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-09-10  4:54 [PATCH] kernfs: allocate the open node outside the open file mutex Shakeel Butt
2026-09-10 21:12 ` Tejun Heo

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