* Re: [PATCH] memcg: decouple memcg_hotplug_cpu_dead from stock_lock
2025-04-10 21:06 [PATCH] memcg: decouple memcg_hotplug_cpu_dead from stock_lock Shakeel Butt
@ 2025-04-11 0:00 ` Roman Gushchin
2025-04-11 5:06 ` Andrew Morton
` (3 subsequent siblings)
4 siblings, 0 replies; 14+ messages in thread
From: Roman Gushchin @ 2025-04-11 0:00 UTC (permalink / raw)
To: Shakeel Butt
Cc: Andrew Morton, Johannes Weiner, Michal Hocko, Muchun Song,
Yosry Ahmed, Waiman Long, Vlastimil Babka, linux-mm, cgroups,
linux-kernel, Meta kernel team
Shakeel Butt <shakeel.butt@linux.dev> writes:
> The function memcg_hotplug_cpu_dead works on the stock of a remote dead
> CPU and drain_obj_stock works on the given stock instead of local stock,
> so there is no need to take local stock_lock anymore.
>
> Signed-off-by: Shakeel Butt <shakeel.butt@linux.dev>
Acked-by: Roman Gushchin <roman.gushchin@linux.dev>
Thanks!
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH] memcg: decouple memcg_hotplug_cpu_dead from stock_lock
2025-04-10 21:06 [PATCH] memcg: decouple memcg_hotplug_cpu_dead from stock_lock Shakeel Butt
2025-04-11 0:00 ` Roman Gushchin
@ 2025-04-11 5:06 ` Andrew Morton
2025-04-14 17:52 ` Shakeel Butt
2025-04-11 8:55 ` Vlastimil Babka
` (2 subsequent siblings)
4 siblings, 1 reply; 14+ messages in thread
From: Andrew Morton @ 2025-04-11 5:06 UTC (permalink / raw)
To: Shakeel Butt
Cc: Johannes Weiner, Michal Hocko, Roman Gushchin, Muchun Song,
Yosry Ahmed, Waiman Long, Vlastimil Babka, linux-mm, cgroups,
linux-kernel, Meta kernel team
On Thu, 10 Apr 2025 14:06:23 -0700 Shakeel Butt <shakeel.butt@linux.dev> wrote:
> The function memcg_hotplug_cpu_dead works on the stock of a remote dead
> CPU and drain_obj_stock works on the given stock instead of local stock,
> so there is no need to take local stock_lock anymore.
>
> @@ -1964,10 +1964,10 @@ static int memcg_hotplug_cpu_dead(unsigned int cpu)
>
> stock = &per_cpu(memcg_stock, cpu);
>
> - /* drain_obj_stock requires stock_lock */
> - local_lock_irqsave(&memcg_stock.stock_lock, flags);
> - drain_obj_stock(stock);
> - local_unlock_irqrestore(&memcg_stock.stock_lock, flags);
> + local_irq_save(flag);
> + /* stock of a remote dead cpu, no need for stock_lock. */
> + __drain_obj_stock(stock);
> + local_irq_restore(flag);
>
s/flag/flags/
Obviously what-i-got isn't what-you-tested. Please check what happened
here,
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH] memcg: decouple memcg_hotplug_cpu_dead from stock_lock
2025-04-11 5:06 ` Andrew Morton
@ 2025-04-14 17:52 ` Shakeel Butt
0 siblings, 0 replies; 14+ messages in thread
From: Shakeel Butt @ 2025-04-14 17:52 UTC (permalink / raw)
To: Andrew Morton
Cc: Johannes Weiner, Michal Hocko, Roman Gushchin, Muchun Song,
Yosry Ahmed, Waiman Long, Vlastimil Babka, linux-mm, cgroups,
linux-kernel, Meta kernel team
(I was trying to reply last week but my email stopped working, so trying
again)
On Thu, Apr 10, 2025 at 10:06:18PM -0700, Andrew Morton wrote:
> On Thu, 10 Apr 2025 14:06:23 -0700 Shakeel Butt <shakeel.butt@linux.dev> wrote:
>
> > The function memcg_hotplug_cpu_dead works on the stock of a remote dead
> > CPU and drain_obj_stock works on the given stock instead of local stock,
> > so there is no need to take local stock_lock anymore.
> >
> > @@ -1964,10 +1964,10 @@ static int memcg_hotplug_cpu_dead(unsigned int cpu)
> >
> > stock = &per_cpu(memcg_stock, cpu);
> >
> > - /* drain_obj_stock requires stock_lock */
> > - local_lock_irqsave(&memcg_stock.stock_lock, flags);
> > - drain_obj_stock(stock);
> > - local_unlock_irqrestore(&memcg_stock.stock_lock, flags);
> > + local_irq_save(flag);
> > + /* stock of a remote dead cpu, no need for stock_lock. */
> > + __drain_obj_stock(stock);
> > + local_irq_restore(flag);
> >
>
> s/flag/flags/
>
> Obviously what-i-got isn't what-you-tested. Please check what happened
> here,
Sorry about that. I tested the previous version where I had
drain_obj_stock() as a separate function but after seeing that there is
just one caller, I manually inlined it but forgot to test before
sending.
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] memcg: decouple memcg_hotplug_cpu_dead from stock_lock
2025-04-10 21:06 [PATCH] memcg: decouple memcg_hotplug_cpu_dead from stock_lock Shakeel Butt
2025-04-11 0:00 ` Roman Gushchin
2025-04-11 5:06 ` Andrew Morton
@ 2025-04-11 8:55 ` Vlastimil Babka
2025-04-11 14:06 ` Sebastian Andrzej Siewior
2025-04-11 17:54 ` Shakeel Butt
2025-04-11 23:18 ` kernel test robot
2025-04-11 23:18 ` kernel test robot
4 siblings, 2 replies; 14+ messages in thread
From: Vlastimil Babka @ 2025-04-11 8:55 UTC (permalink / raw)
To: Shakeel Butt, Andrew Morton, Sebastian Andrzej Siewior
Cc: Johannes Weiner, Michal Hocko, Roman Gushchin, Muchun Song,
Yosry Ahmed, Waiman Long, linux-mm, cgroups, linux-kernel,
Meta kernel team
On 4/10/25 23:06, Shakeel Butt wrote:
> The function memcg_hotplug_cpu_dead works on the stock of a remote dead
> CPU and drain_obj_stock works on the given stock instead of local stock,
> so there is no need to take local stock_lock anymore.
>
> Signed-off-by: Shakeel Butt <shakeel.butt@linux.dev>
> ---
> mm/memcontrol.c | 20 ++++++++++++--------
> 1 file changed, 12 insertions(+), 8 deletions(-)
>
> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> index f23a4d0ad239..2178a051bd09 100644
> --- a/mm/memcontrol.c
> +++ b/mm/memcontrol.c
> @@ -1789,7 +1789,7 @@ static DEFINE_PER_CPU(struct memcg_stock_pcp, memcg_stock) = {
> };
> static DEFINE_MUTEX(percpu_charge_mutex);
>
> -static void drain_obj_stock(struct memcg_stock_pcp *stock);
> +static void __drain_obj_stock(struct memcg_stock_pcp *stock);
> static bool obj_stock_flush_required(struct memcg_stock_pcp *stock,
> struct mem_cgroup *root_memcg);
>
> @@ -1873,7 +1873,7 @@ static void drain_local_stock(struct work_struct *dummy)
> local_lock_irqsave(&memcg_stock.stock_lock, flags);
>
> stock = this_cpu_ptr(&memcg_stock);
> - drain_obj_stock(stock);
> + __drain_obj_stock(stock);
> drain_stock(stock);
> clear_bit(FLUSHING_CACHED_CHARGE, &stock->flags);
>
> @@ -1964,10 +1964,10 @@ static int memcg_hotplug_cpu_dead(unsigned int cpu)
>
> stock = &per_cpu(memcg_stock, cpu);
>
> - /* drain_obj_stock requires stock_lock */
> - local_lock_irqsave(&memcg_stock.stock_lock, flags);
> - drain_obj_stock(stock);
> - local_unlock_irqrestore(&memcg_stock.stock_lock, flags);
> + local_irq_save(flag);
I think for RT this is not great? At least in theory, probably it's not
actually used together with cpu hotplug? As it relies on memcg_stats_lock()
I think no irq save/enable is necessary there. local_lock_irqsave wasn't
actually a irq disable on RT. I don't know if there's a handy wrapper for this.
> + /* stock of a remote dead cpu, no need for stock_lock. */
> + __drain_obj_stock(stock);
> + local_irq_restore(flag);
>
> drain_stock(stock);
>
> @@ -2837,7 +2837,11 @@ static bool consume_obj_stock(struct obj_cgroup *objcg, unsigned int nr_bytes,
> return ret;
> }
>
> -static void drain_obj_stock(struct memcg_stock_pcp *stock)
> +/*
> + * Works on the given stock. The callers are responsible for the proper locking
> + * for the local or remote stocks.
> + */
> +static void __drain_obj_stock(struct memcg_stock_pcp *stock)
> {
> struct obj_cgroup *old = READ_ONCE(stock->cached_objcg);
>
> @@ -2925,7 +2929,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 */
> - drain_obj_stock(stock);
> + __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;
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH] memcg: decouple memcg_hotplug_cpu_dead from stock_lock
2025-04-11 8:55 ` Vlastimil Babka
@ 2025-04-11 14:06 ` Sebastian Andrzej Siewior
2025-04-11 17:54 ` Shakeel Butt
1 sibling, 0 replies; 14+ messages in thread
From: Sebastian Andrzej Siewior @ 2025-04-11 14:06 UTC (permalink / raw)
To: Vlastimil Babka
Cc: Shakeel Butt, Andrew Morton, Johannes Weiner, Michal Hocko,
Roman Gushchin, Muchun Song, Yosry Ahmed, Waiman Long, linux-mm,
cgroups, linux-kernel, Meta kernel team
On 2025-04-11 10:55:31 [+0200], Vlastimil Babka wrote:
> > @@ -1964,10 +1964,10 @@ static int memcg_hotplug_cpu_dead(unsigned int cpu)
> >
> > stock = &per_cpu(memcg_stock, cpu);
> >
> > - /* drain_obj_stock requires stock_lock */
> > - local_lock_irqsave(&memcg_stock.stock_lock, flags);
> > - drain_obj_stock(stock);
> > - local_unlock_irqrestore(&memcg_stock.stock_lock, flags);
> > + local_irq_save(flag);
>
> I think for RT this is not great? At least in theory, probably it's not
> actually used together with cpu hotplug? As it relies on memcg_stats_lock()
> I think no irq save/enable is necessary there. local_lock_irqsave wasn't
> actually a irq disable on RT. I don't know if there's a handy wrapper for this.
No seeing the whole context but memcg_hotplug_cpu_dead() should be
invoked the control CPU while "cpu" is already gone. So the local_lock
should be acquired and the target CPU needs no locks since it is
offline. local_irq_save() will break things.
Sebastian
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] memcg: decouple memcg_hotplug_cpu_dead from stock_lock
2025-04-11 8:55 ` Vlastimil Babka
2025-04-11 14:06 ` Sebastian Andrzej Siewior
@ 2025-04-11 17:54 ` Shakeel Butt
2025-04-11 18:06 ` Vlastimil Babka
1 sibling, 1 reply; 14+ messages in thread
From: Shakeel Butt @ 2025-04-11 17:54 UTC (permalink / raw)
To: Vlastimil Babka, Sebastian Andrzej Siewior
Cc: Shakeel Butt, Andrew Morton, Johannes Weiner, Michal Hocko,
Roman Gushchin, Muchun Song, Yosry Ahmed, Waiman Long, linux-mm,
cgroups, linux-kernel, Meta kernel team
(my migadu/linux.dev stopped working and I have to send through gmail,
sorry for any formatting issue)
On Fri, Apr 11, 2025 at 04:06:46PM +0200, Sebastian Andrzej Siewior wrote:
>
> On 2025-04-11 10:55:31 [+0200], Vlastimil Babka wrote:
>
> > @@ -1964,10 +1964,10 @@ static int memcg_hotplug_cpu_dead(unsigned int cpu)
>
> >
>
> > stock = &per_cpu(memcg_stock, cpu);
>
> >
>
> > - /* drain_obj_stock requires stock_lock */
>
> > - local_lock_irqsave(&memcg_stock.stock_lock, flags);
>
> > - drain_obj_stock(stock);
>
> > - local_unlock_irqrestore(&memcg_stock.stock_lock, flags);
>
> > + local_irq_save(flag);
>
> I think for RT this is not great?
>
This is not a performance critical function, so I would not worry about
that.
>
> At least in theory, probably it's not
>
> actually used together with cpu hotplug? As it relies on memcg_stats_lock()
>
> I think no irq save/enable is necessary there. local_lock_irqsave wasn't
>
> actually a irq disable on RT. I don't know if there's a handy wrapper for this.
>
> No seeing the whole context but memcg_hotplug_cpu_dead() should be
>
> invoked the control CPU while "cpu" is already gone. So the local_lock
>
> should be acquired and the target CPU needs no locks since it is
>
> offline. local_irq_save() will break things.
>
I don't see how local_irq_save() will break anything. We are working on
a stock of a dead remote cpu. We actually don't even need to disable irq
or need local cpu's local_lock. It is actually the calls to
__mod_memcg_lruvec_state() and __mod_memcg_state() in
__drain_obj_stock() which need irq-disabled on non-RT kernels and for
RT-kernels they already have preempt_disable_nested().
Disabling irq even on RT seems excessive but this is not a performance
critical code, so I don't see an issue unless there is
local_lock_irqsave() alternative which does not disables irqs on RT
kernels.
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] memcg: decouple memcg_hotplug_cpu_dead from stock_lock
2025-04-11 17:54 ` Shakeel Butt
@ 2025-04-11 18:06 ` Vlastimil Babka
2025-04-11 18:12 ` Shakeel Butt
0 siblings, 1 reply; 14+ messages in thread
From: Vlastimil Babka @ 2025-04-11 18:06 UTC (permalink / raw)
To: Shakeel Butt, Sebastian Andrzej Siewior
Cc: Shakeel Butt, Andrew Morton, Johannes Weiner, Michal Hocko,
Roman Gushchin, Muchun Song, Yosry Ahmed, Waiman Long, linux-mm,
cgroups, linux-kernel, Meta kernel team
On 4/11/25 19:54, Shakeel Butt wrote:
> (my migadu/linux.dev stopped working and I have to send through gmail,
> sorry for any formatting issue)
>
> I don't see how local_irq_save() will break anything. We are working on
> a stock of a dead remote cpu. We actually don't even need to disable irq
> or need local cpu's local_lock. It is actually the calls to
> __mod_memcg_lruvec_state() and __mod_memcg_state() in
> __drain_obj_stock() which need irq-disabled on non-RT kernels and for
> RT-kernels they already have preempt_disable_nested().
>
> Disabling irq even on RT seems excessive but this is not a performance
> critical code, so I don't see an issue unless there is
> local_lock_irqsave() alternative which does not disables irqs on RT
> kernels.
local_lock_irqsave() does not disable irqs on RT kernels :) so keeping
local_lock as is would do the irq disable on !RT and be more RT-friendly on
RT. It's just wrong from the logical scope of the lock to perform it on a
different cpu than the stock we modify. If one day we have some runtime
checks for that, they would complain.
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] memcg: decouple memcg_hotplug_cpu_dead from stock_lock
2025-04-11 18:06 ` Vlastimil Babka
@ 2025-04-11 18:12 ` Shakeel Butt
2025-04-14 17:55 ` Shakeel Butt
0 siblings, 1 reply; 14+ messages in thread
From: Shakeel Butt @ 2025-04-11 18:12 UTC (permalink / raw)
To: Vlastimil Babka
Cc: Sebastian Andrzej Siewior, Shakeel Butt, Andrew Morton,
Johannes Weiner, Michal Hocko, Roman Gushchin, Muchun Song,
Yosry Ahmed, Waiman Long, linux-mm, cgroups, linux-kernel,
Meta kernel team
On Fri, Apr 11, 2025 at 2:06 PM Vlastimil Babka <vbabka@suse.cz> wrote:
>
> On 4/11/25 19:54, Shakeel Butt wrote:
> > (my migadu/linux.dev stopped working and I have to send through gmail,
> > sorry for any formatting issue)
> >
> > I don't see how local_irq_save() will break anything. We are working on
> > a stock of a dead remote cpu. We actually don't even need to disable irq
> > or need local cpu's local_lock. It is actually the calls to
> > __mod_memcg_lruvec_state() and __mod_memcg_state() in
> > __drain_obj_stock() which need irq-disabled on non-RT kernels and for
> > RT-kernels they already have preempt_disable_nested().
> >
> > Disabling irq even on RT seems excessive but this is not a performance
> > critical code, so I don't see an issue unless there is
> > local_lock_irqsave() alternative which does not disables irqs on RT
> > kernels.
>
> local_lock_irqsave() does not disable irqs on RT kernels :)
Sorry, I wanted to say local_irq_save() here instead of local_lock_irqsave().
> so keeping
> local_lock as is would do the irq disable on !RT and be more RT-friendly on
> RT. It's just wrong from the logical scope of the lock to perform it on a
> different cpu than the stock we modify. If one day we have some runtime
> checks for that, they would complain.
Basically I don't want to use stock_lock here. Maybe I should explore
adding a new local_lock for __mod_memcg_lruvec_state and
__mod_memcg_state.
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] memcg: decouple memcg_hotplug_cpu_dead from stock_lock
2025-04-11 18:12 ` Shakeel Butt
@ 2025-04-14 17:55 ` Shakeel Butt
2025-04-15 6:30 ` Sebastian Andrzej Siewior
0 siblings, 1 reply; 14+ messages in thread
From: Shakeel Butt @ 2025-04-14 17:55 UTC (permalink / raw)
To: Vlastimil Babka, Sebastian Andrzej Siewior
Cc: Andrew Morton, Johannes Weiner, Michal Hocko, Roman Gushchin,
Muchun Song, Yosry Ahmed, Waiman Long, linux-mm, cgroups,
linux-kernel, Meta kernel team
On Fri, Apr 11, 2025 at 02:12:46PM -0400, Shakeel Butt wrote:
> On Fri, Apr 11, 2025 at 2:06 PM Vlastimil Babka <vbabka@suse.cz> wrote:
> >
> > On 4/11/25 19:54, Shakeel Butt wrote:
> > > (my migadu/linux.dev stopped working and I have to send through gmail,
> > > sorry for any formatting issue)
> > >
> > > I don't see how local_irq_save() will break anything. We are working on
> > > a stock of a dead remote cpu. We actually don't even need to disable irq
> > > or need local cpu's local_lock. It is actually the calls to
> > > __mod_memcg_lruvec_state() and __mod_memcg_state() in
> > > __drain_obj_stock() which need irq-disabled on non-RT kernels and for
> > > RT-kernels they already have preempt_disable_nested().
> > >
> > > Disabling irq even on RT seems excessive but this is not a performance
> > > critical code, so I don't see an issue unless there is
> > > local_lock_irqsave() alternative which does not disables irqs on RT
> > > kernels.
> >
> > local_lock_irqsave() does not disable irqs on RT kernels :)
>
> Sorry, I wanted to say local_irq_save() here instead of local_lock_irqsave().
>
> > so keeping
> > local_lock as is would do the irq disable on !RT and be more RT-friendly on
> > RT. It's just wrong from the logical scope of the lock to perform it on a
> > different cpu than the stock we modify. If one day we have some runtime
> > checks for that, they would complain.
>
> Basically I don't want to use stock_lock here. Maybe I should explore
> adding a new local_lock for __mod_memcg_lruvec_state and
> __mod_memcg_state.
Vlastimil & Sebastian, if you don't have a strong opinion/push-back on
this patch then I will keep it as is. However I am planning to rework
the memcg stats (& vmstats) to see if I can use dedicated local_lock for
them and able to modify them in any context.
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] memcg: decouple memcg_hotplug_cpu_dead from stock_lock
2025-04-14 17:55 ` Shakeel Butt
@ 2025-04-15 6:30 ` Sebastian Andrzej Siewior
2025-04-15 17:01 ` Shakeel Butt
0 siblings, 1 reply; 14+ messages in thread
From: Sebastian Andrzej Siewior @ 2025-04-15 6:30 UTC (permalink / raw)
To: Shakeel Butt
Cc: Vlastimil Babka, Andrew Morton, Johannes Weiner, Michal Hocko,
Roman Gushchin, Muchun Song, Yosry Ahmed, Waiman Long, linux-mm,
cgroups, linux-kernel, Meta kernel team
On 2025-04-14 10:55:31 [-0700], Shakeel Butt wrote:
> Vlastimil & Sebastian, if you don't have a strong opinion/push-back on
> this patch then I will keep it as is. However I am planning to rework
> the memcg stats (& vmstats) to see if I can use dedicated local_lock for
> them and able to modify them in any context.
Please don't use local_irq_save().
Sebastian
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] memcg: decouple memcg_hotplug_cpu_dead from stock_lock
2025-04-15 6:30 ` Sebastian Andrzej Siewior
@ 2025-04-15 17:01 ` Shakeel Butt
0 siblings, 0 replies; 14+ messages in thread
From: Shakeel Butt @ 2025-04-15 17:01 UTC (permalink / raw)
To: Sebastian Andrzej Siewior
Cc: Vlastimil Babka, Andrew Morton, Johannes Weiner, Michal Hocko,
Roman Gushchin, Muchun Song, Yosry Ahmed, Waiman Long, linux-mm,
cgroups, linux-kernel, Meta kernel team
On Tue, Apr 15, 2025 at 08:30:54AM +0200, Sebastian Andrzej Siewior wrote:
> On 2025-04-14 10:55:31 [-0700], Shakeel Butt wrote:
> > Vlastimil & Sebastian, if you don't have a strong opinion/push-back on
> > this patch then I will keep it as is. However I am planning to rework
> > the memcg stats (& vmstats) to see if I can use dedicated local_lock for
> > them and able to modify them in any context.
>
> Please don't use local_irq_save().
Sounds good. Andrew, can you please drop this patch (I think it was
picked into mm-new).
BTW I think using local_irq_save() is not wrong and just not preferred
for RT kernel, is that correct?
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] memcg: decouple memcg_hotplug_cpu_dead from stock_lock
2025-04-10 21:06 [PATCH] memcg: decouple memcg_hotplug_cpu_dead from stock_lock Shakeel Butt
` (2 preceding siblings ...)
2025-04-11 8:55 ` Vlastimil Babka
@ 2025-04-11 23:18 ` kernel test robot
2025-04-11 23:18 ` kernel test robot
4 siblings, 0 replies; 14+ messages in thread
From: kernel test robot @ 2025-04-11 23:18 UTC (permalink / raw)
To: Shakeel Butt; +Cc: oe-kbuild-all
Hi Shakeel,
kernel test robot noticed the following build errors:
[auto build test ERROR on akpm-mm/mm-everything]
[cannot apply to linus/master v6.15-rc1 next-20250411]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Shakeel-Butt/memcg-decouple-memcg_hotplug_cpu_dead-from-stock_lock/20250411-050829
base: https://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm.git mm-everything
patch link: https://lore.kernel.org/r/20250410210623.1016767-1-shakeel.butt%40linux.dev
patch subject: [PATCH] memcg: decouple memcg_hotplug_cpu_dead from stock_lock
config: x86_64-buildonly-randconfig-001-20250412 (https://download.01.org/0day-ci/archive/20250412/202504120739.K2ifymlR-lkp@intel.com/config)
compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250412/202504120739.K2ifymlR-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202504120739.K2ifymlR-lkp@intel.com/
All error/warnings (new ones prefixed by >>):
In file included from include/linux/bitops.h:7,
from include/linux/radix-tree.h:11,
from include/linux/idr.h:15,
from include/linux/cgroup-defs.h:13,
from mm/memcontrol.c:28:
mm/memcontrol.c: In function 'memcg_hotplug_cpu_dead':
>> mm/memcontrol.c:1967:24: error: 'flag' undeclared (first use in this function); did you mean 'flags'?
1967 | local_irq_save(flag);
| ^~~~
include/linux/typecheck.h:11:16: note: in definition of macro 'typecheck'
11 | typeof(x) __dummy2; \
| ^
include/linux/irqflags.h:239:38: note: in expansion of macro 'raw_local_irq_save'
239 | #define local_irq_save(flags) do { raw_local_irq_save(flags); } while (0)
| ^~~~~~~~~~~~~~~~~~
mm/memcontrol.c:1967:9: note: in expansion of macro 'local_irq_save'
1967 | local_irq_save(flag);
| ^~~~~~~~~~~~~~
mm/memcontrol.c:1967:24: note: each undeclared identifier is reported only once for each function it appears in
1967 | local_irq_save(flag);
| ^~~~
include/linux/typecheck.h:11:16: note: in definition of macro 'typecheck'
11 | typeof(x) __dummy2; \
| ^
include/linux/irqflags.h:239:38: note: in expansion of macro 'raw_local_irq_save'
239 | #define local_irq_save(flags) do { raw_local_irq_save(flags); } while (0)
| ^~~~~~~~~~~~~~~~~~
mm/memcontrol.c:1967:9: note: in expansion of macro 'local_irq_save'
1967 | local_irq_save(flag);
| ^~~~~~~~~~~~~~
>> include/linux/typecheck.h:12:25: warning: comparison of distinct pointer types lacks a cast
12 | (void)(&__dummy == &__dummy2); \
| ^~
include/linux/irqflags.h:172:17: note: in expansion of macro 'typecheck'
172 | typecheck(unsigned long, flags); \
| ^~~~~~~~~
include/linux/irqflags.h:239:38: note: in expansion of macro 'raw_local_irq_save'
239 | #define local_irq_save(flags) do { raw_local_irq_save(flags); } while (0)
| ^~~~~~~~~~~~~~~~~~
mm/memcontrol.c:1967:9: note: in expansion of macro 'local_irq_save'
1967 | local_irq_save(flag);
| ^~~~~~~~~~~~~~
>> include/linux/typecheck.h:12:25: warning: comparison of distinct pointer types lacks a cast
12 | (void)(&__dummy == &__dummy2); \
| ^~
include/linux/irqflags.h:177:17: note: in expansion of macro 'typecheck'
177 | typecheck(unsigned long, flags); \
| ^~~~~~~~~
include/linux/irqflags.h:240:39: note: in expansion of macro 'raw_local_irq_restore'
240 | #define local_irq_restore(flags) do { raw_local_irq_restore(flags); } while (0)
| ^~~~~~~~~~~~~~~~~~~~~
mm/memcontrol.c:1970:9: note: in expansion of macro 'local_irq_restore'
1970 | local_irq_restore(flag);
| ^~~~~~~~~~~~~~~~~
>> mm/memcontrol.c:1963:23: warning: unused variable 'flags' [-Wunused-variable]
1963 | unsigned long flags;
| ^~~~~
vim +1967 mm/memcontrol.c
1959
1960 static int memcg_hotplug_cpu_dead(unsigned int cpu)
1961 {
1962 struct memcg_stock_pcp *stock;
> 1963 unsigned long flags;
1964
1965 stock = &per_cpu(memcg_stock, cpu);
1966
> 1967 local_irq_save(flag);
1968 /* stock of a remote dead cpu, no need for stock_lock. */
1969 __drain_obj_stock(stock);
1970 local_irq_restore(flag);
1971
1972 drain_stock(stock);
1973
1974 return 0;
1975 }
1976
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH] memcg: decouple memcg_hotplug_cpu_dead from stock_lock
2025-04-10 21:06 [PATCH] memcg: decouple memcg_hotplug_cpu_dead from stock_lock Shakeel Butt
` (3 preceding siblings ...)
2025-04-11 23:18 ` kernel test robot
@ 2025-04-11 23:18 ` kernel test robot
4 siblings, 0 replies; 14+ messages in thread
From: kernel test robot @ 2025-04-11 23:18 UTC (permalink / raw)
To: Shakeel Butt; +Cc: llvm, oe-kbuild-all
Hi Shakeel,
kernel test robot noticed the following build errors:
[auto build test ERROR on akpm-mm/mm-everything]
[cannot apply to linus/master v6.15-rc1 next-20250411]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Shakeel-Butt/memcg-decouple-memcg_hotplug_cpu_dead-from-stock_lock/20250411-050829
base: https://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm.git mm-everything
patch link: https://lore.kernel.org/r/20250410210623.1016767-1-shakeel.butt%40linux.dev
patch subject: [PATCH] memcg: decouple memcg_hotplug_cpu_dead from stock_lock
config: i386-buildonly-randconfig-002-20250412 (https://download.01.org/0day-ci/archive/20250412/202504120759.hMxys5DQ-lkp@intel.com/config)
compiler: clang version 20.1.2 (https://github.com/llvm/llvm-project 58df0ef89dd64126512e4ee27b4ac3fd8ddf6247)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250412/202504120759.hMxys5DQ-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202504120759.hMxys5DQ-lkp@intel.com/
All errors (new ones prefixed by >>):
>> mm/memcontrol.c:1967:17: error: use of undeclared identifier 'flag'; did you mean 'flags'?
1967 | local_irq_save(flag);
| ^~~~
| flags
include/linux/irqflags.h:216:22: note: expanded from macro 'local_irq_save'
216 | raw_local_irq_save(flags); \
| ^
include/linux/irqflags.h:172:28: note: expanded from macro 'raw_local_irq_save'
172 | typecheck(unsigned long, flags); \
| ^
include/linux/typecheck.h:11:9: note: expanded from macro 'typecheck'
11 | typeof(x) __dummy2; \
| ^
mm/memcontrol.c:1963:16: note: 'flags' declared here
1963 | unsigned long flags;
| ^
>> mm/memcontrol.c:1967:17: error: use of undeclared identifier 'flag'; did you mean 'flags'?
1967 | local_irq_save(flag);
| ^~~~
| flags
include/linux/irqflags.h:216:22: note: expanded from macro 'local_irq_save'
216 | raw_local_irq_save(flags); \
| ^
include/linux/irqflags.h:173:3: note: expanded from macro 'raw_local_irq_save'
173 | flags = arch_local_irq_save(); \
| ^
mm/memcontrol.c:1963:16: note: 'flags' declared here
1963 | unsigned long flags;
| ^
>> mm/memcontrol.c:1967:17: error: use of undeclared identifier 'flag'; did you mean 'flags'?
1967 | local_irq_save(flag);
| ^~~~
| flags
include/linux/irqflags.h:217:32: note: expanded from macro 'local_irq_save'
217 | if (!raw_irqs_disabled_flags(flags)) \
| ^
include/linux/irqflags.h:188:28: note: expanded from macro 'raw_irqs_disabled_flags'
188 | typecheck(unsigned long, flags); \
| ^
include/linux/typecheck.h:11:9: note: expanded from macro 'typecheck'
11 | typeof(x) __dummy2; \
| ^
include/linux/compiler.h:55:47: note: expanded from macro 'if'
55 | #define if(cond, ...) if ( __trace_if_var( !!(cond , ## __VA_ARGS__) ) )
| ^
include/linux/compiler.h:57:52: note: expanded from macro '__trace_if_var'
57 | #define __trace_if_var(cond) (__builtin_constant_p(cond) ? (cond) : __trace_if_value(cond))
| ^
mm/memcontrol.c:1963:16: note: 'flags' declared here
1963 | unsigned long flags;
| ^
>> mm/memcontrol.c:1967:17: error: use of undeclared identifier 'flag'; did you mean 'flags'?
1967 | local_irq_save(flag);
| ^~~~
| flags
include/linux/irqflags.h:217:32: note: expanded from macro 'local_irq_save'
217 | if (!raw_irqs_disabled_flags(flags)) \
| ^
include/linux/irqflags.h:189:28: note: expanded from macro 'raw_irqs_disabled_flags'
189 | arch_irqs_disabled_flags(flags); \
| ^
include/linux/compiler.h:55:47: note: expanded from macro 'if'
55 | #define if(cond, ...) if ( __trace_if_var( !!(cond , ## __VA_ARGS__) ) )
| ^
include/linux/compiler.h:57:52: note: expanded from macro '__trace_if_var'
57 | #define __trace_if_var(cond) (__builtin_constant_p(cond) ? (cond) : __trace_if_value(cond))
| ^
mm/memcontrol.c:1963:16: note: 'flags' declared here
1963 | unsigned long flags;
| ^
>> mm/memcontrol.c:1967:17: error: use of undeclared identifier 'flag'; did you mean 'flags'?
1967 | local_irq_save(flag);
| ^~~~
| flags
include/linux/irqflags.h:217:32: note: expanded from macro 'local_irq_save'
217 | if (!raw_irqs_disabled_flags(flags)) \
| ^
include/linux/irqflags.h:188:28: note: expanded from macro 'raw_irqs_disabled_flags'
188 | typecheck(unsigned long, flags); \
| ^
include/linux/typecheck.h:11:9: note: expanded from macro 'typecheck'
11 | typeof(x) __dummy2; \
| ^
include/linux/compiler.h:55:47: note: expanded from macro 'if'
55 | #define if(cond, ...) if ( __trace_if_var( !!(cond , ## __VA_ARGS__) ) )
| ^
include/linux/compiler.h:57:61: note: expanded from macro '__trace_if_var'
57 | #define __trace_if_var(cond) (__builtin_constant_p(cond) ? (cond) : __trace_if_value(cond))
| ^
mm/memcontrol.c:1963:16: note: 'flags' declared here
1963 | unsigned long flags;
| ^
>> mm/memcontrol.c:1967:17: error: use of undeclared identifier 'flag'; did you mean 'flags'?
1967 | local_irq_save(flag);
| ^~~~
| flags
include/linux/irqflags.h:217:32: note: expanded from macro 'local_irq_save'
217 | if (!raw_irqs_disabled_flags(flags)) \
| ^
include/linux/irqflags.h:189:28: note: expanded from macro 'raw_irqs_disabled_flags'
189 | arch_irqs_disabled_flags(flags); \
| ^
include/linux/compiler.h:55:47: note: expanded from macro 'if'
55 | #define if(cond, ...) if ( __trace_if_var( !!(cond , ## __VA_ARGS__) ) )
| ^
include/linux/compiler.h:57:61: note: expanded from macro '__trace_if_var'
57 | #define __trace_if_var(cond) (__builtin_constant_p(cond) ? (cond) : __trace_if_value(cond))
| ^
mm/memcontrol.c:1963:16: note: 'flags' declared here
1963 | unsigned long flags;
| ^
>> mm/memcontrol.c:1967:17: error: use of undeclared identifier 'flag'; did you mean 'flags'?
1967 | local_irq_save(flag);
| ^~~~
| flags
include/linux/irqflags.h:217:32: note: expanded from macro 'local_irq_save'
217 | if (!raw_irqs_disabled_flags(flags)) \
| ^
include/linux/irqflags.h:188:28: note: expanded from macro 'raw_irqs_disabled_flags'
188 | typecheck(unsigned long, flags); \
| ^
include/linux/typecheck.h:11:9: note: expanded from macro 'typecheck'
11 | typeof(x) __dummy2; \
| ^
include/linux/compiler.h:55:47: note: expanded from macro 'if'
55 | #define if(cond, ...) if ( __trace_if_var( !!(cond , ## __VA_ARGS__) ) )
| ^
include/linux/compiler.h:57:86: note: expanded from macro '__trace_if_var'
57 | #define __trace_if_var(cond) (__builtin_constant_p(cond) ? (cond) : __trace_if_value(cond))
| ^
include/linux/compiler.h:68:3: note: expanded from macro '__trace_if_value'
68 | (cond) ? \
| ^
mm/memcontrol.c:1963:16: note: 'flags' declared here
1963 | unsigned long flags;
| ^
>> mm/memcontrol.c:1967:17: error: use of undeclared identifier 'flag'; did you mean 'flags'?
1967 | local_irq_save(flag);
| ^~~~
| flags
include/linux/irqflags.h:217:32: note: expanded from macro 'local_irq_save'
217 | if (!raw_irqs_disabled_flags(flags)) \
| ^
include/linux/irqflags.h:189:28: note: expanded from macro 'raw_irqs_disabled_flags'
189 | arch_irqs_disabled_flags(flags); \
| ^
include/linux/compiler.h:55:47: note: expanded from macro 'if'
55 | #define if(cond, ...) if ( __trace_if_var( !!(cond , ## __VA_ARGS__) ) )
| ^
include/linux/compiler.h:57:86: note: expanded from macro '__trace_if_var'
57 | #define __trace_if_var(cond) (__builtin_constant_p(cond) ? (cond) : __trace_if_value(cond))
| ^
include/linux/compiler.h:68:3: note: expanded from macro '__trace_if_value'
68 | (cond) ? \
| ^
mm/memcontrol.c:1963:16: note: 'flags' declared here
1963 | unsigned long flags;
| ^
mm/memcontrol.c:1970:20: error: use of undeclared identifier 'flag'; did you mean 'flags'?
1970 | local_irq_restore(flag);
| ^~~~
| flags
include/linux/irqflags.h:223:32: note: expanded from macro 'local_irq_restore'
223 | if (!raw_irqs_disabled_flags(flags)) \
| ^
include/linux/irqflags.h:188:28: note: expanded from macro 'raw_irqs_disabled_flags'
188 | typecheck(unsigned long, flags); \
| ^
include/linux/typecheck.h:11:9: note: expanded from macro 'typecheck'
11 | typeof(x) __dummy2; \
| ^
include/linux/compiler.h:55:47: note: expanded from macro 'if'
55 | #define if(cond, ...) if ( __trace_if_var( !!(cond , ## __VA_ARGS__) ) )
| ^
include/linux/compiler.h:57:52: note: expanded from macro '__trace_if_var'
57 | #define __trace_if_var(cond) (__builtin_constant_p(cond) ? (cond) : __trace_if_value(cond))
| ^
mm/memcontrol.c:1963:16: note: 'flags' declared here
1963 | unsigned long flags;
| ^
mm/memcontrol.c:1970:20: error: use of undeclared identifier 'flag'; did you mean 'flags'?
1970 | local_irq_restore(flag);
| ^~~~
| flags
include/linux/irqflags.h:223:32: note: expanded from macro 'local_irq_restore'
223 | if (!raw_irqs_disabled_flags(flags)) \
| ^
include/linux/irqflags.h:189:28: note: expanded from macro 'raw_irqs_disabled_flags'
189 | arch_irqs_disabled_flags(flags); \
| ^
include/linux/compiler.h:55:47: note: expanded from macro 'if'
55 | #define if(cond, ...) if ( __trace_if_var( !!(cond , ## __VA_ARGS__) ) )
| ^
include/linux/compiler.h:57:52: note: expanded from macro '__trace_if_var'
57 | #define __trace_if_var(cond) (__builtin_constant_p(cond) ? (cond) : __trace_if_value(cond))
| ^
mm/memcontrol.c:1963:16: note: 'flags' declared here
1963 | unsigned long flags;
| ^
mm/memcontrol.c:1970:20: error: use of undeclared identifier 'flag'; did you mean 'flags'?
1970 | local_irq_restore(flag);
| ^~~~
| flags
include/linux/irqflags.h:223:32: note: expanded from macro 'local_irq_restore'
223 | if (!raw_irqs_disabled_flags(flags)) \
| ^
include/linux/irqflags.h:188:28: note: expanded from macro 'raw_irqs_disabled_flags'
188 | typecheck(unsigned long, flags); \
| ^
include/linux/typecheck.h:11:9: note: expanded from macro 'typecheck'
11 | typeof(x) __dummy2; \
| ^
include/linux/compiler.h:55:47: note: expanded from macro 'if'
55 | #define if(cond, ...) if ( __trace_if_var( !!(cond , ## __VA_ARGS__) ) )
| ^
include/linux/compiler.h:57:61: note: expanded from macro '__trace_if_var'
57 | #define __trace_if_var(cond) (__builtin_constant_p(cond) ? (cond) : __trace_if_value(cond))
| ^
mm/memcontrol.c:1963:16: note: 'flags' declared here
1963 | unsigned long flags;
| ^
mm/memcontrol.c:1970:20: error: use of undeclared identifier 'flag'; did you mean 'flags'?
1970 | local_irq_restore(flag);
| ^~~~
| flags
include/linux/irqflags.h:223:32: note: expanded from macro 'local_irq_restore'
223 | if (!raw_irqs_disabled_flags(flags)) \
| ^
include/linux/irqflags.h:189:28: note: expanded from macro 'raw_irqs_disabled_flags'
189 | arch_irqs_disabled_flags(flags); \
| ^
include/linux/compiler.h:55:47: note: expanded from macro 'if'
55 | #define if(cond, ...) if ( __trace_if_var( !!(cond , ## __VA_ARGS__) ) )
| ^
include/linux/compiler.h:57:61: note: expanded from macro '__trace_if_var'
57 | #define __trace_if_var(cond) (__builtin_constant_p(cond) ? (cond) : __trace_if_value(cond))
| ^
vim +1967 mm/memcontrol.c
1959
1960 static int memcg_hotplug_cpu_dead(unsigned int cpu)
1961 {
1962 struct memcg_stock_pcp *stock;
1963 unsigned long flags;
1964
1965 stock = &per_cpu(memcg_stock, cpu);
1966
> 1967 local_irq_save(flag);
1968 /* stock of a remote dead cpu, no need for stock_lock. */
1969 __drain_obj_stock(stock);
1970 local_irq_restore(flag);
1971
1972 drain_stock(stock);
1973
1974 return 0;
1975 }
1976
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 14+ messages in thread