Received: by 2002:a25:8b91:0:0:0:0:0 with SMTP id j17csp3639681ybl; Sun, 12 Jan 2020 23:24:33 -0800 (PST) X-Google-Smtp-Source: APXvYqx6oL7YXw1fJcvh1Gc5pMQY1MTHllKs2VkCMncVHfReNGKagmsOD2xX3O1Moexsy8zD0vhG X-Received: by 2002:a9d:7f11:: with SMTP id j17mr12659369otq.281.1578900273780; Sun, 12 Jan 2020 23:24:33 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1578900273; cv=none; d=google.com; s=arc-20160816; b=zc+KGajpFR2jKL6LMd6a0lg7UCUOtTUhVDJwuI8EQ9ix/2lAn8JWPxwHlR319YExSA c9RxajhqYU8fwHCuNxxuQPzQI+Ye+JcIDoZ2j7ONv47JcCO2M2wkDQTCxOQhkDi8Q+9e av9G7fAvKQfBQE8FH5q/Ia/46aTxHcX3HcksWaOXC2mYVYxnebvwcwIPRhP0a0BnTfrk C5BQC0fAElmjAMer6eOT9elR9MeRPAfK5zScyaeWj9Xv8z3RPiCMBkTWRjzhIiP7ktbN BIxFqzhhhA2IGqC0bmkydfh3CHsMp1yG2N6Ahi8p12c7LJ31wae2utH/ua/vpCXxqeqz aN0Q== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:sender:content-transfer-encoding:in-reply-to :mime-version:user-agent:date:message-id:from:references:cc:to :subject; bh=AlR3ktCm06opncaI1bIJ0sbOn4CdqTFUrUwnn+yFf+4=; b=GUo6MH0LC5C4ioBoT9lBw11M/AyeMWf5jfRqbyAkMHprG8sdp8evjSGGTZOn9+OSkk aYssLI1Uf7U5VypS1sFIF2bKOlUMT59uTxhplbcPtXU97zEIviX+fj+7optgS0iF0Kpd G56viJSe3ruPJiTGL0QO2TvQQ3yhEtg4AWF5qLYWlzP2gXVF+BQ7QsAri9JsfW0vTfem lyBAAYtBCwQLzAmQ9LceP4AP7Vhb+VekpX69Xo12faM9toKZ/wbx70zIvByrrAw10K32 CsVwciWuT20gjYYRJEfDIs2cNGOdIUoxeaan1CFsejuk715K5ET+eaaLCmTbnvEom/D+ 5pxQ== ARC-Authentication-Results: i=1; mx.google.com; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=alibaba.com Return-Path: Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id v3si4990974oif.122.2020.01.12.23.24.21; Sun, 12 Jan 2020 23:24:33 -0800 (PST) Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67; Authentication-Results: mx.google.com; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=alibaba.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1728720AbgAMHX3 (ORCPT + 99 others); Mon, 13 Jan 2020 02:23:29 -0500 Received: from out30-57.freemail.mail.aliyun.com ([115.124.30.57]:53427 "EHLO out30-57.freemail.mail.aliyun.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1727555AbgAMHX2 (ORCPT ); Mon, 13 Jan 2020 02:23:28 -0500 X-Alimail-AntiSpam: AC=PASS;BC=-1|-1;BR=01201311R851e4;CH=green;DM=||false|;DS=||;FP=0|-1|-1|-1|0|-1|-1|-1;HT=e01e04426;MF=alex.shi@linux.alibaba.com;NM=1;PH=DS;RN=14;SR=0;TI=SMTPD_---0TnZoR5p_1578900205; Received: from IT-FVFX43SYHV2H.local(mailfrom:alex.shi@linux.alibaba.com fp:SMTPD_---0TnZoR5p_1578900205) by smtp.aliyun-inc.com(127.0.0.1); Mon, 13 Jan 2020 15:23:25 +0800 Subject: Re: [PATCH v7 01/10] mm/vmscan: remove unnecessary lruvec adding To: Konstantin Khlebnikov , cgroups@vger.kernel.org, linux-kernel@vger.kernel.org, linux-mm@kvack.org, akpm@linux-foundation.org, mgorman@techsingularity.net, tj@kernel.org, hughd@google.com, daniel.m.jordan@oracle.com, yang.shi@linux.alibaba.com, willy@infradead.org, shakeelb@google.com, hannes@cmpxchg.org Cc: yun.wang@linux.alibaba.com References: <1577264666-246071-1-git-send-email-alex.shi@linux.alibaba.com> <1577264666-246071-2-git-send-email-alex.shi@linux.alibaba.com> <6c91fb0a-d4e0-d960-1cfd-62bef5cd15a5@yandex-team.ru> From: Alex Shi Message-ID: <8f0f9fd5-56d3-0ec3-e875-f2eb5e1e7971@linux.alibaba.com> Date: Mon, 13 Jan 2020 15:21:58 +0800 User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:68.0) Gecko/20100101 Thunderbird/68.3.1 MIME-Version: 1.0 In-Reply-To: <6c91fb0a-d4e0-d960-1cfd-62bef5cd15a5@yandex-team.ru> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 在 2020/1/10 下午4:39, Konstantin Khlebnikov 写道: > On 25/12/2019 12.04, Alex Shi wrote: >> We don't have to add a freeable page into lru and then remove from it. >> This change saves a couple of actions and makes the moving more clear. >> >> The SetPageLRU needs to be kept here for list intergrity. Otherwise: >> >>      #0 mave_pages_to_lru                #1 release_pages >>                                          if (put_page_testzero()) >>      if (put_page_testzero()) >>                                         !PageLRU //skip lru_lock >>                                                  list_add(&page->lru,); >>      else >>          list_add(&page->lru,) //corrupt >> >> Signed-off-by: Alex Shi >> Cc: Andrew Morton >> Cc: Johannes Weiner >> Cc: Hugh Dickins >> Cc: yun.wang@linux.alibaba.com >> Cc: linux-mm@kvack.org >> Cc: linux-kernel@vger.kernel.org >> --- >>   mm/vmscan.c | 16 +++++++--------- >>   1 file changed, 7 insertions(+), 9 deletions(-) >> >> diff --git a/mm/vmscan.c b/mm/vmscan.c >> index 572fb17c6273..8719361b47a0 100644 >> --- a/mm/vmscan.c >> +++ b/mm/vmscan.c >> @@ -1852,26 +1852,18 @@ static unsigned noinline_for_stack move_pages_to_lru(struct lruvec *lruvec, > > Here is another cleanup: pass only pgdat as argument. > > This function reavaluates lruvec for each page under lru lock. > Probably this is redundant for now but could be used in the future (or your patchset already use that). Thanks a lot for comments, Konstantin! yes, we could use pgdat here, but since to remove the pgdat later, maybe better to save this change? :) > >>       while (!list_empty(list)) { >>           page = lru_to_page(list); >>           VM_BUG_ON_PAGE(PageLRU(page), page); >> +        list_del(&page->lru); >>           if (unlikely(!page_evictable(page))) { >> -            list_del(&page->lru); >>               spin_unlock_irq(&pgdat->lru_lock); >>               putback_lru_page(page); >>               spin_lock_irq(&pgdat->lru_lock); >>               continue; >>           } >> -        lruvec = mem_cgroup_page_lruvec(page, pgdat); >> - > > Please leave a comment that we must set PageLRU before dropping our page reference. Right, I will try to give a comments here. > >>           SetPageLRU(page); >> -        lru = page_lru(page); >> - >> -        nr_pages = hpage_nr_pages(page); >> -        update_lru_size(lruvec, lru, page_zonenum(page), nr_pages); >> -        list_move(&page->lru, &lruvec->lists[lru]); >>             if (put_page_testzero(page)) { >>               __ClearPageLRU(page); >>               __ClearPageActive(page); >> -            del_page_from_lru_list(page, lruvec, lru); >>                 if (unlikely(PageCompound(page))) { >>                   spin_unlock_irq(&pgdat->lru_lock); >> @@ -1880,6 +1872,12 @@ static unsigned noinline_for_stack move_pages_to_lru(struct lruvec *lruvec, >>               } else >>                   list_add(&page->lru, &pages_to_free); >>           } else { >> +            lruvec = mem_cgroup_page_lruvec(page, pgdat); >> +            lru = page_lru(page); >> +            nr_pages = hpage_nr_pages(page); >> + >> +            update_lru_size(lruvec, lru, page_zonenum(page), nr_pages); >> +            list_add(&page->lru, &lruvec->lists[lru]); >>               nr_moved += nr_pages; >>           } > > IMHO It looks better to in this way:> > SetPageLRU() > > if (unlikely(put_page_testzero())) { >   >  continue; > } > > Yes, this looks better! Thanks Alex > >>       } >>