Re: [tip:sched/core] sched: Add 'autogroup' scheduling feature: automated per session task groups

[Bharata B Rao <bharata.rao@gmail.com>]

On Mon, Dec 20, 2010 at 10:09 PM, Mike Galbraith <efault@gmx.de> wrote:
> On Mon, 2010-12-20 at 21:16 +0530, Bharata B Rao wrote:
>> On Mon, Dec 20, 2010 at 6:49 PM, Peter Zijlstra <a.p.zijlstra@chello.nl> wrote:
>> > On Mon, 2010-12-20 at 18:38 +0530, Bharata B Rao wrote:
>> >> The autogroup patchset removes the display of cgroup name from
>> >> sched_debug output.
>> >
>> > Hrmph.. that wasn't supposed to happen, care to send a patch to fix that
>> > up?
>>
>> There are two aspects here:
>>
>> - Printing cgroup name for per-CPU cfs_rqs shouldn't be affected by
>>   autogroup and the old code should work here.
>> - Printing cgroup name for tasks depends on task_group(), which has
>> been changed by autogroup patch. I haven't really looked deep into
>> autogroup patch, but from whatever I can gather, Mike had a reason
>> to remove this bit from sched_debug. The task groups created for
>> autogroups don't have cgroups associated with them and hence no
>> dentries and hence no pathnames.
>
> Mike didn't remove it, but _was_ supposed to get around to it.
>
>> I guess we could do fix this as shown in the attached patch.
>
>
> +#ifdef CONFIG_CGROUP_SCHED
> +       {
> +               char path[64];
> +
>
> ...
>
> +#if defined(CONFIG_CGROUP_SCHED) && defined(CONFIG_FAIR_GROUP_SCHED)
> +       char path[128];
>
>
> One reason it was removed was path[64/128].

Other callers of cgroup_path use PATH_MAX=4096 here. I believe the
original reason for these short path sizes was to be light on stack
and as well as to avoid allocation. Can we have some reasonable length
(256 or 512 ?) and live with truncation (if that ever happens) ?

Also, while displaying group name with tasks, does it make sense to
display autogroup-<id> (the one shown in /proc/<pid>/autogroup) ?

Regards,
Bharata.