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 7B714EB64D9 for ; Mon, 19 Jun 2023 04:34:51 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id E4A188D0002; Mon, 19 Jun 2023 00:34:50 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id DFA1E8D0001; Mon, 19 Jun 2023 00:34:50 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id CC0178D0002; Mon, 19 Jun 2023 00:34:50 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0016.hostedemail.com [216.40.44.16]) by kanga.kvack.org (Postfix) with ESMTP id BDD4B8D0001 for ; Mon, 19 Jun 2023 00:34:50 -0400 (EDT) Received: from smtpin18.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay05.hostedemail.com (Postfix) with ESMTP id 8B05F40136 for ; Mon, 19 Jun 2023 04:34:50 +0000 (UTC) X-FDA: 80918231940.18.CCE09E3 Received: from mail-ej1-f48.google.com (mail-ej1-f48.google.com [209.85.218.48]) by imf04.hostedemail.com (Postfix) with ESMTP id 9DD3140007 for ; Mon, 19 Jun 2023 04:34:48 +0000 (UTC) Authentication-Results: imf04.hostedemail.com; dkim=pass header.d=google.com header.s=20221208 header.b=VrCmPthO; dmarc=pass (policy=reject) header.from=google.com; spf=pass (imf04.hostedemail.com: domain of yosryahmed@google.com designates 209.85.218.48 as permitted sender) smtp.mailfrom=yosryahmed@google.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1687149288; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=fQlhQ/GJgQzUBNcbs/51oGUQCslwDzBZawZLttFe2zs=; b=rZm7KWusHKCNkqGUu+5weFar8L/2IS787dg0oP1dLodXvbWLP6qTSDtol6K24pbcoosRJW Fn83tSKbhdBl2yotNEnds4VK1gSjcuNUTmizQBk5IIWEdjQ6xYuw/KwEhDf6XaMMQAPFAa NPHhbpPI11JaP3WDdfI1Pn2IoDpDlDk= ARC-Authentication-Results: i=1; imf04.hostedemail.com; dkim=pass header.d=google.com header.s=20221208 header.b=VrCmPthO; dmarc=pass (policy=reject) header.from=google.com; spf=pass (imf04.hostedemail.com: domain of yosryahmed@google.com designates 209.85.218.48 as permitted sender) smtp.mailfrom=yosryahmed@google.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1687149288; a=rsa-sha256; cv=none; b=xOu7CEZhL47Bybzy95indJcjWOdJpZ57hc/zVDkiywQoo/5lxS6dXevl9JUIERGB2rOGVr eCd7GuQ1tqpgVRv3JM5xa8ippQLWv5o1sLaIy0nALVHAgg9UVmVYwr0Im0IcqCt4/ihrku WJqmER4ky/5f2XNjkbEi+VmpvqyC1os= Received: by mail-ej1-f48.google.com with SMTP id a640c23a62f3a-9829a5ae978so503286466b.2 for ; Sun, 18 Jun 2023 21:34:48 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20221208; t=1687149287; x=1689741287; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=fQlhQ/GJgQzUBNcbs/51oGUQCslwDzBZawZLttFe2zs=; b=VrCmPthOaGEQb68j4amXLTUaxHBX8uPSPicAT/kBhVCd0MN5X5YX5/UOecABvefhRr naTMzTJ1PrLoyMQfkzHFof9lkTr3iV1J84qHx5ciUnSX+fAU6o/dtK/q/7Pwi785MhSG 6MSCtS4lnJ0vK3AOjq9+wvRQWZok6apk60qmhtthOsitC5vZ5BArnnm5jFQINxp/oNFm QsmB03wEbPHkZaYTmhAWhaUt0SswvngN1vGLVL3/bG3mNajq6vGrVoH+TTqP+7K/y1JN QuYutO2fmAegNJB/XHc/abcYThe6NflLccqqrfyRIuseALvzAJSE7RQwFvaD/GmNjB1T +7UA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1687149287; x=1689741287; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=fQlhQ/GJgQzUBNcbs/51oGUQCslwDzBZawZLttFe2zs=; b=QHSrhSPDj1SB97NHlIWWtoq0BUHXuP5/RxqvXktFa2fiE4sy0Z97Y/lDaZtM+lKXo4 vMIbWSQXFIqxQ890aAisozkW9NCY/Pwkz6VBzfrINqnntMTeHrAYnc0XTEjpFtB3+T9w hGPKads0mC2YTJKmWmr0TmBnTol8tSLrBlaOmY6yIVk9q0LVDxP1yLffSyhRp+qiG9pC ievgN0YAUJm2tBzN2UOX7DezSm1pggFiMn6p86j7Ig1fsxyPruC+0CEUGKy9K6PXBM80 nAXu60A1nZBMaIEZIqRwhr3h8/lW8tP4HbyhbiTrEQubF909nZbkbN3LxjlOfDZp+Imx +VbA== X-Gm-Message-State: AC+VfDwxSshToGB3z6Fd2zZixwhZRa2oxRG4iPaoGPtTpEJQBQHGqwMP BFiHBC6/EH7WkjJixp8EUZY8m0v51fz/HCPJEUTRcg== X-Google-Smtp-Source: ACHHUZ46xnLpaELxLld4ks9SpXhnggAeUh/XO8e/tIt2cOVuJsF0TKeKKur5BMys8pQFSn22vNVVegLcr0wvVTlj0tc= X-Received: by 2002:a17:907:7dab:b0:94e:8559:b5c5 with SMTP id oz43-20020a1709077dab00b0094e8559b5c5mr9094566ejc.64.1687149286991; Sun, 18 Jun 2023 21:34:46 -0700 (PDT) MIME-Version: 1.0 References: <20230618065824.1365750-1-yosryahmed@google.com> <87ilbk8b89.fsf@yhuang6-desk2.ccr.corp.intel.com> <87a5ww84b9.fsf@yhuang6-desk2.ccr.corp.intel.com> In-Reply-To: <87a5ww84b9.fsf@yhuang6-desk2.ccr.corp.intel.com> From: Yosry Ahmed Date: Sun, 18 Jun 2023 21:34:10 -0700 Message-ID: Subject: Re: [RFC PATCH 5/5] Revert "mm/migrate: __unmap_and_move() push good newpage to LRU" To: "Huang, Ying" Cc: Andrew Morton , Yu Zhao , "Jan Alexander Steffens (heftig)" , Steven Barrett , Brian Geffon , "T.J. Alumbaugh" , Gaosheng Cui , Suren Baghdasaryan , "Matthew Wilcox (Oracle)" , "Liam R. Howlett" , David Hildenbrand , Jason Gunthorpe , Mathieu Desnoyers , David Howells , Hugh Dickins , Greg Thelen , linux-mm@kvack.org, linux-kernel@vger.kernel.org Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Rspamd-Server: rspam09 X-Rspamd-Queue-Id: 9DD3140007 X-Stat-Signature: 6s7sdfimm87ubb3q7yic34zyxgc3nwfd X-Rspam-User: X-HE-Tag: 1687149288-362108 X-HE-Meta: U2FsdGVkX1/2ep+UtCr2iZHNnOfuqZ28FyOv08XFd9T4+3Y7bAttuYfuaI5lt6vWknp0TRxdyw6qMSYDMqOrcaztpGSdwrdEP92IA17PDNMav4kDVq7WYAHsVXMohbH0QwfXR43R/laQp3lYMKrTcvXbkP96wklOfE+bjg8N3YmbQGYrTh40/M11X8FkyYpOJmPKz+EP/Lpj1rkhfaQWrbqyYoiTdlpUax2MBm1yame+ZlIh/xrQhvbAjNGW1B9HD1Q0GnaXEcTNIxvOJfj8iSeMeU45geEqMNyHHOrsa+Bv9L/cRSvOcfZDCFyYb72s9bu1SEL0HDhICuUri03uh2MTm7H09JnQHI2vdiqSVnp0GVpseH9TKOROeDrDIjHItFAEM1QjAQqI1kH3MumkAeos0i5fQUuBxf/1qx5MgzR41erpc5skA5hglJYVH9fTiQNjRya7H+7Um3qGIWsSfE+3jii2Y+2BebFwUY9tKyE7ePF0cBNUZt0JDZ5NbgToheuSq5HTUiH40MXxkSz3FlvJmGIjhwlG4RRzWrd/em1Eb/81m8bxjCzepH+N4B0FSktlr4Ory3uOIKluP5h0WOOVRjbhLI/I5yS4zHd5Qs4YfzdPoX54hN2Tw56+XOy0FdQqi/c6nF1QykeGMUqH2Cb+WMQ67cTuQhrkyNpwcD6Na4XpgNdHxQMZC8V5Oe4OOq08MIiFBn7cxO5iFBagx3PRktRUXsDn9uBK8D7QsAdkO5hcL2gAeXNv9827ikvKUg9lBZSpnuiv/WU1HwzxdyxbY+GQAbut2UerKvHsrRf2nJBi/Cyff02Vui16OliFkN3rtzLSE6HhC/MtuT4GUhpi6PXpPjxHKDDAKPwkIPdEquRsDFVkeCOp7xJ2S2TCXIQ7661SfGfuwZWgiKclRHwu/kb2njz8g1tpM0eazSxW/V+gaF1Dwn8kFer4Q0MXP0I7oZevEbxpRQfPZWe wKUrPMRd Ko4fkmyZ3PRdYAoVjftxdxVGSAQAx2bn9AvXt9/xNMpzbjajpIercr0lgy2DLzCoU+syk7I64XrjExT8SdmsLSgTOVjdlu2G5XAUj5o3Ku/cwq/RFXQ/VyucF3euk4zndUvzXfJcDjCXy0m4CUlV4qotebBHpHsPPQDg7Do8JPP5/giqaUVoTg1ykfRBHkwHBT6gmSVOURax/ltBoZtx0LOXz2fBdme4ljlDtUqItr9+gtRBkgH/rxV+C+oDtEygyU7ZNbr4aJ9mAcwePQCIEWisu/UqRnPchYljUIRcdhqHN3eYFTLrMCkBd1g/v9c6mnwpR 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 Sun, Jun 18, 2023 at 9:29=E2=80=AFPM Huang, Ying = wrote: > > Yosry Ahmed writes: > > > On Sun, Jun 18, 2023 at 7:00=E2=80=AFPM Huang, Ying wrote: > >> > >> Hi, Yosry, > >> > >> Yosry Ahmed writes: > >> > >> > This reverts commit c3096e6782b733158bf34f6bbb4567808d4e0740. > >> > > >> > That commit made sure we immediately add the new page to the LRU bef= ore > >> > remove_migration_ptes() is called in migrate_move_folio() (used to b= e > >> > __unmap_and_move() back then), such that the rmap walk will rebuild = the > >> > correct mlock_count for the page again. This was needed because the > >> > mlock_count was lost when the page is isolated. This is no longer th= e > >> > case since mlock_count no longer overlays page->lru. > >> > > >> > Revert the commit (the code was foliated afterward the commit, so th= e > >> > revert is updated as such). > >> > > >> > Signed-off-by: Yosry Ahmed > >> > --- > >> > mm/migrate.c | 24 +++++++++--------------- > >> > 1 file changed, 9 insertions(+), 15 deletions(-) > >> > > >> > diff --git a/mm/migrate.c b/mm/migrate.c > >> > index 01cac26a3127..68f693731865 100644 > >> > --- a/mm/migrate.c > >> > +++ b/mm/migrate.c > >> > @@ -1279,19 +1279,6 @@ static int migrate_folio_move(free_page_t put= _new_page, unsigned long private, > >> > if (unlikely(!is_lru)) > >> > goto out_unlock_both; > >> > >> The patch itself looks good to me! Thanks! > >> > >> Reviewed-by: "Huang, Ying" > > > > Thanks for taking a look! > > > >> > >> And, it seems that we can remove the above 2 lines and "out_unlock_bot= h" > >> label now. That can make the code simpler a little. Right? > > > > I am not familiar with this code. If we remove the above condition > > then pages that have is_lru =3D=3D 0 (i.e __PageMovable(src) is true) a= nd > > page_was_mapped =3D=3D 1 will call remove_migration_ptes(). This wouldn= 't > > happen without removing the above 2 lines. If this combination is > > impossible (is_lru =3D=3D 0 && page_was_mapped =3D=3D 1), then yeah we = can > > remove the above condition. > > > > It looks like __SetPageMovable() is only called by zsmalloc, z3fold, > > and balloon_page_insert(). The former 2 will never have those pages > > mapped into userspace. I am not familiar with balloon_page_insert(), > > but my gut feeling is that those are pages used by the driver and are > > also not mapped into userspace. > > You can take a look at migrate_folio_unmap(), where "page_was_mapped" > will not be set to 1 if !is_lru. Oh I was looking in the wrong place huh, thanks. Will remove it when I resp= in! > > Best Regards, > Huang, Ying > > > So I guess we can just remove the condition, but a confirmation for > > the above would be reassuring :) > > > >> > >> Best Regards, > >> Huang, Ying > >> > >> > - /* > >> > - * When successful, push dst to LRU immediately: so that if it > >> > - * turns out to be an mlocked page, remove_migration_ptes() wi= ll > >> > - * automatically build up the correct dst->mlock_count for it. > >> > - * > >> > - * We would like to do something similar for the old page, whe= n > >> > - * unsuccessful, and other cases when a page has been temporar= ily > >> > - * isolated from the unevictable LRU: but this case is the eas= iest. > >> > - */ > >> > - folio_add_lru(dst); > >> > - if (page_was_mapped) > >> > - lru_add_drain(); > >> > - > >> > if (page_was_mapped) > >> > remove_migration_ptes(src, dst, false); > >> > > >> > @@ -1301,9 +1288,16 @@ static int migrate_folio_move(free_page_t put= _new_page, unsigned long private, > >> > /* > >> > * If migration is successful, decrease refcount of dst, > >> > * which will not free the page because new page owner increas= ed > >> > - * refcounter. > >> > + * refcounter. As well, if it is LRU folio, add the folio to L= RU > >> > + * list in here. Use the old state of the isolated source foli= o to > >> > + * determine if we migrated a LRU folio. dst was already unloc= ked > >> > + * and possibly modified by its owner - don't rely on the foli= o > >> > + * state. > >> > */ > >> > - folio_put(dst); > >> > + if (unlikely(!is_lru)) > >> > + folio_put(dst); > >> > + else > >> > + folio_putback_lru(dst); > >> > > >> > /* > >> > * A folio that has been migrated has all references removed > >> >