Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1425309

Re: [PATCH] mm: memcontrol: fix cgroup creation failure after many small jobs

Show all headers | View raw

On Thu, Jun 16, 2016 at 04:06:17PM -0400, Tejun Heo wrote:
> On Wed, Jun 15, 2016 at 11:42:44PM -0400, Johannes Weiner wrote:
> > @@ -6205,6 +6205,24 @@ struct cgroup *cgroup_get_from_path(const char *path)
> >  }
> >  EXPORT_SYMBOL_GPL(cgroup_get_from_path);
> >  
> > +/**
> > + * css_id_free - relinquish an existing CSS's ID
> > + * @css: the CSS
> > + *
> > + * This releases the @css's ID and allows it to be recycled while the
> > + * CSS continues to exist. This is useful for controllers with state
> > + * that extends past a cgroup's lifetime but doesn't need precious ID
> > + * address space.
> > + *
> > + * This invalidates @css->id, and css_from_id() might return NULL or a
> > + * new css if the ID has been recycled in the meantime.
> > + */
> > +void css_id_free(struct cgroup_subsys_state *css)
> > +{
> > +	cgroup_idr_remove(&css->ss->css_idr, css->id);
> > +	css->id = 0;
> > +}
> 
> I don't quite get why we're trying to free css->id earlier when memcg
> is gonna be using its private id anyway.  From cgroup core side, the
> id space isn't restricted.

For some reason I was thinking of CSS ID being restricted as well, but
of course the only restriction is what's enforced in memcg onlining. I
deleted it.

> > diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> > index 75e74408cc8f..1d8a6dffdc25 100644
> > --- a/mm/memcontrol.c
> > +++ b/mm/memcontrol.c
> ...
> > +static void mem_cgroup_id_put(struct mem_cgroup *memcg)
> > +{
> > +	if (atomic_dec_and_test(&memcg->id.ref)) {
> > +		idr_remove(&mem_cgroup_idr, memcg->id.id);
> 
> Maybe this should do "memcg->id.id = 0"?

Added.

> > +		css_id_free(&memcg->css);
> > +		css_put(&memcg->css);
> > +	}
> > +}
> > +
> > +/**
> > + * mem_cgroup_from_id - look up a memcg from a memcg id
> > + * @id: the memcg id to look up
> > + *
> > + * Caller must hold rcu_read_lock().
> > + */
> > +struct mem_cgroup *mem_cgroup_from_id(unsigned short id)
> > +{
> > +	WARN_ON_ONCE(!rcu_read_lock_held());
> > +	return id > 0 ? idr_find(&mem_cgroup_idr, id) : NULL;
> > +}
> 
> css_from_id() has it too but I don't think id > 0 test is necessary.
> We prolly should take it out of css_from_id() too.

Yeah, idr_find() just returns NULL for index 0 - no warning. I removed
it from my patch and added a patch to remove it in css_from_id().

> It might be useful to add comment explaining why memcg needs private
> ids.

Good point. I put an intro comment above the mem_cgroup_idr definition
that explains why we need a private space.

Thanks

Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread

Thread

[PATCH] mm: memcontrol: fix cgroup creation failure after many small jobs Johannes Weiner <hannes@cmpxchg.org> - 2016-06-16 05:50 +0200
  Re: [PATCH] mm: memcontrol: fix cgroup creation failure after many  small jobs Tejun Heo <tj@kernel.org> - 2016-06-16 22:10 +0200
    [PATCH 3/3] mm: memcontrol: fix cgroup creation failure after many  small jobs Johannes Weiner <hannes@cmpxchg.org> - 2016-06-17 18:30 +0200
      Re: [PATCH 3/3] mm: memcontrol: fix cgroup creation failure after  many small jobs Tejun Heo <tj@kernel.org> - 2016-06-17 20:20 +0200
      Re: [PATCH 3/3] mm: memcontrol: fix cgroup creation failure after  many small jobs Nikolay Borisov <kernel@kyup.com> - 2016-06-20 08:20 +0200
      Re: [PATCH 3/3] mm: memcontrol: fix cgroup creation failure after  many small jobs Vladimir Davydov <vdavydov@virtuozzo.com> - 2016-06-21 12:40 +0200
        Re: [PATCH 3/3] mm: memcontrol: fix cgroup creation failure after  many small jobs Johannes Weiner <hannes@cmpxchg.org> - 2016-06-21 17:50 +0200
    [PATCH 1/3] cgroup: fix idr leak for the first cgroup root Johannes Weiner <hannes@cmpxchg.org> - 2016-06-17 18:30 +0200
    [PATCH 2/3] cgroup: remove unnecessary 0 check from css_from_id() Johannes Weiner <hannes@cmpxchg.org> - 2016-06-17 18:30 +0200
      Re: [PATCH 2/3] cgroup: remove unnecessary 0 check from css_from_id() Tejun Heo <tj@kernel.org> - 2016-06-17 20:20 +0200
    Re: [PATCH] mm: memcontrol: fix cgroup creation failure after many  small jobs Johannes Weiner <hannes@cmpxchg.org> - 2016-06-17 18:30 +0200
  Re: [PATCH] mm: memcontrol: fix cgroup creation failure after many  small jobs Johannes Weiner <hannes@cmpxchg.org> - 2016-06-17 18:50 +0200

csiph-web