From owner-svn-src-head@freebsd.org Thu Jul 19 19:41:39 2018 Return-Path: Delivered-To: svn-src-head@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 A6BC61047349; Thu, 19 Jul 2018 19:41:39 +0000 (UTC) (envelope-from chmeeedalf@gmail.com) Received: from mail-lj1-f174.google.com (mail-lj1-f174.google.com [209.85.208.174]) (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 2050876CC8; Thu, 19 Jul 2018 19:41:39 +0000 (UTC) (envelope-from chmeeedalf@gmail.com) Received: by mail-lj1-f174.google.com with SMTP id 203-v6so9068659ljj.13; Thu, 19 Jul 2018 12:41:39 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=tzQ+xbQI4ObjJQwbYvJHcs3SJ7rX19/ECeyIrvxHRn8=; b=rcXu5Db4axaftS5PXMv8Y07Xx2wofdDMrDjYZsTNIk4/0Gk+yV6X8mgVOVVXl5n70J vUvNOCLYr2flN70/SRRypT5H5Uj3ZFNCWhtW5vxL9p4XiCemMgwPy/HJEYBlQt5aFQ55 rUFIL3ILPXQZi+QiwPr4fIHFBIbsW0vX8LCQposihlz/L2EG/Cx1az7InIfB4e/t2zzF 7qMfICxFcF6cPXU1I5oKUP9oX0uFiKjzt4iMg42VXlWITKB/9y4Ee8jP6F5W6DRRygmt 8HlV+ZMj8Kc2IkD4Gq/p7FuhgVjaNJb6WV2BXcXoEm2SBzm8mxYeG0TmExCyxl8JqI9h sYKA== X-Gm-Message-State: AOUpUlEoTMcbL3cZo9M5V1YPCml++awYnryc8CaIMQNhCVFCxdMpgdvx 4s6KjWVyFKZIsrLF/UjAyuEuSh4y8mM2F2zdX1/9pg== X-Google-Smtp-Source: AAOMgpftdvF5RW4rJZfVraOcQ09gnGYmN57QV8e9Tx6Km2460G08wKy5QaIcwEMU+7ALG2+pLkkiLQ6Ao2UxrhUOK/Q= X-Received: by 2002:a2e:944:: with SMTP id 65-v6mr8600895ljj.30.1532028922149; Thu, 19 Jul 2018 12:35:22 -0700 (PDT) MIME-Version: 1.0 References: <201806061257.w56CvCwq089369@repo.freebsd.org> <20180606140311.GU2450@kib.kiev.ua> <20180608033242.GA54099@pesky> <20180608173755.GJ2450@kib.kiev.ua> <20180608183010.GC65388@pesky> <20180608183732.GK2450@kib.kiev.ua> <0b128417-7107-5090-e65a-afa94fd1aed6@FreeBSD.org> In-Reply-To: <0b128417-7107-5090-e65a-afa94fd1aed6@FreeBSD.org> From: Justin Hibbits Date: Thu, 19 Jul 2018 14:35:08 -0500 Message-ID: Subject: Re: svn commit: r334708 - head/sys/kern To: Bryan Drewery Cc: Konstantin Belousov , Mark Johnston , Ryan Libby , mjguzik@gmail.com, src-committers , svn-src-all@freebsd.org, svn-src-head@freebsd.org Content-Type: text/plain; charset="UTF-8" X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.27 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 19 Jul 2018 19:41:39 -0000 To the best of my understanding, the underlying race condition within the assert has not been solved. I've worked around it for now by simply removing the assert, but that's just a workaround to keep my development going. - Justin On Thu, Jul 19, 2018 at 2:09 PM Bryan Drewery wrote: > > Did this issue get resolved? > > On 6/8/2018 11:37 AM, Konstantin Belousov wrote: > > On Fri, Jun 08, 2018 at 02:30:10PM -0400, Mark Johnston wrote: > >> On Fri, Jun 08, 2018 at 08:37:55PM +0300, Konstantin Belousov wrote: > >>> On Thu, Jun 07, 2018 at 11:02:29PM -0700, Ryan Libby wrote: > >>>> On Thu, Jun 7, 2018 at 10:03 PM, Mateusz Guzik wrote: > >>>>> Checking it without any locks is perfectly valid in this case. It is done > >>>>> after v_holdcnt gets bumped from a non-zero value. So at that time it > >>>>> is at least two. Of course that result is stale as an arbitrary number of > >>>>> other threads could have bumped and dropped the ref past that point. > >>>>> The minimum value is 1 since we hold the ref. But this means the > >>>>> vnode must not be on the free list and that's what the assertion is > >>>>> verifying. > >>>>> > >>>>> The problem is indeed lack of ordering against the code clearing the > >>>>> flag for the case where 2 threads to vhold and one does the 0->1 > >>>>> transition. > >>>>> > >>>>> That said, the fence is required for the assertion to work. > >>>>> > >>>> > >>>> Yeah, I agree with this logic. What I mean is that reordering between > >>>> v_holdcnt 0->1 and v_iflag is normally settled by the release and > >>>> acquisition of the vnode interlock, which we are supposed to hold for > >>>> v_*i*flag. A quick scan seems to show all of the checks of VI_FREE that > >>>> are not asserts do hold the vnode interlock. So, I'm just saying that I > >>>> don't think the possible reordering affects them. > >>> But do we know that only VI_FREE checks are affected ? > >>> > >>> My concern is that users of _vhold() rely on seeing up to date state of the > >>> vnode, and VI_FREE is only an example of the problem. Most likely, the > >>> code which fetched the vnode pointer before _vhold() call, should guarantee > >>> visibility. > >> > >> Wouldn't this be a problem only if we permit lockless accesses of vnode > >> state outside of _vhold() and other vnode subroutines? The current > >> protocol requires that the interlock be held, and this synchronizes with > >> code which performs 0->1 and 1->0 transitions of the hold count. If this > >> requirement is relaxed in the future, then fences would indeed be > >> needed. > > > > I do not claim that my concern is a real problem. I stated it as a > > thing to look at when deciding whether the fences should be added > > (unconditionally ?). > > > > If you argument is that the only current lock-less protocol for the > > struct vnode state is the v_holdcnt transitions for > 1, then I can > > agree with it. > > > > > -- > Regards, > Bryan Drewery >