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=-14.3 required=3.0 tests=DKIMWL_WL_MED,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH, MAILING_LIST_MULTI,SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED, USER_IN_DEF_DKIM_WL 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 39CFEFC6194 for ; Thu, 7 Nov 2019 02:51:03 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id CD9E42187F for ; Thu, 7 Nov 2019 02:51:02 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=google.com header.i=@google.com header.b="pew1rgD8" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org CD9E42187F Authentication-Results: mail.kernel.org; dmarc=fail (p=reject dis=none) header.from=google.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id 7DBC76B0007; Wed, 6 Nov 2019 21:51:02 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 78E126B0008; Wed, 6 Nov 2019 21:51:02 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 6CA926B000A; Wed, 6 Nov 2019 21:51:02 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0098.hostedemail.com [216.40.44.98]) by kanga.kvack.org (Postfix) with ESMTP id 5766E6B0007 for ; Wed, 6 Nov 2019 21:51:02 -0500 (EST) Received: from smtpin17.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay02.hostedemail.com (Postfix) with SMTP id 083573CF8 for ; Thu, 7 Nov 2019 02:51:02 +0000 (UTC) X-FDA: 76127954364.17.place44_8f534a6c6a3b X-HE-Tag: place44_8f534a6c6a3b X-Filterd-Recvd-Size: 13913 Received: from mail-ot1-f68.google.com (mail-ot1-f68.google.com [209.85.210.68]) by imf10.hostedemail.com (Postfix) with ESMTP for ; Thu, 7 Nov 2019 02:51:01 +0000 (UTC) Received: by mail-ot1-f68.google.com with SMTP id d5so722586otp.4 for ; Wed, 06 Nov 2019 18:51:01 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=srXzeN6zSyigYhNuWeb9PFhUGyu0G9M6CV2KGF40AVw=; b=pew1rgD87ZaI/spUbF18bIFhNfZCzl/lmrXyqx3XD33GQMy1NTRrBcDEBORBTtM0fG chp0XP6M6PS+rGJ1t2s/GhyUVIEjETRenL2dLY6BqP7fPdtQPVNHCFanNLXsOQdGbV+p AuxK0oWka7dzV+VRAIqOCc3pi0VUTOBzmKatk9s5cjhBbDgGzHpASdXp4dZiBU+L/6y/ T18GBMdA/R6gQKxSUaoQTU9r3sUZoe86ho6wYzI8uut/hWqxegZUr2uEE5NPEgQG7eSq z3GSj8oI52tMO9J5zu648zrA8aMmiyE8POdFGAfIj9OXppWPhz1mAoQjEvzilrTVWt+v TMcg== 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=srXzeN6zSyigYhNuWeb9PFhUGyu0G9M6CV2KGF40AVw=; b=lNBADwWeTmIDuR9CXVMAywbI0llHL9ot3bBkNzKYbOjQfajOV0mLNREDdzv8ncIAS0 z1qMmltCk7d/4cnYCcTFjq0Yzac9sw0yRomUmWKdD9rfFnW/6EDTnhlEaoWSJDrxW/WD PLnt1L+svMN+U53ajHA1PhCmVLwaxCL0mAAS+Va47Wx7xFC7iUE4OZReq+For/SXLaWT Q6m+LIT1A79744Jhd4WoZmAmTtygRV3YEh35X0dir3io7JSrBKt2Gr2PPy6dW1dwRlrC n/x/dAZn8z+nfHqRn+2JhtFTabDEi8YFVwYFyHkfKhQmaFFb6E1g0dngx7ymEjWAhs82 6YlQ== X-Gm-Message-State: APjAAAV5N/rzRk8TvDkmJ3/tgykC5zPoL+9KTghmB6C+jb68RFzMyYKF JdRoXNJ6S9y1ZqRnJRJnD36EhhNPvSywK26i9sY0dw== X-Google-Smtp-Source: APXvYqwq8GtOb42/vNn0kEpqNRamkE3UI0G4dO+NIpOK+9GgFfvDf8q5Un0kzNBt2z0ztUL616rTkL5uTH5fOYBhCfo= X-Received: by 2002:a9d:3675:: with SMTP id w108mr868519otb.81.1573095060320; Wed, 06 Nov 2019 18:51:00 -0800 (PST) MIME-Version: 1.0 References: <20190603210746.15800-1-hannes@cmpxchg.org> <20190603210746.15800-3-hannes@cmpxchg.org> In-Reply-To: <20190603210746.15800-3-hannes@cmpxchg.org> From: Shakeel Butt Date: Wed, 6 Nov 2019 18:50:49 -0800 Message-ID: Subject: Re: [PATCH 02/11] mm: clean up and clarify lruvec lookup procedure To: Johannes Weiner Cc: Andrew Morton , Andrey Ryabinin , Suren Baghdasaryan , Michal Hocko , Linux MM , Cgroups , LKML , Kernel Team Content-Type: text/plain; charset="UTF-8" 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, Jun 3, 2019 at 3:04 PM Johannes Weiner wrote: > > There is a per-memcg lruvec and a NUMA node lruvec. Which one is being > used is somewhat confusing right now, and it's easy to make mistakes - > especially when it comes to global reclaim. > > How it works: when memory cgroups are enabled, we always use the > root_mem_cgroup's per-node lruvecs. When memory cgroups are not > compiled in or disabled at runtime, we use pgdat->lruvec. > > Document that in a comment. > > Due to the way the reclaim code is generalized, all lookups use the > mem_cgroup_lruvec() helper function, and nobody should have to find > the right lruvec manually right now. But to avoid future mistakes, > rename the pgdat->lruvec member to pgdat->__lruvec and delete the > convenience wrapper that suggests it's a commonly accessed member. > > While in this area, swap the mem_cgroup_lruvec() argument order. The > name suggests a memcg operation, yet it takes a pgdat first and a > memcg second. I have to double check > take every time I call this. Fix that. > > Signed-off-by: Johannes Weiner Reviewed-by: Shakeel Butt > --- > include/linux/memcontrol.h | 26 +++++++++++++------------- > include/linux/mmzone.h | 15 ++++++++------- > mm/memcontrol.c | 6 +++--- > mm/page_alloc.c | 2 +- > mm/vmscan.c | 6 +++--- > mm/workingset.c | 8 ++++---- > 6 files changed, 32 insertions(+), 31 deletions(-) > > diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h > index fa1e8cb1b3e2..fc32cfaebf32 100644 > --- a/include/linux/memcontrol.h > +++ b/include/linux/memcontrol.h > @@ -382,22 +382,22 @@ mem_cgroup_nodeinfo(struct mem_cgroup *memcg, int nid) > } > > /** > - * mem_cgroup_lruvec - get the lru list vector for a node or a memcg zone > - * @node: node of the wanted lruvec > + * mem_cgroup_lruvec - get the lru list vector for a memcg & node > * @memcg: memcg of the wanted lruvec > + * @node: node of the wanted lruvec > * > - * Returns the lru list vector holding pages for a given @node or a given > - * @memcg and @zone. This can be the node lruvec, if the memory controller > - * is disabled. > + * Returns the lru list vector holding pages for a given @memcg & > + * @node combination. This can be the node lruvec, if the memory > + * controller is disabled. > */ > -static inline struct lruvec *mem_cgroup_lruvec(struct pglist_data *pgdat, > - struct mem_cgroup *memcg) > +static inline struct lruvec *mem_cgroup_lruvec(struct mem_cgroup *memcg, > + struct pglist_data *pgdat) > { > struct mem_cgroup_per_node *mz; > struct lruvec *lruvec; > > if (mem_cgroup_disabled()) { > - lruvec = node_lruvec(pgdat); > + lruvec = &pgdat->__lruvec; > goto out; > } > > @@ -716,7 +716,7 @@ static inline void __mod_lruvec_page_state(struct page *page, > return; > } > > - lruvec = mem_cgroup_lruvec(pgdat, page->mem_cgroup); > + lruvec = mem_cgroup_lruvec(page->mem_cgroup, pgdat); > __mod_lruvec_state(lruvec, idx, val); > } > > @@ -887,16 +887,16 @@ static inline void mem_cgroup_migrate(struct page *old, struct page *new) > { > } > > -static inline struct lruvec *mem_cgroup_lruvec(struct pglist_data *pgdat, > - struct mem_cgroup *memcg) > +static inline struct lruvec *mem_cgroup_lruvec(struct mem_cgroup *memcg, > + struct pglist_data *pgdat) > { > - return node_lruvec(pgdat); > + return &pgdat->__lruvec; > } > > static inline struct lruvec *mem_cgroup_page_lruvec(struct page *page, > struct pglist_data *pgdat) > { > - return &pgdat->lruvec; > + return &pgdat->__lruvec; > } > > static inline bool mm_match_cgroup(struct mm_struct *mm, > diff --git a/include/linux/mmzone.h b/include/linux/mmzone.h > index 427b79c39b3c..95d63a395f40 100644 > --- a/include/linux/mmzone.h > +++ b/include/linux/mmzone.h > @@ -761,7 +761,13 @@ typedef struct pglist_data { > #endif > > /* Fields commonly accessed by the page reclaim scanner */ > - struct lruvec lruvec; > + > + /* > + * NOTE: THIS IS UNUSED IF MEMCG IS ENABLED. > + * > + * Use mem_cgroup_lruvec() to look up lruvecs. > + */ > + struct lruvec __lruvec; > > unsigned long flags; > > @@ -784,11 +790,6 @@ typedef struct pglist_data { > #define node_start_pfn(nid) (NODE_DATA(nid)->node_start_pfn) > #define node_end_pfn(nid) pgdat_end_pfn(NODE_DATA(nid)) > > -static inline struct lruvec *node_lruvec(struct pglist_data *pgdat) > -{ > - return &pgdat->lruvec; > -} > - > static inline unsigned long pgdat_end_pfn(pg_data_t *pgdat) > { > return pgdat->node_start_pfn + pgdat->node_spanned_pages; > @@ -826,7 +827,7 @@ static inline struct pglist_data *lruvec_pgdat(struct lruvec *lruvec) > #ifdef CONFIG_MEMCG > return lruvec->pgdat; > #else > - return container_of(lruvec, struct pglist_data, lruvec); > + return container_of(lruvec, struct pglist_data, __lruvec); > #endif > } > > diff --git a/mm/memcontrol.c b/mm/memcontrol.c > index c193aef3ba9e..6de8ca735ee2 100644 > --- a/mm/memcontrol.c > +++ b/mm/memcontrol.c > @@ -1200,7 +1200,7 @@ struct lruvec *mem_cgroup_page_lruvec(struct page *page, struct pglist_data *pgd > struct lruvec *lruvec; > > if (mem_cgroup_disabled()) { > - lruvec = &pgdat->lruvec; > + lruvec = &pgdat->__lruvec; > goto out; > } > > @@ -1518,7 +1518,7 @@ static bool mem_cgroup_out_of_memory(struct mem_cgroup *memcg, gfp_t gfp_mask, > static bool test_mem_cgroup_node_reclaimable(struct mem_cgroup *memcg, > int nid, bool noswap) > { > - struct lruvec *lruvec = mem_cgroup_lruvec(NODE_DATA(nid), memcg); > + struct lruvec *lruvec = mem_cgroup_lruvec(memcg, NODE_DATA(nid)); > > if (lruvec_page_state(lruvec, NR_INACTIVE_FILE) || > lruvec_page_state(lruvec, NR_ACTIVE_FILE)) > @@ -3406,7 +3406,7 @@ static int mem_cgroup_move_charge_write(struct cgroup_subsys_state *css, > static unsigned long mem_cgroup_node_nr_lru_pages(struct mem_cgroup *memcg, > int nid, unsigned int lru_mask) > { > - struct lruvec *lruvec = mem_cgroup_lruvec(NODE_DATA(nid), memcg); > + struct lruvec *lruvec = mem_cgroup_lruvec(memcg, NODE_DATA(nid)); > unsigned long nr = 0; > enum lru_list lru; > > diff --git a/mm/page_alloc.c b/mm/page_alloc.c > index a345418b548e..cd8e64e536f7 100644 > --- a/mm/page_alloc.c > +++ b/mm/page_alloc.c > @@ -6619,7 +6619,7 @@ static void __meminit pgdat_init_internals(struct pglist_data *pgdat) > > pgdat_page_ext_init(pgdat); > spin_lock_init(&pgdat->lru_lock); > - lruvec_init(node_lruvec(pgdat)); > + lruvec_init(&pgdat->__lruvec); > } > > static void __meminit zone_init_internals(struct zone *zone, enum zone_type idx, int nid, > diff --git a/mm/vmscan.c b/mm/vmscan.c > index f396424850aa..853be16ee5e2 100644 > --- a/mm/vmscan.c > +++ b/mm/vmscan.c > @@ -2477,7 +2477,7 @@ static void get_scan_count(struct lruvec *lruvec, struct mem_cgroup *memcg, > static void shrink_node_memcg(struct pglist_data *pgdat, struct mem_cgroup *memcg, > struct scan_control *sc) > { > - struct lruvec *lruvec = mem_cgroup_lruvec(pgdat, memcg); > + struct lruvec *lruvec = mem_cgroup_lruvec(memcg, pgdat); > unsigned long nr[NR_LRU_LISTS]; > unsigned long targets[NR_LRU_LISTS]; > unsigned long nr_to_scan; > @@ -2988,7 +2988,7 @@ static void snapshot_refaults(struct mem_cgroup *root_memcg, pg_data_t *pgdat) > unsigned long refaults; > struct lruvec *lruvec; > > - lruvec = mem_cgroup_lruvec(pgdat, memcg); > + lruvec = mem_cgroup_lruvec(memcg, pgdat); > refaults = lruvec_page_state_local(lruvec, WORKINGSET_ACTIVATE); > lruvec->refaults = refaults; > } while ((memcg = mem_cgroup_iter(root_memcg, memcg, NULL))); > @@ -3351,7 +3351,7 @@ static void age_active_anon(struct pglist_data *pgdat, > > memcg = mem_cgroup_iter(NULL, NULL, NULL); > do { > - struct lruvec *lruvec = mem_cgroup_lruvec(pgdat, memcg); > + struct lruvec *lruvec = mem_cgroup_lruvec(memcg, pgdat); > > if (inactive_list_is_low(lruvec, false, sc, true)) > shrink_active_list(SWAP_CLUSTER_MAX, lruvec, > diff --git a/mm/workingset.c b/mm/workingset.c > index e0b4edcb88c8..2aaa70bea99c 100644 > --- a/mm/workingset.c > +++ b/mm/workingset.c > @@ -233,7 +233,7 @@ void *workingset_eviction(struct page *page) > VM_BUG_ON_PAGE(page_count(page), page); > VM_BUG_ON_PAGE(!PageLocked(page), page); > > - lruvec = mem_cgroup_lruvec(pgdat, memcg); > + lruvec = mem_cgroup_lruvec(memcg, pgdat); > eviction = atomic_long_inc_return(&lruvec->inactive_age); > return pack_shadow(memcgid, pgdat, eviction, PageWorkingset(page)); > } > @@ -280,7 +280,7 @@ void workingset_refault(struct page *page, void *shadow) > memcg = mem_cgroup_from_id(memcgid); > if (!mem_cgroup_disabled() && !memcg) > goto out; > - lruvec = mem_cgroup_lruvec(pgdat, memcg); > + lruvec = mem_cgroup_lruvec(memcg, pgdat); > refault = atomic_long_read(&lruvec->inactive_age); > active_file = lruvec_lru_size(lruvec, LRU_ACTIVE_FILE, MAX_NR_ZONES); > > @@ -345,7 +345,7 @@ void workingset_activation(struct page *page) > memcg = page_memcg_rcu(page); > if (!mem_cgroup_disabled() && !memcg) > goto out; > - lruvec = mem_cgroup_lruvec(page_pgdat(page), memcg); > + lruvec = mem_cgroup_lruvec(memcg, page_pgdat(page)); > atomic_long_inc(&lruvec->inactive_age); > out: > rcu_read_unlock(); > @@ -428,7 +428,7 @@ static unsigned long count_shadow_nodes(struct shrinker *shrinker, > struct lruvec *lruvec; > int i; > > - lruvec = mem_cgroup_lruvec(NODE_DATA(sc->nid), sc->memcg); > + lruvec = mem_cgroup_lruvec(sc->memcg, NODE_DATA(sc->nid)); > for (pages = 0, i = 0; i < NR_LRU_LISTS; i++) > pages += lruvec_page_state_local(lruvec, > NR_LRU_BASE + i); > -- > 2.21.0 >