Hello,
Looks generally good to me. Some comments below.
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.
> 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"?
> + 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.
It might be useful to add comment explaining why memcg needs private
ids.
Thanks.
--
tejun
--
To unsubscribe from this list: send the line "unsubscribe cgroups" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
