From owner-freebsd-current@freebsd.org Thu May 14 15:26:47 2020 Return-Path: Delivered-To: freebsd-current@mailman.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.nyi.freebsd.org (Postfix) with ESMTP id D3E1A2F49A3 for ; Thu, 14 May 2020 15:26:47 +0000 (UTC) (envelope-from markjdb@gmail.com) Received: from mail-qv1-xf42.google.com (mail-qv1-xf42.google.com [IPv6:2607:f8b0:4864:20::f42]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) server-signature RSA-PSS (4096 bits) client-signature RSA-PSS (2048 bits) client-digest SHA256) (Client CN "smtp.gmail.com", Issuer "GTS CA 1O1" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 49NFkv5Df2z4SKv; Thu, 14 May 2020 15:26:47 +0000 (UTC) (envelope-from markjdb@gmail.com) Received: by mail-qv1-xf42.google.com with SMTP id z9so1836213qvi.12; Thu, 14 May 2020 08:26:47 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=sender:date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to; bh=YtioxKq85PWeY7Ea5uAEcaCAuvzeHlH1tmrypRinKFQ=; b=WNMGeM7/KQxqo8gmRiPx+j//s1fWV0yetSCAnSp0y0RP2uwY0LKorX7s9PdNPWgjCt Ww+s7zpKbWJk6SvLOn7+59yQ7TRcxHePoL2ViMa4EIUlO4lNB1X2HPGZP3tRET5MODJp DVxeZ32cElHYekmGrgiIer2LyrK4wjGX3NfZkHJo56TXnINVQ6fPEuyWTh6UEAhzC1qn eUoBkI2uOsMul0UA23tlGia+rWBBc6AJ0wcfSxEsAqwLwNkwAkVFzH75IExydD4jStm7 HLU+griiW75I0TJCfSW1rzm3XPOpgZjL2u81bqtkvNjviJzxTJ1zcZzIY1uNHPyezocG hixw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:sender:date:from:to:cc:subject:message-id :references:mime-version:content-disposition:in-reply-to; bh=YtioxKq85PWeY7Ea5uAEcaCAuvzeHlH1tmrypRinKFQ=; b=o82jn5fEP/YO3N8nUEH8HmTvnoGnWOIgX9XbfTR0DWNUucFcmDb7/oUyCW8C8vZ9x6 VI8IdmI0ECDFyxOe9Xh9fauA2cdOCNWVm1P6B+z0ykYa3Yug92I/MCJ8ZIyKdl177dAo BSeAx9RoB9pNtTwm3LSctjLwiXRu/Z3TZVxsrjWC7EMFQgoIYnSWEbgSv2PkWFhgFKKW y2bXqKKK0wpInycEoeewJren0rfeZu5LmIvcJgRlfBjyvHT8veQ7XTy18fFqnuarU3yi O/RTrP1Q+DHHAUi/Cuo0kuIsfSpWHh20wSfCo9swX4jT1hD4/T3QPoL7MSpfSz1o0B7r kyZQ== X-Gm-Message-State: AOAM530hEAE6ArxnJvzuCjDaTd0424zUlWHYyNuJJjeNBTHHJyThJONS URG7ZDa4YVm4srWwmx8qDEbzNCag X-Google-Smtp-Source: ABdhPJwTVDETUCl4J1q3fpXL1GQLH5i7uBfw1azdn5OPTKsR9rAVrDMpfENutYwYxcaHTkV1Fi/MrA== X-Received: by 2002:ad4:4c92:: with SMTP id bs18mr5427873qvb.67.1589470006268; Thu, 14 May 2020 08:26:46 -0700 (PDT) Received: from raichu (toroon0560w-lp130-15-184-144-87-103.dsl.bell.ca. [184.144.87.103]) by smtp.gmail.com with ESMTPSA id x19sm2667229qkb.136.2020.05.14.08.26.44 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 14 May 2020 08:26:45 -0700 (PDT) Sender: Mark Johnston Date: Thu, 14 May 2020 11:26:43 -0400 From: Mark Johnston To: Andriy Gapon Cc: Bryan Drewery , freebsd-current@freebsd.org Subject: Re: zfs deadlock on r360452 relating to busy vm page Message-ID: <20200514152643.GC4268@raichu> References: <2bdc8563-283b-32cc-8a1a-85ff52aca99e@FreeBSD.org> <0e9cceba-84d0-ec4f-8784-36703452201d@FreeBSD.org> <889cb93b-85c7-3ec4-4ccf-5fb56ec38fa5@FreeBSD.org> <20200513144257.GA24239@raichu> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: X-Rspamd-Queue-Id: 49NFkv5Df2z4SKv X-Spamd-Bar: ----- Authentication-Results: mx1.freebsd.org; none X-Spamd-Result: default: False [-5.98 / 15.00]; NEURAL_HAM_MEDIUM(-0.98)[-0.983,0]; NEURAL_HAM_LONG(-1.00)[-1.000,0]; REPLY(-4.00)[] X-BeenThere: freebsd-current@freebsd.org X-Mailman-Version: 2.1.33 Precedence: list List-Id: Discussions about the use of FreeBSD-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 14 May 2020 15:26:47 -0000 On Thu, May 14, 2020 at 02:16:15PM +0300, Andriy Gapon wrote: > On 13/05/2020 17:42, Mark Johnston wrote: > > On Wed, May 13, 2020 at 10:45:24AM +0300, Andriy Gapon wrote: > >> On 13/05/2020 10:35, Andriy Gapon wrote: > >>> In r329363 I re-worked zfs_getpages and introduced range locking to it. > >>> At the time I believed that it was safe and maybe it was, please see the commit > >>> message. > >>> There, indeed, have been many performance / concurrency improvements to the VM > >>> system and r358443 is one of them. > >> > >> Thinking more about it, it could be r352176. > >> I think that vm_page_grab_valid (and later vm_page_grab_valid_unlocked) are not > >> equivalent to the code that they replaced. > >> The original code would check valid field before any locking and it would > >> attempt any locking / busing if a page is invalid. The object was required to > >> be locked though. > >> The new code tries to busy the page in any case. > >> > >>> I am not sure how to resolve the problem best. Maybe someone who knows the > >>> latest VM code better than me can comment on my assumptions stated in the commit > >>> message. > > > > The general trend has been to use the page busy lock as the single point > > of synchronization for per-page state. As you noted, updates to the > > valid bits were previously interlocked by the object lock, but this is > > coarse-grained and hurts concurrency. I think you are right that the > > range locking in getpages was ok before the recent change, but it seems > > preferable to try and address this in ZFS. > > > >>> In illumos (and, I think, in OpenZFS/ZoL) they don't have the range locking in > >>> this corner of the code because of a similar deadlock a long time ago. > > > > Do they just not implement readahead? > > I think so, but not 100% sure. > I recall seeing a comment in illumos code that they do not care about read-ahead > because there is ZFS prefetch and the data will be cached in ARC. That makes > sense from the I/O point of view, but it does not help with page faults. > > > Can you explain exactly what the > > range lock accomplishes here - is it entirely to ensure that znode block > > size remains stable? > > As far as I can recall, this is the reason indeed. It seems to me that zfs_getpages() could use a non-blocking rangelock_enter() operation to avoid the deadlock. The ZFS rangelock implementation doesn't have one, but it is easy to add. I'm not able to trigger the deadlock with this patch: https://reviews.freebsd.org/D24839