On Fri, 10 Feb 2023 15:49:47 +0000 Yafang Shao <laoar.shao@xxxxxxxxx> wrote: > The extra space which is used to store the obj_cgroup membership is only > valid when kmemcg is enabled. The kmemcg can be disabled via the kernel > parameter "cgroup.memory=nokmem" at runtime. > This helper is also used in non-memcg code, for example the tracepoint, > so we should fix it. > > It was found by code review when I was implementing bpf memory usage[1]. > No real issue happens in production environment. > > ... > > --- a/mm/percpu-internal.h > +++ b/mm/percpu-internal.h > @@ -4,6 +4,7 @@ > > #include <linux/types.h> > #include <linux/percpu.h> > +#include <linux/memcontrol.h> > > /* > * pcpu_block_md is the metadata block struct. > @@ -125,7 +126,8 @@ static inline size_t pcpu_obj_full_size(size_t size) > size_t extra_size = 0; > > #ifdef CONFIG_MEMCG_KMEM > - extra_size += size / PCPU_MIN_ALLOC_SIZE * sizeof(struct obj_cgroup *); > + if (!mem_cgroup_kmem_disabled()) > + extra_size += size / PCPU_MIN_ALLOC_SIZE * sizeof(struct obj_cgroup *); > #endif > > return size * num_possible_cpus() + extra_size; Seems risky at the first look - enabling kmemcg at runtime will make prior calculations based on pcpu_obj_full_size) incorrect. But as long as this is only used for accounting I guess that's OK. What happens if we do a bunch of allocations with kmemcg enabled, then disable kmemcg then free those allocations, or some such thing. Does the accounting end up wrong? The final sentence in the pcpu_obj_full_size() kerneldoc could do with an update - it still implies that the extra_size accounting is unconditional.
