From owner-svn-src-all@freebsd.org Wed Jun 6 13:21:53 2018 Return-Path: Delivered-To: svn-src-all@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 429B2FF5D2A; Wed, 6 Jun 2018 13:21:53 +0000 (UTC) (envelope-from chmeeedalf@gmail.com) Received: from mail-lf0-x241.google.com (mail-lf0-x241.google.com [IPv6:2a00:1450:4010:c07::241]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (Client CN "smtp.gmail.com", Issuer "Google Internet Authority G2" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id A959973DDE; Wed, 6 Jun 2018 13:21:52 +0000 (UTC) (envelope-from chmeeedalf@gmail.com) Received: by mail-lf0-x241.google.com with SMTP id t134-v6so9082487lff.6; Wed, 06 Jun 2018 06:21:52 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:sender:in-reply-to:references:from:date:message-id :subject:to:cc; bh=+hJvypAT4A0EbGhClTw2TYADVRTtwXAnYKVACfHx73A=; b=l3tDYLrKjsJ62m21QCGnLSySf3DSnUjbNmKKw++gbfUrzinv+/Sr/UXt0LOdv+AFBh 2vFpsrd+lpxAgnb6uAWiCKCwZrlBI7GzL+ZONs4N7EdLAHYFfvmZtWqBgP5SI9fmTRuU 7HW1c4WpDpCeVgGzcH540pn/eWuqxRzk8Vm09LjjHwH6gkbLdQh0lCc5q57mN1k3Iueg hjdYYUB8zoP5jWtzeG2xcMAJSJaAccjFWs9iWYQ2NBjqz6KRRCbwTZN241aq9YZ5FFje upmlfjy8zlV/xCSTZXbC4tqrEyNvp6AdlNfCvPWwYeLkfkIf6ZN3TpyLYUHlACxUAwE6 DtQA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:sender:in-reply-to:references:from :date:message-id:subject:to:cc; bh=+hJvypAT4A0EbGhClTw2TYADVRTtwXAnYKVACfHx73A=; b=Grj0/XfYXj0DOrFxJTUISl1Kizt3pqvRtBUP0yeIpYtk7vGELL9QhZ0cq3+V8dNZjM t8mf0BKSfn+xbD+8wKWvZb2/fm8y5N884t59kfanF8Jo1iQwiduFw3rQAMh1VnzXYJ5D gwFXqwib4/DmYtsLx0Jeg8Z7SspUmiS0tFiGu6uzJORC3lJcORaxkBPkruP5RIQptGpF C4Gehmcg+Qvgp9/5S8Lqrgh0s4LAN8hvg1fXEj15yuaUcpTiPfdrBA9nbMWifDUXIvj9 ijTnoUHrBNekb83teO8LfcFBn3768U5B+qHVxA354Cyc0kXGLWXdkp4njfR6FWLcgnYI Jt4Q== X-Gm-Message-State: APt69E1nKQEXmaElZY1sMTtBFzqnyBXOuSv0LX7fCgKMyrffVawCBIlr DIvc01RZgzrDaKNzTm1rffii7FBwMv3g+ywUTxI= X-Google-Smtp-Source: ADUXVKKSqOlkK9deZ3UOYSgPLxtKbc4UxnOS0uyuLiZpyRYprxv94SddutVKFI+X+V7OGv6LCXGBo6JaEu+5ms3eRwg= X-Received: by 2002:a2e:8605:: with SMTP id a5-v6mr2075853lji.43.1528291310541; Wed, 06 Jun 2018 06:21:50 -0700 (PDT) MIME-Version: 1.0 Sender: chmeeedalf@gmail.com Received: by 2002:a2e:1702:0:0:0:0:0 with HTTP; Wed, 6 Jun 2018 06:21:49 -0700 (PDT) In-Reply-To: References: <201806061257.w56CvCwq089369@repo.freebsd.org> From: Justin Hibbits Date: Wed, 6 Jun 2018 09:21:49 -0400 X-Google-Sender-Auth: XtfUlyrqa-bZgkiF_4K2An84IpA Message-ID: Subject: Re: svn commit: r334708 - head/sys/kern To: Warner Losh Cc: src-committers , svn-src-all@freebsd.org, svn-src-head@freebsd.org Content-Type: text/plain; charset="UTF-8" X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.26 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 06 Jun 2018 13:21:53 -0000 On Wed, Jun 6, 2018 at 9:02 AM, Warner Losh wrote: > > > On Wed, Jun 6, 2018 at 8:57 AM, Justin Hibbits wrote: >> >> Author: jhibbits >> Date: Wed Jun 6 12:57:11 2018 >> New Revision: 334708 >> URL: 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? > > Warner I'm not entirely sure. I had never seen this before, it's only cropped up on my POWER9 system. The refcount_acquire doesn't include a memory barrier, and mjg is reluctant to add one, since most refcount users don't care about ordering. Adding the memory barrier appeases the VNASSERT(). It may only be necessary for the VNASSERT(), and may not be strictly necessary for proper operation, but I think it's safest this way, until better performance metrics can be done. - Justin