linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* Writing "+pids" to cgroup.subtree_control flie yields EINVAL
@ 2017-12-04 21:35 Michael Kerrisk (man-pages)
  2017-12-04 21:47 ` Tejun Heo
  0 siblings, 1 reply; 10+ messages in thread
From: Michael Kerrisk (man-pages) @ 2017-12-04 21:35 UTC (permalink / raw)
  To: Tejun Heo; +Cc: mtk.manpages, lkml

Hello Tejun,

I was trying to do some simple testing ot the CPU controller
that is merged into 4.15, and ran immediately into some confusion.
In the root cgroup on a freshly booted 4.150-rc1, I try the following:

# pwd
/sys/fs/cgroup/unified
# echo '+cpu' > cgroup.subtree_control 
sh: echo: write error: Invalid argument

What am I missing> I presume I'm missing something obvious, although
nothing jumped out at me as I read the cgroups-v2.txt file.

Cheers,

Michael

-- 
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: Writing "+pids" to cgroup.subtree_control flie yields EINVAL
  2017-12-04 21:35 Writing "+pids" to cgroup.subtree_control flie yields EINVAL Michael Kerrisk (man-pages)
@ 2017-12-04 21:47 ` Tejun Heo
  2017-12-05  7:45   ` Michael Kerrisk (man-pages)
  0 siblings, 1 reply; 10+ messages in thread
From: Tejun Heo @ 2017-12-04 21:47 UTC (permalink / raw)
  To: Michael Kerrisk (man-pages); +Cc: lkml

Hello, Michael.

On Mon, Dec 04, 2017 at 10:35:13PM +0100, Michael Kerrisk (man-pages) wrote:
> I was trying to do some simple testing ot the CPU controller
> that is merged into 4.15, and ran immediately into some confusion.
> In the root cgroup on a freshly booted 4.150-rc1, I try the following:
> 
> # pwd
> /sys/fs/cgroup/unified
> # echo '+cpu' > cgroup.subtree_control 
> sh: echo: write error: Invalid argument
>
> What am I missing> I presume I'm missing something obvious, although
> nothing jumped out at me as I read the cgroups-v2.txt file.

Checking whether I messed up something really basic... hmmm doesn't
seem that way.  What do /sys/fs/cgroup/unified/cgroup.controllers and
/proc/cgroups say?

Thanks.

-- 
tejun

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: Writing "+pids" to cgroup.subtree_control flie yields EINVAL
  2017-12-04 21:47 ` Tejun Heo
@ 2017-12-05  7:45   ` Michael Kerrisk (man-pages)
  2017-12-05 16:00     ` Tejun Heo
  0 siblings, 1 reply; 10+ messages in thread
From: Michael Kerrisk (man-pages) @ 2017-12-05  7:45 UTC (permalink / raw)
  To: Tejun Heo; +Cc: mtk.manpages, lkml, Lennart Poettering

[dropping Lennart into CC]

Hello Tejun,

On 12/04/2017 10:47 PM, Tejun Heo wrote:
> Hello, Michael.
> 
> On Mon, Dec 04, 2017 at 10:35:13PM +0100, Michael Kerrisk (man-pages) wrote:
>> I was trying to do some simple testing ot the CPU controller
>> that is merged into 4.15, and ran immediately into some confusion.
>> In the root cgroup on a freshly booted 4.150-rc1, I try the following:
>>
>> # pwd
>> /sys/fs/cgroup/unified
>> # echo '+cpu' > cgroup.subtree_control 
>> sh: echo: write error: Invalid argument
>>
>> What am I missing> I presume I'm missing something obvious, although
>> nothing jumped out at me as I read the cgroups-v2.txt file.
> 
> Checking whether I messed up something really basic... hmmm doesn't
> seem that way.  What do /sys/fs/cgroup/unified/cgroup.controllers and
> /proc/cgroups say?

Oh -- they're all sensible:

In the root cgroup:

# cat cgroup.controllers 
cpu io memory pids

$ cat /proc/cgroups 
#subsys_name	hierarchy	num_cgroups	enabled
cpuset	0	142	1
cpu	0	142	1
cpuacct	0	142	1
blkio	0	142	1
memory	0	142	1
devices	0	142	1
freezer	0	142	1
net_cls	0	142	1
perf_event	0	142	1
net_prio	0	142	1
hugetlb	0	142	1
pids	0	142	1

But, I through some trial and error and printk() I worked out

a) If I first move all tasks to the root cgroup, then I can
write '+cpu' to the cgroup.subtree_control file in the root
cgroup.

b) The reason for my initial problems was this test in
the kernel in cpu_cgroup_can_attach():

#ifdef CONFIG_RT_GROUP_SCHED
                if (!sched_rt_can_attach(css_tg(css), task))
                        return -EINVAL;
#else
                /* We don't support RT-tasks being in separate groups */
                if (task->sched_class != &fair_sched_class)
                        return -EINVAL;
#endif

I don't have CONFIG_RT_GROUP_SCHED, and the second 'if' was yielding
false because of some SCHED_RR processes that are in some of the nonroot
cgroups created by systemd, namely:

# ps ax -L -o 'pid tid cls rtprio comm'|grep RR
  685   723  RR     99 rtkit-daemon
  972   979  RR      5 alsa-sink-ALC26
  972   982  RR      5 alsa-source-ALC
 1594  1597  RR      5 alsa-sink-ALC26
 1594  1600  RR      5 alsa-source-ALC

So, one solution is to move those processes to the root cgroup,
and then it's possible to write '+pids' to cgroup.subtree_control.

Is enabling CONFIG_RT_GROUP_SCHED also a solution? (I have
not had a chance to test that yet.)

Anyway, it seems like this should be documented somewhere in the
kernel Documentation files, since it may be that others will run
into this as well. I'm not quite sure what should be added to the
documentation. Do you have some idea?

Thanks,

Michael

-- 
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: Writing "+pids" to cgroup.subtree_control flie yields EINVAL
  2017-12-05  7:45   ` Michael Kerrisk (man-pages)
@ 2017-12-05 16:00     ` Tejun Heo
  2017-12-05 16:44       ` Michael Kerrisk (man-pages)
  0 siblings, 1 reply; 10+ messages in thread
From: Tejun Heo @ 2017-12-05 16:00 UTC (permalink / raw)
  To: Michael Kerrisk (man-pages); +Cc: lkml, Lennart Poettering

Hello, Michael.

On Tue, Dec 05, 2017 at 08:45:19AM +0100, Michael Kerrisk (man-pages) wrote:
> b) The reason for my initial problems was this test in
> the kernel in cpu_cgroup_can_attach():
> 
> #ifdef CONFIG_RT_GROUP_SCHED
>                 if (!sched_rt_can_attach(css_tg(css), task))
>                         return -EINVAL;
> #else
>                 /* We don't support RT-tasks being in separate groups */
>                 if (task->sched_class != &fair_sched_class)
>                         return -EINVAL;
> #endif
> 
> I don't have CONFIG_RT_GROUP_SCHED, and the second 'if' was yielding
> false because of some SCHED_RR processes that are in some of the nonroot
> cgroups created by systemd, namely:

I see.  Thanks for tracking it down.  Yeah, the RT side of things
isn't too well thought out yet.

> # ps ax -L -o 'pid tid cls rtprio comm'|grep RR
>   685   723  RR     99 rtkit-daemon
>   972   979  RR      5 alsa-sink-ALC26
>   972   982  RR      5 alsa-source-ALC
>  1594  1597  RR      5 alsa-sink-ALC26
>  1594  1600  RR      5 alsa-source-ALC
> 
> So, one solution is to move those processes to the root cgroup,
> and then it's possible to write '+pids' to cgroup.subtree_control.

You mean '+cpu", right?

> Is enabling CONFIG_RT_GROUP_SCHED also a solution? (I have
> not had a chance to test that yet.)

We aren't yet sure about how we should handle RT and haven't enabled
RT part on cgroup2 yet.  You can test the same scenario in cgroup1 tho
and would have to configure RT shares all along the hierarchy to the
leaf cgroup.

> Anyway, it seems like this should be documented somewhere in the
> kernel Documentation files, since it may be that others will run
> into this as well. I'm not quite sure what should be added to the
> documentation. Do you have some idea?

I think the only thing we can say right now is that RT processes
should be in the root cgroup.  :(

Thanks a lot.

-- 
tejun

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: Writing "+pids" to cgroup.subtree_control flie yields EINVAL
  2017-12-05 16:00     ` Tejun Heo
@ 2017-12-05 16:44       ` Michael Kerrisk (man-pages)
  2017-12-05 17:13         ` [PATCH cgroup/for-4.15-fixes] cgroup: add warning about RT not being supported on cgroup2 Tejun Heo
  0 siblings, 1 reply; 10+ messages in thread
From: Michael Kerrisk (man-pages) @ 2017-12-05 16:44 UTC (permalink / raw)
  To: Tejun Heo; +Cc: lkml, Lennart Poettering

Hello Tejun,

On 5 December 2017 at 17:00, Tejun Heo <tj@kernel.org> wrote:
> Hello, Michael.
>
> On Tue, Dec 05, 2017 at 08:45:19AM +0100, Michael Kerrisk (man-pages) wrote:
>> b) The reason for my initial problems was this test in
>> the kernel in cpu_cgroup_can_attach():
>>
>> #ifdef CONFIG_RT_GROUP_SCHED
>>                 if (!sched_rt_can_attach(css_tg(css), task))
>>                         return -EINVAL;
>> #else
>>                 /* We don't support RT-tasks being in separate groups */
>>                 if (task->sched_class != &fair_sched_class)
>>                         return -EINVAL;
>> #endif
>>
>> I don't have CONFIG_RT_GROUP_SCHED, and the second 'if' was yielding
>> false because of some SCHED_RR processes that are in some of the nonroot
>> cgroups created by systemd, namely:
>
> I see.  Thanks for tracking it down.  Yeah, the RT side of things
> isn't too well thought out yet.
>
>> # ps ax -L -o 'pid tid cls rtprio comm'|grep RR
>>   685   723  RR     99 rtkit-daemon
>>   972   979  RR      5 alsa-sink-ALC26
>>   972   982  RR      5 alsa-source-ALC
>>  1594  1597  RR      5 alsa-sink-ALC26
>>  1594  1600  RR      5 alsa-source-ALC
>>
>> So, one solution is to move those processes to the root cgroup,
>> and then it's possible to write '+pids' to cgroup.subtree_control.
>
> You mean '+cpu", right?

D'oh! Yes. I was also doing some tests with the 'pids' controller, and
mistyped. (I see I even manage to mistitle this mail thread :-(.)

>> Is enabling CONFIG_RT_GROUP_SCHED also a solution? (I have
>> not had a chance to test that yet.)
>
> We aren't yet sure about how we should handle RT and haven't enabled
> RT part on cgroup2 yet.  You can test the same scenario in cgroup1 tho
> and would have to configure RT shares all along the hierarchy to the
> leaf cgroup.

Okay. Thanks for the confirmation that this is at least surprising behavior.

>> Anyway, it seems like this should be documented somewhere in the
>> kernel Documentation files, since it may be that others will run
>> into this as well. I'm not quite sure what should be added to the
>> documentation. Do you have some idea?
>
> I think the only thing we can say right now is that RT processes
> should be in the root cgroup.  :(

Okay. (Will you add that to the v2 doc file?)

Cheers,

Michael

--
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH cgroup/for-4.15-fixes] cgroup: add warning about RT not being supported on cgroup2
  2017-12-05 16:44       ` Michael Kerrisk (man-pages)
@ 2017-12-05 17:13         ` Tejun Heo
  2017-12-05 18:24           ` Michael Kerrisk (man-pages)
  0 siblings, 1 reply; 10+ messages in thread
From: Tejun Heo @ 2017-12-05 17:13 UTC (permalink / raw)
  To: Michael Kerrisk (man-pages); +Cc: lkml, Lennart Poettering

From: Tejun Heo <tj@kernel.org>
Date: Tue, 5 Dec 2017 09:10:17 -0800

We haven't yet figured out what to do with RT threads on cgroup2.
Document the limitation.

Signed-off-by: Tejun Heo <tj@kernel.org>
Reported-by: "Michael Kerrisk (man-pages)" <mtk.manpages@gmail.com>
---
 Documentation/cgroup-v2.txt | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/Documentation/cgroup-v2.txt b/Documentation/cgroup-v2.txt
index 779211f..55a28f4 100644
--- a/Documentation/cgroup-v2.txt
+++ b/Documentation/cgroup-v2.txt
@@ -898,6 +898,10 @@ controller implements weight and absolute bandwidth limit models for
 normal scheduling policy and absolute bandwidth allocation model for
 realtime scheduling policy.
 
+WARNING: cgroup2 doesn't yet support control of realtime processes and
+the cpu controller can only be enabled when all RT processes are in
+the root cgroup.
+
 
 CPU Interface Files
 ~~~~~~~~~~~~~~~~~~~
-- 
2.9.5

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH cgroup/for-4.15-fixes] cgroup: add warning about RT not being supported on cgroup2
  2017-12-05 17:13         ` [PATCH cgroup/for-4.15-fixes] cgroup: add warning about RT not being supported on cgroup2 Tejun Heo
@ 2017-12-05 18:24           ` Michael Kerrisk (man-pages)
  2017-12-05 18:29             ` [PATCH v2 " Tejun Heo
  0 siblings, 1 reply; 10+ messages in thread
From: Michael Kerrisk (man-pages) @ 2017-12-05 18:24 UTC (permalink / raw)
  To: Tejun Heo; +Cc: lkml, Lennart Poettering

Hello Tejun,

On 5 December 2017 at 18:13, Tejun Heo <tj@kernel.org> wrote:
> From: Tejun Heo <tj@kernel.org>
> Date: Tue, 5 Dec 2017 09:10:17 -0800
>
> We haven't yet figured out what to do with RT threads on cgroup2.
> Document the limitation.
>
> Signed-off-by: Tejun Heo <tj@kernel.org>
> Reported-by: "Michael Kerrisk (man-pages)" <mtk.manpages@gmail.com>
> ---
>  Documentation/cgroup-v2.txt | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/Documentation/cgroup-v2.txt b/Documentation/cgroup-v2.txt
> index 779211f..55a28f4 100644
> --- a/Documentation/cgroup-v2.txt
> +++ b/Documentation/cgroup-v2.txt
> @@ -898,6 +898,10 @@ controller implements weight and absolute bandwidth limit models for
>  normal scheduling policy and absolute bandwidth allocation model for
>  realtime scheduling policy.
>
> +WARNING: cgroup2 doesn't yet support control of realtime processes and
> +the cpu controller can only be enabled when all RT processes are in
> +the root cgroup.
> +

The documentation file already carries this useful text:

[[
During transition to v2, system management software might still
automount the v1 cgroup filesystem and so hijack all controllers
during boot, before manual intervention is possible. To make testing
and experimenting easier, the kernel parameter cgroup_no_v1= allows
disabling controllers in v1 and make them always available in v2.
]]

In that spirit, it may be worth alerting the reader that "system
management software" might already have put some RT processes in
nonroot cgroups. Maybe something like this:

[[
WARNING: cgroup2 doesn't yet support control of realtime processes and
the cpu controller can only be enabled when all RT processes are in
the root cgroup. Be aware that that system management software may
already have placed RT processes into nonroot cgroups during the system
boot process, and these processes may need to be moved to the root
cgroup before the cpu controller can be enabled.
]]

Thanks,

Michael

-- 
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH v2 cgroup/for-4.15-fixes] cgroup: add warning about RT not being supported on cgroup2
  2017-12-05 18:24           ` Michael Kerrisk (man-pages)
@ 2017-12-05 18:29             ` Tejun Heo
  2017-12-05 18:53               ` Michael Kerrisk (man-pages)
  0 siblings, 1 reply; 10+ messages in thread
From: Tejun Heo @ 2017-12-05 18:29 UTC (permalink / raw)
  To: Michael Kerrisk (man-pages); +Cc: lkml, Lennart Poettering

>From f0a821260f24ea9378ed0e0c149e3bbc1a1bd008 Mon Sep 17 00:00:00 2001
From: Tejun Heo <tj@kernel.org>
Date: Tue, 5 Dec 2017 09:10:17 -0800

We haven't yet figured out what to do with RT threads on cgroup2.
Document the limitation.

v2: Included the warning about system management software behavior as
    suggested by Michael.

Signed-off-by: Tejun Heo <tj@kernel.org>
Reported-by: "Michael Kerrisk (man-pages)" <mtk.manpages@gmail.com>
---
 Documentation/cgroup-v2.txt | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/Documentation/cgroup-v2.txt b/Documentation/cgroup-v2.txt
index 779211f..c5e3c21 100644
--- a/Documentation/cgroup-v2.txt
+++ b/Documentation/cgroup-v2.txt
@@ -898,6 +898,13 @@ controller implements weight and absolute bandwidth limit models for
 normal scheduling policy and absolute bandwidth allocation model for
 realtime scheduling policy.
 
+WARNING: cgroup2 doesn't yet support control of realtime processes and
+the cpu controller can only be enabled when all RT processes are in
+the root cgroup.  Be aware that that system management software may
+already have placed RT processes into nonroot cgroups during the
+system boot process, and these processes may need to be moved to the
+root cgroup before the cpu controller can be enabled.
+
 
 CPU Interface Files
 ~~~~~~~~~~~~~~~~~~~
-- 
2.9.5

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 cgroup/for-4.15-fixes] cgroup: add warning about RT not being supported on cgroup2
  2017-12-05 18:29             ` [PATCH v2 " Tejun Heo
@ 2017-12-05 18:53               ` Michael Kerrisk (man-pages)
  2017-12-05 19:48                 ` Tejun Heo
  0 siblings, 1 reply; 10+ messages in thread
From: Michael Kerrisk (man-pages) @ 2017-12-05 18:53 UTC (permalink / raw)
  To: Tejun Heo; +Cc: lkml, Lennart Poettering

On 5 December 2017 at 19:29, Tejun Heo <tj@kernel.org> wrote:
> From f0a821260f24ea9378ed0e0c149e3bbc1a1bd008 Mon Sep 17 00:00:00 2001
> From: Tejun Heo <tj@kernel.org>
> Date: Tue, 5 Dec 2017 09:10:17 -0800
>
> We haven't yet figured out what to do with RT threads on cgroup2.
> Document the limitation.
>
> v2: Included the warning about system management software behavior as
>     suggested by Michael.
>
> Signed-off-by: Tejun Heo <tj@kernel.org>
> Reported-by: "Michael Kerrisk (man-pages)" <mtk.manpages@gmail.com>
> ---
>  Documentation/cgroup-v2.txt | 7 +++++++
>  1 file changed, 7 insertions(+)
>
> diff --git a/Documentation/cgroup-v2.txt b/Documentation/cgroup-v2.txt
> index 779211f..c5e3c21 100644
> --- a/Documentation/cgroup-v2.txt
> +++ b/Documentation/cgroup-v2.txt
> @@ -898,6 +898,13 @@ controller implements weight and absolute bandwidth limit models for
>  normal scheduling policy and absolute bandwidth allocation model for
>  realtime scheduling policy.
>
> +WARNING: cgroup2 doesn't yet support control of realtime processes and
> +the cpu controller can only be enabled when all RT processes are in
> +the root cgroup.  Be aware that that system management software may

Sorry, there was a fault in my text: s/that that/that/

Thanks,

Michael



-- 
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 cgroup/for-4.15-fixes] cgroup: add warning about RT not being supported on cgroup2
  2017-12-05 18:53               ` Michael Kerrisk (man-pages)
@ 2017-12-05 19:48                 ` Tejun Heo
  0 siblings, 0 replies; 10+ messages in thread
From: Tejun Heo @ 2017-12-05 19:48 UTC (permalink / raw)
  To: Michael Kerrisk (man-pages); +Cc: lkml, Lennart Poettering

On Tue, Dec 05, 2017 at 07:53:51PM +0100, Michael Kerrisk (man-pages) wrote:
> > +WARNING: cgroup2 doesn't yet support control of realtime processes and
> > +the cpu controller can only be enabled when all RT processes are in
> > +the root cgroup.  Be aware that that system management software may
> 
> Sorry, there was a fault in my text: s/that that/that/

Heh, fixed.

Thanks.

-- 
tejun

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2017-12-05 19:48 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-12-04 21:35 Writing "+pids" to cgroup.subtree_control flie yields EINVAL Michael Kerrisk (man-pages)
2017-12-04 21:47 ` Tejun Heo
2017-12-05  7:45   ` Michael Kerrisk (man-pages)
2017-12-05 16:00     ` Tejun Heo
2017-12-05 16:44       ` Michael Kerrisk (man-pages)
2017-12-05 17:13         ` [PATCH cgroup/for-4.15-fixes] cgroup: add warning about RT not being supported on cgroup2 Tejun Heo
2017-12-05 18:24           ` Michael Kerrisk (man-pages)
2017-12-05 18:29             ` [PATCH v2 " Tejun Heo
2017-12-05 18:53               ` Michael Kerrisk (man-pages)
2017-12-05 19:48                 ` Tejun Heo

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).