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 E8FB2C433F5 for ; Mon, 31 Jan 2022 21:37:28 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id B97BD6B0116; Mon, 31 Jan 2022 16:37:27 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id B20426B0117; Mon, 31 Jan 2022 16:37:27 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 99B3F8D0024; Mon, 31 Jan 2022 16:37:27 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0141.hostedemail.com [216.40.44.141]) by kanga.kvack.org (Postfix) with ESMTP id 8816C6B0116 for ; Mon, 31 Jan 2022 16:37:27 -0500 (EST) Received: from smtpin22.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay03.hostedemail.com (Postfix) with ESMTP id 4531882EB780 for ; Mon, 31 Jan 2022 21:37:27 +0000 (UTC) X-FDA: 79091893734.22.9C4AE59 Received: from mail-ej1-f43.google.com (mail-ej1-f43.google.com [209.85.218.43]) by imf22.hostedemail.com (Postfix) with ESMTP id E670EC0003 for ; Mon, 31 Jan 2022 21:37:26 +0000 (UTC) Received: by mail-ej1-f43.google.com with SMTP id ah7so47073750ejc.4 for ; Mon, 31 Jan 2022 13:37:26 -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=9eAtHYfw+/lT8OWoIpphlqOzT4nSqWECsjs9ahLjkbw=; b=s/Bxs4zdolC7/C0cnjeR6DBgNzcmI/HhxGvzNgg0QUH7B/R3JRWVYyUi4i8NRwgwtb bOfBerN+MxfcWU7s+3cir8msw2qpYcRdcmcEebZA5uXNeY0HWy6osbR7Xxi2tMHNdchL Z1pof6grX5bhk0/Mi4cp6PgYhHh7Tk7TrWFD7qYu4rDrcV9HWdLB9FdfNIcshtDy/ONa VZ48tGWZptTW8mYOyiB2+Rage6XmCIksZ3ZczLQcrmhAMPImixlcU4L7W0e+Fm7ppo2+ EvTMwGq26okhfqJ7wGne5ZshpXZIlDNQZZQmbIR/GFAb6j8SHP1pAYC+pAfemPaOYHHy kOBg== 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=9eAtHYfw+/lT8OWoIpphlqOzT4nSqWECsjs9ahLjkbw=; b=ug/fhal/4jaBs/39JtPIyECahM5ASolvxcesGg9Ll2QKQBOTkZT5iVQCNEE9crEeWc GqHM3K2yUHGJuNjALwwtvXEb3sarzX1PCd9pTBerMBoOYCe+WtETC2EKmFMYykxL+e9o 6wspw0c4b5+DGf2PRwbKok/lVjsh+dM2RApKpckZr8uLnPatS/k16kcU8Q2Q1EXQCAs8 0uKnBFTF2wSQ11HqNLc3v8jxqlf/ULFStQaxUl0KATOSiBfTjgaAOosYXpDI2ORI28zf LThXyl1GRXYrPLffoknKmE2M1ob49OE8rPRytf5/fSx1qBvR2tpINY4aGIle1gqRGHXf Y39g== X-Gm-Message-State: AOAM532jxWFnbLd2ETDbeVPYxulDmv3do46hfHJEkU0wCZ3px3QWLzCV 0Fe+yA+zZ754HwMshIq9sz5Ob2S19Un5ksNMUfZ3Mw== X-Google-Smtp-Source: ABdhPJyAJ8VRWefS1wIU5l6oqnj1HmOvBTN0xWCGW1WwFsJFSSfT9GVPhOVM+FX4T8PVHaSxwhNSU5D3rp1VONSQZT0= X-Received: by 2002:a17:907:608f:: with SMTP id ht15mr18617793ejc.484.1643665045448; Mon, 31 Jan 2022 13:37:25 -0800 (PST) MIME-Version: 1.0 References: <20220131203504.3458775-1-willmcvicker@google.com> <27e5f98a-0709-1a80-18ed-e4ccaaf39fe6@nvidia.com> In-Reply-To: <27e5f98a-0709-1a80-18ed-e4ccaaf39fe6@nvidia.com> From: Will McVicker Date: Mon, 31 Jan 2022 13:37:09 -0800 Message-ID: Subject: Re: [PATCH v1 1/1] mm/gup: skip pinnable check for refs==1 To: John Hubbard Cc: Andrew Morton , "Cc: Android Kernel" , Minchan Kim , linux-mm@kvack.org, Linux Kernel Mailing List Content-Type: text/plain; charset="UTF-8" X-Rspamd-Queue-Id: E670EC0003 X-Rspam-User: nil Authentication-Results: imf22.hostedemail.com; dkim=pass header.d=google.com header.s=20210112 header.b="s/Bxs4zd"; dmarc=pass (policy=reject) header.from=google.com; spf=pass (imf22.hostedemail.com: domain of willmcvicker@google.com designates 209.85.218.43 as permitted sender) smtp.mailfrom=willmcvicker@google.com X-Stat-Signature: xt7dzt4bnwrdkyqgtk66nkeof97me44p X-Rspamd-Server: rspam08 X-HE-Tag: 1643665046-465071 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 Mon, Jan 31, 2022 at 12:49 PM John Hubbard wrote: > > On 1/31/22 12:35, Will McVicker wrote: > > This fixes commit 54d516b1d62f ("mm/gup: small refactoring: simplify > > try_grab_page()") which refactors try_grab_page() to call > > try_grab_compound_head() with refs=1. The refactor commit is causing > > pin_user_pages() to return -ENOMEM when we try to pin one user page that > > is migratable and not in the movable zone. Previously, try_grab_page() > > didn't check if the page was pinnable for FOLL_PIN. To match the same > > functionality, this fix adds the check `refs > 1 &&` to skip the call to > > is_pinnable_page(). > > > > That's a clear write-up of what you're seeing, what caused it, and how > you'd like to correct it. The previous code had a loophole, and you want > to keep that loophole. More below... > > > This issue is reproducible with the Pixel 6 on the 5.15 LTS kernel. Here > > is the call stack to reproduce the -ENOMEM error: > ... > > Fixes: 54d516b1d62f ("mm/gup: small refactoring: simplify try_grab_page()") > > Cc: John Hubbard > > Cc: Minchan Kim > > Signed-off-by: Will McVicker > > --- > > mm/gup.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/mm/gup.c b/mm/gup.c > > index f0af462ac1e2..0509c49c46a3 100644 > > --- a/mm/gup.c > > +++ b/mm/gup.c > > @@ -135,7 +135,7 @@ struct page *try_grab_compound_head(struct page *page, > > * right zone, so fail and let the caller fall back to the slow > > * path. > > */ > > - if (unlikely((flags & FOLL_LONGTERM) && > > + if (refs > 1 && unlikely((flags & FOLL_LONGTERM) && > > !is_pinnable_page(page))) > > return NULL; > > > > ...but are you really sure that this is the best way to "fix" the > problem? This trades correctness for "bug-for-bug compatibility" with > the previous code. It says, "it's OK to violate the pinnable and > longterm checks, as long as you do it one page at a time, rather than in > larger chunks. > > Wouldn't it be better to try to fix up the calling code so that it's > not in violation of these zone rules? > > > thanks, > -- > John Hubbard > NVIDIA Hi John, Thanks for the prompt response! I'm not super familiar with what PIN+LONGTERM conditions require, but if this was previously a bug, then I definitely don't want to re-introduce it. Since you're confirming that, let me sync-up with the driver owner to see how I can fix this on the side. Thanks! Will