diff mbox series

controllers/cpuset: Make mount failure TCONF

Message ID 20231027104951.32464-1-rpalethorpe@suse.com
State Accepted
Headers show
Series controllers/cpuset: Make mount failure TCONF | expand

Commit Message

Richard Palethorpe Oct. 27, 2023, 10:49 a.m. UTC
It appears that if the subsystem is available under V2 then mounting
the V1 can fail. Probably when the V2 has processes assigned to a
group it controls.

The test should scan the system and find the existing CGroup
hierarchies and work with those. This can be done by converting the
test to tst_cgctl or rewriting it in C. It's not clear what the best
course of action would be.

For now, this commit just changes the result to TCONF.

Signed-off-by: Richard Palethorpe <rpalethorpe@suse.com>
---
 testcases/kernel/controllers/cpuset/cpuset_funcs.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Petr Vorel Oct. 27, 2023, 11:01 a.m. UTC | #1
Hi Richie,

> It appears that if the subsystem is available under V2 then mounting
> the V1 can fail. Probably when the V2 has processes assigned to a
> group it controls.

Sounds likely, but it would be good to check this in the kernel code or with
kernel cgroup developers.

> The test should scan the system and find the existing CGroup
> hierarchies and work with those. This can be done by converting the
> test to tst_cgctl or rewriting it in C. It's not clear what the best
> course of action would be.

> For now, this commit just changes the result to TCONF.

+1 (suppose it's the case of already mounted under v2)

Reviewed-by: Petr Vorel <pvorel@suse.cz>

> Signed-off-by: Richard Palethorpe <rpalethorpe@suse.com>
> ---
>  testcases/kernel/controllers/cpuset/cpuset_funcs.sh | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

> diff --git a/testcases/kernel/controllers/cpuset/cpuset_funcs.sh b/testcases/kernel/controllers/cpuset/cpuset_funcs.sh
> index 0cfa0c17e..312654a9d 100755
> --- a/testcases/kernel/controllers/cpuset/cpuset_funcs.sh
> +++ b/testcases/kernel/controllers/cpuset/cpuset_funcs.sh
> @@ -165,7 +165,7 @@ setup()
>  	mount -t cgroup -o cpuset cpuset "$CPUSET" 2> /dev/null
>  	if [ $? -ne 0 ]; then
>  		cleanup
> -		tst_brkm TFAIL "Could not mount cgroup filesystem with"\
> +		tst_brkm TCONF "Could not mount cgroup filesystem with"\
>  					" cpuset on $CPUSET..Exiting test"

nit: I'd cleanup the message.

		tst_brkm TCONF "Could not mount cgroup filesystem with cpuset on $CPUSET"

Kind regards,
Petr

>  	fi
Cyril Hrubis Oct. 27, 2023, 2:23 p.m. UTC | #2
Hi!
> > It appears that if the subsystem is available under V2 then mounting
> > the V1 can fail. Probably when the V2 has processes assigned to a
> > group it controls.
> 
> Sounds likely, but it would be good to check this in the kernel code or with
> kernel cgroup developers.

I'm pretty sure that controller can be used either in v1, or in v2 but
not from both at the same time by choice. Some of the changes between v1
and v2 are incompatible so this couldn't work either way.

I think that once the controller name is written to
/sys/fs/cgroup/unified/cgroup.subtree_controll mounting it to v1 should
fail, so maybe we should just try to grep the fail instead and exit the
test with TCONF if we found the controller bound to v2 already.
Petr Vorel Oct. 27, 2023, 5:45 p.m. UTC | #3
> Hi!
> > > It appears that if the subsystem is available under V2 then mounting
> > > the V1 can fail. Probably when the V2 has processes assigned to a
> > > group it controls.

> > Sounds likely, but it would be good to check this in the kernel code or with
> > kernel cgroup developers.

> I'm pretty sure that controller can be used either in v1, or in v2 but
> not from both at the same time by choice. Some of the changes between v1
> and v2 are incompatible so this couldn't work either way.

> I think that once the controller name is written to
> /sys/fs/cgroup/unified/cgroup.subtree_controll mounting it to v1 should
> fail, so maybe we should just try to grep the fail instead and exit the
> test with TCONF if we found the controller bound to v2 already.

Sounds like a good idea.

Kind regards,
Petr
Richard Palethorpe Oct. 30, 2023, 10:39 a.m. UTC | #4
Hello,

Petr Vorel <pvorel@suse.cz> writes:

>> Hi!
>> > > It appears that if the subsystem is available under V2 then mounting
>> > > the V1 can fail. Probably when the V2 has processes assigned to a
>> > > group it controls.
>
>> > Sounds likely, but it would be good to check this in the kernel code or with
>> > kernel cgroup developers.
>
>> I'm pretty sure that controller can be used either in v1, or in v2 but
>> not from both at the same time by choice. Some of the changes between v1
>> and v2 are incompatible so this couldn't work either way.
>
>> I think that once the controller name is written to
>> /sys/fs/cgroup/unified/cgroup.subtree_controll mounting it to v1 should
>> fail, so maybe we should just try to grep the fail instead and exit the
>> test with TCONF if we found the controller bound to v2 already.
>
> Sounds like a good idea.

I tried a quick experiment. Adding it to cgroup.subtree_control is not
enough. However adding a process to a cgroup where the controller is
enabled causes mount to fail with EBUSY. There could be other ways. I
would assume that sort of thing can change as well between kernel versions.

If we want to avoid attempting the mount then maybe even just scanning
cgroup.controllers is best. This is practically what we do in tst_cgroup
lib. However there is also just the option of not running the test on
systems where only V2 will be used. So this is just a minimal fix to
keep the test limping along until they are obsolete or rewritten
entirely.

Another option would be to split the tests into V1 only and everything
else. That would work for us if distros are practically stopping support for
V1 on newer versions. Then there is no need to make these tests detect
V2, we just don't run them on those systems.
diff mbox series

Patch

diff --git a/testcases/kernel/controllers/cpuset/cpuset_funcs.sh b/testcases/kernel/controllers/cpuset/cpuset_funcs.sh
index 0cfa0c17e..312654a9d 100755
--- a/testcases/kernel/controllers/cpuset/cpuset_funcs.sh
+++ b/testcases/kernel/controllers/cpuset/cpuset_funcs.sh
@@ -165,7 +165,7 @@  setup()
 	mount -t cgroup -o cpuset cpuset "$CPUSET" 2> /dev/null
 	if [ $? -ne 0 ]; then
 		cleanup
-		tst_brkm TFAIL "Could not mount cgroup filesystem with"\
+		tst_brkm TCONF "Could not mount cgroup filesystem with"\
 					" cpuset on $CPUSET..Exiting test"
 	fi