Re: [PATCH] memcg: enable accounting for pids in nested pid namespaces

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, Apr 23, 2021 at 05:53:43AM +0300, Vasily Averin wrote:
> On 4/23/21 5:30 AM, Roman Gushchin wrote:
> > On Fri, Apr 23, 2021 at 05:09:01AM +0300, Vasily Averin wrote:
> >> On 4/23/21 4:00 AM, Roman Gushchin wrote:
> >>> On Thu, Apr 22, 2021 at 08:44:15AM +0300, Vasily Averin wrote:
> >>>> init_pid_ns.pid_cachep have enabled memcg accounting, though this
> >>>> setting was disabled for nested pid namespaces.
> >>>>
> >>>> Signed-off-by: Vasily Averin <vvs@xxxxxxxxxxxxx>
> >>>> ---
> >>>>  kernel/pid_namespace.c | 3 ++-
> >>>>  1 file changed, 2 insertions(+), 1 deletion(-)
> >>>>
> >>>> diff --git a/kernel/pid_namespace.c b/kernel/pid_namespace.c
> >>>> index 6cd6715..a46a372 100644
> >>>> --- a/kernel/pid_namespace.c
> >>>> +++ b/kernel/pid_namespace.c
> >>>> @@ -51,7 +51,8 @@ static struct kmem_cache *create_pid_cachep(unsigned int level)
> >>>>  	mutex_lock(&pid_caches_mutex);
> >>>>  	/* Name collision forces to do allocation under mutex. */
> >>>>  	if (!*pkc)
> >>>> -		*pkc = kmem_cache_create(name, len, 0, SLAB_HWCACHE_ALIGN, 0);
> >>>> +		*pkc = kmem_cache_create(name, len, 0,
> >>>> +					 SLAB_HWCACHE_ALIGN | SLAB_ACCOUNT, 0);
> >>>>  	mutex_unlock(&pid_caches_mutex);
> >>>>  	/* current can fail, but someone else can succeed. */
> >>>>  	return READ_ONCE(*pkc);
> >>>> -- 
> >>>> 1.8.3.1
> >>>>
> >>>
> >>> It looks good to me! It makes total sense to apply the same rules to the root
> >>> and non-root levels.
> >>>
> >>> Acked-by: Roman Gushchin <guro@xxxxxx>
> >>>
> >>> Btw, is there any reason why this patch is not included into the series?
> >>
> >> It is a bugfix and I think it should be added to upstream ASAP.
> > 
> > Then it would be really useful to add some details on why it's a bug,
> > what kind of problems it causes, etc. If it has to be backported to
> > stable, please, add cc stable/fixes tag.
> 
> I mean, in this case we already decided to account pids, but forget to do it.
> In another cases we did not have final decision about accounting.
> 
> I doubt we specially denied accounting for pids frem nested pid namespaces,
> especially because they consumes more memory.
> We can expect that all pids are accounted -- but it does not happen in fact.

As Roman said you should probably explain this in the cover letter and
essentially justify why this should be backported. The thing with
changes such as this is that it's easy for someone to reply and go
"noone noticed for <n> years so why do we care now?". Otherwise:

Acked-by: Christian Brauner <christian.brauner@xxxxxxxxxx>

Christian



[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux OMAP]     [Linux MIPS]     [eCos]     [Asterisk Internet PBX]     [Linux API]     [Monitors]

  Powered by Linux