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 X-Spam-Level: X-Spam-Status: No, score=-10.1 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY, SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 34563C433DF for ; Wed, 5 Aug 2020 22:43:41 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id CC3CC2076E for ; Wed, 5 Aug 2020 22:43:40 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="WX7ocpjD" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org CC3CC2076E Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=gmail.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id 400CD6B0003; Wed, 5 Aug 2020 18:43:40 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 3B2266B0005; Wed, 5 Aug 2020 18:43:40 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 253FD6B0006; Wed, 5 Aug 2020 18:43:40 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0009.hostedemail.com [216.40.44.9]) by kanga.kvack.org (Postfix) with ESMTP id 0E19F6B0003 for ; Wed, 5 Aug 2020 18:43:40 -0400 (EDT) Received: from smtpin03.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay01.hostedemail.com (Postfix) with ESMTP id 8F31F180AD804 for ; Wed, 5 Aug 2020 22:43:39 +0000 (UTC) X-FDA: 77117993358.03.drum43_3c013d126fb2 Received: from filter.hostedemail.com (10.5.16.251.rfc1918.com [10.5.16.251]) by smtpin03.hostedemail.com (Postfix) with ESMTP id 589AD28A4E9 for ; Wed, 5 Aug 2020 22:43:39 +0000 (UTC) X-HE-Tag: drum43_3c013d126fb2 X-Filterd-Recvd-Size: 8171 Received: from mail-il1-f194.google.com (mail-il1-f194.google.com [209.85.166.194]) by imf09.hostedemail.com (Postfix) with ESMTP for ; Wed, 5 Aug 2020 22:43:38 +0000 (UTC) Received: by mail-il1-f194.google.com with SMTP id t18so38845205ilh.2 for ; Wed, 05 Aug 2020 15:43:38 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=yfR6T7oCaeLyjQ9SxVMOY64V3krO/aimxcAQ4bPIbX8=; b=WX7ocpjDWB4JEf5Vxc+bT7cgMR54ylfB0nbKx/qzh7bkB0tA+cJZEHy8v92TB+FSli yAb8tqjCXpaKNxV3c4mVPitLtQ3GD8PsOmxbGmkwASJI3nHicBPfYsD/QCMGwO3JVlye Ww5LE/35K3MqDoMsJAWFZfJwev8RbOSsDnXI8ZTFYy9lUdHnRYwb7UCucAl4RI+tZulZ HCP2BE4fPKbsUQhGEXIVV4K9GsxbJ9Ut9j/UmvlFtSA8fRMx1FbumPmln8vOY+WnKQJ4 jIcjtWftqgCVQ9Gdd6ewkRxrIktn3qEczHiAjDFDh1Qforkax2YUAkzOni17zqB6W3bY 4M7g== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=yfR6T7oCaeLyjQ9SxVMOY64V3krO/aimxcAQ4bPIbX8=; b=RBAgIY/hOFvaGUY09nU+aD/utrarcY+MkyGH6QYVUXFiiuh4wKAxq/PdAAhPr9b9Me hB+WhdFHq973u3fqikeuGOvELOM1N6pGbSMykSwtJSA8ZYRXZoBhVOKEm5V9SOL+y5P+ JM4bzs7mgeP3+dE2XSfh/HS8s2iHaQWL/LrXfkTmSL8MNRzO7Kr24MjuvuuHyMYlbRzV ysIoGwyGAdd1FeGY5CYJBPUINO7dj+Ch4npqOtqfHvfnG9BLnJ56bsA8MYsw8TzMiwoW FRJlsICnYZ6BAHSuji5XF4pBINDjSCVmD+SBpb9Zu8UeR3Scx2AreFGZuO+lmff0QOVe 0Uew== X-Gm-Message-State: AOAM532poAsNQ3nURAQRIwQgOjp+Hr/ZIZQZ9CmIquD9SDYfPzQPtcVI ouslvUW32j4aszHLcLLkvWpKdho3vN6cWNRXB7Y= X-Google-Smtp-Source: ABdhPJy1iTjipZHLfFa2tpUoCXh3KbOtUScwfAP9RwbBuwBH2LowKi2GRFNbixTAX5YssLItPPQWb9LObtijLB7HC78= X-Received: by 2002:a05:6e02:143:: with SMTP id j3mr5940509ilr.97.1596667417710; Wed, 05 Aug 2020 15:43:37 -0700 (PDT) MIME-Version: 1.0 References: <1595681998-19193-1-git-send-email-alex.shi@linux.alibaba.com> <1595681998-19193-14-git-send-email-alex.shi@linux.alibaba.com> <9b906469-38fb-8a4e-9a47-d617c7669579@linux.alibaba.com> In-Reply-To: <9b906469-38fb-8a4e-9a47-d617c7669579@linux.alibaba.com> From: Alexander Duyck Date: Wed, 5 Aug 2020 15:43:26 -0700 Message-ID: Subject: Re: [PATCH v17 13/21] mm/lru: introduce TestClearPageLRU To: Alex Shi Cc: Andrew Morton , Mel Gorman , Tejun Heo , Hugh Dickins , Konstantin Khlebnikov , Daniel Jordan , Yang Shi , Matthew Wilcox , Johannes Weiner , kbuild test robot , linux-mm , LKML , cgroups@vger.kernel.org, Shakeel Butt , Joonsoo Kim , Wei Yang , "Kirill A. Shutemov" , Rong Chen , Michal Hocko , Vladimir Davydov Content-Type: text/plain; charset="UTF-8" X-Rspamd-Queue-Id: 589AD28A4E9 X-Spamd-Result: default: False [0.00 / 100.00] X-Rspamd-Server: rspam02 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, Jul 28, 2020 at 8:53 PM Alex Shi wrote: > > rewrite the commit log. > > From 9310c359b0049e3cc9827b771dc583d504bbf022 Mon Sep 17 00:00:00 2001 > From: Alex Shi > Date: Sat, 25 Apr 2020 12:03:30 +0800 > Subject: [PATCH v17 13/23] mm/lru: introduce TestClearPageLRU > > Currently lru_lock still guards both lru list and page's lru bit, that's > ok. but if we want to use specific lruvec lock on the page, we need to > pin down the page's lruvec/memcg during locking. Just taking lruvec > lock first may be undermined by the page's memcg charge/migration. To > fix this problem, we could clear the lru bit out of locking and use > it as pin down action to block the page isolation in memcg changing. > > So now a standard steps of page isolation is following: > 1, get_page(); #pin the page avoid to be free > 2, TestClearPageLRU(); #block other isolation like memcg change > 3, spin_lock on lru_lock; #serialize lru list access > 4, delete page from lru list; > The step 2 could be optimzed/replaced in scenarios which page is > unlikely be accessed or be moved between memcgs. > > This patch start with the first part: TestClearPageLRU, which combines > PageLRU check and ClearPageLRU into a macro func TestClearPageLRU. This > function will be used as page isolation precondition to prevent other > isolations some where else. Then there are may !PageLRU page on lru > list, need to remove BUG() checking accordingly. > > There 2 rules for lru bit now: > 1, the lru bit still indicate if a page on lru list, just in some > temporary moment(isolating), the page may have no lru bit when > it's on lru list. but the page still must be on lru list when the > lru bit set. > 2, have to remove lru bit before delete it from lru list. > > Hugh Dickins pointed that when a page is in free path and no one is > possible to take it, non atomic lru bit clearing is better, like in > __page_cache_release and release_pages. > And no need get_page() before lru bit clear in isolate_lru_page, > since it '(1) Must be called with an elevated refcount on the page'. > > As Andrew Morton mentioned this change would dirty cacheline for page > isn't on LRU. But the lost would be acceptable with Rong Chen > report: > https://lkml.org/lkml/2020/3/4/173 > > Suggested-by: Johannes Weiner > Signed-off-by: Alex Shi > Cc: Hugh Dickins > Cc: Johannes Weiner > Cc: Michal Hocko > Cc: Vladimir Davydov > Cc: Andrew Morton > Cc: linux-kernel@vger.kernel.org > Cc: cgroups@vger.kernel.org > Cc: linux-mm@kvack.org > --- > include/linux/page-flags.h | 1 + > mm/mlock.c | 3 +-- > mm/swap.c | 6 ++---- > mm/vmscan.c | 18 +++++++----------- > 4 files changed, 11 insertions(+), 17 deletions(-) > > diff --git a/mm/swap.c b/mm/swap.c > index f645965fde0e..5092fe9c8c47 100644 > --- a/mm/swap.c > +++ b/mm/swap.c > @@ -83,10 +83,9 @@ static void __page_cache_release(struct page *page) > struct lruvec *lruvec; > unsigned long flags; > > + __ClearPageLRU(page); > spin_lock_irqsave(&pgdat->lru_lock, flags); > lruvec = mem_cgroup_page_lruvec(page, pgdat); > - VM_BUG_ON_PAGE(!PageLRU(page), page); > - __ClearPageLRU(page); > del_page_from_lru_list(page, lruvec, page_off_lru(page)); > spin_unlock_irqrestore(&pgdat->lru_lock, flags); > } > @@ -878,9 +877,8 @@ void release_pages(struct page **pages, int nr) > spin_lock_irqsave(&locked_pgdat->lru_lock, flags); > } > > - lruvec = mem_cgroup_page_lruvec(page, locked_pgdat); > - VM_BUG_ON_PAGE(!PageLRU(page), page); > __ClearPageLRU(page); > + lruvec = mem_cgroup_page_lruvec(page, locked_pgdat); > del_page_from_lru_list(page, lruvec, page_off_lru(page)); > } > The more I look at this piece it seems like this change wasn't really necessary. If anything it seems like it could catch potential bugs as it was testing for the PageLRU flag before and then clearing it manually anyway. In addition it doesn't reduce the critical path by any significant amount so I am not sure these changes are providing any benefit.