> On Dec 2, 2022, at 06:10, Mina Almasry <almasrymina@xxxxxxxxxx> wrote: > > On Thu, Dec 1, 2022 at 1:32 PM Shakeel Butt <shakeelb@xxxxxxxxxx> 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. >
