GNOME Bugzilla – Bug 635119
[PATCH] automatically create a subcgroup in the cpu hierarchy for each widget
Last modified: 2020-04-27 20:23:09 UTC
Created attachment 174724 [details] [review] the patch As a follow-up about the recent discussion on LKML, Phoronix, Slashdot and friends regarding tty-based CPU autogrouping here's a patch that does the same in userspace. It's pretty minimal but does what is necessary. Note that for this to work you need the cpu cgroup hierarchy mounted to /sys/fs/cgroup/cpu. systemd systems have that mounted anyway, for others you might need to do this manually. Also, your access to the cgroup fs must be set properly so that normal users can create subgroups of the cgroups their processes run it. systemd sets this up properly out-of-the-box now, on other machines might need to do this manually. On a non-systemd system do the following: mount -t tmpfs tmpfs /sys/fs/cgroup mkdir /sys/fs/cgroup/cpu mount -t cgroup cgroup /sys/fs/cgroup/cpu -o cpu chmod 777 /sys/fs/cgroup/cpu (Of course, the last line is dangerous, this needs to managed properly, which systemd does. But for a preview on non-F15 systems the lines above should suffice)
(oh yepp, this patch doesn't cleanup the cgroup afterwards, but well, systemd does that anyway)
I think this is going to need to be both: 1) A configure option (I don't care about the default though) 2) A GConf key
An alternative to dynamically creating cgroups is to explicitly specify them. For example, for the desktop, we could always have (at least) two: "desktop" and "background" Then this patch would simply tell VTE to put things in "background". This would be better for say IDEs; they could simply check whether a "background" cgroup exists, and if so use it, rather than having to make them up.
Not sure I follow what you mean by that. I think putting apps into their own cgroups is a distinct problem of putting the VTE shells into seperate subgroups. When we use systemd as session manager, then the all apps will be in their own groups. However, each terminal window that the gnome-terminal process shows still would need its own subgroup from that. Due to that this patch should be pretty future-proof, since it just creates a subgroup of what it already is in. The other problem, i.e. giving the foreground app more CPU time is a matter of somehow teaching gnome-shell to find the foreground app, then find the cgroup of it and then changing the cpushares value of the cgroup. Each time the foreground app changes the cpushares of the previous one will be reset and the new one will get a bump. Of course, for some apps you want default cpushare settings. Those we can definitely configure defaults in the standard .desktop or systemd .service files.
Jeez, my English is getting worse with every word I type.
Not sure if vte should do this. The proposed behavior is quite arbitrary to say the least. We definitely already allow users of vte to get a callback and setup environment however they wish. ChPe, what do you think?
(In reply to comment #4) > Not sure I follow what you mean by that. I think putting apps into their own > cgroups is a distinct problem of putting the VTE shells into seperate > subgroups. I'm simply suggesting there should be one explicit "background" group that any program can add to, rather than one special for VTE. To make this clear, I've attached a modified version of your patch to do this. Also, it adds a flag to allow embedders to opt-in to this behavior.
Created attachment 174732 [details] [review] Add VTE_PTY_BACKGROUND_GROUP
(In reply to comment #6) > Not sure if vte should do this. The proposed behavior is quite arbitrary to > say the least. We definitely already allow users of vte to get a callback and > setup environment however they wish. Putting it in gnome-terminal is a possibility for sure.
I don't really follow. If you do this then running gnome-terminal, and opening two terminals from it will give you the same cgroup in both. That's not desirable. One of the features of the original kernel patch was that it was finegrained, i.e. by pty, so that if you ran make -j in one terminal, and "mplayer" in the other, you'd end up with two distinct cgroups for them, so that mplayer would get the same amount of CPU as all of make -j. With your modified patch both terminals would be in the very same cgroup and hence this defeats the whole point of my original patch.
(In reply to comment #10) > I don't really follow. If you do this then running gnome-terminal, and opening > two terminals from it will give you the same cgroup in both. That's not > desirable. One of the features of the original kernel patch was that it was > finegrained, i.e. by pty, so that if you ran make -j in one terminal, and > "mplayer" in the other, you'd end up with two distinct cgroups for them, so > that mplayer would get the same amount of CPU as all of make -j. With your > modified patch both terminals would be in the very same cgroup and hence this > defeats the whole point of my original patch. The bug here I will argue is running mplayer as a child process of a terminal (and we need some way to avoid desktop apps being run from a terminal). If you're in the scenario of launching firefox from the desktop UI and gnome-terminal + make -j 64, then my patch works just as well as yours does.
Even if the code lives in g-t, it can create separate cgroups per vte terminal. It does that in a callback received from vte after fork and before exec.
> The bug here I will argue is running mplayer as a child process of a terminal Why is that a bug? mplayer has a perfectly fine tty interface (plus a graphical window for the output of course). > (and we need some way to avoid desktop apps being run from a terminal). Are you serious? Do you have any specific reason for that? I agree that the effect of this patch is a bit "magic" (e.g. if I start two compilations and I fine tune the shortest to use "make -j2" and the longest to use "make -j8", that won't work). However, I don't see what's the relation between that and disliking desktop apps being run from a terminal.
(In reply to comment #11) > (In reply to comment #10) > > I don't really follow. If you do this then running gnome-terminal, and opening > > two terminals from it will give you the same cgroup in both. That's not > > desirable. One of the features of the original kernel patch was that it was > > finegrained, i.e. by pty, so that if you ran make -j in one terminal, and > > "mplayer" in the other, you'd end up with two distinct cgroups for them, so > > that mplayer would get the same amount of CPU as all of make -j. With your > > modified patch both terminals would be in the very same cgroup and hence this > > defeats the whole point of my original patch. > > The bug here I will argue is running mplayer as a child process of a terminal > (and we need some way to avoid desktop apps being run from a terminal). Well, this is a terminal. A terminal is for weirdo hackers like us. Those are the same people who will yell loud if they can't spawn processes anymore... > If you're in the scenario of launching firefox from the desktop UI and > gnome-terminal + make -j 64, then my patch works just as well as yours does. Well, that might be true. But the way I want to cover this is make systemd actually take the role of gnome-session and the backend of dbus activation. Then, following Ryans suggestion to spawn processes via bus activation by default systemd would create the cpu cgroup anyway. Done.
(In reply to comment #13) > > The bug here I will argue is running mplayer as a child process of a terminal > > Why is that a bug? mplayer has a perfectly fine tty interface (plus a > graphical window for the output of course). > > > (and we need some way to avoid desktop apps being run from a terminal). > > Are you serious? Do you have any specific reason for that? I meant, as child processes; basically we remote to dbus. > I agree that the effect of this patch is a bit "magic" (e.g. if I start two > compilations and I fine tune the shortest to use "make -j2" and the longest to > use "make -j8", that won't work). I'm not sure it really matters if we break that, though.
(In reply to comment #14) > > Well, this is a terminal. A terminal is for weirdo hackers like us. Those are > the same people who will yell loud if they can't spawn processes anymore... One can certainly still spawn processes...but doing it right would be more reliable. Also note that, if we are talking about people running desktop apps from a terminal, it's probably just as likely people do: firefox /path/to/some/docs & make -j In which case you're back in the same boat.
> > I agree that the effect of this patch is a bit "magic" (e.g. if I start two > > compilations and I fine tune the shortest to use "make -j2" and the longest to > > use "make -j8", that won't work). > > I'm not sure it really matters if we break that, though. FWIW I agree.
I prefer the approach from the 2nd patch, that is, adding a new flag for this (and possibly also a string property to specify the name of the cgroup?). However, as to actually setting this cgroup up, I'm not sure vte is the right place to do this. We already have a gnome-pty-helper process that does create the pty and updates lastlog and [uw]tmp; wouldn't that fit better in there? BTW, I have been meaning to ask if these tasks of gnome-pty-helper could be taken over by systemd or ConsoleKit (which I think Lennart has said he wants to merge with systemd) ?
I proposed to let the session systemd handle this; see http://lists.freedesktop.org/archives/systemd-devel/2011-June/002698.html and Lennart's reply at http://lists.freedesktop.org/archives/systemd-devel/2011-July/002802.html .
vte 0.60 moves the child process to their own systemd scope, so this should be fixed.