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 F2D38C433EF for ; Tue, 1 Feb 2022 18:33:16 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 1A5EC8D007A; Tue, 1 Feb 2022 13:33:16 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 12E688D006D; Tue, 1 Feb 2022 13:33:16 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id EE9918D007A; Tue, 1 Feb 2022 13:33:15 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0103.hostedemail.com [216.40.44.103]) by kanga.kvack.org (Postfix) with ESMTP id D7E868D006D for ; Tue, 1 Feb 2022 13:33:15 -0500 (EST) Received: from smtpin28.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay03.hostedemail.com (Postfix) with ESMTP id 6EC2D815A79A for ; Tue, 1 Feb 2022 18:33:15 +0000 (UTC) X-FDA: 79095058350.28.3ACC4CA Received: from mail-ej1-f43.google.com (mail-ej1-f43.google.com [209.85.218.43]) by imf01.hostedemail.com (Postfix) with ESMTP id 0162F4000C for ; Tue, 1 Feb 2022 18:33:14 +0000 (UTC) Received: by mail-ej1-f43.google.com with SMTP id jx6so57343209ejb.0 for ; Tue, 01 Feb 2022 10:33:14 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20210112; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=+g6PLOYAprOC0tmznRa4CnfNMzJ64s7o3yrfJR0yNSo=; b=HS44/SRDY7S67H6RxAurzppCSFvIh0Osj71xUDW2a5zZUyI5Hcu08OsL0ROVxDaIB3 Sfqt9b5AE5h9hppkg5CmuzzEkx+Wf2grMT6XsKliabH+p53VeF6sVdYNS0JOPBLQ26G/ wWv2S3lsXFtbMYZ2ZFM4w85Q59DHtNkZZxKMBQtfEcrn0SWNTugEqY0TXt9/dnYTwcuR z8vxI34y9D5oTEiEKfata5fIFYH77RnP02mG0y0UXWvZnbuWDbfy2rFVIFrp97Wj4hGa wcXICDS6MN4vWR54c+Ebj1a7awVUnC9FJp8mZlV4Foyv0oCqSnSsY2mGvFUDX7i9gbc5 2yBA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=+g6PLOYAprOC0tmznRa4CnfNMzJ64s7o3yrfJR0yNSo=; b=wqS/qIcg9mVNjM8PR2pALyxBHaLAXBHQP5jLavrvHvOssvL9lxAYy/WYgktLThK9jn U01rixj4xW3N8l1QyNZQLyiZIN3W4b9AmKzpccWuGqs4LLKMch1iWv8osPIJ6dx3/Mcu Mb2X4T6bZU9HWiqu6ezlgmRVW9rVxnZVPqeIBI29Yl12oLook/2D19MSDGtf2Prm8DEb r7mWD27GA6N0TM+IUqZS7mA6oPBtI5FD39aTHIBoDtgwxXSmV7w0uIVp09MdnHPjAfkw k8uKyShjDBjT7+P8Q3LrsKKEfOzmtlyFsHKTiKMkXdNWuAhxd6oOqWpQFaIxBunggnET owhQ== X-Gm-Message-State: AOAM531wAkJjXFKcd4A0ShGIsha2l2BfJsBxm7opvIr0HngGHIoZ7xvU VSLIdgmPGz6DJ67l4Zivf0AQMhWOqCde/PsE3wdKMQ== X-Google-Smtp-Source: ABdhPJx+rHqFGXeIOt8FzMC764v1w59ZK9SMShq4PJIyAXqvaJPsA/e6WPlsIzYqkh1l4VHGFdMUShMVBihsVo6Z5og= X-Received: by 2002:a17:907:9689:: with SMTP id hd9mr13302667ejc.240.1643740393524; Tue, 01 Feb 2022 10:33:13 -0800 (PST) MIME-Version: 1.0 References: <20220201092927.242254-1-jhubbard@nvidia.com> In-Reply-To: From: Will McVicker Date: Tue, 1 Feb 2022 10:32:57 -0800 Message-ID: Subject: Re: [PATCH] Revert mm/gup: small refactoring: simplify try_grab_page() To: John Hubbard Cc: Andrew Morton , LKML , linux-mm@kvack.org, Christoph Hellwig , Minchan Kim , Matthew Wilcox , Christian Borntraeger , Heiko Carstens , Vasily Gorbik , Linus Torvalds , stable Content-Type: text/plain; charset="UTF-8" X-Rspamd-Server: rspam06 X-Rspamd-Queue-Id: 0162F4000C X-Stat-Signature: efymgp9875hod4kwrng8d67xo1hoqjbn Authentication-Results: imf01.hostedemail.com; dkim=pass header.d=google.com header.s=20210112 header.b="HS44/SRD"; dmarc=pass (policy=reject) header.from=google.com; spf=pass (imf01.hostedemail.com: domain of willmcvicker@google.com designates 209.85.218.43 as permitted sender) smtp.mailfrom=willmcvicker@google.com X-Rspam-User: nil X-HE-Tag: 1643740394-106098 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, Feb 1, 2022 at 10:00 AM Will McVicker wrote: > > On Tue, Feb 1, 2022 at 1:29 AM John Hubbard wrote: > > > > This reverts commit 54d516b1d62ff8f17cee2da06e5e4706a0d00b8a > > > > That commit did a refactoring that effectively combined fast and slow > > gup paths (again). And that was again incorrect, for two reasons: > > > > a) Fast gup and slow gup get reference counts on pages in different ways > > and with different goals: see Linus' writeup in commit cd1adf1b63a1 > > ("Revert "mm/gup: remove try_get_page(), call try_get_compound_head() > > directly""), and > > > > b) try_grab_compound_head() also has a specific check for "FOLL_LONGTERM > > && !is_pinned(page)", that assumes that the caller can fall back to slow > > gup. This resulted in new failures, as recently report by Will McVicker > > [1]. > > > > But (a) has problems too, even though they may not have been reported > > yet. So just revert this. > > > > [1] https://lore.kernel.org/r/20220131203504.3458775-1-willmcvicker@google.com > > > > Fixes: 54d516b1d62f ("mm/gup: small refactoring: simplify try_grab_page()") > > Cc: Christoph Hellwig > > Cc: Will McVicker > > Cc: Minchan Kim > > Cc: Matthew Wilcox > > Cc: Christian Borntraeger > > Cc: Heiko Carstens > > Cc: Vasily Gorbik > > Cc: Linus Torvalds > > Signed-off-by: John Hubbard > > --- > > mm/gup.c | 35 ++++++++++++++++++++++++++++++----- > > 1 file changed, 30 insertions(+), 5 deletions(-) > > > > diff --git a/mm/gup.c b/mm/gup.c > > index f0af462ac1e2..a9d4d724aef7 100644 > > --- a/mm/gup.c > > +++ b/mm/gup.c > > @@ -124,8 +124,8 @@ static inline struct page *try_get_compound_head(struct page *page, int refs) > > * considered failure, and furthermore, a likely bug in the caller, so a warning > > * is also emitted. > > */ > > -struct page *try_grab_compound_head(struct page *page, > > - int refs, unsigned int flags) > > +__maybe_unused struct page *try_grab_compound_head(struct page *page, > > + int refs, unsigned int flags) > > { > > if (flags & FOLL_GET) > > return try_get_compound_head(page, refs); > > @@ -208,10 +208,35 @@ static void put_compound_head(struct page *page, int refs, unsigned int flags) > > */ > > bool __must_check try_grab_page(struct page *page, unsigned int flags) > > { > > - if (!(flags & (FOLL_GET | FOLL_PIN))) > > - return true; > > + WARN_ON_ONCE((flags & (FOLL_GET | FOLL_PIN)) == (FOLL_GET | FOLL_PIN)); > > > > - return try_grab_compound_head(page, 1, flags); > > + if (flags & FOLL_GET) > > + return try_get_page(page); > > + else if (flags & FOLL_PIN) { > > + int refs = 1; > > + > > + page = compound_head(page); > > + > > + if (WARN_ON_ONCE(page_ref_count(page) <= 0)) > > + return false; > > + > > + if (hpage_pincount_available(page)) > > + hpage_pincount_add(page, 1); > > + else > > + refs = GUP_PIN_COUNTING_BIAS; > > + > > + /* > > + * Similar to try_grab_compound_head(): even if using the > > + * hpage_pincount_add/_sub() routines, be sure to > > + * *also* increment the normal page refcount field at least > > + * once, so that the page really is pinned. > > + */ > > + page_ref_add(page, refs); > > + > > + mod_node_page_state(page_pgdat(page), NR_FOLL_PIN_ACQUIRED, 1); > > + } > > + > > + return true; > > } > > > > /** > > > > base-commit: 26291c54e111ff6ba87a164d85d4a4e134b7315c > > -- > > 2.35.1 > > > > Thanks John! I verified this works on the Pixel 6 with the 5.15 kernel > for my camera use-case. Free free to include: > > Tested-by: Will McVicker > > Thanks, > Will And just so we don't miss this, I'd also like to request this be pulled into the 5.15 stable branch please. Cc: stable@vger.kernel.org # 5.15 Thanks, Will