Re: [PATCH, v10 3/3] cgroups: introduce timer slack controller

[Peter Zijlstra]

On Sat, 2011-10-15 at 13:20 +0200, Lennart Poettering wrote:
> On Fri, 14.10.11 15:43, Andrew Morton (akpm@xxxxxxxxxxxxxxxxxxxx) wrote:
> 
> > > cgroup subsys "timer_slack" implements timer slack controller. It
> > > provides a way to set minimal timer slack value for a group of tasks.
> > > If a task belongs to a cgroup with minimal timer slack value higher than
> > > task's value, cgroup's value will be applied.
> > > 
> > > Timer slack controller allows to implement setting timer slack value of
> > > a process based on a policy. For example, you can create foreground and
> > > background cgroups and move tasks between them based on system state.
> > 
> > I'm having trouble understanding the value of this feature.  Users can
> > presently control the timer-slack of a group of processes via
> > inherit-over-fork.
> > 
> > Perhaps there's a case for providing a way for process A to set process
> > B's slack.  And perhaps B's children.  That would be a simpler patch
> > and would have the considerable advantage that it doesn't require
> > cgroups.
> > 
> > So.... why should we merge this?
> 
> Our usecase is basically this:
> 
> consider you have one or more desktop user sessions logged in, each one
> in a timer slack cgroup. Now, userspace already tracks when sessions
> become idle (i.e. currently desktop userspace then starts a screensaver,
> or turns off the screen, or similar), and we'd like to increase the timer slack
> for the session cgroups individually as the individual session becomes
> idle, and decrease it again if the session stops being idle.

> What matters for us here is that the timer slack cgroup controller
> provides us with a very nice way to change the timerslack for the whole
> set of session processes in one go and from the outside. i.e. the idle
> logic in userspace would be trivially easy to implement, since we
> already track per-session idle states and with the timerlsack cgroup
> controller we'd have to touch only a single kernel file to make our
> changes.

I would argue this is an excellent reason not to merge this. This really
is in the camp of lets paper over shitty userspace instead of fix it.

Such a scheme takes away the immediate need to fix crap and therefore
crap will not get fixed. Why not focus on creating tools (I think
powertop really already does everything you need) and track down WTF
apps are firing so many timers when the screen if off.

Also, the downside of your approach is that you treat all applications
the same, what if there is a genuine need for reasonable timer response
and your 'policy' wrecks things?



--
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/