Message ID | 20250314061511.1308152-8-shakeel.butt@linux.dev (mailing list archive) |
---|---|
State | New |
Headers | show |
Series | memcg: stock code cleanups | expand |
On 3/14/25 07:15, Shakeel Butt wrote: > For non-PREEMPT_RT kernels, drain_obj_stock() is always called with irq > disabled, so we can use __mod_memcg_state() instead of > mod_memcg_state(). For PREEMPT_RT, we need to add memcg_stats_[un]lock > in __mod_memcg_state(). > > Signed-off-by: Shakeel Butt <shakeel.butt@linux.dev> Maybe it'll make sense later but as of this patch itself it begs a question why put memcg_stats_lock()/unlock() in __mod_memcg_state itself and not just around the call in drain_obj_stock()? > --- > mm/memcontrol.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/mm/memcontrol.c b/mm/memcontrol.c > index 3c4de384b5a0..dfe9c2eb7816 100644 > --- a/mm/memcontrol.c > +++ b/mm/memcontrol.c > @@ -707,10 +707,12 @@ void __mod_memcg_state(struct mem_cgroup *memcg, enum memcg_stat_item idx, > if (WARN_ONCE(BAD_STAT_IDX(i), "%s: missing stat item %d\n", __func__, idx)) > return; > > + memcg_stats_lock(); > __this_cpu_add(memcg->vmstats_percpu->state[i], val); > val = memcg_state_val_in_pages(idx, val); > memcg_rstat_updated(memcg, val); > trace_mod_memcg_state(memcg, idx, val); > + memcg_stats_unlock(); > } > > #ifdef CONFIG_MEMCG_V1 > @@ -2845,7 +2847,7 @@ static void drain_obj_stock(struct memcg_stock_pcp *stock) > > memcg = get_mem_cgroup_from_objcg(old); > > - mod_memcg_state(memcg, MEMCG_KMEM, -nr_pages); > + __mod_memcg_state(memcg, MEMCG_KMEM, -nr_pages); > memcg1_account_kmem(memcg, -nr_pages); > if (!mem_cgroup_is_root(memcg)) > memcg_uncharge(memcg, nr_pages);
On 2025-03-13 23:15:08 [-0700], Shakeel Butt wrote: > For non-PREEMPT_RT kernels, drain_obj_stock() is always called with irq > disabled, so we can use __mod_memcg_state() instead of > mod_memcg_state(). For PREEMPT_RT, we need to add memcg_stats_[un]lock > in __mod_memcg_state(). > > Signed-off-by: Shakeel Butt <shakeel.butt@linux.dev> Reviewed-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de> Sebastian
On 2025-03-14 11:27:40 [+0100], Vlastimil Babka wrote: > On 3/14/25 07:15, Shakeel Butt wrote: > > For non-PREEMPT_RT kernels, drain_obj_stock() is always called with irq > > disabled, so we can use __mod_memcg_state() instead of > > mod_memcg_state(). For PREEMPT_RT, we need to add memcg_stats_[un]lock > > in __mod_memcg_state(). > > > > Signed-off-by: Shakeel Butt <shakeel.butt@linux.dev> > > Maybe it'll make sense later but as of this patch itself it begs a question > why put memcg_stats_lock()/unlock() in __mod_memcg_state itself and not just > around the call in drain_obj_stock()? The memcg_stats_lock() were introduce to protect the per-CPU counters (vmstats_percpu) on PREEMPT_RT which are protected on !PREEMPT_RT by disabling interrupts. Other modifier have this already except for __mod_memcg_state() because mod_memcg_state() was the only user and already disables interrupt for the operation. Sebastian
diff --git a/mm/memcontrol.c b/mm/memcontrol.c index 3c4de384b5a0..dfe9c2eb7816 100644 --- a/mm/memcontrol.c +++ b/mm/memcontrol.c @@ -707,10 +707,12 @@ void __mod_memcg_state(struct mem_cgroup *memcg, enum memcg_stat_item idx, if (WARN_ONCE(BAD_STAT_IDX(i), "%s: missing stat item %d\n", __func__, idx)) return; + memcg_stats_lock(); __this_cpu_add(memcg->vmstats_percpu->state[i], val); val = memcg_state_val_in_pages(idx, val); memcg_rstat_updated(memcg, val); trace_mod_memcg_state(memcg, idx, val); + memcg_stats_unlock(); } #ifdef CONFIG_MEMCG_V1 @@ -2845,7 +2847,7 @@ static void drain_obj_stock(struct memcg_stock_pcp *stock) memcg = get_mem_cgroup_from_objcg(old); - mod_memcg_state(memcg, MEMCG_KMEM, -nr_pages); + __mod_memcg_state(memcg, MEMCG_KMEM, -nr_pages); memcg1_account_kmem(memcg, -nr_pages); if (!mem_cgroup_is_root(memcg)) memcg_uncharge(memcg, nr_pages);
For non-PREEMPT_RT kernels, drain_obj_stock() is always called with irq disabled, so we can use __mod_memcg_state() instead of mod_memcg_state(). For PREEMPT_RT, we need to add memcg_stats_[un]lock in __mod_memcg_state(). Signed-off-by: Shakeel Butt <shakeel.butt@linux.dev> --- mm/memcontrol.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)