On 9/22/25 04:54, Mateusz Guzik wrote:
On Mon, Sep 22, 2025 at 10:41 AM John Baldwin <j...@freebsd.org> wrote:
On 9/19/25 10:19, Konstantin Belousov wrote:
The branch main has been updated by kib:
URL:
https://cgit.FreeBSD.org/src/commit/?id=40a42785dbba93cc5196178fc49d340c1a89cabe
commit 40a42785dbba93cc5196178fc49d340c1a89cabe
Author: Konstantin Belousov <k...@freebsd.org>
AuthorDate: 2025-09-11 10:05:04 +0000
Commit: Konstantin Belousov <k...@freebsd.org>
CommitDate: 2025-09-19 14:19:13 +0000
fcntl(F_SETFL): only allow one thread to perform F_SETFL
Use f_vflags file locking for this.
Allowing more than one thread handling F_SETFL might cause de-sync
between real driver state and flags.
Reviewed by: markj
Tested by: pho
Sponsored by: The FreeBSD Foundation
MFC after: 2 weeks
Differential revision: https://reviews.freebsd.org/D52487
Thanks for fixing this. I still slightly worry that "home-grown" locks
aren't visible to WITNESS and it's checking.
Another problem with these is that they don't do adaptive spinning.
In particular for file offset, it *is* putting threads off cpu in real
workloads when it plausibly could be avoided.
I think the real thing to do here is to drop the hand-rolled machinery
and use an sx lock.
Currently struct file is 80 bytes which is a very nasty size from
caching standpoint.
Locks are 32 bytes in size, which is another problem, but ultimately
one can be added here without growing the struct past 128 bytes.
The only issue here is that files are marked as NOFREE, so this memory
can *never* be reclaimed.
One could be tempted to use smr here, but the cost of smr_enter is
prohibitive. There is a lazy variant which does not do atomics, which
perhaps could work, but that 0 users in the tree and was probably
never tested.
With 32-bit archs going away I don't think it's a big deal though.
For interested, on Linux the struct is 256 bytes.
I had suggested in an earlier review adding an sx-pool similar to our
existing mtxpool and using that. That would avoid bloating the structure
with a dedicated lock.
--
John Baldwin
