Re: [RFC][PATCH] cgroup: Fix cgroup_subsys::exit callback

[Paul Menage]

On Mon, Feb 7, 2011 at 12:02 PM, Peter Zijlstra <peterz@xxxxxxxxxxxxx> 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@xxxxxxxxxxxxx> 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
--
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/