Re: [PATCH v4] sched: automated per session task groups

Mike Galbraith
2010-12-01T05:57:54Z

On Tue, 2010-11-30 at 14:36 -0500, Vivek Goyal wrote:

> Few things.
>
> - This /proc/<pid>/autogroup is good for doing this single thing but when
> I start thinking of possible extensions of it down the line, it creates
> issues.
>
> - Once we have some kind of uppper limit support in cpu controller, these
> autogroups are beyond control. If you want to impose some kind of
> limits on them then you shall have to extend parallel interface
> /proc/<pid>/autogroup to also speicify upper limit (like nice levels).
>
> - Similiarly if this autgroup notion is extended to other cgroup
> controllers, then you shall have to again extend /proc/<pid>/autogroup
> to be able to specify these additional parameters.

Yes, if it evolves, it's interface will need to evolve as well. It
could have been a directory containing buttons, knobs and statistics,
but KISS won.

> - If there is a monitoring tool which is monitoring the system for
> resource usage by the groups, then I think these autogroups are beyond
> reach and any stats exported by cgroup interface will not be available.
> (though right now I can't see any stats being exported by cgroup files
> in cpu controller but other controllers like block and memory do.).

If you're manually assigning bandwidth et al from userland, there's not
much point to in-kernel automation is there?

If I had married the two, the first thing that would have happened is
gripes about things appearing and disappearing in cgroups directories,
resulting in mayhem and confusion for scripts and tools.

> - I am doing some testing with the patch and w.r.t. cgroup interface some
> things don't seem right.
>
> I have applied your patch and enabled CONFIG_AUTO_GROUP. Now I boot
> into the kernel and open a new ssh connection to the machine.
>
> # echo $$
> 3555
> # cat /proc/3555/autogroup
> /autogroup-63 nice 0
>
> IIUC, task 3555 has been moved into an autogroup. Now I mount the cpu
> controller and this task is visible in root cgroup.
>
> # mount -t cgroup -o cpu none /cgroup/cpu
> # cat /cgroup/cpu/tasks | grep 3555
> 3555
>
> First of all this gives user a wrong impression that task 3555 is in
> root cgroup.

It is in the root cgroup. It is not in the root autogroup is not
auto-cgroups group.

> Now I create a child group test1 and move the task there and also change
> the weight/shares of the cgroup to 10240.
>
> # mkdir test1
> # echo 3555 > test1/tasks
> # echo 10240 > test1/cpu.shares
> # cat /proc/3555/cgroup
> 3:cpu:/test1
> # cat /proc/3555/autogroup
> /autogroup-63 nice 0
>
> So again, user will think that task is in cgroup test1 and is being
> controlled by the respective weight but that's not the case.

It is the case here.

PID USER PR NI VIRT RES SHR S %CPU %MEM TIME+ P COMMAND
7573 root 20 0 7996 340 256 R 50 0.0 3:35.86 3 pert
7572 root 20 0 7996 340 256 R 50 0.0 9:21.68 3 pert
...
marge:/cgroups/test # echo 7572 > tasks
marge:/cgroups/test # echo 4096 > cpu.shares

PID USER PR NI VIRT RES SHR S %CPU %MEM TIME+ P COMMAND
7572 root 20 0 7996 340 256 R 80 0.0 10:06.92 3 pert
7573 root 20 0 7996 340 256 R 20 0.0 4:05.80 3 pert

When you move a task into a cgroup, it still has an autogroup
association, as all tasks (processes actually) do, but it's not used.

> Even if we prevent autogroup task from being visible in cpu controller
> root group, then comes the question what happens if cpu and some other
> controller is comounted. Say cpuset. Now in that case will task be
> visible in root group task file and can one operate on that. Now showing
> up there does not make much sense as task should still be controllable
> by other controllers and its policies.

The user has to specifically ask for it in his config, can turn it on or
off on the fly or at boot..

> So yes, creating a /proc/<pid>/autogroup is dirt cheap and makes the life
> easier in terms of implementation of this patch and it should work well.
> But it is also a new user interface which does not sound too extensible and
> does not seem to cooperate well with cgroup interface

..it has a different mission, with different users being targeted, so
why does it need to hold hands?

> It also introduces this new notion of niceness for task groups which is sort
> of equivalent to cpu.shares in cpu controller. First of all why should we
> not stick to shares notion even for autogroup. Even if we introduce the notion
> of niceness for groups, IMHO, it should be through cgroup interface instead of
> group niceness for autogroup and shares/weights for cgroup despite the
> fact that in the background they do similar things.

IMHO, cgroups should have been 'nice' from the start, but the folks who
wrote it did what they thought best. I like nice a lot better than
shares, so I used nice.

> I think above concerns can possibly be reason enough to think about about
> the wedding.

Perhaps in future, they'll get married, and perhaps they should, but in
the here and now, I think they have similar but not identical missions.
If you turn on one, turn off the other. Maybe that should be automated.

Systemd thingy may make autogroup short lived anyway. I had a query
from an embedded guy (hm, which I spaced) suggesting autogroup may be
quite nice for handheld stuff though, so who knows.

-Mike