From mboxrd@z Thu Jan 1 00:00:00 1970 From: Muchun Song Subject: Re: [RFC PATCH v2] mm: Add nodes= arg to memory.reclaim Date: Fri, 2 Dec 2022 14:04:45 +0800 Message-ID: References: <20221130020328.1009347-1-almasrymina@google.com> <20221201213202.ycdaymwojd5plyrk@google.com> Mime-Version: 1.0 (Mac OS X Mail 16.0 \(3731.200.110.1.12\)) Content-Transfer-Encoding: 8BIT Return-path: In-Reply-To: List-ID: Content-Type: text/plain; charset="us-ascii" To: Mina Almasry Cc: Shakeel Butt , Huang Ying , Yang Shi , Yosry Ahmed , Tim Chen , weixugc@google.com, gthelen@google.com, fvdl@google.com, Tejun Heo , Zefan Li , Johannes Weiner , Jonathan Corbet , Michal Hocko , Roman Gushchin , Muchun Song , Andrew Morton , cgroups@vger.kernel.org, Linux Doc Mailing List , linux-kernel@vger.kernel.org, Linux Memory Management List > On Dec 2, 2022, at 06:10, Mina Almasry wrote: > > On Thu, Dec 1, 2022 at 1:32 PM Shakeel Butt wrote: >> >> On Tue, Nov 29, 2022 at 06:03:27PM -0800, Mina Almasry wrote: >> [...] >>> diff --git a/mm/vmscan.c b/mm/vmscan.c >>> index 7b8e8e43806b..23fc5b523764 100644 >>> --- a/mm/vmscan.c >>> +++ b/mm/vmscan.c >>> @@ -6735,7 +6735,8 @@ unsigned long mem_cgroup_shrink_node(struct mem_cgroup *memcg, >>> unsigned long try_to_free_mem_cgroup_pages(struct mem_cgroup *memcg, >>> unsigned long nr_pages, >>> gfp_t gfp_mask, >>> - unsigned int reclaim_options) >>> + unsigned int reclaim_options, >>> + nodemask_t nodemask) >> >> Can you please make this parameter a nodemask_t* and pass NULL instead >> of NODE_MASK_ALL? > > Thank you very much for the review. I sure can in the next version. To > be honest I thought about that and made the parameter nodemask_t > because I thought the call sites would be more readable. I.e. this: > > try_to_free_mem_cgroup_pages(memcg, 1, GFP_KERNEL, > MEMCG_RECLAIM_MAY_SWAP, NODE_MASK_ALL); nodemask_t is an array, which can be large depending on CONFIG_NODES_SHIFT. I don't think passing a big array is an efficient way. So I agree with Shakeel. Thanks. > > Would be more readable than this: > > try_to_free_mem_cgroup_pages(memcg, 1, GFP_KERNEL, > MEMCG_RECLAIM_MAY_SWAP, NULL); > > But the tradeoff is that the callers need include/linux/nodemask.h. > But yes I can fix in the next version. >