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 791B5C433EF for ; Tue, 1 Feb 2022 18:00:25 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id E20728D0078; Tue, 1 Feb 2022 13:00:24 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id DA9328D0077; Tue, 1 Feb 2022 13:00:24 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id C22B38D0078; Tue, 1 Feb 2022 13:00:24 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (relay032.a.hostedemail.com [64.99.140.32]) by kanga.kvack.org (Postfix) with ESMTP id ACB368D0077 for ; Tue, 1 Feb 2022 13:00:24 -0500 (EST) Received: from smtpin01.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay06.hostedemail.com (Postfix) with ESMTP id 5F2792257C for ; Tue, 1 Feb 2022 18:00:24 +0000 (UTC) X-FDA: 79094975568.01.A42DBFA Received: from mail-ej1-f50.google.com (mail-ej1-f50.google.com [209.85.218.50]) by imf10.hostedemail.com (Postfix) with ESMTP id D9353C0006 for ; Tue, 1 Feb 2022 18:00:23 +0000 (UTC) Received: by mail-ej1-f50.google.com with SMTP id s13so56819738ejy.3 for ; Tue, 01 Feb 2022 10:00:23 -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=zYWw5fKgw0+fWLBM5WcyalO3iWi3rJrQY6A3mcVhAdo=; b=TKTqg/+Xu+IFbxw6/1tRT+p0hRJIx3p6IA2gs5gSCrTEoiqBRKrTi4/3XBam3+FSnr SlQ32EqBVY97MehrYwFxhimvO6VnkqMTL331KfZdJPMHotjgNZBCRjyV6f19K6IQUfLO eGH2OiMr3/pAnhvu31AwmpMg8cVig9tjIcaY9lQZgZCunSnf1nMLIWsBH22/EPuVG6Dv zq+Pk/P2ZGibGTEwv7jcTkjZZoZ2UscoHNlnIHL7BA2GmN52BCeGJquZuXr8Sp6rErDW HLQsD24AhpZ2g7Rs1GxfBY34jc2aLtWWlcH/F3pCvwxv3gxCTv2r9yx1ds0ohbojLtYv xBig== 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=zYWw5fKgw0+fWLBM5WcyalO3iWi3rJrQY6A3mcVhAdo=; b=rBAw0Tu5Eo4J0pDOB5XfO8v4TbKTLQHf/uWvW0b+1pzKfz+6kWxgqQfui45wpqsXMj a8Wfuze3S1QQJ9DwcUrm/ZZk/TBCiFGgOkNyObaYY+wWEY9Ys8w88fF1L/os4bj2UMrG 39rqNR7pWE2Rv++90M6xtSxIqVeFpw4aotWJjM6J5Dh9IuabFHO0qz47dxVV50ILqqJQ rq2/TpDne2CiYRS7z2bYvTxcldd7W4QN8B1glg2pp8jEVaC+EML4zbIzBQAMv8zGy0lu EVAuLXQhoBaq3bhA7A1+ag2QTtv+8BQVaYi+DfywekM0bbXpjMsb32MdlPvvIwdjY9tz Wg9Q== X-Gm-Message-State: AOAM533awv40YUIfcbsAwhfbAerhatG7IhFuKejmIKa11CQ2iICH9R6D L2CZsEWjdHQCpt0WFCWRPgv7L6HRQ5QSykKzRPcgZA== X-Google-Smtp-Source: ABdhPJzwVOy/dYFlWpX4/fHn02mGi/7KQNhjiJr0zXz2v5On9RsoqgNga+ar2ZZxn1BwjKpzLl51iGMaqN0UNfvIXgw= X-Received: by 2002:a17:907:608f:: with SMTP id ht15mr22076700ejc.484.1643738422292; Tue, 01 Feb 2022 10:00:22 -0800 (PST) MIME-Version: 1.0 References: <20220201092927.242254-1-jhubbard@nvidia.com> In-Reply-To: <20220201092927.242254-1-jhubbard@nvidia.com> From: Will McVicker Date: Tue, 1 Feb 2022 10:00:06 -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 Content-Type: text/plain; charset="UTF-8" Authentication-Results: imf10.hostedemail.com; dkim=pass header.d=google.com header.s=20210112 header.b="TKTqg/+X"; spf=pass (imf10.hostedemail.com: domain of willmcvicker@google.com designates 209.85.218.50 as permitted sender) smtp.mailfrom=willmcvicker@google.com; dmarc=pass (policy=reject) header.from=google.com X-Rspam-User: nil X-Rspamd-Queue-Id: D9353C0006 X-Stat-Signature: q9h3713upg387xu8gubgpw1ttzffgj7t X-Rspamd-Server: rspam12 X-HE-Tag: 1643738423-768255 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 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