[RFC/POC]: Make cpuset.cpus.effective independent of cpuset.cpus

[RFC/POC]: Make cpuset.cpus.effective independent of cpuset.cpus

[Petr Malat]

Hi!
I have tried to use the new remote cgroup feature and I find the
interface unfriendly - requiring cpuset.cpus.exclusive to be a subset
of cpuset.cpus requires the program, which wants to isolate a CPU for
some RT activity, to know what CPUs all ancestor cgroups want to use.

For example consider cgroup hierarchy c1/c2/c3 where my program is
running and wants to isolate CPU N, so
 - It creates new c1/c2/c3/rt cgroup
 - It adds N to cpuset.cpus.exclusive of rt, c3 and c2 cgroup
   (cpuset.cpus.exclusive |= N)
 - Now it should do the same with cpuset.cpus, but that's not possible
   if ancestors cpuset.cpus is empty, which is common configuration and
   there is no good way how to set it in that case.

My proposal is to
 - Not require cpuset.cpus.exclusive to be a subset of cpuset.cpus
 - Create remote cgroup if cpuset.cpus is empty and local cgroup if it's
   set, to give the user explicit control on what cgroup is created.

I have prepared change to test my idea (the next mail). I haven't tested it
thoroughly yet, but I wanted to open the discussion on this topic to know
if such a change could be accepted and I should burn more time on it.

BR,
  Petr

[PATCH] cgroup/cpuset: Make cpuset.cpus.effective independent of cpuset.cpus

[Petr Malat]

Requiring cpuset.cpus.effective to be a subset of cpuset.cpus makes it
hard to use as one is forced to configure cpuset.cpus of current and all
ancestor cgroups, which requires a knowledge about all other units
sharing the same cgroup subtree. Also, it doesn't allow using empty
cpuset.cpus.

Do not require cpuset.cpus.effective to be a subset of cpuset.cpus and
create remote cgroup only if cpuset.cpus is empty, to make it easier for
the user to control which cgroup is being created.

Signed-off-by: Petr Malat <oss@malat.biz>
---
 kernel/cgroup/cpuset.c | 17 ++++++-----------
 1 file changed, 6 insertions(+), 11 deletions(-)

diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
index b8000240a1476..72ec7ef0eabc8 100644
--- a/kernel/cgroup/cpuset.c
+++ b/kernel/cgroup/cpuset.c
@@ -1459,7 +1459,7 @@ static bool compute_effective_exclusive_cpumask(struct cpuset *cs,
 		xcpus = cs->effective_xcpus;
 
 	if (!cpumask_empty(cs->exclusive_cpus))
-		cpumask_and(xcpus, cs->exclusive_cpus, cs->cpus_allowed);
+		cpumask_copy(xcpus, cs->exclusive_cpus);
 	else
 		cpumask_copy(xcpus, cs->cpus_allowed);
 
@@ -2987,18 +2987,13 @@ static int update_prstate(struct cpuset *cs, int new_prs)
 		 * cpus_allowed cannot be empty.
 		 */
 		if (cpumask_empty(cs->cpus_allowed)) {
+			if (remote_partition_enable(cs, &tmpmask))
+				goto out;
 			err = PERR_CPUSEMPTY;
-			goto out;
+		} else {
+			err = update_parent_effective_cpumask(cs,
+					partcmd_enable, NULL, &tmpmask);
 		}
-
-		err = update_parent_effective_cpumask(cs, partcmd_enable,
-						      NULL, &tmpmask);
-		/*
-		 * If an attempt to become local partition root fails,
-		 * try to become a remote partition root instead.
-		 */
-		if (err && remote_partition_enable(cs, &tmpmask))
-			err = 0;
 	} else if (old_prs && new_prs) {
 		/*
 		 * A change in load balance state only, no change in cpumasks.
-- 
2.42.0

Re: [RFC/POC]: Make cpuset.cpus.effective independent of cpuset.cpus

[Waiman Long]

On 3/21/24 17:33, Petr Malat wrote:
> Hi!
> I have tried to use the new remote cgroup feature and I find the
> interface unfriendly - requiring cpuset.cpus.exclusive to be a subset
> of cpuset.cpus requires the program, which wants to isolate a CPU for
> some RT activity, to know what CPUs all ancestor cgroups want to use.
>
> For example consider cgroup hierarchy c1/c2/c3 where my program is
> running and wants to isolate CPU N, so
>   - It creates new c1/c2/c3/rt cgroup
>   - It adds N to cpuset.cpus.exclusive of rt, c3 and c2 cgroup
>     (cpuset.cpus.exclusive |= N)
>   - Now it should do the same with cpuset.cpus, but that's not possible
>     if ancestors cpuset.cpus is empty, which is common configuration and
>     there is no good way how to set it in that case.
>
> My proposal is to
>   - Not require cpuset.cpus.exclusive to be a subset of cpuset.cpus
>   - Create remote cgroup if cpuset.cpus is empty and local cgroup if it's
>     set, to give the user explicit control on what cgroup is created.

I think we can make cpuset.cpus.exclusive independent of cpuset.cpus as 
a separate hierarchy to make creation of remote partitions easier. I 
need some more time to think through it. I don't think your test patch 
is enough for making this change. BTW, you confuse cpuset.cpus.exclusive 
with cpuset.cpus.effective which are two completely different things.

Cheers,
Longman

Re: [RFC/POC]: Make cpuset.cpus.effective independent of cpuset.cpus

[Petr Malat]

Hi,
On Thu, Mar 21, 2024 at 09:41:43PM -0400, Waiman Long wrote:
> On 3/21/24 17:33, Petr Malat wrote:
> > Hi!
> > I have tried to use the new remote cgroup feature and I find the
> > interface unfriendly - requiring cpuset.cpus.exclusive to be a subset
> > of cpuset.cpus requires the program, which wants to isolate a CPU for
> > some RT activity, to know what CPUs all ancestor cgroups want to use.
> > 
> > For example consider cgroup hierarchy c1/c2/c3 where my program is
> > running and wants to isolate CPU N, so
> >   - It creates new c1/c2/c3/rt cgroup
> >   - It adds N to cpuset.cpus.exclusive of rt, c3 and c2 cgroup
> >     (cpuset.cpus.exclusive |= N)
> >   - Now it should do the same with cpuset.cpus, but that's not possible
> >     if ancestors cpuset.cpus is empty, which is common configuration and
> >     there is no good way how to set it in that case.
> > 
> > My proposal is to
> >   - Not require cpuset.cpus.exclusive to be a subset of cpuset.cpus
> >   - Create remote cgroup if cpuset.cpus is empty and local cgroup if it's
> >     set, to give the user explicit control on what cgroup is created.
> 
> I think we can make cpuset.cpus.exclusive independent of cpuset.cpus as a
> separate hierarchy to make creation of remote partitions easier. I need some
> more time to think through it. I don't think your test patch is enough for
> making this change. BTW, you confuse cpuset.cpus.exclusive with
> cpuset.cpus.effective which are two completely different things.

The exclusive/effective confusion is a copy and paste mistake in the
description, the code should make cpuset.cpus.exclusive independent on
cpuset.cpus, how is described in the initial mail. I have pasted it
from my clipboard history and apparently haven't read the whole string.
  P.

Re: [PATCH] cgroup/cpuset: Make cpuset.cpus.effective independent of cpuset.cpus

[Tejun Heo]

On Thu, Mar 21, 2024 at 10:39:45PM +0100, Petr Malat wrote:
> Requiring cpuset.cpus.effective to be a subset of cpuset.cpus makes it
> hard to use as one is forced to configure cpuset.cpus of current and all
> ancestor cgroups, which requires a knowledge about all other units
> sharing the same cgroup subtree. Also, it doesn't allow using empty
> cpuset.cpus.
> 
> Do not require cpuset.cpus.effective to be a subset of cpuset.cpus and
> create remote cgroup only if cpuset.cpus is empty, to make it easier for
> the user to control which cgroup is being created.
> 
> Signed-off-by: Petr Malat <oss@malat.biz>

Waiman, what do you think?

Thanks.

-- 
tejun

Re: [PATCH] cgroup/cpuset: Make cpuset.cpus.effective independent of cpuset.cpus

[Waiman Long]

On 3/25/24 16:12, Tejun Heo wrote:
> On Thu, Mar 21, 2024 at 10:39:45PM +0100, Petr Malat wrote:
>> Requiring cpuset.cpus.effective to be a subset of cpuset.cpus makes it
>> hard to use as one is forced to configure cpuset.cpus of current and all
>> ancestor cgroups, which requires a knowledge about all other units
>> sharing the same cgroup subtree. Also, it doesn't allow using empty
>> cpuset.cpus.
>>
>> Do not require cpuset.cpus.effective to be a subset of cpuset.cpus and
>> create remote cgroup only if cpuset.cpus is empty, to make it easier for
>> the user to control which cgroup is being created.
>>
>> Signed-off-by: Petr Malat <oss@malat.biz>
> Waiman, what do you think?

I think it is possible to make cpuset.cpus.exclusive independent of 
cpuset.cpus. There are probably more places that need to be changed 
including the cgroup-v2.rst file.

Cheers,
Longman

>
> Thanks.
>

Re: [PATCH] cgroup/cpuset: Make cpuset.cpus.effective independent of cpuset.cpus

[Tejun Heo]

Hello,

On Tue, Mar 26, 2024 at 11:14:53AM -0400, Waiman Long wrote:
> On 3/25/24 16:12, Tejun Heo wrote:
> > On Thu, Mar 21, 2024 at 10:39:45PM +0100, Petr Malat wrote:
> > > Requiring cpuset.cpus.effective to be a subset of cpuset.cpus makes it
> > > hard to use as one is forced to configure cpuset.cpus of current and all
> > > ancestor cgroups, which requires a knowledge about all other units
> > > sharing the same cgroup subtree. Also, it doesn't allow using empty
> > > cpuset.cpus.
> > > 
> > > Do not require cpuset.cpus.effective to be a subset of cpuset.cpus and
> > > create remote cgroup only if cpuset.cpus is empty, to make it easier for
> > > the user to control which cgroup is being created.
> > > 
> > > Signed-off-by: Petr Malat <oss@malat.biz>
> > Waiman, what do you think?
> 
> I think it is possible to make cpuset.cpus.exclusive independent of
> cpuset.cpus. There are probably more places that need to be changed
> including the cgroup-v2.rst file.

I really like the idea of making this more easier to configure. It'd be
great if you could Petr so that this can land.

Thank you.

-- 
tejun

Re: [PATCH] cgroup/cpuset: Make cpuset.cpus.effective independent of cpuset.cpus

[Michal Koutný]

[-- Attachment #1: Type: text/plain, Size: 900 bytes --]

Hello.

On Thu, Mar 21, 2024 at 10:39:45PM +0100, Petr Malat <oss@malat.biz> wrote:
> Requiring cpuset.cpus.effective to be a subset of cpuset.cpus makes it
> hard to use as one is forced to configure cpuset.cpus of current and all
> ancestor cgroups, which requires a knowledge about all other units
> sharing the same cgroup subtree.

> Also, it doesn't allow using empty cpuset.cpus.
                               ^^^^^^^^^^^^^^^^^
                          _this_ is what cpuset has been missing IMO

I think cpuset v2 should allow empty value in cpuset.cpus (not only
default but also as a reset (to the default)) which would implicitely
mean using whatever CPUs were passed from parent(s).

Does that make sense to you too?

Thus the patch(es) seems to need to be extended to handle a case when
empty cpuset.cpus is assigned but no cpuset.cpus.exclusive are
specified neither.

Thanks,
Michal

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH] cgroup/cpuset: Make cpuset.cpus.effective independent of cpuset.cpus

[Petr Malat]

Hi,
On Tue, Apr 02, 2024 at 07:04:58PM +0200, Michal Koutn?? wrote:
> Hello.
> 
> On Thu, Mar 21, 2024 at 10:39:45PM +0100, Petr Malat <oss@malat.biz> wrote:
> > Requiring cpuset.cpus.effective to be a subset of cpuset.cpus makes it
> > hard to use as one is forced to configure cpuset.cpus of current and all
> > ancestor cgroups, which requires a knowledge about all other units
> > sharing the same cgroup subtree.
> 
> > Also, it doesn't allow using empty cpuset.cpus.
>                                ^^^^^^^^^^^^^^^^^
>                           _this_ is what cpuset has been missing IMO
> 
> I think cpuset v2 should allow empty value in cpuset.cpus (not only
> default but also as a reset (to the default)) which would implicitely
> mean using whatever CPUs were passed from parent(s).
> 
> Does that make sense to you too?
> 
> Thus the patch(es) seems to need to be extended to handle a case when
> empty cpuset.cpus is assigned but no cpuset.cpus.exclusive are
> specified neither.

I don't see how this could be useful - consider hierarchy A/B, where A
configures the cpuset.cpus and B doesn't and inherits if from A.

If B is then made root partition it will use exactly the same CPUs as
A, thus these CPUs will not be available in A. Also, there can't be
a sibling of B, because there are no CPUs left for it. As B is then
the only working child of A, no resource distribution can happen on A.

So there is no point in creating B and one could use A directly.
  Petr

Re: Re: [PATCH] cgroup/cpuset: Make cpuset.cpus.effective independent of cpuset.cpus

[Michal Koutný]

[-- Attachment #1: Type: text/plain, Size: 1584 bytes --]

On Thu, Apr 04, 2024 at 06:36:56AM +0200, Petr Malat <oss@malat.biz> wrote:
> So there is no point in creating B and one could use A directly.

There indeed isn't in this example.

Consider siblings A/B and A/C. B would have configured cpus (and
possibly be a partition root) while C is generally uninterested in
cpuset so would not be configured. (Setup like this is encountered more
easily on unified hierarchy where the tree is already organized without
cpuset considerations.)

If B took all A's CPUs, it would be an invalid partition, otherwise C
would use the remaining CPUs implicitly.

Documentation/admin-guide/cgroup-v2.rst already tries to describe
something like that:

        An empty value indicates that the cgroup is using the same
        setting as the nearest cgroup ancestor with a non-empty
        "cpuset.cpus" or all the available CPUs if none is found.

But it doesn't work like that (kernel 6.7.9):
	# cd /sys/fs/cgroup 
	# echo +cpuset >cgroup.subtree_control
	# cat init.scope/cpuset.cpus
	   (empty is possible)
	# cat init.scope/cpuset.cpus.effective
	0-7
	
	# echo 3 >init.scope/cpuset.cpus
	# cat init.scope/cpuset.cpus.effective
	3
	echo "" >init.scope/cpuset.cpus
	# cat init.scope/cpuset.cpus
	3  (I'd expect empty again)

IOW, cpuset cgroup can have empty cpuset.cpus when it's freshly created
but it seems it cannot be reset back to such an indifferent state.

(To match it to the previous example A/C==init.scope, A/B would be some
foo.service (under -.slice) that requires configured cpuset.)

Michal

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]