[PATCH 06/12] cgroup, memcg: move cgroup_event implementation to memcg

[Michal Hocko]

On Tue 27-08-13 16:00:02, Tejun Heo wrote:
> Hey, Michal.
> 
> On Tue, Aug 27, 2013 at 04:20:02PM +0200, Michal Hocko wrote:
[...]
> > We have been trying to reduce memcg specific things in the past and
> > this will add non trivial chunk of code. I would at least expect some
> > justification _why_ moving the maintenance burden is worth it. It
> > certainly won't make memcg live easier. I can bite a bullet though if
> > this is the roadblock for making important changes in the cgroup core.
> > But you didn't tell us anything like that, except that you do not like
> > the interface because like other parts it is over-engineered thus bad.
> 
> Yes, it is a road block in two ways.
> 
> * Everything is being made per-css which is necessary as css's
>   lifetime would no longer coincide with cgroup's.  Keeping this in
>   cgroup proper would mean that it needs to be updated so that it's
>   attached to css instead of cgroup.
> 
> * The file system interface of cgroup will go through sysfs so that
>   cgroup doesn't have to worry about inode locking maze.  This is a
>   major issue for nested subsys enable / disable and implementating
>   migration as currently we end up having to lock inodes which belong
>   to different subtrees and vfs doesn't define locking order between
>   them.  So, short of meddling with vfs rename mutex, it'll deadlock.

This is totally new to me. You haven't said this would be a road _block_
before. I am not familiar with the current core cgroup development to
figure the above out myself as you can see.
[...]
> > And we might end up having that code there for ever because your new and
> > yet to be shown interface might turn out to be not the best fit for the
> > current users.
> 
> We're gonna keep that code for years no matter what and you know that
> the existing interface has fundamental issues.  Regardless of what we
> do in the future, it needs to go.  That much is clear, isn't it?

Yes. And I have already said that I do not insist on the interface. I do
not like it much either.  I hope that is clear as well.  Writing magic
fd into a file and getting events was quite weird also from the user
POV. But that was the interface we had for a long time and people are
using it.
I was merely objecting to moving code somewhere where it doesn't belong
IMO. Your changelog lacked the most important information _why_ it has
to move from cgroup core. At least that wasn't obvious to me. Your
general conclusion:
"
cgroup_event is way over-designed and tries to build a generic
flexible event mechanism into cgroup - fully customizable event
specification for each user of the interface.  This is utterly
unnecessary and overboard especially in the light of the planned
unified hierarchy as there's gonna be single agent. Simply generating
events at fixed points, or if that's too restrictive, configureable
cadence or single set of configureable points should be enough.
"
didn't explain that to me and sounds more like, it is bad so shove it
off to the user of the interface.

Update the changelog with the more specific information you have
provided in this email and I will have no problem accepting the patch as
the outcome is clearly higher than the maintenance burden in memcg.

[...]

Thanks for the clarification.
-- 
Michal Hocko
SUSE Labs