Linux filesystem development
 help / color / mirror / Atom feed
* fstests: Verify behaviour of "seek" in a directory
@ 2026-08-27 23:36 NeilBrown
  2026-08-27 23:36 ` [PATCH] fstests: generic: Add test of seek in directories NeilBrown
  0 siblings, 1 reply; 14+ messages in thread
From: NeilBrown @ 2026-08-27 23:36 UTC (permalink / raw)
  To: fstests; +Cc: linux-fsdevel, Chuck Lever

Hi,

 I would like to propose the following test to be added to fstests.  It
 checks the behaviour of a directory with respect to seek.  As described
 in the patch and the code, it expects names which are not removed or
 moved to have a stable position in the result of getdents64 with
 respect to each other and with respect to any seek offset (d_off)
 reported of any entry.  This means that if we stop and any point while
 scanning a directory, remember the most recent d_off, close the
 directory, then later open and seek to that offset and keep reading,
 then we will find all the names that existed for the whole time and see
 those only once.

 Names that are added or removed are not tests as we expect them
 possibly appear not at all or even multiple times.

 My particular motivation is to confirm that the current tmpfs doesn't
 behave correctly, and to test an alternate implementation that I will
 propose which does pass this test.

 I discovered that while xfs and ext4 pass the test, btrfs doesn't.
 It appears that with btrfs the target of a rename does not keep its
 position in the directory listing order.

Thanks,
NeilBrown

 [PATCH] fstests: generic: Add test of seek in directories

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

* [PATCH] fstests: generic: Add test of seek in directories
  2026-08-27 23:36 fstests: Verify behaviour of "seek" in a directory NeilBrown
@ 2026-08-27 23:36 ` NeilBrown
  2026-08-28  4:13   ` Matthew Wilcox
                     ` (2 more replies)
  0 siblings, 3 replies; 14+ messages in thread
From: NeilBrown @ 2026-08-27 23:36 UTC (permalink / raw)
  To: fstests; +Cc: linux-fsdevel, Chuck Lever

From: NeilBrown <neil@brown.name>

Add a test for consistency of readdir (getdents64) results.
The expectation is that name which exist before a readdir starts, and
still exist when the readdir completes, and which have not been removed
or renamed away and re-created, should appear exactly once.
Also if the fd on the file is closed in the middle of a readdir and
re-opened with a seek to some offset previously reported, the same
sequence should be reported as if the file was kept open with no
seeking.  This last is important for nfsd.

Names which are the target of a rename have not been removed or renamed
away, and so they should be stable too.

The test involves creating a large number of names and designating
half as "stable".  Those which aren't "stable" may be removed,
recreated, or renamed to something else.  Names which are stable
might be the target of a rename, but will always exist.

The test then reads the directory to find the order of the stable name,
which should never change. It also find the "d_off" of all names.
These d_off should be usable as a "seek" offset to find at least all
the stable names that came after that point in the original listing.

After creating the names and finding the order, a loop which repeatly:
  - makes random changes to unstable names
  - checks the complete order of stable names in a new readdir
  - checks what appears after a seek() to a randomly chosen offet

In Linux 7.2 this test always passes for xfs and ext4 but fails
for btrfs unless we suppress renames with a "stable" name as target.
It also fails for tmpfs.

To support NFS it is possible to suppress RENAME_EXCHANGE tests.

Signed-off-by: NeilBrown <neil@brown.name>
---
 .gitignore            |   1 +
 src/Makefile          |   2 +-
 src/t_dir_seek.c      | 545 ++++++++++++++++++++++++++++++++++++++++++
 tests/generic/799     |  25 ++
 tests/generic/799.out |   3 +
 5 files changed, 575 insertions(+), 1 deletion(-)
 create mode 100644 src/t_dir_seek.c
 create mode 100755 tests/generic/799
 create mode 100644 tests/generic/799.out

diff --git a/.gitignore b/.gitignore
index 0b6b94529892..8645520cbe85 100644
--- a/.gitignore
+++ b/.gitignore
@@ -144,6 +144,7 @@ tags
 /src/t_create_short_dirs
 /src/t_dir_offset
 /src/t_dir_offset2
+/src/t_dir_seek
 /src/t_dir_type
 /src/t_encrypted_d_revalidate
 /src/t_enospc
diff --git a/src/Makefile b/src/Makefile
index 76cf50c3e169..17e7f1560033 100644
--- a/src/Makefile
+++ b/src/Makefile
@@ -36,7 +36,7 @@ LINUX_TARGETS = xfsctl bstat t_mtab getdevicesize preallo_rw_pattern_reader \
 	fscrypt-crypt-util bulkstat_null_ocount splice-test chprojid_fail \
 	detached_mounts_propagation ext4_resize t_readdir_3 splice2pipe \
 	uuid_ioctl t_snapshot_deleted_subvolume fiemap-fault min_dio_alignment \
-	rw_hint fs-monitor btrfs_ioctl
+	rw_hint fs-monitor btrfs_ioctl t_dir_seek
 
 EXTRA_EXECS = dmerror fill2attr fill2fs fill2fs_check scaleread.sh \
 	      btrfs_crc32c_forged_name.py popdir.pl popattr.py \
diff --git a/src/t_dir_seek.c b/src/t_dir_seek.c
new file mode 100644
index 000000000000..3b05c7eb900a
--- /dev/null
+++ b/src/t_dir_seek.c
@@ -0,0 +1,545 @@
+// SPDX-License-Identifier: GPL-2.0
+
+// Copyright (c) 2026 NeilBrown <neil@brown.name>
+/*
+ * This test aims to verify that seekdir (or lseek() on a directory)
+ * is reliable, including after closing and re-openning the directory.
+ * This is important for nfsd operation, among other things.
+ *
+ * The expected behaviour is that a sequence of getdents64 calls
+ * which start at a seek offset of "0" and continue until EOF
+ * is reported (getdents64 returns 0) will report all "stable" names
+ * precisely once and if that process is repeated then the order
+ * of "stable" name will stay the same.
+ * Further, if the process starts at some other seek offset which was
+ * returned in a getdents() together with a particular "stable" name,
+ * then scanning with getdents64 to the end will return all "stable" names
+ * that appear after the particular name in the known order, and none of the
+ * "stable" names which appears at or before the particular name in that order.
+ *
+ * Here a name is "stable" over a period of time if it exists at the
+ * start of the period and at the end of the period and was not unlinked
+ * or renamed during that period.  If a stable name is the target of a
+ * rename of the source of a RENAME_EXCHANGE rename, then it remains stable.
+ *
+ * The behavour of the test is to create some number (5000 by default) of names
+ * in a directory and to designate half of them as stable.
+ * Then to read the directory and establish the order and reported d_off lseek
+ * offsets.
+ * Then to repeatedly:
+ *  - randomly modify unstable names including:
+ *        + unlink existing name
+ *        + create file an non-exising name
+ *        + rename existing name to non-existing name
+ *        + RENAME_EXCHANGE two existing names
+ *        + rename existing unstable name to a stable name
+ *        + RENAME_EXCHANGE two stable names
+ *  - scan the directory from the start and validate the order of stable name
+ *  - scan the directory from some random stable name to the end and
+ *    verify the order.
+ *  This repeats 100 times by default
+ *
+ * Each name used is 12 chars long consisting of 6 random alphabetics,
+ * a hyphen, and 5 digits. which spell a sequence number of 00000 upwards.
+ */
+
+#include <unistd.h>
+#include <stdlib.h>
+#include <stdio.h>
+#include <stdint.h>
+#include <stdbool.h>
+#include <fcntl.h>
+#include <string.h>
+#include <time.h>
+#include <sys/stat.h>
+#include <sys/syscall.h>
+
+#define BUF_SIZE 4096
+struct linux_dirent64 {
+	uint64_t	d_ino;
+	uint64_t	d_off;
+	unsigned short	d_reclen;
+	unsigned char	d_type;
+	char		d_name[0];
+};
+
+typedef char fname[6+1+5+1];
+
+fname *names;
+struct flist {
+	int num;
+	int *list;
+} stable, unstable, unused;
+
+struct fstatus {
+	bool stable;
+	int index;
+	uint64_t pos;
+} *status;
+
+int num_names = 5000;
+int num_loops = 100;
+int num_ops = 10;
+bool trace = false;
+
+int homefd;
+char *dir = "testdir";
+
+enum {
+	op_unlink,
+	op_create,
+	op_rename, // rename to unused name
+	op_replace, // rename to unstable name
+	op_exchange,
+	op_replace_stable,
+	op_exchange_stable,
+	NUM_OPS
+};
+
+#define OP_STABLE ((1 << op_replace_stable) | (1 << op_exchange_stable))
+#define OP_EXCH ((1 << op_exchange) | (1 << op_exchange_stable))
+int disabled_ops = 0;
+
+static char nchars[] = "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ";
+
+static void make_name(fname name, int num)
+{
+
+	for (int i = 0; i < 6; i++)
+		name[i] = nchars[random()%(sizeof(nchars)-1)];
+	name[6] = '-';
+	sprintf(name+7, "%05d", num);
+}
+
+static int num_of(char *name)
+{
+	char *ep = NULL;
+	int num;
+
+	if (strlen(name) != 7+5)
+		return -1;
+	num = strtoul(name + 7, &ep, 10);
+	if (ep && *ep == '\0')
+		return num;
+	return -1;
+}
+
+static void make_names(void)
+{
+	names = calloc(num_names, sizeof(*names));
+	for (int i = 0; i <num_names; i++)
+		make_name(names[i], i);
+}
+
+static void alloc_flist(struct flist *list)
+{
+	list->num = 0;
+	list->list = calloc(num_names, sizeof(list->list[0]));
+}
+
+static void alloc_status(void)
+{
+	status = calloc(num_names, sizeof(*status));
+}
+
+static int get_file(struct flist *list)
+{
+	int i;
+	int ret;
+
+	if (list->num <= 0)
+		return -1;
+	i = random() % list->num;
+	ret = list->list[i];
+	list->list[i] = list->list[list->num-1];
+	list->num -= 1;
+	return ret;
+}
+
+static void put_file(struct flist *list, int f)
+{
+	list->list[list->num] = f;
+	list->num += 1;
+}
+
+static void clean_files(void)
+{
+	int n;
+
+	while ( (n = get_file(&unstable)) >= 0)
+		unlink(names[n]);
+	while ( (n = get_file(&stable)) >= 0)
+		unlink(names[n]);
+
+	fchdir(homefd);
+	if (rmdir(dir))
+		perror(dir);
+}
+
+static void fail(char *op, char *name)
+{
+	fprintf(stderr, "FATAL: %s failed on %s\n", op, name);
+	clean_files();
+	exit(3);
+}
+
+static int getdents(int fd, char *buf, int bufsize)
+{
+	return syscall(SYS_getdents64, fd, buf, bufsize);
+}
+
+static void get_order(void)
+{
+	char buf[BUF_SIZE];
+	int index = 0;
+	int fd;
+	int n;
+
+	fd = open(".", O_RDONLY | O_DIRECTORY);
+	if (fd < 0)
+		fail("open", ".");
+
+	lseek(fd, 0, SEEK_SET);
+	for (int i = 0; i < num_names; i++)
+		status[i].index = -1;
+	while ((n = getdents(fd, buf, BUF_SIZE)) > 0) {
+		int pos = 0;
+		while (pos < n) {
+			struct linux_dirent64 *d = (void*)buf + pos;
+			int i = num_of(d->d_name);
+
+			if (i >= 0) {
+				if (status[i].stable)
+					index++;
+				status[i].index = index;
+				status[i].pos = d->d_off;
+				if (trace)
+					printf("get_order %s %sstable index %d pos %lu\n",
+					       d->d_name, status[i].stable ?"":"un",
+					       status[i].index, d->d_off);
+			}
+			pos += d->d_reclen;
+		}
+	}
+	close(fd);
+}
+
+static bool check_order(int seen, uint64_t off)
+{
+	char buf[BUF_SIZE];
+	int fd = open(".", O_RDONLY | O_DIRECTORY);
+	int n;
+
+	if (fd < 0)
+		fail("open", ".");
+
+	if (trace)
+		printf("check order at offset %lu expecting index %d\n",
+		       off, seen+1);
+	lseek(fd, off, SEEK_SET);
+	while ((n = getdents(fd, buf, BUF_SIZE)) > 0) {
+		int pos = 0;
+		while (pos < n) {
+			struct linux_dirent64 *d = (void*)buf + pos;
+			int i = num_of(d->d_name);
+
+			if (i >= 0 && status[i].stable) {
+				if (status[i].index != seen + 1) {
+					fprintf(stderr, "order wrong at %s: expected index %d got %d\n",
+						d->d_name, seen+1, status[i].index);
+					close(fd);
+					return false;
+				}
+				seen += 1;
+			}
+			pos += d->d_reclen;
+		}
+	}
+	close(fd);
+	if (seen != num_names / 2) {
+		fprintf(stderr, "last stable name seen was %d, not %d\n", seen,
+			num_names/2);
+		return false;
+	}
+	return true;
+}
+
+static void create_files(void)
+{
+	for (int i = 0; i < num_names; i++) {
+		if (mknod(names[i], S_IFREG | 0600, 0) < 0)
+			fail("create", names[i]);
+
+		if (i & 1) {
+			put_file(&stable, i);
+			status[i].stable = true;
+		} else {
+			put_file(&unstable, i);
+			status[i].stable = false;
+		}
+	}
+}
+
+static void run_ops(int num)
+{
+	for (int i = 0; i < num;) {
+		int n, n2;
+		int op = random() % NUM_OPS;
+
+		if (disabled_ops & (1<<op))
+			continue;
+
+		switch (op) {
+		case op_unlink:
+			n = get_file(&unstable);
+			if (n >= 0) {
+				if (unlink(names[n]) == 0)
+					put_file(&unused, n);
+				else
+					fail("unlink", names[n]);
+				i++;
+				if (trace)
+					printf("unlink unstable %s\n", names[n]);
+			}
+			break;
+		case op_create:
+			n = get_file(&unused);
+			if (n > 0) {
+				if (mknod(names[n], S_IFREG | 0600, 0) == 0)
+					put_file(&unstable, n);
+				else
+					fail("create", names[n]);
+				i++;
+				if (trace)
+					printf("create unstable %s\n", names[n]);
+			}
+			break;
+		case op_rename:
+			n = get_file(&unstable);
+			if (n < 0)
+				break;
+			n2 = get_file(&unused);
+			if (n2 < 0) {
+				put_file(&unstable, n);
+				break;
+			}
+			if (rename(names[n], names[n2]) == 0) {
+				put_file(&unused, n);
+				put_file(&unstable, n2);
+				i++;
+			} else
+				fail("rename-to", names[n2]);
+			if (trace)
+				printf("rename unstable %s to unused %s\n",
+				       names[n], names[n2]);
+			break;
+		case op_replace:
+			n = get_file(&unstable);
+			if (n < 0)
+				break;
+			n2 = get_file(&unstable);
+			if (n2 < 0) {
+				put_file(&unstable, n);
+				break;
+			}
+			if (rename(names[n], names[n2]) == 0) {
+				put_file(&unused, n);
+				put_file(&unstable, n2);
+				i++;
+			} else
+				fail("replace", names[n2]);
+			if (trace)
+				printf("rename unstable %s to unstable %s\n",
+				       names[n], names[n2]);
+			break;
+		case op_exchange:
+			n = get_file(&unstable);
+			if (n < 0)
+				break;
+			n2 = get_file(&unstable);
+			if (n2 < 0) {
+				put_file(&unstable, n);
+				break;
+			}
+			if (renameat2(AT_FDCWD, names[n],
+				      AT_FDCWD, names[n2],
+				      RENAME_EXCHANGE) == 0) {
+				put_file(&unstable, n);
+				put_file(&unstable, n2);
+				i++;
+			} else
+				fail("exchange", names[n]);
+			if (trace)
+				printf("exchange unstable %s with unstable %s\n",
+				       names[n], names[n2]);
+			break;
+		case op_replace_stable:
+			n = get_file(&unstable);
+			if (n < 0)
+				break;
+			n2 = get_file(&stable);
+			if (n2 < 0) {
+				put_file(&unstable, n);
+				break;
+			}
+			if (rename(names[n], names[n2]) == 0) {
+				put_file(&unused, n);
+				put_file(&stable, n2);
+				i++;
+			} else
+				fail("replace", names[n2]);
+			if (trace)
+				printf("rename unstable %s to stable %s\n",
+				       names[n], names[n2]);
+			break;
+		case op_exchange_stable:
+			n = get_file(&stable);
+			if (n < 0)
+				break;
+			n2 = get_file(&stable);
+			if (n2 < 0) {
+				put_file(&stable, n);
+				break;
+			}
+			if (renameat2(AT_FDCWD, names[n],
+				      AT_FDCWD, names[n2],
+				      RENAME_EXCHANGE) == 0) {
+				put_file(&stable, n);
+				put_file(&stable, n2);
+				i++;
+			} else
+				fail("exchange", names[n2]);
+			if (trace)
+				printf("exchange stable %s with stable %s\n",
+				       names[n], names[n2]);
+			break;
+		}
+	}
+}
+
+static void usage(void)
+{
+	fprintf(stderr, "Usage: t_dir_seek -n numfiles -l numloops -o numops\n");
+	fprintf(stderr, "                  -p working-path -d temp-dir -S randomseed\n");
+	fprintf(stderr, "                  -T -O ops\n");
+	fprintf(stderr, " -T enables tracing\n");
+	fprintf(stderr, " -O disables ops. Currently:\n");
+	fprintf(stderr, "           \"nostable\" - disables ops affecting stable names\n");
+	fprintf(stderr, "           \"noexchange\" - disables RENAME_EXCHANGE\n");
+	fprintf(stderr, " random seed default to seconds since epoch\n");
+	fprintf(stderr, " temp-dir default to \"testdir\" and is removed on completion\n");
+	fprintf(stderr, " other defaults: -n 5000 -l 100 -p 10\n");
+}
+
+int main(int argc, char *argv[])
+{
+	int opt;
+	int loop;
+	unsigned int seed = time(NULL);
+
+	while ( (opt = getopt(argc, argv, "n:l:o:p:d:S:TO:")) > 0) {
+		switch (opt) {
+		case 'n': /* number of names */
+			num_names = atoi(optarg);
+			if (num_names < 10 || num_names > 100000) {
+				fprintf(stderr, "-N: number of names must be 10..100000\n");
+				exit(2);
+			}
+			break;
+		case 'l': /* number of loops */
+			num_loops = atoi(optarg);
+			if (num_loops < 1 || num_loops > 10000) {
+				fprintf(stderr, "-l: number of loops must be 1..10000\n");
+				exit(2);
+			}
+			break;
+		case 'o': /* number of ops per loop */
+			num_ops = atoi(optarg);
+			if (num_ops < 1 || num_ops > 100) {
+				fprintf(stderr, "-l: number of ops must be 1..100\n");
+				exit(2);
+			}
+			break;
+		case 'p': /* name of dir to work in */
+			if (chdir(optarg) != 0) {
+				perror(optarg);
+				exit(2);
+			}
+			break;
+		case 'd': /* Directory to create and use */
+			if (strlen(optarg) == 0 ||
+			    strchr(optarg, '/') != NULL ||
+			    optarg[0] == '.') {
+				fprintf(stderr, "illegal directory name \"%s\"\n",
+					optarg);
+				exit(2);
+			}
+			dir = optarg;
+			break;
+		case 'S':
+			seed = atoi(optarg);
+			break;
+		case 'T':
+			trace = true;
+			break;
+		case 'O':
+			if (strcmp(optarg, "nostable") == 0)
+				disabled_ops |= OP_STABLE;
+			else if (strcmp(optarg, "noexchange") == 0)
+				disabled_ops |= OP_EXCH;
+			else {
+				fprintf(stderr, "-O %s - not recognised\n", optarg);
+				exit(2);
+			}
+			break;
+		case '?':
+		default:
+			usage();
+			exit(2);
+		}
+	}
+
+	if (optind < argc) {
+		fprintf(stderr, "Unrecognised argument: %s\n", argv[optind]);
+		usage();
+		exit(2);
+	}
+
+	homefd = open(".", O_RDONLY | O_DIRECTORY);
+	if (homefd < 0)
+		fail("open", ".");
+	if (mkdir(dir, 0755)) {
+		perror(dir);
+		exit(1);
+	}
+	chdir(dir);
+	srandom(seed);
+	make_names();
+	alloc_flist(&stable);
+	alloc_flist(&unstable);
+	alloc_flist(&unused);
+	alloc_status();
+
+	create_files();
+	get_order();
+	for (loop = 0; loop < num_loops; loop++) {
+		int n;
+
+		run_ops(num_ops);
+		if (!check_order(0, 0))
+			break;
+
+		n = random() % num_names;
+		if (!check_order(status[n].index, status[n].pos))
+			break;
+	}
+	clean_files();
+	if (loop == num_loops) {
+		printf("Passed all %d loops with seed %u\n", loop, seed);
+		exit(0);
+	} else {
+		printf("Failed on loop %d of %d with seed %u\n", loop,
+		       num_loops, seed);
+		exit(1);
+	}
+}
diff --git a/tests/generic/799 b/tests/generic/799
new file mode 100755
index 000000000000..07279394ef43
--- /dev/null
+++ b/tests/generic/799
@@ -0,0 +1,25 @@
+#! /bin/bash
+# SPDX-License-Identifier: GPL-2.0
+# Copyright (c) 2025 NeilBrown
+#
+# Check dir list and offsets are stable in the fast of churn.
+# Name that are not unlinks or renamed should rename in-order w.r.t
+# each other and any seek positions reported.
+
+. ./common/preamble
+_begin_fstest auto dir quick
+
+_require_test
+_require_test_program "t_dir_seek"
+
+extra=""
+if [ "$FSTYP" == "nfs" ]; then
+    extra="-O noexchange"
+fi
+
+# provide a seed as it is reported in the output
+$here/src/t_dir_seek -p $TEST_DIR -S 1234567 $extra
+
+# success, all done
+echo "*** done"
+status=0
diff --git a/tests/generic/799.out b/tests/generic/799.out
new file mode 100644
index 000000000000..3acd51a1847b
--- /dev/null
+++ b/tests/generic/799.out
@@ -0,0 +1,3 @@
+QA output created by 799
+Passed all 100 loops with seed 1234567
+*** done

base-commit: 56c410ad0f69da5b13c5807bc47b4876dcfa02b2
-- 
2.50.0.107.gf914562f5916.dirty


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

* Re: [PATCH] fstests: generic: Add test of seek in directories
  2026-08-27 23:36 ` [PATCH] fstests: generic: Add test of seek in directories NeilBrown
@ 2026-08-28  4:13   ` Matthew Wilcox
  2026-08-28 11:12     ` NeilBrown
  2026-08-30 23:13   ` Chuck Lever
  2026-08-31 12:59   ` Christoph Hellwig
  2 siblings, 1 reply; 14+ messages in thread
From: Matthew Wilcox @ 2026-08-28  4:13 UTC (permalink / raw)
  To: NeilBrown; +Cc: fstests, linux-fsdevel, Chuck Lever

On Fri, Aug 28, 2026 at 09:36:10AM +1000, NeilBrown wrote:
> Add a test for consistency of readdir (getdents64) results.
> The expectation is that name which exist before a readdir starts, and
> still exist when the readdir completes, and which have not been removed
> or renamed away and re-created, should appear exactly once.
> Also if the fd on the file is closed in the middle of a readdir and
> re-opened with a seek to some offset previously reported, the same
> sequence should be reported as if the file was kept open with no
> seeking.  This last is important for nfsd.

I'm not sure it's required by POSIX:

	The behavior is unspecified if lseek() is used to set the file
	offset to a value other than zero or a value returned by a
	previous call to lseek() on the same open file description."

https://pubs.opengroup.org/onlinepubs/9799919799/functions/posix_getdents.html

We can of course require stronger semantics from Linux filesystems
than POSIX requires,  We should discuss the benefits & costs of doing so
(and I have no idea what the costs to btrfs or tmpfs might be).

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

* Re: [PATCH] fstests: generic: Add test of seek in directories
  2026-08-28  4:13   ` Matthew Wilcox
@ 2026-08-28 11:12     ` NeilBrown
  2026-08-28 14:55       ` Theodore Tso
  2026-08-31 13:16       ` Christoph Hellwig
  0 siblings, 2 replies; 14+ messages in thread
From: NeilBrown @ 2026-08-28 11:12 UTC (permalink / raw)
  To: Matthew Wilcox; +Cc: fstests, linux-fsdevel, Chuck Lever

On Fri, 28 Aug 2026, Matthew Wilcox wrote:
> On Fri, Aug 28, 2026 at 09:36:10AM +1000, NeilBrown wrote:
> > Add a test for consistency of readdir (getdents64) results.
> > The expectation is that name which exist before a readdir starts, and
> > still exist when the readdir completes, and which have not been removed
> > or renamed away and re-created, should appear exactly once.
> > Also if the fd on the file is closed in the middle of a readdir and
> > re-opened with a seek to some offset previously reported, the same
> > sequence should be reported as if the file was kept open with no
> > seeking.  This last is important for nfsd.
> 
> I'm not sure it's required by POSIX:
> 
> 	The behavior is unspecified if lseek() is used to set the file
> 	offset to a value other than zero or a value returned by a
> 	previous call to lseek() on the same open file description."
> 
> https://pubs.opengroup.org/onlinepubs/9799919799/functions/posix_getdents.html

If you happened to know the sizes of the all the names in the directory,
you could make a sequence of getdents calls which each return precisely
1 entry,  You could then use lseek to determine the seek offset at every
point.
Linux getdents64 makes this a bit simpler by returning exactly that same
number (the seek offset to the next name) in the d_off field.

So posix certainly allows, indirectly, seeking to each d_off.

Posix refers to one directory entry "immediately following" another
which clearly implies a well defined sequence (if you exclude names
added and removed during the read).

Also glibc currently expects exactly this behaviour for implementing
telldir/seekdir.

> 
> We can of course require stronger semantics from Linux filesystems
> than POSIX requires,  We should discuss the benefits & costs of doing so
> (and I have no idea what the costs to btrfs or tmpfs might be).
> 

I think a strong argument for Linux needing something is that NFS needs
working directory offsets to be able to support READDIR, because there
is no "OPEN" request for directories.

I agree it would be good to discuss this and hear other perspectives,
but I cannot see a lot of flexibility if we want a system that works
consistently.

Fixing tmpfs is easy.  I have a patch which makes it work much better
with less code.  Chuck quite reasonably wanted some objective measure of
"better" and suggested I write some test code for fstests.  Hence this
patch.

I know less about btrfs.  It appears to always add new names to the end
of the directory listing.  When you rename over an existing name, that
existing name is relocated to the end.  I can only guess why it might do
that.  I have no idea if it "needs" to do that.

Fun fact: This behaviour of btrfs (which I think is perfectly defensible
for new names) resulted in generic/736 (which I think is an unreasonable
test to impose) which btrfs "fixed" with a mechanism that doesn't work
over NFS.  i.e.  it doesn't work if you close and re-open the file for
each getdents call.
If I mount a btrfs filesystem over nfs with rsize=4096, then generic/736
fails.


The link you provided says:
  If a sequence of calls to posix_getdents() is made that reads from
  offset zero to end-of-file and a file is removed from or added to the
  directory between the first and last of those calls, whether the
  sequence of calls returns an entry for that file is unspecified. 

which unfortunately doesn't cover what happens when a file is renamed.
When renamed to a non-existing name, it might be reasonable to describe
this as "removed from" and "added to" so its appearance could be
unspecified.
When renamed to an existing name, I don't think it reasonable to
describe the target name a being either "removed from" or "added to" the
directory.

Thanks for taking an interest,
NeilBrown

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

* Re: [PATCH] fstests: generic: Add test of seek in directories
  2026-08-28 11:12     ` NeilBrown
@ 2026-08-28 14:55       ` Theodore Tso
  2026-08-30 22:22         ` NeilBrown
  2026-08-31 13:16       ` Christoph Hellwig
  1 sibling, 1 reply; 14+ messages in thread
From: Theodore Tso @ 2026-08-28 14:55 UTC (permalink / raw)
  To: NeilBrown; +Cc: Matthew Wilcox, fstests, linux-fsdevel, Chuck Lever

The key requirements come from readdir() and seekdir().  To quote from
readdir() specification:

   If a file is removed from or added to the directory after the most
   recent call to opendir() or rewinddir(), whether a subsequent call
   to readdir() returns an entry for that file is unspecified.

And then from seekdir():

   The seekdir() function shall set the position of the next readdir()
   operation on the directory stream specified by dirp to the position
   specified by loc. The value of loc should have been returned from
   an earlier call to telldir(). The new position reverts to the one
   associated with the directory stream when telldir() was performed.

   If the value of loc was not obtained from an earlier call to
   telldir(), or if a call to rewinddir() occurred between the call to
   telldir() and the call to seekdir(), the results of subsequent
   calls to readdir() are unspecified.

Telldir() is specified to return a "cookie".  It does not have to be a
offset, and the DIR * returned by opendir() and consumed by
readdir() does not have to be backed by a file descriptor.

Historically, jfs implemented an whole an extra on-disk btree in order
to implement the insane POSIX seekdir()/telldir() semantics.  (Meaning
every single directory insert or deletion required at least one extra
disk seek.)  Ext4 returns directory entries in a very sub-optimal hash
tree order, requiring lots of extra disk seeks, in order to provide
this guarantee.  This is also something that historically has been
enforced by Posix Compliance Test Suites (PCTS) for those people who
believed that Posix compliance was relevant.

And, yes we also go through all of this pain and performance hit not
just because of Posix compliance, but also because NFS requires it.
Personally, supporting NFS is *way* more important than Posix
compliance, but people of good will can have different priorities.  :-)

For this reason, I would support having a test to provide the
gaurantee needed for Posix and NFS compatibility.  Not for Posix's
sake, but for NFS's.

Cheers,

					- Ted

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

* Re: [PATCH] fstests: generic: Add test of seek in directories
  2026-08-28 14:55       ` Theodore Tso
@ 2026-08-30 22:22         ` NeilBrown
  2026-08-31  1:37           ` Theodore Tso
  0 siblings, 1 reply; 14+ messages in thread
From: NeilBrown @ 2026-08-30 22:22 UTC (permalink / raw)
  To: Theodore Tso; +Cc: Matthew Wilcox, fstests, linux-fsdevel, Chuck Lever

On Sat, 29 Aug 2026, Theodore Tso wrote:
> The key requirements come from readdir() and seekdir().  To quote from
> readdir() specification:
> 
>    If a file is removed from or added to the directory after the most
>    recent call to opendir() or rewinddir(), whether a subsequent call
>    to readdir() returns an entry for that file is unspecified.
> 
> And then from seekdir():
> 
>    The seekdir() function shall set the position of the next readdir()
>    operation on the directory stream specified by dirp to the position
>    specified by loc. The value of loc should have been returned from
>    an earlier call to telldir(). The new position reverts to the one
>    associated with the directory stream when telldir() was performed.
> 
>    If the value of loc was not obtained from an earlier call to
>    telldir(), or if a call to rewinddir() occurred between the call to
>    telldir() and the call to seekdir(), the results of subsequent
>    calls to readdir() are unspecified.
> 
> Telldir() is specified to return a "cookie".  It does not have to be a
> offset, and the DIR * returned by opendir() and consumed by
> readdir() does not have to be backed by a file descriptor.
> 
> Historically, jfs implemented an whole an extra on-disk btree in order
> to implement the insane POSIX seekdir()/telldir() semantics.  (Meaning
> every single directory insert or deletion required at least one extra
> disk seek.)  Ext4 returns directory entries in a very sub-optimal hash
> tree order, requiring lots of extra disk seeks, in order to provide
> this guarantee.  This is also something that historically has been
> enforced by Posix Compliance Test Suites (PCTS) for those people who
> believed that Posix compliance was relevant.
> 
> And, yes we also go through all of this pain and performance hit not
> just because of Posix compliance, but also because NFS requires it.
> Personally, supporting NFS is *way* more important than Posix
> compliance, but people of good will can have different priorities.  :-)

Given that the requirements predates the htree design, isn't this "pain
and performance hit" a deliberate design choice?  Or maybe an
unfortunate design omission?  Such things can be fixed.

Presumably as well as the "extra disk seeks" the current design prevents
read-ahead from being effective.

ext4 already has two indexing schemes - indirect and extent-based.  How
hard would it be to add a third that effectively moved the current
directory index blocks out of the "file" and into the "file-index"?
Obviously the 32 block address in dx_entry would need to become a 48 bit
device address etc, and there are bound to be details I've glossed over,
but this approach would amortise all metadata access of a lot data
access, and would allow read-ahead.

So if there is really a problem, I'm sure it can be fixed.

> 
> For this reason, I would support having a test to provide the
> gaurantee needed for Posix and NFS compatibility.  Not for Posix's
> sake, but for NFS's.

Thanks for the support.

NeilBrown

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

* Re: [PATCH] fstests: generic: Add test of seek in directories
  2026-08-27 23:36 ` [PATCH] fstests: generic: Add test of seek in directories NeilBrown
  2026-08-28  4:13   ` Matthew Wilcox
@ 2026-08-30 23:13   ` Chuck Lever
  2026-08-31 12:59   ` Christoph Hellwig
  2 siblings, 0 replies; 14+ messages in thread
From: Chuck Lever @ 2026-08-30 23:13 UTC (permalink / raw)
  To: NeilBrown, fstests; +Cc: linux-fsdevel


On Thu, Aug 27, 2026, at 7:36 PM, NeilBrown wrote:
> Add a test for consistency of readdir (getdents64) results.

Thanks for writing this!

Note that it asserts more than exactly-once: the relative order of
every stable name must hold for the life of the directory, across
independent opens. That is the right property needed for NFS. Either
the internal documentation or the commit message should say that,
and explain why: between two READDIRs there is no open state, so the
server can be handed any cookie against any later state of the
directory. POSIX does not require this, certainly, but NFS does. 

The seek check picks n from all names, including unstable ones
unlinked many loops earlier, using the d_off from the first scan.
That is the "rm -rf over NFS" case and the most valuable check here,
but it reads like an oversight. Can you add a comment saying it is
deliberate?

The seven op classes run mixed in one invocation, so a failure says
"loop 37 with seed 1234567" and not which *semantic* broke. btrfs
fails only on renames onto a stable name, and nobody can tell that
from the output without rerunning with -T. It might be nicer to have
the wrapper invoke the binary once per class, or at least once each
for churn, rename onto an existing name, and RENAME_EXCHANGE. Then
put exchange in its own test gated by _require_renameat2 exchange,
which replaces the FSTYP == nfs check and _notruns everywhere the
flag is unsupported.

> +_begin_fstest auto dir quick

Add rename, perhaps?

> +$here/src/t_dir_seek -p $TEST_DIR -S 1234567 $extra

$TEST_DIR/testdir is not scoped to $seq and there is no _cleanup().
A run interrupted before clean_files() makes the next one fail in
mkdir with exit status 1. Use $TEST_DIR/$seq-dir and rm -rf it before
the run and in _cleanup().

> +		case op_create:
> +			n = get_file(&unused);
> +			if (n > 0) {

n >= 0. Name 0 is a valid index. Drawn here it is removed from unused
and never put back anywhere.

> +	lseek(fd, off, SEEK_SET);

For both get_order() and check_order(), a rejected cookie should fail
with the errno, not proceed from 0 and report bad order.

Nits: copyright 2025 vs 2026; typos in both header comments; %lu for
uint64_t; usage says "-p 10" for -o; the -n and -o range errors name
the wrong flags; d_name[] rather than d_name[0].

With the wrapper and op_create fixes:

Reviewed-by: Chuck Lever <cel@kernel.org>

I will also note that, despite the new failures, users of NFS-
exported tmpfs and btrfs filesystems have not observed or reported
problems. So the severity of these failures is not high, IMO, but it
would still be good to correct them. It's great to see more of the
specific NFS requirements for directories materialized in a set of
unit tests.


-- 
Chuck Lever (Come to NFS bake-a-thon! https://nfsv4bat.org)

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

* Re: [PATCH] fstests: generic: Add test of seek in directories
  2026-08-30 22:22         ` NeilBrown
@ 2026-08-31  1:37           ` Theodore Tso
  0 siblings, 0 replies; 14+ messages in thread
From: Theodore Tso @ 2026-08-31  1:37 UTC (permalink / raw)
  To: NeilBrown; +Cc: Matthew Wilcox, fstests, linux-fsdevel, Chuck Lever

On Mon, Aug 31, 2026 at 08:22:01AM -0500, NeilBrown wrote:
> Given that the requirements predates the htree design, isn't this "pain
> and performance hit" a deliberate design choice?  Or maybe an
> unfortunate design omission?  Such things can be fixed.

The requirements assumed that the directory was a linear structure,
much like the V7 and BSD FFS structure.  That's why
telldir()/seekdir() interface used a 32-biit cookie --- e.g., a seek
offset.  This doesn't deal well with a b-tree or equivalent structure
where when you insert an item into the tree, the tree might require a
node to be split or the tree to be rebalanced.

> ext4 already has two indexing schemes - indirect and extent-based.  How
> hard would it be to add a third that effectively moved the current
> directory index blocks out of the "file" and into the "file-index"?
> Obviously the 32 block address in dx_entry would need to become a 48 bit
> device address etc, and there are bound to be details I've glossed over,
> but this approach would amortise all metadata access of a lot data
> access, and would allow read-ahead.

The primary detail that you've glossed over is that the tree would
have to be a B-tree.  Ext4 currently uses a b+tree which means that we
don't need to store the file name (key) in the index nodes.  The index
node in the htree is a fixed size, and compact, and this allows for a
much greater fanout (340 nodes in each node).  This makes the tree
much shallower, and reduces the number of seeks when doing lookups.

Switching to a b-tree would almost double the size of the overhead of
the tree, and the tree would be deeper, and thus require more seeks
when doing a lookup.

So it's a tradeoff, and it depends on whether you think readdir or
directory lookups are more frequent.

Cheers,

						- Ted

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

* Re: [PATCH] fstests: generic: Add test of seek in directories
  2026-08-27 23:36 ` [PATCH] fstests: generic: Add test of seek in directories NeilBrown
  2026-08-28  4:13   ` Matthew Wilcox
  2026-08-30 23:13   ` Chuck Lever
@ 2026-08-31 12:59   ` Christoph Hellwig
  2026-08-31 22:47     ` NeilBrown
  2 siblings, 1 reply; 14+ messages in thread
From: Christoph Hellwig @ 2026-08-31 12:59 UTC (permalink / raw)
  To: NeilBrown; +Cc: fstests, linux-fsdevel, Chuck Lever, linux-btrfs

Please include the mainling list for the file system you think is
buggy or at least odd.  Done now.

On Fri, Aug 28, 2026 at 09:36:10AM +1000, NeilBrown wrote:
> The test then reads the directory to find the order of the stable name,
> which should never change. It also find the "d_off" of all names.
> These d_off should be usable as a "seek" offset to find at least all
> the stable names that came after that point in the original listing.
> 
> After creating the names and finding the order, a loop which repeatly:
>   - makes random changes to unstable names
>   - checks the complete order of stable names in a new readdir
>   - checks what appears after a seek() to a randomly chosen offet
> 
> In Linux 7.2 this test always passes for xfs and ext4 but fails
> for btrfs unless we suppress renames with a "stable" name as target.
> It also fails for tmpfs.

There is no requirement where the entry for a renamed entry is placed.
Posix requires telldir/seekdir to provide a stable cookie, but how
that cookie behaves when the directory is modified is completely
undefined.  The same is true for NFS which requires basically the
same, but more stateless than local telldir/seekdir.

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

* Re: [PATCH] fstests: generic: Add test of seek in directories
  2026-08-28 11:12     ` NeilBrown
  2026-08-28 14:55       ` Theodore Tso
@ 2026-08-31 13:16       ` Christoph Hellwig
  2026-09-01  1:05         ` NeilBrown
  1 sibling, 1 reply; 14+ messages in thread
From: Christoph Hellwig @ 2026-08-31 13:16 UTC (permalink / raw)
  To: NeilBrown
  Cc: Matthew Wilcox, fstests, linux-fsdevel, Chuck Lever, linux-btrfs

On Fri, Aug 28, 2026 at 09:12:41PM +1000, NeilBrown wrote:
> If you happened to know the sizes of the all the names in the directory,
> you could make a sequence of getdents calls which each return precisely
> 1 entry,  You could then use lseek to determine the seek offset at every
> point.

No, you can't.  Despite the historic naming d_off is not an offset, but
a cookie.  You can not arithmetics on it.

> Linux getdents64 makes this a bit simpler by returning exactly that same
> number (the seek offset to the next name) in the d_off field.
> 
> So posix certainly allows, indirectly, seeking to each d_off.

Posix and Linux allow to seekdir to each cookie returned from telldir,
it does not allow to do arithmetics on it.

> 
> Posix refers to one directory entry "immediately following" another
> which clearly implies a well defined sequence (if you exclude names
> added and removed during the read).

I think you're talking about posix_getdents here, which isn't really
Posix as we know it, but was added in the 2024 edition without actually
having relevant implementation so far.  It does however implement
the syscall-level API in most modern Unixes.  It does not mention
d_off at all, just the lseek-able file offset, though.

> I think a strong argument for Linux needing something is that NFS needs
> working directory offsets to be able to support READDIR, because there
> is no "OPEN" request for directories.

I don't think anyone disputes the need for stable directory offsets,
and we should (*knock on wood*) have implementation of native
file systems that fail this.

What the test tries to force is a specific behavior for rename onto
existing file names, replacing the original file name entry with
a new one of the same name.  The test expects that to reuse the
previous d_off, which is not required by any real or de facto standard.

> I know less about btrfs.  It appears to always add new names to the end
> of the directory listing.  When you rename over an existing name, that
> existing name is relocated to the end.  I can only guess why it might do
> that.  I have no idea if it "needs" to do that.

btrfs doesn't manage freespace for the d_off space (doing so is quite
complicated and requires a lot of code in XFS for example), so it
simply uses a monotonically increasing counter for the value reported
in d_off.  This simplifies things a lot, and should work well on 64-bit
systems were you are basically impossible to round out of d_off values.
It might be a lot more problematic on 32-bit systems because the
seekdir/telldir cookie is a long and not a guaranteed 64-bit value.

> Fun fact: This behaviour of btrfs (which I think is perfectly defensible
> for new names) resulted in generic/736 (which I think is an unreasonable
> test to impose) which btrfs "fixed" with a mechanism that doesn't work
> over NFS.  i.e.  it doesn't work if you close and re-open the file for
> each getdents call.
> If I mount a btrfs filesystem over nfs with rsize=4096, then generic/736
> fails.

That code and the tests looks a bit questionable, as readdir by
definition can't every complete in other file systems either if you add
new entries faster than the pace of readdir calls.  I.e. if you replace
the rename there with link calls adding new entries you run into the
same issue with every file system.

> The link you provided says:
>   If a sequence of calls to posix_getdents() is made that reads from
>   offset zero to end-of-file and a file is removed from or added to the
>   directory between the first and last of those calls, whether the
>   sequence of calls returns an entry for that file is unspecified. 
> 
> which unfortunately doesn't cover what happens when a file is renamed.
> When renamed to a non-existing name, it might be reasonable to describe
> this as "removed from" and "added to" so its appearance could be
> unspecified.
> When renamed to an existing name, I don't think it reasonable to
> describe the target name a being either "removed from" or "added to" the
> directory.

If we are talking about files, it is very clear that the target over
which is renamed is removed from the directory.  And if the source
was outside the directory it also is very clearly added.  The only
gray area is a source file that already was in the same directory.


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

* Re: [PATCH] fstests: generic: Add test of seek in directories
  2026-08-31 12:59   ` Christoph Hellwig
@ 2026-08-31 22:47     ` NeilBrown
  2026-09-01  9:09       ` Christoph Hellwig
  0 siblings, 1 reply; 14+ messages in thread
From: NeilBrown @ 2026-08-31 22:47 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: fstests, linux-fsdevel, Chuck Lever, linux-btrfs

On Mon, 31 Aug 2026, Christoph Hellwig wrote:
> Please include the mainling list for the file system you think is
> buggy or at least odd.  Done now.

Thanks.

> 
> On Fri, Aug 28, 2026 at 09:36:10AM +1000, NeilBrown wrote:
> > The test then reads the directory to find the order of the stable name,
> > which should never change. It also find the "d_off" of all names.
> > These d_off should be usable as a "seek" offset to find at least all
> > the stable names that came after that point in the original listing.
> > 
> > After creating the names and finding the order, a loop which repeatly:
> >   - makes random changes to unstable names
> >   - checks the complete order of stable names in a new readdir
> >   - checks what appears after a seek() to a randomly chosen offet
> > 
> > In Linux 7.2 this test always passes for xfs and ext4 but fails
> > for btrfs unless we suppress renames with a "stable" name as target.
> > It also fails for tmpfs.
> 
> There is no requirement where the entry for a renamed entry is placed.
> Posix requires telldir/seekdir to provide a stable cookie, but how
> that cookie behaves when the directory is modified is completely
> undefined.  The same is true for NFS which requires basically the
> same, but more stateless than local telldir/seekdir.
> 
> 

I don't think the behaviour is "completely" undefined in the face of
change.

https://pubs.opengroup.org/onlinepubs/007908799/xsh/readdir.html

says the directory stream represents "all the directory entries in a
particular directory" and makes exceptions:

  If a file is removed from or added to the directory after the most
  recent call to opendir() or rewinddir(), whether a subsequent call to
  readdir() returns an entry for that file is unspecified. 

so "all" doesn't need to includes things that were added or removed.
Does a rename over an existing file "add" or "remove"?
It depends on how you understand "file".

We are told "Directory entries represent files" so we need to understand
"files" in that context.
If we considered "file" to mean "filesystem object", then the above
would allow readdir to ignore multiple hard-links to a file reporting
only one of them.  Hopefully we all agree that would be wrong.

So I think "file" in this context must mean "name" (that is the main
part of a "directory entry").

So, when we rename over an existing name, is that name added or removed?
I think not. It is critical to rename(2) that the replacement is atomic.

With btrfs at present, if a name is the target of a rename while a
readdir is happening, that name might not be reported.  This is because
btrfs iterate_shared deliberately skips any names that are "new" since
the start of the readdir, and it considers a name replaced in a rename
as "new".  I think this is incorrect behaviour and could be harmful.

Prior to 
Commit: 9b378f6ad48c ("btrfs: fix infinite directory reads")

btrfs could report the target of a rename twice in a readdir listing
(but always at least once).
While I think the duplication is unnecessary it is harder to criticise.

NeilBrown

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

* Re: [PATCH] fstests: generic: Add test of seek in directories
  2026-08-31 13:16       ` Christoph Hellwig
@ 2026-09-01  1:05         ` NeilBrown
  2026-09-01  9:18           ` Christoph Hellwig
  0 siblings, 1 reply; 14+ messages in thread
From: NeilBrown @ 2026-09-01  1:05 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Matthew Wilcox, fstests, linux-fsdevel, Chuck Lever, linux-btrfs

On Mon, 31 Aug 2026, Christoph Hellwig wrote:
> On Fri, Aug 28, 2026 at 09:12:41PM +1000, NeilBrown wrote:
> > If you happened to know the sizes of the all the names in the directory,
> > you could make a sequence of getdents calls which each return precisely
> > 1 entry,  You could then use lseek to determine the seek offset at every
> > point.
> 
> No, you can't.  Despite the historic naming d_off is not an offset, but
> a cookie.  You can not arithmetics on it.
> 
> > Linux getdents64 makes this a bit simpler by returning exactly that same
> > number (the seek offset to the next name) in the d_off field.
> > 
> > So posix certainly allows, indirectly, seeking to each d_off.
> 
> Posix and Linux allow to seekdir to each cookie returned from telldir,
> it does not allow to do arithmetics on it.

I agree. No arithmetic, but seek is allowed.
The cookies should all be unique across a single readdir pass, and none
of them may be zero as d_off is a seek address *after* the current name,
and the seek address zero is *before* all names.

> 
> > 
> > Posix refers to one directory entry "immediately following" another
> > which clearly implies a well defined sequence (if you exclude names
> > added and removed during the read).
> 
> I think you're talking about posix_getdents here, which isn't really
> Posix as we know it, but was added in the 2024 edition without actually
> having relevant implementation so far.  It does however implement
> the syscall-level API in most modern Unixes.  It does not mention
> d_off at all, just the lseek-able file offset, though.

The d_off provided by Linux is precisely a seek-address with a
granularity of one name, rather than a granularity of one getdents call.
This is implicit in the VFS implementation.

> 
> > I think a strong argument for Linux needing something is that NFS needs
> > working directory offsets to be able to support READDIR, because there
> > is no "OPEN" request for directories.
> 
> I don't think anyone disputes the need for stable directory offsets,
> and we should (*knock on wood*) have implementation of native
> file systems that fail this.

"should not" ??

> 
> What the test tries to force is a specific behavior for rename onto
> existing file names, replacing the original file name entry with
> a new one of the same name.  The test expects that to reuse the
> previous d_off, which is not required by any real or de facto standard.

That's debatable.  The documents that I have found don't mention rename.
Does that mean anything goes, or do that mean it doesn't get an
exemption from the general rule that all names must be listed?

> 
> > I know less about btrfs.  It appears to always add new names to the end
> > of the directory listing.  When you rename over an existing name, that
> > existing name is relocated to the end.  I can only guess why it might do
> > that.  I have no idea if it "needs" to do that.
> 
> btrfs doesn't manage freespace for the d_off space (doing so is quite
> complicated and requires a lot of code in XFS for example), so it
> simply uses a monotonically increasing counter for the value reported
> in d_off.  This simplifies things a lot, and should work well on 64-bit
> systems were you are basically impossible to round out of d_off values.
> It might be a lot more problematic on 32-bit systems because the
> seekdir/telldir cookie is a long and not a guaranteed 64-bit value.

I think that always allocating the next unused number when adding a name
to a directory is perfectly reasonable when 64bit seek addressing is
available.  The only question is on whether a replacing rename involves
"adding a name".

> 
> > Fun fact: This behaviour of btrfs (which I think is perfectly defensible
> > for new names) resulted in generic/736 (which I think is an unreasonable
> > test to impose) which btrfs "fixed" with a mechanism that doesn't work
> > over NFS.  i.e.  it doesn't work if you close and re-open the file for
> > each getdents call.
> > If I mount a btrfs filesystem over nfs with rsize=4096, then generic/736
> > fails.
> 
> That code and the tests looks a bit questionable, as readdir by
> definition can't every complete in other file systems either if you add
> new entries faster than the pace of readdir calls.  I.e. if you replace
> the rename there with link calls adding new entries you run into the
> same issue with every file system.

According to
  https://www.spinics.net/lists/linux-btrfs/msg138653.html
this seem to come from
  https://github.com/landley/toybox/issues/306
which suggests that

  toybox find testdir -type f -print0 | xargs -0 -n1 sed -i s/a/b/ 

runs forever on btrfs.  Certainly this is unexpected behaviour.  Should
it be fixed in the filesystem or in toybox/find?

I can understand toybox wanting to be as simple as possible.  But
readdir() doesn't make any promises about terminating.

I wouldn't object to btrfs "fixing" this by never reporting new names
except that:
 1/ the interaction with rename/replace is problematic
 2/ the current fix doesn't work over NFS.

I think the rename issue can be fixed by simply reusing the existing
offset when reusing a name.  I might try a patch if a find some time.

I think a fix to make it work over NFS would be to report the entries in
reverse order - most recently added to least recently added.  This would
effectively encode the timestamp (last_index) in the seek cookie instead
of in btrfs_file_private.
Changing how the seek-cookie is interpreted could cause a hiccup
for an NFS client which was in the middle of a READDIR when a server
we reboot to a new verion of btrfs, but that is likely not significant.

Note that
  https://pubs.opengroup.org/onlinepubs/9799919799/functions/posix_getdents.html

uses the word "incremented" in 

  the directory entry immediately following the last entry whose
  information was returned

but I don't think anyone seems interested in that detail.

> 
> > The link you provided says:
> >   If a sequence of calls to posix_getdents() is made that reads from
> >   offset zero to end-of-file and a file is removed from or added to the
> >   directory between the first and last of those calls, whether the
> >   sequence of calls returns an entry for that file is unspecified. 
> > 
> > which unfortunately doesn't cover what happens when a file is renamed.
> > When renamed to a non-existing name, it might be reasonable to describe
> > this as "removed from" and "added to" so its appearance could be
> > unspecified.
> > When renamed to an existing name, I don't think it reasonable to
> > describe the target name a being either "removed from" or "added to" the
> > directory.
> 
> If we are talking about files, it is very clear that the target over
> which is renamed is removed from the directory.  And if the source
> was outside the directory it also is very clearly added.  The only
> gray area is a source file that already was in the same directory.
> 
> 

As I explained in another email, I don't think it is reasonable to
interpret "files" as mentioned in the documentation as "inodes" but only
as "names".  I think the text is sloppy and should be more explicit but
if you uniformly assume "file" to mean "filesystem object" is doesn't
make sense.

Thanks,
NeilBrown

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

* Re: [PATCH] fstests: generic: Add test of seek in directories
  2026-08-31 22:47     ` NeilBrown
@ 2026-09-01  9:09       ` Christoph Hellwig
  0 siblings, 0 replies; 14+ messages in thread
From: Christoph Hellwig @ 2026-09-01  9:09 UTC (permalink / raw)
  To: NeilBrown
  Cc: Christoph Hellwig, fstests, linux-fsdevel, Chuck Lever,
	linux-btrfs

On Tue, Sep 01, 2026 at 08:47:58AM +1000, NeilBrown wrote:
> > Posix requires telldir/seekdir to provide a stable cookie, but how
> > that cookie behaves when the directory is modified is completely
> > undefined.  The same is true for NFS which requires basically the
> > same, but more stateless than local telldir/seekdir.
> 
> I don't think the behaviour is "completely" undefined in the face of
> change.
> 
> https://pubs.opengroup.org/onlinepubs/007908799/xsh/readdir.html
> 
> says the directory stream represents "all the directory entries in a
> particular directory" and makes exceptions:
> 
>   If a file is removed from or added to the directory after the most
>   recent call to opendir() or rewinddir(), whether a subsequent call to
>   readdir() returns an entry for that file is unspecified. 
> 
> so "all" doesn't need to includes things that were added or removed.
> Does a rename over an existing file "add" or "remove"?
> It depends on how you understand "file".
> 
> We are told "Directory entries represent files" so we need to understand
> "files" in that context.
> If we considered "file" to mean "filesystem object", then the above
> would allow readdir to ignore multiple hard-links to a file reporting
> only one of them.  Hopefully we all agree that would be wrong.
> 
> So I think "file" in this context must mean "name" (that is the main
> part of a "directory entry").

It does not.  Posix very precisely uses "directory entry" when referring
to directory entries, and files refer to what is an inode in Linux.
This is very clear in the rename documentation:

    The rename() function shall change the name of a file. The old
    argument points to the pathname of the file to be renamed. The new
    argument points to the new pathname of the file.
    If the new argument does not resolve to an existing directory
    entry...

https://pubs.opengroup.org/onlinepubs/9799919799/functions/rename.html

> So, when we rename over an existing name, is that name added or removed?
> I think not. It is critical to rename(2) that the replacement is atomic.

The directory entry for the old file is removed, and the directory entry
for the file is added in one atomic transaction.

> With btrfs at present, if a name is the target of a rename while a
> readdir is happening, that name might not be reported.  This is because
> btrfs iterate_shared deliberately skips any names that are "new" since
> the start of the readdir, and it considers a name replaced in a rename
> as "new".  I think this is incorrect behaviour and could be harmful.
> 
> Prior to 
> Commit: 9b378f6ad48c ("btrfs: fix infinite directory reads")
> 
> btrfs could report the target of a rename twice in a readdir listing
> (but always at least once).
> While I think the duplication is unnecessary it is harder to criticise.

I'm in agreement here.


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

* Re: [PATCH] fstests: generic: Add test of seek in directories
  2026-09-01  1:05         ` NeilBrown
@ 2026-09-01  9:18           ` Christoph Hellwig
  0 siblings, 0 replies; 14+ messages in thread
From: Christoph Hellwig @ 2026-09-01  9:18 UTC (permalink / raw)
  To: NeilBrown
  Cc: Christoph Hellwig, Matthew Wilcox, fstests, linux-fsdevel,
	Chuck Lever, linux-btrfs

On Tue, Sep 01, 2026 at 11:05:35AM +1000, NeilBrown wrote:
> > > I think a strong argument for Linux needing something is that NFS needs
> > > working directory offsets to be able to support READDIR, because there
> > > is no "OPEN" request for directories.
> > 
> > I don't think anyone disputes the need for stable directory offsets,
> > and we should (*knock on wood*) have implementation of native
> > file systems that fail this.
> 
> "should not" ??

Yes.

> > What the test tries to force is a specific behavior for rename onto
> > existing file names, replacing the original file name entry with
> > a new one of the same name.  The test expects that to reuse the
> > previous d_off, which is not required by any real or de facto standard.
> 
> That's debatable.  The documents that I have found don't mention rename.
> Does that mean anything goes, or do that mean it doesn't get an
> exemption from the general rule that all names must be listed?

The Posix definition of rename is very explicit that the old directory
entry shall be removed:

    Otherwise, if the directory entry named by new exists, it shall be
    removed and old renamed to new. In this case, a directory entry named
    new shall remain visible to other threads throughout the renaming
    operation and refer either to the file referred to by new or old
    before the operation began.

So the previous directory entry for "new" shall be removed.  If a
file system reuses the same d_off for the renamed file, it just created
a new entry that happens to instantly reuse the d_off in the same atomic
operation.

> I think a fix to make it work over NFS would be to report the entries in
> reverse order - most recently added to least recently added.  This would
> effectively encode the timestamp (last_index) in the seek cookie instead
> of in btrfs_file_private.
> Changing how the seek-cookie is interpreted could cause a hiccup
> for an NFS client which was in the middle of a READDIR when a server
> we reboot to a new verion of btrfs, but that is likely not significant.
> 
> Note that
>   https://pubs.opengroup.org/onlinepubs/9799919799/functions/posix_getdents.html
> 
> uses the word "incremented" in 
> 
>   the directory entry immediately following the last entry whose
>   information was returned
> 
> but I don't think anyone seems interested in that detail.

I would suggest to ignore posix_getdents or the whole Issue 8 base spec,
as unlike previous versions I did not try to document and norm existing
behavior, but instead comes up with it's own things.  If we need a
justification we should look at historic Linux and Unix behaviors and
older Posix specs that haven't drifted away from the purpose as much
as the current one.


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

end of thread, other threads:[~2026-09-01  9:18 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-27 23:36 fstests: Verify behaviour of "seek" in a directory NeilBrown
2026-08-27 23:36 ` [PATCH] fstests: generic: Add test of seek in directories NeilBrown
2026-08-28  4:13   ` Matthew Wilcox
2026-08-28 11:12     ` NeilBrown
2026-08-28 14:55       ` Theodore Tso
2026-08-30 22:22         ` NeilBrown
2026-08-31  1:37           ` Theodore Tso
2026-08-31 13:16       ` Christoph Hellwig
2026-09-01  1:05         ` NeilBrown
2026-09-01  9:18           ` Christoph Hellwig
2026-08-30 23:13   ` Chuck Lever
2026-08-31 12:59   ` Christoph Hellwig
2026-08-31 22:47     ` NeilBrown
2026-09-01  9:09       ` Christoph Hellwig

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