On Sat, Feb 11, 2023 at 6:05 AM Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx> wrote: > > 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? > That won't happen, because we can only enable and disable kmemcg at boot time. > 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. > Sure. -- Regards Yafang
