[PATCH v1] sched: Mention autogroup disabled behavior

Phil Auld posted 1 patch 2 weeks, 6 days ago
man/man7/sched.7 | 2 ++
1 file changed, 2 insertions(+)
[PATCH v1] sched: Mention autogroup disabled behavior
Posted by Phil Auld 2 weeks, 6 days ago
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.  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@redhat.com>
To: Alejandro Colomar <alx@kernel.org>
Cc: <linux-man@vger.kernel.org>
Cc: LKML <linux-kernel@vger.kernel.org>

---
 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
+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
Re: [PATCH v1] sched: Mention autogroup disabled behavior
Posted by Alejandro Colomar 2 weeks, 6 days ago
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

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 can be confusing
> to users. Add a sentence to this effect to sched.7 to
> point this out.
> 
> Signed-off-by: Phil Auld <pauld@redhat.com>
> To: Alejandro Colomar <alx@kernel.org>
> Cc: <linux-man@vger.kernel.org>
> Cc: LKML <linux-kernel@vger.kernel.org>

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/>
Re: [PATCH v1] sched: Mention autogroup disabled behavior
Posted by Phil Auld 2 weeks, 6 days ago
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@redhat.com>
> > To: Alejandro Colomar <alx@kernel.org>
> > Cc: <linux-man@vger.kernel.org>
> > Cc: LKML <linux-kernel@vger.kernel.org>
> 
> 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/>



--
Re: [PATCH v1] sched: Mention autogroup disabled behavior
Posted by Alejandro Colomar 2 weeks, 6 days ago
Hi Phil,

On Thu, Jan 16, 2025 at 08:53:15AM -0500, Phil Auld wrote:
> 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?

Both.  :)

The more information we have in the commit message, the better (in case
someone needs to check in the future, that will give more context).


Cheers,
Alex

-- 
<https://www.alejandro-colomar.es/>
Re: [PATCH v1] sched: Mention autogroup disabled behavior
Posted by Phil Auld 2 weeks, 6 days ago
On Thu, Jan 16, 2025 at 02:59:20PM +0100 Alejandro Colomar wrote:
> Hi Phil,
> 
> On Thu, Jan 16, 2025 at 08:53:15AM -0500, Phil Auld wrote:
> > 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?
> 
> Both.  :)
> 
> The more information we have in the commit message, the better (in case
> someone needs to check in the future, that will give more context).
>

Okay, fair enough. Will do.  It was just hard for me to show that the
listed autogroup is not really being used. 

I'll put the above in there and send v2.

Thanks!

Cheers,
Phil


> 
> Cheers,
> Alex
> 
> -- 
> <https://www.alejandro-colomar.es/>



--