[Sorry for the late reply, I was mostly offline last week] On Thu 15-08-13 12:02:24, Tejun Heo wrote: > 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. I guess you are talking about thresholds here. Having a configurable static table of them will probably work out fine. But, how do I tell such an interface that I want to get only MEDIUM vmpressure events? Do I have a special file for each pressure level? Doing notification and read() every time might be a concern for embedded-world who are worried about too many wakeups. We have already seen suggestions to do a different modes of event triggering to reduce wake up costs because of the power consumption (e.g. http://comments.gmane.org/gmane.linux.kernel.mm/101628). > Thankfully, memcg is the only user and gets to keep it. Replacing it > with something simpler on sane_behavior is strongly recommended. You just forgot to tell us what is that "something simpler". Does it exist yet? What is the semantic? 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. You have mentioned it will help you clean up code further in the past but I do not see any mention about it in this patch neither in the leader email. Could you be more specific? How much? Is this piece of code blocking those cleanups? That is what I _really_ dislike about this patch and why I am really reluctant to ack it. > This patch moves cgroup_event and "cgroup.event_control" > implementation to mm/memcontrol.c. 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. Tejun, you have done _a lot_ of great work on cleaning up cgroup core mess and I really appreciate that! I was supporting you in some of those, I wish I had more time to do more. But I think you are deprecating some things too easily without carrying much about the current users assuming they will cope with that somehow and that a magic central authority will do everything for them in a sane way. I am quite skeptical, to be honest. -- Michal Hocko SUSE Labs _______________________________________________ Containers mailing list Containers@xxxxxxxxxxxxxxxxxxxxxxxxxx https://lists.linuxfoundation.org/mailman/listinfo/containers
