mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH 0/3] kernfs: don't hold kernfs_rwsem across dir_emit()
@ 2026-09-10  0:36 Shakeel Butt
  2026-09-10  0:36 ` [PATCH 1/3] kernfs: don't repeat or skip an entry when readdir resumes Shakeel Butt
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: Shakeel Butt @ 2026-09-10  0:36 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_fop_readdir() takes kernfs_rwsem for reading and holds it for the
whole listing, dir_emit() included.  dir_emit() copies into a userspace
buffer, so it can fault, and under memory pressure that fault goes to
reclaim.

We are hitting this case on the Meta fleet very regularly.  Many times it
is below[1], a monitoring daemon.  It walks the cgroup tree and faults on
its own getdents(2) buffer with the lock held for reading.

	below: page allocation stall for 120 secs: order:0,
	mode:0x140dca(GFP_HIGHUSER_MOVABLE|__GFP_ZERO|__GFP_COMP)
	nodemask=(null),cpuset=hostcritical.slice,mems_allowed=0
	Call Trace:
	 <TASK>
	 dump_stack_lvl+0x5d/0x80
	 __alloc_frozen_pages_noprof+0x5f4d/0x6300
	 ? memcg_list_lru_alloc+0x73/0x320
	 ? ima_file_check+0xd0/0x7d0
	 vma_alloc_folio_noprof+0x145/0x560
	 handle_mm_fault+0x17c9/0x2720
	 ? find_vma+0x27/0x30
	 do_user_addr_fault+0x39f/0x6e0
	 exc_page_fault+0x8f/0x110
	 asm_exc_page_fault+0x22/0x30
	RIP: 0010:filldir64+0xd7/0x1a0
	[Code:/RSP:/RAX:..R15: register block elided]
	 kernfs_fop_readdir+0x2de/0x420
	 iterate_dir+0x8c/0x1f0
	 __se_sys_getdents64+0x61/0xe0
	 ? copy_page_from_iter+0x860/0x860
	 do_syscall_64+0x6a/0x250
	 entry_SYSCALL_64_after_hwframe+0x4b/0x53
	 </TASK>

kernfs_rwsem is the lock every create, remove and rename in the hierarchy
needs, and sysfs and cgroupfs have one per machine.  More importantly, the
userspace OOM killers such as systemd-oomd also traverse the cgroupfs
hierarchy and can get stuck behind below, which is stuck in reclaim.  They
are supposed to relieve memory pressure on the system, but can get stuck
themselves.

The fix is to not hold the lock across dir_emit().  Copy the name while the
lock is held, drop the lock, emit, and take it again.

That opens a window.  With the lock dropped, the entry the listing stopped
on can be removed before the listing picks up again.  readdir remembers its
place as a name hash, and the search for a hash that is no longer there can
land on the entry after the missing one.  That entry is then stepped over
and never reported.

So patch 1 closes that race, patch 2 is the fix, and patch 3 adds tests.

Patch 1 is worth having on its own.  Without patch 2 the same search can
land on the entry before the missing one instead, and report it a second
time, between two getdents(2) calls.

Patches 1 and 2 apply to the vfs-7.4.kernfs branch of the vfs tree.
Patch 3 applies on top of "kernfs: three standalone fixes" [2], which is
still pending on the same branch.

[1] https://github.com/facebookincubator/below
[2] https://lore.kernel.org/20260905191613.3143937-1-shakeel.butt@linux.dev/

Shakeel Butt (3):
  kernfs: don't repeat or skip an entry when readdir resumes
  kernfs: don't hold kernfs_rwsem across dir_emit()
  selftests: cover readdir resuming at a removed entry

 fs/kernfs/dir.c                               |  77 +++++--
 .../selftests/filesystems/kernfs_test.c       | 200 ++++++++++++++++++
 2 files changed, 256 insertions(+), 21 deletions(-)

-- 
2.53.0-Meta


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

* [PATCH 1/3] kernfs: don't repeat or skip an entry when readdir resumes
  2026-09-10  0:36 [PATCH 0/3] kernfs: don't hold kernfs_rwsem across dir_emit() Shakeel Butt
@ 2026-09-10  0:36 ` Shakeel Butt
  2026-09-10  0:36 ` [PATCH 2/3] kernfs: don't hold kernfs_rwsem across dir_emit() Shakeel Butt
  2026-09-10  0:36 ` [PATCH 3/3] selftests: cover readdir resuming at a removed entry Shakeel Butt
  2 siblings, 0 replies; 4+ messages in thread
From: Shakeel Butt @ 2026-09-10  0:36 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Tejun Heo, Christian Brauner
  Cc: Sebastian Andrzej Siewior, Meta kernel team, linux-fsdevel,
	driver-core, linux-kernel

readdir keeps its place as a name hash in ctx->pos and pins the entry in
file->private_data.  If that entry is gone when the listing comes back,
kernfs_dir_pos() searches the rbtree for the hash and keeps whatever
node the descent stopped on.  That is the entry before or after the
missing one, depending on the shape of the tree.

Landing before it repeats an entry the previous getdents(2) call already
reported.  Landing after it, kernfs_dir_next_pos() calls rb_next() and
steps over an unreported entry.

With children A(10), B(20), C(30):

  report A, ctx->pos = 10
                                    A removed
  kernfs_dir_next_pos(10, A)
    A is gone, the search for 10 stops at B
    rb_next(B) -> C, so B is never reported

Only the repeat happens today, between two getdents(2) calls.  The skip
needs the pinned entry to go away inside one call, which the next patch
allows when it drops kernfs_rwsem around dir_emit().

Before the commit 4e4d6d860b93 the descent kept a node only on the way
left, which is a search for the first entry at or after the hash.  That
commit moved the assignment to the top of the loop, where it runs on
right turns too.  Restore that search, and step forward only when the
pinned entry is still there rather than when the hash matches, since two
entries in one directory can share a hash.

While here, kernfs_dir_next_pos() called the hash @ino.  It is never an
inode number, so name it @hash.

Fixes: 4e4d6d860b93 ("sysfs: Add s_hash to sysfs_dirent and order directory entries by hash")
Assisted-by: LLM
Signed-off-by: Shakeel Butt <shakeel.butt@linux.dev>
---
 fs/kernfs/dir.c | 43 ++++++++++++++++++++++++++++---------------
 1 file changed, 28 insertions(+), 15 deletions(-)

diff --git a/fs/kernfs/dir.c b/fs/kernfs/dir.c
index 07abf59f0264..24a927c85123 100644
--- a/fs/kernfs/dir.c
+++ b/fs/kernfs/dir.c
@@ -1903,9 +1903,16 @@ static int kernfs_dir_fop_release(struct inode *inode, struct file *filp)
 	return 0;
 }
 
+/*
+ * Find where a listing left off.  @resumed says whether @pos is still that
+ * entry; if not, the first entry at or after @hash is returned instead.
+ */
 static struct kernfs_node *kernfs_dir_pos(const struct ns_common *ns,
-	struct kernfs_node *parent, loff_t hash, struct kernfs_node *pos)
+	struct kernfs_node *parent, loff_t hash, struct kernfs_node *pos,
+	bool *resumed)
 {
+	if (resumed)
+		*resumed = false;
 	if (pos) {
 		int valid = kernfs_active(pos) &&
 			rcu_access_pointer(pos->__parent) == parent &&
@@ -1913,23 +1920,26 @@ static struct kernfs_node *kernfs_dir_pos(const struct ns_common *ns,
 		kernfs_put(pos);
 		if (!valid)
 			pos = NULL;
+		else if (resumed)
+			*resumed = true;
 	}
 	if (!pos && (hash > 1) && (hash < INT_MAX)) {
 		struct rb_node *node = parent->dir.children.rb_node;
-		u64 ns_id = kernfs_ns_id(ns);
+
+		/*
+		 * Keep a node only on the way left, so the search ends on the
+		 * first entry at or after @hash.  The empty name sorts before
+		 * every entry sharing the hash, so it lands on the first.
+		 */
 		while (node) {
-			pos = rb_to_kn(node);
+			struct kernfs_node *kn = rb_to_kn(node);
 
-			if (hash < pos->hash)
+			if (kernfs_name_compare(hash, "", ns, kn) < 0) {
+				pos = kn;
 				node = node->rb_left;
-			else if (hash > pos->hash)
+			} else {
 				node = node->rb_right;
-			else if (ns_id < kernfs_ns_id(pos->ns))
-				node = node->rb_left;
-			else if (ns_id > kernfs_ns_id(pos->ns))
-				node = node->rb_right;
-			else
-				break;
+			}
 		}
 	}
 	/* Skip over entries which are dying/dead or in the wrong namespace */
@@ -1945,10 +1955,13 @@ static struct kernfs_node *kernfs_dir_pos(const struct ns_common *ns,
 }
 
 static struct kernfs_node *kernfs_dir_next_pos(const struct ns_common *ns,
-	struct kernfs_node *parent, ino_t ino, struct kernfs_node *pos)
+	struct kernfs_node *parent, loff_t hash, struct kernfs_node *pos)
 {
-	pos = kernfs_dir_pos(ns, parent, ino, pos);
-	if (pos) {
+	bool resumed;
+
+	pos = kernfs_dir_pos(ns, parent, hash, pos, &resumed);
+	/* Step over @pos only if it survived; two entries can share a hash. */
+	if (pos && resumed) {
 		do {
 			struct rb_node *node = rb_next(&pos->rb);
 			if (!node)
@@ -1978,7 +1991,7 @@ static int kernfs_fop_readdir(struct file *file, struct dir_context *ctx)
 	if (kernfs_ns_enabled(parent))
 		ns = kernfs_info(dentry->d_sb)->ns;
 
-	for (pos = kernfs_dir_pos(ns, parent, ctx->pos, pos);
+	for (pos = kernfs_dir_pos(ns, parent, ctx->pos, pos, NULL);
 	     pos;
 	     pos = kernfs_dir_next_pos(ns, parent, ctx->pos, pos)) {
 		const char *name = kernfs_rcu_name(pos);
-- 
2.53.0-Meta


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

* [PATCH 2/3] kernfs: don't hold kernfs_rwsem across dir_emit()
  2026-09-10  0:36 [PATCH 0/3] kernfs: don't hold kernfs_rwsem across dir_emit() Shakeel Butt
  2026-09-10  0:36 ` [PATCH 1/3] kernfs: don't repeat or skip an entry when readdir resumes Shakeel Butt
@ 2026-09-10  0:36 ` Shakeel Butt
  2026-09-10  0:36 ` [PATCH 3/3] selftests: cover readdir resuming at a removed entry Shakeel Butt
  2 siblings, 0 replies; 4+ messages in thread
From: Shakeel Butt @ 2026-09-10  0:36 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_fop_readdir() holds kernfs_rwsem for reading across the whole
listing, dir_emit() included.  dir_emit() copies to userspace, so it can
fault into reclaim while holding the lock that every create, remove and
rename in the hierarchy needs.  sysfs and cgroupfs have one per machine.

Under memory pressure the monitoring daemons fault on their own
getdents(2) buffer with it held:

        below: page allocation stall for 120 secs: order:0,
        mode:0x140dca(GFP_HIGHUSER_MOVABLE|__GFP_ZERO|__GFP_COMP)
        nodemask=(null),cpuset=hostcritical.slice,mems_allowed=0
        Call Trace:
         <TASK>
         dump_stack_lvl+0x5d/0x80
         __alloc_frozen_pages_noprof+0x5f4d/0x6300
         ? memcg_list_lru_alloc+0x73/0x320
         ? ima_file_check+0xd0/0x7d0
         vma_alloc_folio_noprof+0x145/0x560
         handle_mm_fault+0x17c9/0x2720
         ? find_vma+0x27/0x30
         do_user_addr_fault+0x39f/0x6e0
         exc_page_fault+0x8f/0x110
         asm_exc_page_fault+0x22/0x30
        RIP: 0010:filldir64+0xd7/0x1a0
        [Code:/RSP:/RAX:..R15: register block elided]
         kernfs_fop_readdir+0x2de/0x420
         iterate_dir+0x8c/0x1f0
         __se_sys_getdents64+0x61/0xe0
         ? copy_page_from_iter+0x860/0x860
         do_syscall_64+0x6a/0x250
         entry_SYSCALL_64_after_hwframe+0x4b/0x53
         </TASK>

Commit 9aab10a0249e ("kernfs: Don't re-lock kernfs_root::kernfs_rwsem in
kernfs_fop_readdir().") took the lock drop out because dir_emit() was
handed kernfs_node::name, which a rename can free.  So copy the name
under the lock and emit the copy, and pass it to the resume so that a
resume within one call keys on (hash, ns_id, name) and not on the hash
alone.

A listing is no longer atomic within one getdents(2) call, which for
most sysfs and cgroup directories is all of it.  POSIX leaves that
unspecified for an entry added or removed since opendir(3).

Fixes: 9aab10a0249e ("kernfs: Don't re-lock kernfs_root::kernfs_rwsem in kernfs_fop_readdir().")
Assisted-by: LLM
Signed-off-by: Shakeel Butt <shakeel.butt@linux.dev>
---
 fs/kernfs/dir.c | 52 +++++++++++++++++++++++++++++++++++--------------
 1 file changed, 37 insertions(+), 15 deletions(-)

diff --git a/fs/kernfs/dir.c b/fs/kernfs/dir.c
index 24a927c85123..f7cd2be67e1a 100644
--- a/fs/kernfs/dir.c
+++ b/fs/kernfs/dir.c
@@ -1905,11 +1905,11 @@ static int kernfs_dir_fop_release(struct inode *inode, struct file *filp)
 
 /*
  * Find where a listing left off.  @resumed says whether @pos is still that
- * entry; if not, the first entry at or after @hash is returned instead.
+ * entry; if not, the search falls back to @hash, keyed by @name if given.
  */
 static struct kernfs_node *kernfs_dir_pos(const struct ns_common *ns,
 	struct kernfs_node *parent, loff_t hash, struct kernfs_node *pos,
-	bool *resumed)
+	const char *name, bool *resumed)
 {
 	if (resumed)
 		*resumed = false;
@@ -1928,13 +1928,13 @@ static struct kernfs_node *kernfs_dir_pos(const struct ns_common *ns,
 
 		/*
 		 * Keep a node only on the way left, so the search ends on the
-		 * first entry at or after @hash.  The empty name sorts before
-		 * every entry sharing the hash, so it lands on the first.
+		 * first entry after the key.  An empty @name sorts before all
+		 * entries sharing the hash, so it lands on the first of them.
 		 */
 		while (node) {
 			struct kernfs_node *kn = rb_to_kn(node);
 
-			if (kernfs_name_compare(hash, "", ns, kn) < 0) {
+			if (kernfs_name_compare(hash, name ?: "", ns, kn) < 0) {
 				pos = kn;
 				node = node->rb_left;
 			} else {
@@ -1955,12 +1955,13 @@ static struct kernfs_node *kernfs_dir_pos(const struct ns_common *ns,
 }
 
 static struct kernfs_node *kernfs_dir_next_pos(const struct ns_common *ns,
-	struct kernfs_node *parent, loff_t hash, struct kernfs_node *pos)
+	struct kernfs_node *parent, loff_t hash, struct kernfs_node *pos,
+	const char *name)
 {
 	bool resumed;
 
-	pos = kernfs_dir_pos(ns, parent, hash, pos, &resumed);
-	/* Step over @pos only if it survived; two entries can share a hash. */
+	pos = kernfs_dir_pos(ns, parent, hash, pos, name, &resumed);
+	/* Step over @pos only if it survived; @name handles it if not. */
 	if (pos && resumed) {
 		do {
 			struct rb_node *node = rb_next(&pos->rb);
@@ -1979,34 +1980,55 @@ static int kernfs_fop_readdir(struct file *file, struct dir_context *ctx)
 	struct dentry *dentry = file->f_path.dentry;
 	struct kernfs_node *parent = kernfs_dentry_node(dentry);
 	struct kernfs_node *pos = file->private_data;
+	char *name __free(kfree) = NULL;
 	struct kernfs_root *root;
 	const struct ns_common *ns = NULL;
 
 	if (!dir_emit_dots(file, ctx))
 		return 0;
 
+	/*
+	 * One buffer for the call, so each name can be copied out before
+	 * dropping kernfs_rwsem.  PATH_MAX: kernfs bounds no single name.
+	 */
+	name = kmalloc(PATH_MAX, GFP_KERNEL);
+	if (!name)
+		return -ENOMEM;
+
 	root = kernfs_root(parent);
 	down_read(&root->kernfs_rwsem);
 
 	if (kernfs_ns_enabled(parent))
 		ns = kernfs_info(dentry->d_sb)->ns;
 
-	for (pos = kernfs_dir_pos(ns, parent, ctx->pos, pos, NULL);
+	for (pos = kernfs_dir_pos(ns, parent, ctx->pos, pos, NULL, NULL);
 	     pos;
-	     pos = kernfs_dir_next_pos(ns, parent, ctx->pos, pos)) {
-		const char *name = kernfs_rcu_name(pos);
+	     pos = kernfs_dir_next_pos(ns, parent, ctx->pos, pos, name)) {
 		unsigned int type = fs_umode_to_dtype(pos->mode);
-		int len = strlen(name);
 		ino_t ino = kernfs_ino(pos);
+		int len;
+
+		len = strscpy(name, kernfs_rcu_name(pos), PATH_MAX);
 
 		ctx->pos = pos->hash;
 		file->private_data = pos;
 		kernfs_get(pos);
 
-		if (!dir_emit(ctx, name, len, ino, type)) {
-			up_read(&root->kernfs_rwsem);
+		/*
+		 * getname() caps a path, so only an in-kernel caller can get
+		 * here.  Skip the entry rather than report a truncated name.
+		 */
+		if (WARN_ON_ONCE(len < 0))
+			continue;
+
+		/*
+		 * dir_emit() can fault, so run it unlocked.  @pos is pinned
+		 * above and kernfs_dir_pos() rechecks it on the way back.
+		 */
+		up_read(&root->kernfs_rwsem);
+		if (!dir_emit(ctx, name, len, ino, type))
 			return 0;
-		}
+		down_read(&root->kernfs_rwsem);
 	}
 	up_read(&root->kernfs_rwsem);
 	file->private_data = NULL;
-- 
2.53.0-Meta


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

* [PATCH 3/3] selftests: cover readdir resuming at a removed entry
  2026-09-10  0:36 [PATCH 0/3] kernfs: don't hold kernfs_rwsem across dir_emit() Shakeel Butt
  2026-09-10  0:36 ` [PATCH 1/3] kernfs: don't repeat or skip an entry when readdir resumes Shakeel Butt
  2026-09-10  0:36 ` [PATCH 2/3] kernfs: don't hold kernfs_rwsem across dir_emit() Shakeel Butt
@ 2026-09-10  0:36 ` Shakeel Butt
  2 siblings, 0 replies; 4+ messages in thread
From: Shakeel Butt @ 2026-09-10  0:36 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Tejun Heo, Christian Brauner
  Cc: Sebastian Andrzej Siewior, Meta kernel team, linux-fsdevel,
	driver-core, linux-kernel

The kernfs tests list only directories that do not change, so nothing
covers the entry a listing stopped on being gone when it comes back.

readdir_resume_at_removed_entry takes a telldir(3) cookie for every
entry, then removes each entry in turn, seeks to its cookie and reads
the rest; nothing reported before it may come back.  This is a resume
between two getdents(2) calls, and it fails without "kernfs: don't
repeat or skip an entry when readdir resumes".

readdir_resume_vs_internal_remove is a resume inside one call, which
"kernfs: don't hold kernfs_rwsem across dir_emit()" opens.  It churns
cgroup.subtree_control rather than calling rmdir(2), which cannot reach
that window because iterate_dir() holds the listed directory's i_rwsem
for the whole listing.  It is a stress test, has not been seen to catch
the ordering bug, and keeps the window busy for lockdep and KASAN.  It
fails if the churn died and skips if the listings never overlapped it,
so it cannot pass having listed a static directory.

Assisted-by: LLM
Signed-off-by: Shakeel Butt <shakeel.butt@linux.dev>
---
 .../selftests/filesystems/kernfs_test.c       | 200 ++++++++++++++++++
 1 file changed, 200 insertions(+)

diff --git a/tools/testing/selftests/filesystems/kernfs_test.c b/tools/testing/selftests/filesystems/kernfs_test.c
index 6e74da91ebca..01cd58e5e41e 100644
--- a/tools/testing/selftests/filesystems/kernfs_test.c
+++ b/tools/testing/selftests/filesystems/kernfs_test.c
@@ -8,6 +8,7 @@
 #include <limits.h>
 #include <net/if.h>
 #include <sched.h>
+#include <signal.h>
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
@@ -17,6 +18,7 @@
 #include <sys/mount.h>
 #include <sys/socket.h>
 #include <sys/stat.h>
+#include <sys/syscall.h>
 #include <sys/xattr.h>
 
 #include "kselftest_harness.h"
@@ -472,6 +474,204 @@ TEST_F(kernfs_cgroup, readdir_no_duplicates)
 			EXPECT_STRNE(names[i], names[j]);
 }
 
+#define RESUME_DIRS	24
+
+/*
+ * Resuming at an entry that has gone must carry on after it, never before.
+ * Take a cookie for every entry, then remove each one, seek to its cookie
+ * and read the rest; nothing already reported may come back.
+ */
+TEST_F(kernfs_cgroup, readdir_resume_at_removed_entry)
+{
+	/* The cgroup's own control files are listed alongside ours. */
+	char names[128][NAME_MAX + 1];
+	long pos[128];
+	char path[PATH_MAX];
+	struct dirent *de;
+	int n = 0, i, j;
+	DIR *d;
+
+	for (i = 0; i < RESUME_DIRS; i++) {
+		snprintf(path, sizeof(path), "%s/e%02d", self->scratch, i);
+		ASSERT_EQ(mkdir(path, 0755), 0);
+	}
+
+	/* Record the cookie before reading each entry, with its name. */
+	d = opendir(self->scratch);
+	ASSERT_NE(d, NULL);
+	while (1) {
+		long here = telldir(d);
+
+		de = readdir(d);
+		if (!de)
+			break;
+		if (!strcmp(de->d_name, ".") || !strcmp(de->d_name, ".."))
+			continue;
+		ASSERT_LT(n, (int)ARRAY_SIZE(pos));
+		pos[n] = here;
+		strncpy(names[n], de->d_name, NAME_MAX);
+		names[n][NAME_MAX] = '\0';
+		n++;
+	}
+	closedir(d);
+	ASSERT_GT(n, 1);
+
+	for (i = 0; i < n; i++) {
+		/* Only the directories we made can be removed and put back. */
+		if (strncmp(names[i], "e", 1))
+			continue;
+
+		snprintf(path, sizeof(path), "%s/%s", self->scratch, names[i]);
+		ASSERT_EQ(rmdir(path), 0);
+
+		/* Reopen so the seek has to reach the kernel. */
+		d = opendir(self->scratch);
+		ASSERT_NE(d, NULL);
+		seekdir(d, pos[i]);
+		while ((de = readdir(d))) {
+			if (!strcmp(de->d_name, ".") || !strcmp(de->d_name, ".."))
+				continue;
+			for (j = 0; j < i; j++)
+				ASSERT_STRNE(de->d_name, names[j])
+					TH_LOG("resuming at %s (gone) went back to %s",
+					       names[i], names[j]);
+		}
+		closedir(d);
+
+		ASSERT_EQ(mkdir(path, 0755), 0);
+	}
+
+	for (i = 0; i < RESUME_DIRS; i++) {
+		snprintf(path, sizeof(path), "%s/e%02d", self->scratch, i);
+		EXPECT_EQ(rmdir(path), 0);
+	}
+}
+
+#define CHURN_ROUNDS	400
+#define CHURN_BUFSZ	512	/* small, so a listing takes several calls */
+
+/*
+ * The files appear at the end of the enabling write and go at the start of
+ * the disabling one, so the window where they exist is the short one.
+ */
+#define CHURN_DWELL_ON	2000
+#define CHURN_DWELL_OFF	200
+
+struct kernfs_dirent64 {
+	unsigned long long	d_ino;
+	long long		d_off;
+	unsigned short		d_reclen;
+	unsigned char		d_type;
+	char			d_name[];
+};
+
+/*
+ * The same resume, but inside one getdents(2) call.  rmdir(2) cannot reach
+ * that window because iterate_dir() holds the listed directory's i_rwsem
+ * for the whole listing; cgroup.subtree_control can, having no VFS
+ * operation on the names it adds and removes.  The files that are not the
+ * controller's stay throughout, so each must appear exactly once.
+ *
+ * A stress test: it has not been seen to catch the ordering bug, and is
+ * here to keep the unlocked window under load for lockdep and KASAN.
+ */
+TEST_F(kernfs_cgroup, readdir_resume_vs_internal_remove)
+{
+	char buf[CHURN_BUFSZ] __attribute__((aligned(8)));
+	char stable[128][NAME_MAX + 1];
+	int nstable = 0, i, r;
+	int withctl = 0, without = 0;
+	int seen[128], status;
+	pid_t churner;
+	DIR *d;
+
+	/* With the controller off, whatever is left is what must persist. */
+	ASSERT_EQ(write_file(self->scratch_sc, self->disable), 0);
+	d = opendir(self->child);
+	ASSERT_NE(d, NULL);
+	for (;;) {
+		struct dirent *de = readdir(d);
+
+		if (!de)
+			break;
+		if (!strcmp(de->d_name, ".") || !strcmp(de->d_name, ".."))
+			continue;
+		ASSERT_LT(nstable, (int)ARRAY_SIZE(stable));
+		strncpy(stable[nstable], de->d_name, NAME_MAX);
+		stable[nstable][NAME_MAX] = '\0';
+		nstable++;
+	}
+	closedir(d);
+	ASSERT_GT(nstable, 0);
+
+	churner = fork();
+	ASSERT_GE(churner, 0);
+	if (churner == 0) {
+		for (;;) {
+			if (write_file(self->scratch_sc, self->enable))
+				_exit(10);
+			usleep(CHURN_DWELL_ON);
+			if (write_file(self->scratch_sc, self->disable))
+				_exit(11);
+			usleep(CHURN_DWELL_OFF);
+		}
+	}
+
+	for (r = 0; r < CHURN_ROUNDS; r++) {
+		int fd = open(self->child, O_RDONLY | O_DIRECTORY);
+		int extra = 0;
+		int n;
+
+		ASSERT_GE(fd, 0);
+		memset(seen, 0, sizeof(seen));
+
+		while ((n = syscall(SYS_getdents64, fd, buf, sizeof(buf))) > 0) {
+			int off = 0;
+
+			while (off < n) {
+				struct kernfs_dirent64 *de = (void *)(buf + off);
+				bool known = false;
+
+				off += de->d_reclen;
+				for (i = 0; i < nstable; i++)
+					if (!strcmp(de->d_name, stable[i])) {
+						seen[i]++;
+						known = true;
+					}
+				if (!known && strcmp(de->d_name, ".") &&
+				    strcmp(de->d_name, ".."))
+					extra++;
+			}
+		}
+		ASSERT_GE(n, 0);
+		EXPECT_EQ(close(fd), 0);
+
+		if (extra)
+			withctl++;
+		else
+			without++;
+
+		for (i = 0; i < nstable; i++)
+			ASSERT_EQ(seen[i], 1)
+				TH_LOG("round %d: %s seen %d times",
+				       r, stable[i], seen[i]);
+	}
+
+	/* The churn must have been running, or the listings prove nothing. */
+	EXPECT_EQ(kill(churner, SIGKILL), 0);
+	ASSERT_EQ(waitpid(churner, &status, 0), churner);
+	ASSERT_TRUE(WIFSIGNALED(status) && WTERMSIG(status) == SIGKILL)
+		TH_LOG("churner exited on its own: status %d", status);
+
+	/*
+	 * They also have to have overlapped it.  How much depends on the
+	 * machine, so say the race could not be arranged rather than fail.
+	 */
+	if (!withctl || !without)
+		SKIP(return, "listings did not span the churn: %d with, %d without",
+		     withctl, without);
+}
+
 /*
  * A telldir() cookie must resolve back to the same entry after seekdir().
  * kernfs encodes the cookie as the node's name hash, so this covers
-- 
2.53.0-Meta


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

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

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-09-10  0:36 [PATCH 0/3] kernfs: don't hold kernfs_rwsem across dir_emit() Shakeel Butt
2026-09-10  0:36 ` [PATCH 1/3] kernfs: don't repeat or skip an entry when readdir resumes Shakeel Butt
2026-09-10  0:36 ` [PATCH 2/3] kernfs: don't hold kernfs_rwsem across dir_emit() Shakeel Butt
2026-09-10  0:36 ` [PATCH 3/3] selftests: cover readdir resuming at a removed entry Shakeel Butt

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®