[PATCH 04/46] mm/memfd: make f_pos accesses atomic
Gregory Price
gourry at gourry.net
Mon Oct 26 20:34:28 PDT 2026
On 2026-10-27 00:17 +0100, David Hildenbrand (Arm) wrote:
> From: Josef Bacik <josef at toxicpanda.com>
>
> Commit 9c225f2655e3 ("vfs: atomic f_pos accesses as per POSIX") made
> read(), write() and lseek() on a shared open file description of a regular
> file serialize on ->f_pos_lock. The flag that asks for it,
> FMODE_ATOMIC_POS, is set in do_dentry_open(). A memfd is a regular file
> too, but its struct file comes from alloc_file_pseudo() and never passes
> through do_dentry_open(), so it does not get the flag:
>
... snip ...
>
> Fixes: 9183df25fe7b ("shm: add memfd_create() syscall")
> Cc: stable at vger.kernel.org
> Signed-off-by: Josef Bacik <josef at toxicpanda.com>
> [ david: minor description tweaks ]
> Signed-off-by: David Hildenbrand (Arm) <david at kernel.org>
Reviewed-by: Gregory Price (Meta) <gourry at gourry.net>
Tested-by: Gregory Price (Meta) <gourry at gourry.net>
This one arguably should have at least a trivial selftest.
Test attatched
-- >8 --
From: Gregory Price <gourry at gourry.net>
Subject: [PATCH] selftests/memfd: test shared file position atomicity
Without FMODE_ATOMIC_POS, concurrent reads or writes on a shared memfd
can use the same file position. Two writers can overwrite each other's
records:
CPU 1 CPU 2
pos = file->f_pos
pos = file->f_pos
write a record at pos
write a record at pos
file->f_pos = pos + size
file->f_pos = pos + size
Add separate shared-read and shared-write tests using the kselftest
harness. Check the final offset, file size and exactly-once record delivery
rather than requiring a particular interleaving.
Signed-off-by: Gregory Price (Meta) <gourry at gourry.net>
---
tools/testing/selftests/memfd/.gitignore | 1 +
tools/testing/selftests/memfd/Makefile | 3 +-
tools/testing/selftests/memfd/memfd_pos.c | 136 ++++++++++++++++++++++
3 files changed, 139 insertions(+), 1 deletion(-)
create mode 100644 tools/testing/selftests/memfd/memfd_pos.c
diff --git a/tools/testing/selftests/memfd/.gitignore b/tools/testing/selftests/memfd/.gitignore
index dd9a051f608e2..574ab97ebd982 100644
--- a/tools/testing/selftests/memfd/.gitignore
+++ b/tools/testing/selftests/memfd/.gitignore
@@ -2,4 +2,5 @@
fuse_mnt
fuse_test
memfd_test
+memfd_pos
memfd-test-file
diff --git a/tools/testing/selftests/memfd/Makefile b/tools/testing/selftests/memfd/Makefile
index 163b6f68631c4..67a95c34541cd 100644
--- a/tools/testing/selftests/memfd/Makefile
+++ b/tools/testing/selftests/memfd/Makefile
@@ -2,7 +2,7 @@
CFLAGS += -D_FILE_OFFSET_BITS=64
CFLAGS += $(KHDR_INCLUDES)
-TEST_GEN_PROGS := memfd_test
+TEST_GEN_PROGS := memfd_test memfd_pos
TEST_PROGS := run_fuse_test.sh run_hugetlbfs_test.sh
TEST_GEN_FILES := fuse_test fuse_mnt
@@ -22,6 +22,7 @@ include ../lib.mk
$(OUTPUT)/fuse_mnt: LDLIBS += $(VAR_LDLIBS)
+$(OUTPUT)/memfd_pos: LDLIBS += -pthread
$(OUTPUT)/memfd_test: memfd_test.c common.c
$(OUTPUT)/fuse_test: fuse_test.c common.c
diff --git a/tools/testing/selftests/memfd/memfd_pos.c b/tools/testing/selftests/memfd/memfd_pos.c
new file mode 100644
index 0000000000000..5f3749d13f561
--- /dev/null
+++ b/tools/testing/selftests/memfd/memfd_pos.c
@@ -0,0 +1,136 @@
+// SPDX-License-Identifier: GPL-2.0
+#define _GNU_SOURCE
+#include <errno.h>
+#include <pthread.h>
+#include <stdatomic.h>
+#include <stdint.h>
+#include <stdlib.h>
+#include <sys/mman.h>
+#include <sys/stat.h>
+#include <unistd.h>
+
+#include "../kselftest_harness.h"
+
+#define THREADS 4
+#define RECORDS 16384
+#define TOTAL (THREADS * RECORDS)
+
+struct worker {
+ int fd;
+ unsigned int id;
+ bool reading;
+ pthread_barrier_t *barrier;
+ atomic_uint *seen;
+ int error;
+};
+
+static void *transfer(void *arg)
+{
+ struct worker *w = arg;
+ unsigned int i;
+
+ pthread_barrier_wait(w->barrier);
+ for (i = 0; i < RECORDS; i++) {
+ uint32_t value = w->id * RECORDS + i;
+ ssize_t n;
+
+ do {
+ n = w->reading ? read(w->fd, &value, sizeof(value)) :
+ write(w->fd, &value, sizeof(value));
+ } while (n < 0 && errno == EINTR);
+ if (n != sizeof(value) || value >= TOTAL) {
+ w->error = 1;
+ break;
+ }
+ if (w->reading)
+ atomic_fetch_add(&w->seen[value], 1);
+ }
+ return NULL;
+}
+
+FIXTURE(memfd_pos) {
+ int fd;
+ atomic_uint *seen;
+};
+
+FIXTURE_SETUP(memfd_pos)
+{
+ self->fd = memfd_create("position", MFD_CLOEXEC);
+ if (self->fd < 0 && errno == ENOSYS)
+ SKIP(return, "memfd_create unavailable");
+ ASSERT_GE(self->fd, 0);
+ self->seen = calloc(TOTAL, sizeof(*self->seen));
+ ASSERT_NE(self->seen, NULL);
+ for (unsigned int i = 0; i < TOTAL; i++)
+ atomic_init(&self->seen[i], 0);
+}
+
+FIXTURE_TEARDOWN(memfd_pos)
+{
+ close(self->fd);
+ free(self->seen);
+}
+
+static int run_workers(int fd, bool reading, atomic_uint *seen)
+{
+ pthread_barrier_t barrier;
+ pthread_t threads[THREADS];
+ struct worker workers[THREADS];
+ int ret = 0;
+ unsigned int i;
+
+ if (pthread_barrier_init(&barrier, NULL, THREADS))
+ return 1;
+ for (i = 0; i < THREADS; i++) {
+ workers[i] = (struct worker) {
+ .fd = fd, .id = i, .reading = reading,
+ .barrier = &barrier, .seen = seen,
+ };
+ /* The harness isolates this test in a child; abort on setup failure. */
+ if (pthread_create(&threads[i], NULL, transfer, &workers[i]))
+ abort();
+ }
+ for (i = 0; i < THREADS; i++) {
+ if (pthread_join(threads[i], NULL) || workers[i].error)
+ ret = 1;
+ }
+ pthread_barrier_destroy(&barrier);
+ return ret;
+}
+
+/* Concurrent writes sharing an open file description must not overlap. */
+TEST_F(memfd_pos, shared_write)
+{
+ struct stat st;
+ uint32_t value;
+ unsigned int i;
+
+ ASSERT_EQ(run_workers(self->fd, false, self->seen), 0);
+ ASSERT_EQ(fstat(self->fd, &st), 0);
+ ASSERT_EQ(st.st_size, (off_t)(TOTAL * sizeof(value)));
+ ASSERT_EQ(lseek(self->fd, 0, SEEK_CUR), st.st_size);
+ ASSERT_EQ(lseek(self->fd, 0, SEEK_SET), 0);
+ for (i = 0; i < TOTAL; i++) {
+ ASSERT_EQ(read(self->fd, &value, sizeof(value)), sizeof(value));
+ ASSERT_LT(value, TOTAL);
+ atomic_fetch_add(&self->seen[value], 1);
+ }
+ for (i = 0; i < TOTAL; i++)
+ ASSERT_EQ(atomic_load(&self->seen[i]), 1);
+}
+
+/* Concurrent reads must consume every record exactly once. */
+TEST_F(memfd_pos, shared_read)
+{
+ uint32_t i;
+
+ for (i = 0; i < TOTAL; i++)
+ ASSERT_EQ(write(self->fd, &i, sizeof(i)), sizeof(i));
+ ASSERT_EQ(lseek(self->fd, 0, SEEK_SET), 0);
+ ASSERT_EQ(run_workers(self->fd, true, self->seen), 0);
+ ASSERT_EQ(lseek(self->fd, 0, SEEK_CUR), (off_t)(TOTAL * sizeof(i)));
+ for (i = 0; i < TOTAL; i++)
+ ASSERT_EQ(atomic_load(&self->seen[i]), 1);
+}
+
+TEST_HARNESS_MAIN
--
Gregory Price <gourry at gourry.net>
More information about the kexec
mailing list