commit 3f7d2a970f31232006ccd73a054b52b4149b316c (HEAD -> vlock2)
Author: Mateusz Guzik <mjg@FreeBSD.org>
Date: Sun Oct 5 17:03:54 2025 +0000
vfs offset: fix assertion failure in face of racing ffofset and setfl locking
Both use the same 16 bit var to store their locked and waiters bits,
then this in file_v_unlock:
state = atomic_load_16(flagsp);
if ((state & lock_wait_bit) == 0 &&
atomic_cmpset_rel_16(flagsp, state, state & ~lock_bit))
return;
can fail if for example foffset is being unlocked while setfl is getting
locked.
Afterwards the code assumes there are blocked waiters on foffset.
commit d87d378eae3bebd167dd9bb71450a9e44b2717a1
Author: Mateusz Guzik <mjg@FreeBSD.org>
Date: Sun Oct 5 17:02:01 2025 +0000
vfs foffset: drop weird commentary about offset protection
It is readily apparent the FOFFSET_LOCKED flag protects it because the
read followed its use.
It also does not matter who said how it was protected earlier.Details
Details
- Reviewers
kib markj - Commits
- rG9bea2657c453: vfs offset: fix assertion failure in face of racing ffofset and setfl locking
rG57bf745be964: vfs offset: fix assertion failure in face of racing ffofset and setfl locking
rGf43e19db6cea: vfs offset: fix assertion failure in face of racing ffofset and setfl locking
rGb1de02c415de: vfs offset: fix assertion failure in face of racing ffofset and setfl locking
rGf16178e0bba8: vfs foffset: drop weird commentary about offset protection
This panicked for me while running poudriere.
I created a testcase based on will-it-scale which reproduces it within seconds. No issues with my patch.
#include <string.h>
#include <fcntl.h>
#include <stdlib.h>
#include <unistd.h>
#include <assert.h>
#define BUFLEN 4096
#define FILESIZE (1 * 1024 * 1024)
static char tmpfile[] = "/tmp/willitscale.XXXXXX";
char *testcase_description = "Same file read";
int fd;
void testcase_prepare(unsigned long nr_tasks)
{
char buf[FILESIZE];
fd = mkstemp(tmpfile);
memset(buf, 0, sizeof(buf));
assert(fd >= 0);
assert(write(fd, buf, sizeof(buf)) == sizeof(buf));
}
void testcase(unsigned long long *iterations, unsigned long nr)
{
char buf[BUFLEN];
if (nr == 0) {
while (1) {
int ret = read(fd, buf, BUFLEN);
assert(ret >= 0);
if (ret == 0)
lseek(fd, 0, SEEK_SET);
(*iterations)++;
}
} else {
while (1) {
int ret = fcntl(fd, F_SETFL, FRDAHEAD);
assert(ret == 0);
}
}
}
void testcase_cleanup(void)
{
unlink(tmpfile);
}Diff Detail
Diff Detail
- Repository
- rG FreeBSD src repository
- Lint
Lint Not Applicable - Unit
Tests Not Applicable
Event Timeline
| sys/kern/vfs_vnops.c | ||
|---|---|---|
| 851 | This assertion is still racy. It should operate on state rather than *flagsp. | |
Comment Actions
This seems ok to me. We could also just remove the assertion in question instead.
| sys/kern/vfs_vnops.c | ||
|---|---|---|
| 851 | Oh, never mind, once the wait bit is set it won't be cleared. | |