From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id 6E332C433EF for ; Sat, 8 Jan 2022 01:37:14 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id B83EF6B0072; Fri, 7 Jan 2022 20:37:13 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id B32BC6B0073; Fri, 7 Jan 2022 20:37:13 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id A230D6B0074; Fri, 7 Jan 2022 20:37:13 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0102.hostedemail.com [216.40.44.102]) by kanga.kvack.org (Postfix) with ESMTP id 940526B0072 for ; Fri, 7 Jan 2022 20:37:13 -0500 (EST) Received: from smtpin21.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay04.hostedemail.com (Postfix) with ESMTP id 329BA97319 for ; Sat, 8 Jan 2022 01:37:13 +0000 (UTC) X-FDA: 79005406746.21.2B82CD4 Received: from casper.infradead.org (casper.infradead.org [90.155.50.34]) by imf07.hostedemail.com (Postfix) with ESMTP id 664B440006 for ; Sat, 8 Jan 2022 01:37:12 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=infradead.org; s=casper.20170209; h=In-Reply-To:Content-Type:MIME-Version: References:Message-ID:Subject:Cc:To:From:Date:Sender:Reply-To: Content-Transfer-Encoding:Content-ID:Content-Description; bh=jWDdJG1OSoriMhq6iJ5wsf/+w8u/4OhRAQfK3KUWIOY=; b=AcChi+T+pL4mjssQuot0+00IIu dXZJcsqWEIQz2xMo6L5DX1SSc3Dz8UpEQBn9RfILOBmxcW36WOXSfXqDrSuII72I7Sg14VrYuNWAZ S9cQ93WqwQU5W3Tk7TIQk9CES/DJuszGx1l9X4yB6KLjVqb6RF81PITcoJFW55xlRq1z29LosWYo6 5d9558zxzvr1lpiEacbOnkmDsthZPHy/GgpTlAtXVtvD6vkULluBgzLFrNjiC5p8c7obnt38jAv2s 3dM8R7p14gSwX5n4ZLMo1fKPd6W8LFg1RemoHD4zdrQejh8LwKpcBkcwfTYK3VreiO+E1FT/kQF1k GAGrDnDw==; Received: from willy by casper.infradead.org with local (Exim 4.94.2 #2 (Red Hat Linux)) id 1n60fN-000KYe-Sr; Sat, 08 Jan 2022 01:37:10 +0000 Date: Sat, 8 Jan 2022 01:37:09 +0000 From: Matthew Wilcox To: John Hubbard Cc: linux-mm@kvack.org, Andrew Morton Subject: Re: [PATCH 05/17] gup: Add try_get_folio() Message-ID: References: <20220102215729.2943705-1-willy@infradead.org> <20220102215729.2943705-6-willy@infradead.org> <3ac8af50-dff6-4a0f-dba6-8b8fe5f611d4@nvidia.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <3ac8af50-dff6-4a0f-dba6-8b8fe5f611d4@nvidia.com> X-Stat-Signature: uyenf1at7amfo89tgny8wjkezx8wpqnb X-Rspamd-Server: rspam01 X-Rspamd-Queue-Id: 664B440006 Authentication-Results: imf07.hostedemail.com; dkim=pass header.d=infradead.org header.s=casper.20170209 header.b=AcChi+T+; spf=none (imf07.hostedemail.com: domain of willy@infradead.org has no SPF policy when checking 90.155.50.34) smtp.mailfrom=willy@infradead.org; dmarc=none X-HE-Tag: 1641605832-980820 X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: On Tue, Jan 04, 2022 at 05:25:07PM -0800, John Hubbard wrote: > On 1/2/22 13:57, Matthew Wilcox (Oracle) wrote: > > + folio = page_folio(page); > > + if (WARN_ON_ONCE(folio_ref_count(folio) < 0)) > > return NULL; > > - if (unlikely(!page_cache_add_speculative(head, refs))) > > + if (unlikely(!folio_ref_try_add_rcu(folio, refs))) > > I'm a little lost about the meaning and intended use of the _rcu aspects > of folio_ref_try_add_rcu() here. For example, try_get_folio() does not > require that callers are in an rcu read section, right? This is probably > just a documentation question, sorry if it's obvious--I wasn't able to > work it out on my own. page_cache_add_speculative() always assumed that you were at least as protected as RCU. Quoting v5.4's pagemap.h: * speculatively take a reference to a page. * If the page is free (_refcount == 0), then _refcount is untouched, and 0 * is returned. Otherwise, _refcount is incremented by 1 and 1 is returned. * * This function must be called inside the same rcu_read_lock() section as has * been used to lookup the page in the pagecache radix-tree (or page table): * this allows allocators to use a synchronize_rcu() to stabilize _refcount. ... so is it the addition of "rcu" in the name that's scared you? If so, that seems like a good thing, because previously you were using page_cache_add_speculative() without realising that RCU protection was required. I think there is sufficient protection; either we have interrupts disabled in gup-fast (which prevents RCU from finishing a grace period), or we have the mmap_sem held in gup-slow (which prevents pages from being removed from the page tables). But maybe I've missed something?