Re: [PATCH v1] sched: Mention autogroup disabled behavior

[Phil Auld]

Hi Alejandro,

On Thu, Jan 16, 2025 at 02:06:26PM +0100 Alejandro Colomar wrote:
> Hi Phil,
> 
> > Subject: sched: Mention autogroup disabled behavior
> 
> Please use the pathname of the modified file as a prefix:
> 
> 	man/man7/sched.7: Mention autogroup disabled behavior

ack

> 
> On Thu, Jan 16, 2025 at 12:46:54PM +0000, Phil Auld wrote:
> > The autogroup feature can be contolled at runtime when
> > built into the kernel. Disabling it in this case still
> > creates autogroups and still shows the autogroup membership
> > for the task in /proc.  The scheduler code will just not
> > use the the autogroup task group.
> 
> Would you mind showing (in the commit message) a shell session that
> demonstrates this?

This is actually part of the problem. It's very hard to see this
from userspace. I can show a shell session that shows that autogroup
is disabled and that my task has an autogroup in /proc but determining
that the autogroup is not being used not so much. (I may be missing
something obvious but I could not find it).

I had to look at the kernel code:

kernel/sched/autogroup.h:
static inline struct task_group *
autogroup_task_group(struct task_struct *p, struct task_group *tg)
{
        extern unsigned int sysctl_sched_autogroup_enabled;
        int enabled = READ_ONCE(sysctl_sched_autogroup_enabled);

        if (enabled && task_wants_autogroup(p, tg))
                return p->signal->autogroup->tg;

        return tg;
}

bool task_wants_autogroup(struct task_struct *p, struct task_group *tg)
{
        if (tg != &root_task_group)
                return false;
    ...

}

The former being called from sched_group_fork() and sched_get_task_group().

I suppose looking at /proc/pid/cgroup and seeing it report not "0::/"
is part of it since it then won't be in root task group.

To some extent any systemd based system these days is not really
using autogroup at all anyway. 

I can put some of the above in there or just something like:

# cat /proc/sys/kernel/sched_autogroup_enabled 
0
# cat /proc/$$/autogroup 
/autogroup-112 nice 0


Thoughts?



Cheers,
Phil


> 
> >  This can be confusing
> > to users. Add a sentence to this effect to sched.7 to
> > point this out.
> > 
> > Signed-off-by: Phil Auld <pauld@xxxxxxxxxx>
> > To: Alejandro Colomar <alx@xxxxxxxxxx>
> > Cc: <linux-man@xxxxxxxxxxxxxxx>
> > Cc: LKML <linux-kernel@xxxxxxxxxxxxxxx>
> 
> Thanks!
> 
> > 
> > ---
> >  man/man7/sched.7 | 2 ++
> >  1 file changed, 2 insertions(+)
> > 
> > diff --git a/man/man7/sched.7 b/man/man7/sched.7
> > index 71f098e48..f0a708cd7 100644
> > --- a/man/man7/sched.7
> > +++ b/man/man7/sched.7
> > @@ -724,6 +724,8 @@ in the group terminates.
> >  .P
> >  When autogrouping is enabled, all of the members of an autogroup
> >  are placed in the same kernel scheduler "task group".
> > +When disabled the group creation happens as above, and autogroup membership
> 
> s/disabled/&,/
> 
> Also, please use semantic newlines.  See man-pages(7):
> 
> $ MANWIDTH=72 man man-pages | sed -n '/Use semantic newlines/,/^$/p'
>    Use semantic newlines
>      In the source of a manual page, new sentences should be started on
>      new lines, long sentences should be split  into  lines  at  clause
>      breaks  (commas,  semicolons, colons, and so on), and long clauses
>      should be split at phrase boundaries.  This convention,  sometimes
>      known as "semantic newlines", makes it easier to see the effect of
>      patches, which often operate at the level of individual sentences,
>      clauses, or phrases.
> 
> 
> Have a lovely day!
> Alex
> 
> > +is still visible in /proc, but the autogroups are not used.
> >  The CFS scheduler employs an algorithm that equalizes the
> >  distribution of CPU cycles across task groups.
> >  The benefits of this for interactive desktop performance
> > -- 
> > 2.47.0
> > 
> 
> -- 
> <https://www.alejandro-colomar.es/>



--