Date: Mon, 22 Sep 2025 10:54:27 +0200 From: Mateusz Guzik <mjguzik@gmail.com> To: John Baldwin <jhb@freebsd.org> Cc: Konstantin Belousov <kib@freebsd.org>, src-committers@freebsd.org, dev-commits-src-all@freebsd.org, dev-commits-src-main@freebsd.org Subject: Re: git: 40a42785dbba - main - fcntl(F_SETFL): only allow one thread to perform F_SETFL Message-ID: <CAGudoHGZs3iOLmbRBwhanNHtDRmd5BE%2Buorq8onCAbCkFw39iw@mail.gmail.com> In-Reply-To: <92831372-745d-4612-b38f-aeb235dd8cca@FreeBSD.org> References: <202509191419.58JEJsvj031867@gitrepo.freebsd.org> <92831372-745d-4612-b38f-aeb235dd8cca@FreeBSD.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On Mon, Sep 22, 2025 at 10:41=E2=80=AFAM John Baldwin <jhb@freebsd.org> wro= te: > > 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=3D40a42785dbba93cc5196178f= c49d340c1a89cabe > > > > commit 40a42785dbba93cc5196178fc49d340c1a89cabe > > Author: Konstantin Belousov <kib@FreeBSD.org> > > AuthorDate: 2025-09-11 10:05:04 +0000 > > Commit: Konstantin Belousov <kib@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.
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CAGudoHGZs3iOLmbRBwhanNHtDRmd5BE%2Buorq8onCAbCkFw39iw>