Date: Mon, 6 Oct 2025 21:33:05 +0200 From: Mateusz Guzik <mjguzik@gmail.com> To: Olivier Certner <olce@freebsd.org> Cc: src-committers@freebsd.org, dev-commits-src-all@freebsd.org, dev-commits-src-main@freebsd.org Subject: Re: git: 09f925b57aeb - main - nullfs: Slightly reduce contention by reducing concurrent sections Message-ID: <CAGudoHEKNSi8LJz9wFEP-BM27btZO9%2B-wq7LBeJ-vuHqy7KsPg@mail.gmail.com> In-Reply-To: <202510061522.596FMk6Q043852@gitrepo.freebsd.org> References: <202510061522.596FMk6Q043852@gitrepo.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On Mon, Oct 6, 2025 at 5:22=E2=80=AFPM Olivier Certner <olce@freebsd.org> w= rote: > > The branch main has been updated by olce: > > URL: https://cgit.FreeBSD.org/src/commit/?id=3D09f925b57aeb171318a9d54df5= 00bf22b4cdd986 > > commit 09f925b57aeb171318a9d54df500bf22b4cdd986 > Author: Olivier Certner <olce@FreeBSD.org> > AuthorDate: 2025-10-06 13:22:13 +0000 > Commit: Olivier Certner <olce@FreeBSD.org> > CommitDate: 2025-10-06 15:21:45 +0000 > > nullfs: Slightly reduce contention by reducing concurrent sections > > In null_lock_prep_with_smr(), initialize 'lvp' outside of the > SMR-protected section. > > In null_lock(), if after locking the lower vnode we notice that we ha= ve > been reclaimed, we have to unlock the lower vnode and then relock our > own now that the lock isn't shared anymore. Call VOP_UNLOCK() on the > lower vnode as soon as this condition is known. > [snip] > diff --git a/sys/fs/nullfs/null_vnops.c b/sys/fs/nullfs/null_vnops.c > index 375b6aa27531..ec8a6b10b13f 100644 > --- a/sys/fs/nullfs/null_vnops.c > +++ b/sys/fs/nullfs/null_vnops.c > @@ -788,10 +788,10 @@ null_lock_prep_with_smr(struct vop_lock1_args *ap) > struct null_node *nn; > struct vnode *lvp; > > - vfs_smr_enter(); > - > lvp =3D NULL; > > + vfs_smr_enter(); > + This bit is basically cosmetics. > nn =3D VTONULL_SMR(ap->a_vp); > if (__predict_true(nn !=3D NULL)) { > lvp =3D nn->null_lowervp; > @@ -855,6 +855,8 @@ null_lock(struct vop_lock1_args *ap) > * case by reacquiring correct lock in requested mode. > */ > if (VTONULL(ap->a_vp) =3D=3D NULL && error =3D=3D 0) { > + VOP_UNLOCK(lvp); > + > flags =3D ap->a_flags; > ap->a_flags &=3D ~LK_TYPE_MASK; > switch (flags & LK_TYPE_MASK) { > @@ -869,7 +871,6 @@ null_lock(struct vop_lock1_args *ap) > panic("Unsupported lock request %d\n", > flags); > } > - VOP_UNLOCK(lvp); > error =3D vop_stdlock(ap); > } > vdrop(lvp); This does not shorten the hold time in any capacity for real workloads because the vnode getting doomed from under you is a just a corner case you are not expected to run into. If someone is looking to microoptimize this, __predict true/false could be sprinkled through the entire thing. If someone is looking to create a speed up measurable in a microbenchmark, then nullfs vs vhold/vdrop business could be reworked so that locking always holds and unlocking always vdrops. Then any lock-protected VOP does not have to repeat the dance. A real-world win would come from implementing write-free lookup (modulo the last path component), like for zfs/ufs/tmpfs. It could work by lower filesystems opting it with a dedicated vop for lookups. Then you would do the lookup using the namecache entry from the lower fs and use the nullfs hash to get back to a nullfs vnode. Rinse & repeat until encountering a mount point or finding the final vnode.
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CAGudoHEKNSi8LJz9wFEP-BM27btZO9%2B-wq7LBeJ-vuHqy7KsPg>