Re: git: 09f925b57aeb - main - nullfs: Slightly reduce contention by reducing concurrent sections

On Mon, Oct 6, 2025 at 5:22 PM Olivier Certner <olce@freebsd.org> wrote:
>
> The branch main has been updated by olce:
>
> URL: https://cgit.FreeBSD.org/src/commit/?id=09f925b57aeb171318a9d54df500bf22b4cdd986
>
> 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 have
>     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 = NULL;
>
> +       vfs_smr_enter();
> +

This bit is basically cosmetics.

>         nn = VTONULL_SMR(ap->a_vp);
>         if (__predict_true(nn != NULL)) {
>                 lvp = 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) == NULL && error == 0) {
> +               VOP_UNLOCK(lvp);
> +
>                 flags = ap->a_flags;
>                 ap->a_flags &= ~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 = 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.