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=-15.8 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER, INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS 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 5B3E5C433B4 for ; Wed, 14 Apr 2021 09:27:16 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id E15FA60698 for ; Wed, 14 Apr 2021 09:27:15 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org E15FA60698 Authentication-Results: mail.kernel.org; dmarc=fail (p=quarantine dis=none) header.from=suse.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id 612806B0070; Wed, 14 Apr 2021 05:27:15 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 59B1A6B0071; Wed, 14 Apr 2021 05:27:15 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 417636B0072; Wed, 14 Apr 2021 05:27:15 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0081.hostedemail.com [216.40.44.81]) by kanga.kvack.org (Postfix) with ESMTP id 1CB6C6B0070 for ; Wed, 14 Apr 2021 05:27:15 -0400 (EDT) Received: from smtpin28.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay04.hostedemail.com (Postfix) with ESMTP id D37314E908ED for ; Wed, 14 Apr 2021 09:27:14 +0000 (UTC) X-FDA: 78030443988.28.C613008 Received: from mx2.suse.de (mx2.suse.de [195.135.220.15]) by imf19.hostedemail.com (Postfix) with ESMTP id C78C890009EF for ; Wed, 14 Apr 2021 09:26:58 +0000 (UTC) X-Virus-Scanned: by amavisd-new at test-mx.suse.de DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.com; s=susede1; t=1618392433; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=I4M0tkmcqKTvrAzd8ABxiVydKwG24FDTFmuy+1YGYSU=; b=N2M1K3wQJyemuE66ZlLCYBTRJ2pc+RcYLwo5N9tkPh603oA31ldIeEXp6BXNizpFAFOEse pFtZQU8P0YOdQ2927y6UOD31TXWlBHC+kEs6ZZSSL7a0BUNY6IHLj67f9mgmwRSPuWSXgM E7PybdvZ0BB0zzmOTqEEM5/Qzl/8MwQ= Received: from relay2.suse.de (unknown [195.135.221.27]) by mx2.suse.de (Postfix) with ESMTP id 2D37FACC4; Wed, 14 Apr 2021 09:27:13 +0000 (UTC) Date: Wed, 14 Apr 2021 11:27:12 +0200 From: Michal Hocko To: Muchun Song Cc: guro@fb.com, hannes@cmpxchg.org, akpm@linux-foundation.org, shakeelb@google.com, vdavydov.dev@gmail.com, linux-kernel@vger.kernel.org, linux-mm@kvack.org, duanxiongchun@bytedance.com, fam.zheng@bytedance.com Subject: Re: [PATCH 3/7] mm: memcontrol: remove the pgdata parameter of mem_cgroup_page_lruvec Message-ID: References: <20210413065153.63431-1-songmuchun@bytedance.com> <20210413065153.63431-4-songmuchun@bytedance.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20210413065153.63431-4-songmuchun@bytedance.com> X-Rspamd-Queue-Id: C78C890009EF X-Stat-Signature: yqxo4uizpewmmti76rqgamwxschqzph8 X-Rspamd-Server: rspam02 Received-SPF: none (suse.com>: No applicable sender policy available) receiver=imf19; identity=mailfrom; envelope-from=""; helo=mx2.suse.de; client-ip=195.135.220.15 X-HE-DKIM-Result: pass/pass X-HE-Tag: 1618392418-827065 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 13-04-21 14:51:49, Muchun Song wrote: > All the callers of mem_cgroup_page_lruvec() just pass page_pgdat(page) > as the 2nd parameter to it (except isolate_migratepages_block()). But > for isolate_migratepages_block(), the page_pgdat(page) is also equal > to the local variable of @pgdat. So mem_cgroup_page_lruvec() do not > need the pgdat parameter. Just remove it to simplify the code. > > Signed-off-by: Muchun Song > Acked-by: Johannes Weiner I like this. Two arguments where one can be directly inferred from the first one can just lead to subtle bugs. In this case it even doesn't give any advantage for most callers. Acked-by: Michal Hocko > --- > include/linux/memcontrol.h | 10 +++++----- > mm/compaction.c | 2 +- > mm/memcontrol.c | 9 +++------ > mm/swap.c | 2 +- > mm/workingset.c | 2 +- > 5 files changed, 11 insertions(+), 14 deletions(-) > > diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h > index c960fd49c3e8..4f49865c9958 100644 > --- a/include/linux/memcontrol.h > +++ b/include/linux/memcontrol.h > @@ -743,13 +743,12 @@ static inline struct lruvec *mem_cgroup_lruvec(struct mem_cgroup *memcg, > /** > * mem_cgroup_page_lruvec - return lruvec for isolating/putting an LRU page > * @page: the page > - * @pgdat: pgdat of the page > * > * This function relies on page->mem_cgroup being stable. > */ > -static inline struct lruvec *mem_cgroup_page_lruvec(struct page *page, > - struct pglist_data *pgdat) > +static inline struct lruvec *mem_cgroup_page_lruvec(struct page *page) > { > + pg_data_t *pgdat = page_pgdat(page); > struct mem_cgroup *memcg = page_memcg(page); > > VM_WARN_ON_ONCE_PAGE(!memcg && !mem_cgroup_disabled(), page); > @@ -1223,9 +1222,10 @@ static inline struct lruvec *mem_cgroup_lruvec(struct mem_cgroup *memcg, > return &pgdat->__lruvec; > } > > -static inline struct lruvec *mem_cgroup_page_lruvec(struct page *page, > - struct pglist_data *pgdat) > +static inline struct lruvec *mem_cgroup_page_lruvec(struct page *page) > { > + pg_data_t *pgdat = page_pgdat(page); > + > return &pgdat->__lruvec; > } > > diff --git a/mm/compaction.c b/mm/compaction.c > index caa4c36c1db3..e7da342003dd 100644 > --- a/mm/compaction.c > +++ b/mm/compaction.c > @@ -1033,7 +1033,7 @@ isolate_migratepages_block(struct compact_control *cc, unsigned long low_pfn, > if (!TestClearPageLRU(page)) > goto isolate_fail_put; > > - lruvec = mem_cgroup_page_lruvec(page, pgdat); > + lruvec = mem_cgroup_page_lruvec(page); > > /* If we already hold the lock, we can skip some rechecking */ > if (lruvec != locked) { > diff --git a/mm/memcontrol.c b/mm/memcontrol.c > index 9cbfff59b171..1f807448233e 100644 > --- a/mm/memcontrol.c > +++ b/mm/memcontrol.c > @@ -1177,9 +1177,8 @@ void lruvec_memcg_debug(struct lruvec *lruvec, struct page *page) > struct lruvec *lock_page_lruvec(struct page *page) > { > struct lruvec *lruvec; > - struct pglist_data *pgdat = page_pgdat(page); > > - lruvec = mem_cgroup_page_lruvec(page, pgdat); > + lruvec = mem_cgroup_page_lruvec(page); > spin_lock(&lruvec->lru_lock); > > lruvec_memcg_debug(lruvec, page); > @@ -1190,9 +1189,8 @@ struct lruvec *lock_page_lruvec(struct page *page) > struct lruvec *lock_page_lruvec_irq(struct page *page) > { > struct lruvec *lruvec; > - struct pglist_data *pgdat = page_pgdat(page); > > - lruvec = mem_cgroup_page_lruvec(page, pgdat); > + lruvec = mem_cgroup_page_lruvec(page); > spin_lock_irq(&lruvec->lru_lock); > > lruvec_memcg_debug(lruvec, page); > @@ -1203,9 +1201,8 @@ struct lruvec *lock_page_lruvec_irq(struct page *page) > struct lruvec *lock_page_lruvec_irqsave(struct page *page, unsigned long *flags) > { > struct lruvec *lruvec; > - struct pglist_data *pgdat = page_pgdat(page); > > - lruvec = mem_cgroup_page_lruvec(page, pgdat); > + lruvec = mem_cgroup_page_lruvec(page); > spin_lock_irqsave(&lruvec->lru_lock, *flags); > > lruvec_memcg_debug(lruvec, page); > diff --git a/mm/swap.c b/mm/swap.c > index a75a8265302b..e0d5699213cc 100644 > --- a/mm/swap.c > +++ b/mm/swap.c > @@ -313,7 +313,7 @@ void lru_note_cost(struct lruvec *lruvec, bool file, unsigned int nr_pages) > > void lru_note_cost_page(struct page *page) > { > - lru_note_cost(mem_cgroup_page_lruvec(page, page_pgdat(page)), > + lru_note_cost(mem_cgroup_page_lruvec(page), > page_is_file_lru(page), thp_nr_pages(page)); > } > > diff --git a/mm/workingset.c b/mm/workingset.c > index b7cdeca5a76d..4f7a306ce75a 100644 > --- a/mm/workingset.c > +++ b/mm/workingset.c > @@ -408,7 +408,7 @@ void workingset_activation(struct page *page) > memcg = page_memcg_rcu(page); > if (!mem_cgroup_disabled() && !memcg) > goto out; > - lruvec = mem_cgroup_page_lruvec(page, page_pgdat(page)); > + lruvec = mem_cgroup_page_lruvec(page); > workingset_age_nonresident(lruvec, thp_nr_pages(page)); > out: > rcu_read_unlock(); > -- > 2.11.0 -- Michal Hocko SUSE Labs