Messages in this thread | | | From | Paul Menage <> | Date | Mon, 7 Feb 2011 13:21:35 -0800 | Subject | Re: [RFC][PATCH] cgroup: Fix cgroup_subsys::exit callback |
| |
On Mon, Feb 7, 2011 at 12:02 PM, Peter Zijlstra <peterz@infradead.org> wrote: > On Mon, 2011-02-07 at 11:28 -0800, Paul Menage wrote: >> On Mon, Feb 7, 2011 at 8:10 AM, Peter Zijlstra <peterz@infradead.org> wrote: >> > >> > Make the ::exit method act like ::attach, it is after all very nearly >> > the same thing. >> >> The major difference between attach and exit is that the former is >> only triggered in response to user cgroup-management action, whereas >> the latter is triggered whenever a task exits, even if cgroups aren't >> set up. > > And the major likeness is that they both migrate a task from one cgroup > to another. You cannot simply ignore that.
True, and the exit path for cgroups has always been a bit fuzzy - that was kind of inherited from cpusets, where this worked out OK, but the CPU subsystem has more interesting requirements.
An important semantic difference between attach and exit is that in the exit case the destination is always the root, and the task in question is going to be exiting before doing anything else interesting. So it should be possible to optimize that path a lot compared to the regular attach (many things related to resource usage can be ignored, since the task won't have time to actually use any non-trivial amount of resources).
> > If maybe you're like respond more often than about once every two months > I might actually care what you think.
Yes, sadly since switching groups at Google, cgroups has become pretty much just a spare-time activity for me - but that in itself doesn't automatically invalidate my opinion when I do have time to respond. It's still the case that cgroup_mutex is an incredibly heavyweight mutex that has no business being in the task exit path. Or do you just believe that ad hominem is a valid style of argument?
How about if the exit callback was moved before the preceding task_unlock()? Since I think the scheduler is still the only user of the exit callback, redefining the locking semantics should be fine.
Paul
| |