svn commit: r367631 - in head/sys: kern sys

[Konstantin Belousov]

On Fri, Nov 13, 2020 at 08:24:30AM -0800, Conrad Meyer wrote:
> Hi Konstantin,
> 
> On Fri, Nov 13, 2020 at 1:32 AM Konstantin Belousov <kib at freebsd.org> wrote:
> >
> > Author: kib
> > Date: Fri Nov 13 09:31:57 2020
> > New Revision: 367631
> > URL: https://svnweb.freebsd.org/changeset/base/367631
> >
> > Log:
> >   Implement vn_lock_pair().
> >
> > Modified: head/sys/kern/vfs_vnops.c
> > ==============================================================================
> > --- head/sys/kern/vfs_vnops.c   Fri Nov 13 02:05:45 2020        (r367630)
> > +++ head/sys/kern/vfs_vnops.c   Fri Nov 13 09:31:57 2020        (r367631)
> > @@ -3317,4 +3325,92 @@ vn_fallocate(struct file *fp, off_t offset, off_t len,
> > ...
> > +
> > +static void
> > +vn_lock_pair_pause(const char *wmesg)
> > +{
> > +       atomic_add_long(&vn_lock_pair_pause_cnt, 1);
> > +       pause(wmesg, prng32_bounded(hz / 10));
> > +}
> 
> This function is called when the try-lock of the second lock in the
> pair (either order) fails.  The back-off period is up to 100ms,
> expected average 50ms.  That seems really high?
It is called only in deadlock avoidance case, where we already have to drop
to slow path for other reasons (this is coming in the patch that I hope to
commit today).

My selection of numbers were driven by more or less realistic estimates
of the vnode lock ownership time for sync io on very busy HDD, there was
a short discussion of it with Mark in review.

> 
> Separately: prng32_bounded() may return 0, which is transparently
> converted to the equivalent of 1 by pause_sbt(9).  This means a 1 tick
> pause is marginally more likely than any other possible duration.  It
> probably doesn't matter.
I do not quite understand the point.  Do you want the 0->1 conversion
be explicit there ?  Or something else ?