* [PATCH -v2 next 0/4] Some cleanup for memcg
@ 2025-01-14 12:25 Chen Ridong
2025-01-14 12:25 ` [PATCH -v2 next 1/4] memcg: use OFP_PEAK_UNSET instead of -1 Chen Ridong
` (3 more replies)
0 siblings, 4 replies; 12+ messages in thread
From: Chen Ridong @ 2025-01-14 12:25 UTC (permalink / raw)
To: akpm, mhocko, hannes, yosryahmed, roman.gushchin, shakeel.butt,
muchun.song, davidf, vbabka, mkoutny
Cc: linux-mm, linux-kernel, cgroups, chenridong, wangweiyang2
From: Chen Ridong <chenridong@huawei.com>
Some cleanup for memcg.
---
v1->v2:
- drop the patch 'simplify the mem_cgroup_update_lru_size function'.
- for patch 3, rename '__refill_obj_stock' to replace_stock_objcg, and
keep the 'objcg equal' check in the calling functions.
Chen Ridong (4):
memcg: use OFP_PEAK_UNSET instead of -1
memcg: call the free function when allocation of pn fails
memcg: factor out the replace_stock_objcg function
memcg: factor out stat(event)/stat_local(event_local) reading
functions
mm/memcontrol.c | 127 ++++++++++++++++++++++--------------------------
1 file changed, 59 insertions(+), 68 deletions(-)
--
2.34.1
^ permalink raw reply [flat|nested] 12+ messages in thread* [PATCH -v2 next 1/4] memcg: use OFP_PEAK_UNSET instead of -1 2025-01-14 12:25 [PATCH -v2 next 0/4] Some cleanup for memcg Chen Ridong @ 2025-01-14 12:25 ` Chen Ridong 2025-01-14 18:36 ` Roman Gushchin 2025-01-14 12:25 ` [PATCH -v2 next 2/4] memcg: call the free function when allocation of pn fails Chen Ridong ` (2 subsequent siblings) 3 siblings, 1 reply; 12+ messages in thread From: Chen Ridong @ 2025-01-14 12:25 UTC (permalink / raw) To: akpm, mhocko, hannes, yosryahmed, roman.gushchin, shakeel.butt, muchun.song, davidf, vbabka, mkoutny Cc: linux-mm, linux-kernel, cgroups, chenridong, wangweiyang2 From: Chen Ridong <chenridong@huawei.com> The 'OFP_PEAK_UNSET' has been defined, use it instead of '-1'. Signed-off-by: Chen Ridong <chenridong@huawei.com> Reviewed-by: Michal Koutný <mkoutny@suse.com> Acked-by: David Finkel <davidf@vimeo.com> Acked-by: Shakeel Butt <shakeel.butt@linux.dev> --- mm/memcontrol.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mm/memcontrol.c b/mm/memcontrol.c index 46f8b372d212..05a32c860554 100644 --- a/mm/memcontrol.c +++ b/mm/memcontrol.c @@ -4004,7 +4004,7 @@ static ssize_t peak_write(struct kernfs_open_file *of, char *buf, size_t nbytes, WRITE_ONCE(peer_ctx->value, usage); /* initial write, register watcher */ - if (ofp->value == -1) + if (ofp->value == OFP_PEAK_UNSET) list_add(&ofp->list, watchers); WRITE_ONCE(ofp->value, usage); -- 2.34.1 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH -v2 next 1/4] memcg: use OFP_PEAK_UNSET instead of -1 2025-01-14 12:25 ` [PATCH -v2 next 1/4] memcg: use OFP_PEAK_UNSET instead of -1 Chen Ridong @ 2025-01-14 18:36 ` Roman Gushchin 0 siblings, 0 replies; 12+ messages in thread From: Roman Gushchin @ 2025-01-14 18:36 UTC (permalink / raw) To: Chen Ridong Cc: akpm, mhocko, hannes, yosryahmed, shakeel.butt, muchun.song, davidf, vbabka, mkoutny, linux-mm, linux-kernel, cgroups, chenridong, wangweiyang2 On Tue, Jan 14, 2025 at 12:25:16PM +0000, Chen Ridong wrote: > From: Chen Ridong <chenridong@huawei.com> > > The 'OFP_PEAK_UNSET' has been defined, use it instead of '-1'. > > Signed-off-by: Chen Ridong <chenridong@huawei.com> > Reviewed-by: Michal Koutný <mkoutny@suse.com> > Acked-by: David Finkel <davidf@vimeo.com> > Acked-by: Shakeel Butt <shakeel.butt@linux.dev> Reviewed-by: Roman Gushchin <roman.gushchin@linux.dev> ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH -v2 next 2/4] memcg: call the free function when allocation of pn fails 2025-01-14 12:25 [PATCH -v2 next 0/4] Some cleanup for memcg Chen Ridong 2025-01-14 12:25 ` [PATCH -v2 next 1/4] memcg: use OFP_PEAK_UNSET instead of -1 Chen Ridong @ 2025-01-14 12:25 ` Chen Ridong 2025-01-14 18:36 ` Roman Gushchin 2025-01-14 12:25 ` [PATCH -v2 next 3/4] memcg: factor out the replace_stock_objcg function Chen Ridong 2025-01-14 12:25 ` [PATCH -v2 next 4/4] memcg: factor out stat(event)/stat_local(event_local) reading functions Chen Ridong 3 siblings, 1 reply; 12+ messages in thread From: Chen Ridong @ 2025-01-14 12:25 UTC (permalink / raw) To: akpm, mhocko, hannes, yosryahmed, roman.gushchin, shakeel.butt, muchun.song, davidf, vbabka, mkoutny Cc: linux-mm, linux-kernel, cgroups, chenridong, wangweiyang2 From: Chen Ridong <chenridong@huawei.com> The 'free_mem_cgroup_per_node_info' function is used to free the 'mem_cgroup_per_node' struct. Using 'pn' as the input for the free_mem_cgroup_per_node_info function will be much clearer. Call 'free_mem_cgroup_per_node_info' when 'alloc_mem_cgroup_per_node_info' fails, to free 'pn' as a whole, which makes the code more cohesive. Signed-off-by: Chen Ridong <chenridong@huawei.com> Reviewed-by: Michal Koutný <mkoutny@suse.com> Acked-by: Shakeel Butt <shakeel.butt@linux.dev> --- mm/memcontrol.c | 27 ++++++++++++--------------- 1 file changed, 12 insertions(+), 15 deletions(-) diff --git a/mm/memcontrol.c b/mm/memcontrol.c index 05a32c860554..98f84a9fa228 100644 --- a/mm/memcontrol.c +++ b/mm/memcontrol.c @@ -3424,6 +3424,16 @@ struct mem_cgroup *mem_cgroup_get_from_ino(unsigned long ino) } #endif +static void free_mem_cgroup_per_node_info(struct mem_cgroup_per_node *pn) +{ + if (!pn) + return; + + free_percpu(pn->lruvec_stats_percpu); + kfree(pn->lruvec_stats); + kfree(pn); +} + static bool alloc_mem_cgroup_per_node_info(struct mem_cgroup *memcg, int node) { struct mem_cgroup_per_node *pn; @@ -3448,23 +3458,10 @@ static bool alloc_mem_cgroup_per_node_info(struct mem_cgroup *memcg, int node) memcg->nodeinfo[node] = pn; return true; fail: - kfree(pn->lruvec_stats); - kfree(pn); + free_mem_cgroup_per_node_info(pn); return false; } -static void free_mem_cgroup_per_node_info(struct mem_cgroup *memcg, int node) -{ - struct mem_cgroup_per_node *pn = memcg->nodeinfo[node]; - - if (!pn) - return; - - free_percpu(pn->lruvec_stats_percpu); - kfree(pn->lruvec_stats); - kfree(pn); -} - static void __mem_cgroup_free(struct mem_cgroup *memcg) { int node; @@ -3472,7 +3469,7 @@ static void __mem_cgroup_free(struct mem_cgroup *memcg) obj_cgroup_put(memcg->orig_objcg); for_each_node(node) - free_mem_cgroup_per_node_info(memcg, node); + free_mem_cgroup_per_node_info(memcg->nodeinfo[node]); memcg1_free_events(memcg); kfree(memcg->vmstats); free_percpu(memcg->vmstats_percpu); -- 2.34.1 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH -v2 next 2/4] memcg: call the free function when allocation of pn fails 2025-01-14 12:25 ` [PATCH -v2 next 2/4] memcg: call the free function when allocation of pn fails Chen Ridong @ 2025-01-14 18:36 ` Roman Gushchin 0 siblings, 0 replies; 12+ messages in thread From: Roman Gushchin @ 2025-01-14 18:36 UTC (permalink / raw) To: Chen Ridong Cc: akpm, mhocko, hannes, yosryahmed, shakeel.butt, muchun.song, davidf, vbabka, mkoutny, linux-mm, linux-kernel, cgroups, chenridong, wangweiyang2 On Tue, Jan 14, 2025 at 12:25:17PM +0000, Chen Ridong wrote: > From: Chen Ridong <chenridong@huawei.com> > > The 'free_mem_cgroup_per_node_info' function is used to free > the 'mem_cgroup_per_node' struct. Using 'pn' as the input for the > free_mem_cgroup_per_node_info function will be much clearer. > Call 'free_mem_cgroup_per_node_info' when 'alloc_mem_cgroup_per_node_info' > fails, to free 'pn' as a whole, which makes the code more cohesive. > > Signed-off-by: Chen Ridong <chenridong@huawei.com> > Reviewed-by: Michal Koutný <mkoutny@suse.com> > Acked-by: Shakeel Butt <shakeel.butt@linux.dev> Reviewed-by: Roman Gushchin <roman.gushchin@linux.dev> ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH -v2 next 3/4] memcg: factor out the replace_stock_objcg function 2025-01-14 12:25 [PATCH -v2 next 0/4] Some cleanup for memcg Chen Ridong 2025-01-14 12:25 ` [PATCH -v2 next 1/4] memcg: use OFP_PEAK_UNSET instead of -1 Chen Ridong 2025-01-14 12:25 ` [PATCH -v2 next 2/4] memcg: call the free function when allocation of pn fails Chen Ridong @ 2025-01-14 12:25 ` Chen Ridong 2025-01-14 18:39 ` Roman Gushchin 2025-01-16 0:56 ` Shakeel Butt 2025-01-14 12:25 ` [PATCH -v2 next 4/4] memcg: factor out stat(event)/stat_local(event_local) reading functions Chen Ridong 3 siblings, 2 replies; 12+ messages in thread From: Chen Ridong @ 2025-01-14 12:25 UTC (permalink / raw) To: akpm, mhocko, hannes, yosryahmed, roman.gushchin, shakeel.butt, muchun.song, davidf, vbabka, mkoutny Cc: linux-mm, linux-kernel, cgroups, chenridong, wangweiyang2 From: Chen Ridong <chenridong@huawei.com> Factor out the 'replace_stock_objcg' function to make the code more cohesive. Signed-off-by: Chen Ridong <chenridong@huawei.com> --- mm/memcontrol.c | 26 ++++++++++++++++---------- 1 file changed, 16 insertions(+), 10 deletions(-) diff --git a/mm/memcontrol.c b/mm/memcontrol.c index 98f84a9fa228..b10e0a8f3375 100644 --- a/mm/memcontrol.c +++ b/mm/memcontrol.c @@ -2691,6 +2691,20 @@ void __memcg_kmem_uncharge_page(struct page *page, int order) obj_cgroup_put(objcg); } +/* Replace the stock objcg with objcg, return the old objcg */ +static struct obj_cgroup *replace_stock_objcg(struct memcg_stock_pcp *stock, + struct obj_cgroup *objcg) +{ + struct obj_cgroup *old = NULL; + + old = drain_obj_stock(stock); + obj_cgroup_get(objcg); + stock->nr_bytes = atomic_read(&objcg->nr_charged_bytes) + ? atomic_xchg(&objcg->nr_charged_bytes, 0) : 0; + WRITE_ONCE(stock->cached_objcg, objcg); + return old; +} + static void mod_objcg_state(struct obj_cgroup *objcg, struct pglist_data *pgdat, enum node_stat_item idx, int nr) { @@ -2708,11 +2722,7 @@ static void mod_objcg_state(struct obj_cgroup *objcg, struct pglist_data *pgdat, * changes. */ if (READ_ONCE(stock->cached_objcg) != objcg) { - old = drain_obj_stock(stock); - obj_cgroup_get(objcg); - stock->nr_bytes = atomic_read(&objcg->nr_charged_bytes) - ? atomic_xchg(&objcg->nr_charged_bytes, 0) : 0; - WRITE_ONCE(stock->cached_objcg, objcg); + old = replace_stock_objcg(stock, objcg); stock->cached_pgdat = pgdat; } else if (stock->cached_pgdat != pgdat) { /* Flush the existing cached vmstat data */ @@ -2866,11 +2876,7 @@ static void refill_obj_stock(struct obj_cgroup *objcg, unsigned int nr_bytes, stock = this_cpu_ptr(&memcg_stock); if (READ_ONCE(stock->cached_objcg) != objcg) { /* reset if necessary */ - old = drain_obj_stock(stock); - obj_cgroup_get(objcg); - WRITE_ONCE(stock->cached_objcg, objcg); - stock->nr_bytes = atomic_read(&objcg->nr_charged_bytes) - ? atomic_xchg(&objcg->nr_charged_bytes, 0) : 0; + old = replace_stock_objcg(stock, objcg); allow_uncharge = true; /* Allow uncharge when objcg changes */ } stock->nr_bytes += nr_bytes; -- 2.34.1 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH -v2 next 3/4] memcg: factor out the replace_stock_objcg function 2025-01-14 12:25 ` [PATCH -v2 next 3/4] memcg: factor out the replace_stock_objcg function Chen Ridong @ 2025-01-14 18:39 ` Roman Gushchin 2025-01-16 0:56 ` Shakeel Butt 1 sibling, 0 replies; 12+ messages in thread From: Roman Gushchin @ 2025-01-14 18:39 UTC (permalink / raw) To: Chen Ridong Cc: akpm, mhocko, hannes, yosryahmed, shakeel.butt, muchun.song, davidf, vbabka, mkoutny, linux-mm, linux-kernel, cgroups, chenridong, wangweiyang2 On Tue, Jan 14, 2025 at 12:25:18PM +0000, Chen Ridong wrote: > From: Chen Ridong <chenridong@huawei.com> > > Factor out the 'replace_stock_objcg' function to make the code more > cohesive. > > Signed-off-by: Chen Ridong <chenridong@huawei.com> Reviewed-by: Roman Gushchin <roman.gushchin@linux.dev> ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH -v2 next 3/4] memcg: factor out the replace_stock_objcg function 2025-01-14 12:25 ` [PATCH -v2 next 3/4] memcg: factor out the replace_stock_objcg function Chen Ridong 2025-01-14 18:39 ` Roman Gushchin @ 2025-01-16 0:56 ` Shakeel Butt 1 sibling, 0 replies; 12+ messages in thread From: Shakeel Butt @ 2025-01-16 0:56 UTC (permalink / raw) To: Chen Ridong Cc: akpm, mhocko, hannes, yosryahmed, roman.gushchin, muchun.song, davidf, vbabka, mkoutny, linux-mm, linux-kernel, cgroups, chenridong, wangweiyang2 On Tue, Jan 14, 2025 at 12:25:18PM +0000, Chen Ridong wrote: > From: Chen Ridong <chenridong@huawei.com> > > Factor out the 'replace_stock_objcg' function to make the code more > cohesive. > > Signed-off-by: Chen Ridong <chenridong@huawei.com> Acked-by: Shakeel Butt <shakeel.butt@linux.dev> ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH -v2 next 4/4] memcg: factor out stat(event)/stat_local(event_local) reading functions 2025-01-14 12:25 [PATCH -v2 next 0/4] Some cleanup for memcg Chen Ridong ` (2 preceding siblings ...) 2025-01-14 12:25 ` [PATCH -v2 next 3/4] memcg: factor out the replace_stock_objcg function Chen Ridong @ 2025-01-14 12:25 ` Chen Ridong 2025-01-14 18:45 ` Roman Gushchin 3 siblings, 1 reply; 12+ messages in thread From: Chen Ridong @ 2025-01-14 12:25 UTC (permalink / raw) To: akpm, mhocko, hannes, yosryahmed, roman.gushchin, shakeel.butt, muchun.song, davidf, vbabka, mkoutny Cc: linux-mm, linux-kernel, cgroups, chenridong, wangweiyang2 From: Chen Ridong <chenridong@huawei.com> The only difference between 'lruvec_page_state' and 'lruvec_page_state_local' is that they read 'state' and 'state_local', respectively. Factor out an inner functions to make the code more concise. Do the same for reading 'memcg_page_stat' and 'memcg_events'. Signed-off-by: Chen Ridong <chenridong@huawei.com> --- mm/memcontrol.c | 72 +++++++++++++++++++++---------------------------- 1 file changed, 30 insertions(+), 42 deletions(-) diff --git a/mm/memcontrol.c b/mm/memcontrol.c index b10e0a8f3375..14541610cad0 100644 --- a/mm/memcontrol.c +++ b/mm/memcontrol.c @@ -375,7 +375,8 @@ struct lruvec_stats { long state_pending[NR_MEMCG_NODE_STAT_ITEMS]; }; -unsigned long lruvec_page_state(struct lruvec *lruvec, enum node_stat_item idx) +static unsigned long __lruvec_page_state(struct lruvec *lruvec, + enum node_stat_item idx, bool local) { struct mem_cgroup_per_node *pn; long x; @@ -389,7 +390,8 @@ unsigned long lruvec_page_state(struct lruvec *lruvec, enum node_stat_item idx) return 0; pn = container_of(lruvec, struct mem_cgroup_per_node, lruvec); - x = READ_ONCE(pn->lruvec_stats->state[i]); + x = local ? READ_ONCE(pn->lruvec_stats->state_local[i]) : + READ_ONCE(pn->lruvec_stats->state[i]); #ifdef CONFIG_SMP if (x < 0) x = 0; @@ -397,27 +399,16 @@ unsigned long lruvec_page_state(struct lruvec *lruvec, enum node_stat_item idx) return x; } + +unsigned long lruvec_page_state(struct lruvec *lruvec, enum node_stat_item idx) +{ + return __lruvec_page_state(lruvec, idx, false); +} + unsigned long lruvec_page_state_local(struct lruvec *lruvec, enum node_stat_item idx) { - struct mem_cgroup_per_node *pn; - long x; - int i; - - if (mem_cgroup_disabled()) - return node_page_state(lruvec_pgdat(lruvec), idx); - - i = memcg_stats_index(idx); - if (WARN_ONCE(BAD_STAT_IDX(i), "%s: missing stat item %d\n", __func__, idx)) - return 0; - - pn = container_of(lruvec, struct mem_cgroup_per_node, lruvec); - x = READ_ONCE(pn->lruvec_stats->state_local[i]); -#ifdef CONFIG_SMP - if (x < 0) - x = 0; -#endif - return x; + return __lruvec_page_state(lruvec, idx, true); } /* Subset of vm_event_item to report for memcg event stats */ @@ -651,7 +642,7 @@ static void flush_memcg_stats_dwork(struct work_struct *w) queue_delayed_work(system_unbound_wq, &stats_flush_dwork, FLUSH_TIME); } -unsigned long memcg_page_state(struct mem_cgroup *memcg, int idx) +static unsigned long __memcg_page_state(struct mem_cgroup *memcg, int idx, bool local) { long x; int i = memcg_stats_index(idx); @@ -659,7 +650,9 @@ unsigned long memcg_page_state(struct mem_cgroup *memcg, int idx) if (WARN_ONCE(BAD_STAT_IDX(i), "%s: missing stat item %d\n", __func__, idx)) return 0; - x = READ_ONCE(memcg->vmstats->state[i]); + x = local ? READ_ONCE(memcg->vmstats->state_local[i]) : + READ_ONCE(memcg->vmstats->state[i]); + #ifdef CONFIG_SMP if (x < 0) x = 0; @@ -667,6 +660,11 @@ unsigned long memcg_page_state(struct mem_cgroup *memcg, int idx) return x; } +unsigned long memcg_page_state(struct mem_cgroup *memcg, int idx) +{ + return __memcg_page_state(memcg, idx, false); +} + static int memcg_page_state_unit(int item); /* @@ -709,18 +707,7 @@ void __mod_memcg_state(struct mem_cgroup *memcg, enum memcg_stat_item idx, /* idx can be of type enum memcg_stat_item or node_stat_item. */ unsigned long memcg_page_state_local(struct mem_cgroup *memcg, int idx) { - long x; - int i = memcg_stats_index(idx); - - if (WARN_ONCE(BAD_STAT_IDX(i), "%s: missing stat item %d\n", __func__, idx)) - return 0; - - x = READ_ONCE(memcg->vmstats->state_local[i]); -#ifdef CONFIG_SMP - if (x < 0) - x = 0; -#endif - return x; + return __memcg_page_state(memcg, idx, true); } static void __mod_memcg_lruvec_state(struct lruvec *lruvec, @@ -859,24 +846,25 @@ void __count_memcg_events(struct mem_cgroup *memcg, enum vm_event_item idx, memcg_stats_unlock(); } -unsigned long memcg_events(struct mem_cgroup *memcg, int event) +static unsigned long __memcg_events(struct mem_cgroup *memcg, int event, bool local) { int i = memcg_events_index(event); if (WARN_ONCE(BAD_STAT_IDX(i), "%s: missing stat item %d\n", __func__, event)) return 0; - return READ_ONCE(memcg->vmstats->events[i]); + return local ? READ_ONCE(memcg->vmstats->events_local[i]) : + READ_ONCE(memcg->vmstats->events[i]); } -unsigned long memcg_events_local(struct mem_cgroup *memcg, int event) +unsigned long memcg_events(struct mem_cgroup *memcg, int event) { - int i = memcg_events_index(event); - - if (WARN_ONCE(BAD_STAT_IDX(i), "%s: missing stat item %d\n", __func__, event)) - return 0; + return __memcg_events(memcg, event, false); +} - return READ_ONCE(memcg->vmstats->events_local[i]); +unsigned long memcg_events_local(struct mem_cgroup *memcg, int event) +{ + return __memcg_events(memcg, event, true); } struct mem_cgroup *mem_cgroup_from_task(struct task_struct *p) -- 2.34.1 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH -v2 next 4/4] memcg: factor out stat(event)/stat_local(event_local) reading functions 2025-01-14 12:25 ` [PATCH -v2 next 4/4] memcg: factor out stat(event)/stat_local(event_local) reading functions Chen Ridong @ 2025-01-14 18:45 ` Roman Gushchin 2025-01-16 0:57 ` Shakeel Butt 0 siblings, 1 reply; 12+ messages in thread From: Roman Gushchin @ 2025-01-14 18:45 UTC (permalink / raw) To: Chen Ridong Cc: akpm, mhocko, hannes, yosryahmed, shakeel.butt, muchun.song, davidf, vbabka, mkoutny, linux-mm, linux-kernel, cgroups, chenridong, wangweiyang2 On Tue, Jan 14, 2025 at 12:25:19PM +0000, Chen Ridong wrote: > From: Chen Ridong <chenridong@huawei.com> > > The only difference between 'lruvec_page_state' and > 'lruvec_page_state_local' is that they read 'state' and 'state_local', > respectively. Factor out an inner functions to make the code more concise. > Do the same for reading 'memcg_page_stat' and 'memcg_events'. > > Signed-off-by: Chen Ridong <chenridong@huawei.com> > --- > mm/memcontrol.c | 72 +++++++++++++++++++++---------------------------- > 1 file changed, 30 insertions(+), 42 deletions(-) > > diff --git a/mm/memcontrol.c b/mm/memcontrol.c > index b10e0a8f3375..14541610cad0 100644 > --- a/mm/memcontrol.c > +++ b/mm/memcontrol.c > @@ -375,7 +375,8 @@ struct lruvec_stats { > long state_pending[NR_MEMCG_NODE_STAT_ITEMS]; > }; > > -unsigned long lruvec_page_state(struct lruvec *lruvec, enum node_stat_item idx) > +static unsigned long __lruvec_page_state(struct lruvec *lruvec, > + enum node_stat_item idx, bool local) > { > struct mem_cgroup_per_node *pn; > long x; > @@ -389,7 +390,8 @@ unsigned long lruvec_page_state(struct lruvec *lruvec, enum node_stat_item idx) > return 0; > > pn = container_of(lruvec, struct mem_cgroup_per_node, lruvec); > - x = READ_ONCE(pn->lruvec_stats->state[i]); > + x = local ? READ_ONCE(pn->lruvec_stats->state_local[i]) : > + READ_ONCE(pn->lruvec_stats->state[i]); > #ifdef CONFIG_SMP > if (x < 0) > x = 0; > @@ -397,27 +399,16 @@ unsigned long lruvec_page_state(struct lruvec *lruvec, enum node_stat_item idx) > return x; > } > > + > +unsigned long lruvec_page_state(struct lruvec *lruvec, enum node_stat_item idx) > +{ > + return __lruvec_page_state(lruvec, idx, false); > +} I'd move these wrapper function definitions to memcontrol.h and make them static inline. Other than that, lgtm. Thank you! ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH -v2 next 4/4] memcg: factor out stat(event)/stat_local(event_local) reading functions 2025-01-14 18:45 ` Roman Gushchin @ 2025-01-16 0:57 ` Shakeel Butt 2025-01-16 1:21 ` Chen Ridong 0 siblings, 1 reply; 12+ messages in thread From: Shakeel Butt @ 2025-01-16 0:57 UTC (permalink / raw) To: Roman Gushchin Cc: Chen Ridong, akpm, mhocko, hannes, yosryahmed, muchun.song, davidf, vbabka, mkoutny, linux-mm, linux-kernel, cgroups, chenridong, wangweiyang2 On Tue, Jan 14, 2025 at 06:45:20PM +0000, Roman Gushchin wrote: > On Tue, Jan 14, 2025 at 12:25:19PM +0000, Chen Ridong wrote: > > From: Chen Ridong <chenridong@huawei.com> > > > > The only difference between 'lruvec_page_state' and > > 'lruvec_page_state_local' is that they read 'state' and 'state_local', > > respectively. Factor out an inner functions to make the code more concise. > > Do the same for reading 'memcg_page_stat' and 'memcg_events'. > > > > Signed-off-by: Chen Ridong <chenridong@huawei.com> > > --- > > mm/memcontrol.c | 72 +++++++++++++++++++++---------------------------- > > 1 file changed, 30 insertions(+), 42 deletions(-) > > > > diff --git a/mm/memcontrol.c b/mm/memcontrol.c > > index b10e0a8f3375..14541610cad0 100644 > > --- a/mm/memcontrol.c > > +++ b/mm/memcontrol.c > > @@ -375,7 +375,8 @@ struct lruvec_stats { > > long state_pending[NR_MEMCG_NODE_STAT_ITEMS]; > > }; > > > > -unsigned long lruvec_page_state(struct lruvec *lruvec, enum node_stat_item idx) > > +static unsigned long __lruvec_page_state(struct lruvec *lruvec, > > + enum node_stat_item idx, bool local) > > { > > struct mem_cgroup_per_node *pn; > > long x; > > @@ -389,7 +390,8 @@ unsigned long lruvec_page_state(struct lruvec *lruvec, enum node_stat_item idx) > > return 0; > > > > pn = container_of(lruvec, struct mem_cgroup_per_node, lruvec); > > - x = READ_ONCE(pn->lruvec_stats->state[i]); > > + x = local ? READ_ONCE(pn->lruvec_stats->state_local[i]) : > > + READ_ONCE(pn->lruvec_stats->state[i]); > > #ifdef CONFIG_SMP > > if (x < 0) > > x = 0; > > @@ -397,27 +399,16 @@ unsigned long lruvec_page_state(struct lruvec *lruvec, enum node_stat_item idx) > > return x; > > } > > > > + > > +unsigned long lruvec_page_state(struct lruvec *lruvec, enum node_stat_item idx) > > +{ > > + return __lruvec_page_state(lruvec, idx, false); > > +} > > I'd move these wrapper function definitions to memcontrol.h and make them > static inline. +1 > > Other than that, lgtm. > > Thank you! ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH -v2 next 4/4] memcg: factor out stat(event)/stat_local(event_local) reading functions 2025-01-16 0:57 ` Shakeel Butt @ 2025-01-16 1:21 ` Chen Ridong 0 siblings, 0 replies; 12+ messages in thread From: Chen Ridong @ 2025-01-16 1:21 UTC (permalink / raw) To: Shakeel Butt, Roman Gushchin Cc: akpm, mhocko, hannes, yosryahmed, muchun.song, davidf, vbabka, mkoutny, linux-mm, linux-kernel, cgroups, chenridong, wangweiyang2 On 2025/1/16 8:57, Shakeel Butt wrote: > On Tue, Jan 14, 2025 at 06:45:20PM +0000, Roman Gushchin wrote: >> On Tue, Jan 14, 2025 at 12:25:19PM +0000, Chen Ridong wrote: >>> From: Chen Ridong <chenridong@huawei.com> >>> >>> The only difference between 'lruvec_page_state' and >>> 'lruvec_page_state_local' is that they read 'state' and 'state_local', >>> respectively. Factor out an inner functions to make the code more concise. >>> Do the same for reading 'memcg_page_stat' and 'memcg_events'. >>> >>> Signed-off-by: Chen Ridong <chenridong@huawei.com> >>> --- >>> mm/memcontrol.c | 72 +++++++++++++++++++++---------------------------- >>> 1 file changed, 30 insertions(+), 42 deletions(-) >>> >>> diff --git a/mm/memcontrol.c b/mm/memcontrol.c >>> index b10e0a8f3375..14541610cad0 100644 >>> --- a/mm/memcontrol.c >>> +++ b/mm/memcontrol.c >>> @@ -375,7 +375,8 @@ struct lruvec_stats { >>> long state_pending[NR_MEMCG_NODE_STAT_ITEMS]; >>> }; >>> >>> -unsigned long lruvec_page_state(struct lruvec *lruvec, enum node_stat_item idx) >>> +static unsigned long __lruvec_page_state(struct lruvec *lruvec, >>> + enum node_stat_item idx, bool local) >>> { >>> struct mem_cgroup_per_node *pn; >>> long x; >>> @@ -389,7 +390,8 @@ unsigned long lruvec_page_state(struct lruvec *lruvec, enum node_stat_item idx) >>> return 0; >>> >>> pn = container_of(lruvec, struct mem_cgroup_per_node, lruvec); >>> - x = READ_ONCE(pn->lruvec_stats->state[i]); >>> + x = local ? READ_ONCE(pn->lruvec_stats->state_local[i]) : >>> + READ_ONCE(pn->lruvec_stats->state[i]); >>> #ifdef CONFIG_SMP >>> if (x < 0) >>> x = 0; >>> @@ -397,27 +399,16 @@ unsigned long lruvec_page_state(struct lruvec *lruvec, enum node_stat_item idx) >>> return x; >>> } >>> >>> + >>> +unsigned long lruvec_page_state(struct lruvec *lruvec, enum node_stat_item idx) >>> +{ >>> + return __lruvec_page_state(lruvec, idx, false); >>> +} >> >> I'd move these wrapper function definitions to memcontrol.h and make them >> static inline. > > +1 > Thank you. Will update. Best regards, Ridong >> >> Other than that, lgtm. >> >> Thank you! ^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2025-01-16 1:21 UTC | newest] Thread overview: 12+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2025-01-14 12:25 [PATCH -v2 next 0/4] Some cleanup for memcg Chen Ridong 2025-01-14 12:25 ` [PATCH -v2 next 1/4] memcg: use OFP_PEAK_UNSET instead of -1 Chen Ridong 2025-01-14 18:36 ` Roman Gushchin 2025-01-14 12:25 ` [PATCH -v2 next 2/4] memcg: call the free function when allocation of pn fails Chen Ridong 2025-01-14 18:36 ` Roman Gushchin 2025-01-14 12:25 ` [PATCH -v2 next 3/4] memcg: factor out the replace_stock_objcg function Chen Ridong 2025-01-14 18:39 ` Roman Gushchin 2025-01-16 0:56 ` Shakeel Butt 2025-01-14 12:25 ` [PATCH -v2 next 4/4] memcg: factor out stat(event)/stat_local(event_local) reading functions Chen Ridong 2025-01-14 18:45 ` Roman Gushchin 2025-01-16 0:57 ` Shakeel Butt 2025-01-16 1:21 ` Chen Ridong
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).