Re: [PATCH 06/51] memcg: add mem_cgroup_root_css

[Tejun Heo]

Hey, Michal.

On Wed, Jun 17, 2015 at 04:56:42PM +0200, Michal Hocko wrote:
> On Fri 22-05-15 17:13:20, Tejun Heo wrote:
> > Add global mem_cgroup_root_css which points to the root memcg css.
> 
> Is there any reason to using css rather than mem_cgroup other than the
> structure is not visible outside of memcontrol.c? Because I have a
> patchset which exports it. It is not merged yet so a move to mem_cgroup
> could be done later. I am just interested whether there is a stronger
> reason.

It doesn't really matter either way but I think it makes a bit more
sense to use css as the common type when external code interacts with
cgroup controllers.  e.g. cgroup writeback interacts with both memcg
and blkcg and in most cases it doesn't know or care about their
internal states.  Most of what it wants is tracking them and doing
some common css operations (refcnting, printing and so on) on them.

> > This will be used by cgroup writeback support.  If memcg is disabled,
> > it's defined as ERR_PTR(-EINVAL).
> 
> Hmm. Why EINVAL? I can see only mm/backing-dev.c (in
> review-cgroup-writeback-switch-20150528 branch) which uses it and that
> shouldn't even try to compile if !CONFIG_MEMCG no? Otherwise we would
> simply blow up.

Hmm... the code maybe has changed inbetween but there was something
which depended on the root css being defined when
!CONFIG_CGROUP_WRITEBACK or maybe it was on blkcg_root_css and memcg
side was added for consistency.  An ERR_PTR value is non-zero, which
is an invariant which is often depended upon, while guaranteeing oops
when deref'd.

Thanks.

-- 
tejun
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/