Skip site navigation (1)Skip section navigation (2)
Date:      Wed, 6 Jun 2018 16:22:51 +0300
From:      Andriy Gapon <avg@FreeBSD.org>
To:        Warner Losh <imp@bsdimp.com>, Justin Hibbits <jhibbits@freebsd.org>
Cc:        src-committers <src-committers@freebsd.org>, svn-src-all@freebsd.org, svn-src-head@freebsd.org
Subject:   Re: svn commit: r334708 - head/sys/kern
Message-ID:  <600c9c66-fda6-de4c-3db7-0a79ef833f62@FreeBSD.org>
In-Reply-To: <CANCZdfq0E9u-_GiSdBrZWsH53pV%2Bp_cp1FESRzfDzepDZgkTcQ@mail.gmail.com>
References:  <201806061257.w56CvCwq089369@repo.freebsd.org> <CANCZdfq0E9u-_GiSdBrZWsH53pV%2Bp_cp1FESRzfDzepDZgkTcQ@mail.gmail.com>

next in thread | previous in thread | raw e-mail | index | archive | help
On 06/06/2018 16:02, Warner Losh wrote:
> 
> 
> On Wed, Jun 6, 2018 at 8:57 AM, Justin Hibbits <jhibbits@freebsd.org
> <mailto:jhibbits@freebsd.org>> wrote:
> 
>     Author: jhibbits
>     Date: Wed Jun  6 12:57:11 2018
>     New Revision: 334708
>     URL: https://svnweb.freebsd.org/changeset/base/334708
>     <https://svnweb.freebsd.org/changeset/base/334708>;
> 
>     Log:
>       Add a memory barrier after taking a reference on the vnode holdcnt in _vhold
> 
>       This is needed to avoid a race between the VNASSERT() below, and another
>       thread updating the VI_FREE flag, on weakly-ordered architectures.
> 
>       On a 72-thread POWER9, without this barrier a 'make -j72 buildworld' would
>       panic on the assert regularly.
> 
>       It may be possible to use a weaker barrier, and I'll investigate that once
>       all stability issues are worked out on POWER9.
> 
>     Modified:
>       head/sys/kern/vfs_subr.c
> 
>     Modified: head/sys/kern/vfs_subr.c
>     ==============================================================================
>     --- head/sys/kern/vfs_subr.c    Wed Jun  6 10:46:24 2018        (r334707)
>     +++ head/sys/kern/vfs_subr.c    Wed Jun  6 12:57:11 2018        (r334708)
>     @@ -2807,6 +2807,9 @@ _vhold(struct vnode *vp, bool locked)
>             CTR2(KTR_VFS, "%s: vp %p", __func__, vp);
>             if (!locked) {
>                     if (refcount_acquire_if_not_zero(&vp->v_holdcnt)) {
>     +#if !defined(__amd64__) && !defined(__i386__)
>     +                       mb();
>     +#endif
>                             VNASSERT((vp->v_iflag & VI_FREE) == 0, vp,
>                                 ("_vhold: vnode with holdcnt is free"));
>                             return;
> 
> 
> So why isn't the refcount_acquire() enough?

This is the wrong "acquire", "acquire" in a sense of "get" or "increment", not
in a sense of a memory barrier.



-- 
Andriy Gapon



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?600c9c66-fda6-de4c-3db7-0a79ef833f62>