Re: [PATCH RFC cgroup/for-3.7] cgroup: mark subsystems with broken hierarchy support and whine if cgroups are nested for them

[Vivek Goyal <vgoyal@redhat.com>]

On Tue, Sep 11, 2012 at 10:55:15AM -0700, Tejun Heo wrote:
> Hello, Vivek.
> 
> On Tue, Sep 11, 2012 at 01:35:25PM -0400, Vivek Goyal wrote:
> > It is kind of strange. First kernel allows creation of hiearchy for
> > non-hierarchical controllers and it also gives warning for user space to
> > not do that.
> > 
> > If creating hiearchy for flat controllers is wrong then kernel should
> > not allow it in first place and enforce it, instead of just giving a
> > warning to user space to not create the hierarchy.
> > 
> > Initially I had blocked the creation of hierarchy deeper than 1 level
> > but later had to remove it as people wanted libvirt to use blkio
> > controller and seemed to be fine with flat support. In fact there
> > were people who insisted on flat support as they thought that made
> > more sense.
> 
> Yes, it is an ugly situation and we'll have to drag ourselves out of
> this mess gradually.  I hope it hadn't happened like this but what
> happened already happened and I can't see a better way out.  If you
> can, please share.
> 
> > Sure. Just that CFQ code now has become really complicated and messy
> > (especially with that 3 service trees per group) so making it hierarchical
> > is significant amount of effort.
> 
> I think the wording for the warning wasn't entirely accurate.  The
> thing that we wanna warn about is that the hierarchy behavior isn't
> complete yet and it may change in the future.  If we can absolutely
> declare that cfq is and will stay broken in terms of hierarchy
> support, that could work too but I don't really think that's a good
> idea.  It's something we need to do one way or the other.
> 
> > And regarding change of behavior, we can always intoduce a .hierarchy
> > file like cgroup which needs to be explicitly set to make controller
> > truly hiearchical. That way behavior does not change in a subtle manner
> > in future kernel releases. (Not that I am a fan of hierarchy file, just
> > that it is cost we pay for not implementing hierarchical controller to
> > begin with).
> 
> I think that way of thinking is what led to this horrible mess in the
> first place.  More flexibility doesn't equal better.  We can't keep
> piling things on top.  I'd like to at least have an exit strategy.
> use_hierarchy drives us further away from where we wanna be.

Ok, so whole point of warning seems to be so that we can change the
behavior in future and say to user space they few kernel releases back we
had started printing a warning that creating hierarchy is wrong and 
move to a flat setup. So don't complain to us now.?

Are you planning to get rid of .user_hierarchy file from memory cgroup
too? If you are planning not to put such a file in blkio controller,
then it will make sense to remove it from mem_cgorup too.

.use_hierarchy was not set by default in memory controller and I think one
of the reasons was increased overhead of accounting in deep hierarchies.
Has that been sorted out?

The point I am trying to make is that deep hierarchies (5-6 levels) are
/going to be a reality and if accounting overhead is not manageable then
enabling hierarchy by default might not be a practical solution even
if you implement hierarchy support (like memory cgroup), and in that
case retaining .use_hierarchy will make sense.

IIUC, are you saying that now none of the controller will have flat
hiearchy support because there is no way to be able to create flat 
hierarchy. (Any new group is child of root group). So are we moving
towards a model where every controller is hierarhical and there is
no concept of flat hierarchy.

Thanks
Vivek