Updated check_config with cgroupv2 controllers by gunadhya · Pull Request #41897 · moby/moby

gunadhya · 2021-01-19T13:53:24Z

- What I did
Added cgroupv2 available controllers and check for CONFIG_CGROUP_BPF in contrib/check-config.sh
- How I did it

Reading the /sys/fs/cgroup/cgroup.controllers file for available controllers
Added a new CheckFlag for CONFIG_CGROUP_BPF to BPF_CGROUP_DEVICE support
- How to verify it
Output :
Ran on Linux 5.4.0-62-generic

- Description for the changelog

Update in contrib/check-config.sh cgroupv2 controllers and CONFIG_CGROUP_BPF

- A picture of a cute animal (not mandatory but encouraged)

thaJeztah · 2021-01-19T15:37:27Z

Thanks!

@AkihiroSuda @tianon ptal 🤗

contrib/check-config.sh

tianon · 2021-01-19T20:23:39Z

contrib/check-config.sh

This file (fairly consistently) uses camelCase for variables (and snake_case for functions). I think we should also consider it a failure case if we don't find the referenced file:

Suggested change

cgroupv2_controller_loc="/sys/fs/cgroup/cgroup.controllers"

# Prints available cgroupv2 controllers if the file exists

if test -f "$cgroupv2_controller_loc"; then

mapfile -t cgroupv2_controllers < $cgroupv2_controller_loc

wrap_good "- cgroupv2 controllers" "${cgroupv2_controllers[*]}"

else

echo "$cgroupv2_controller_loc not found."

fi

cgroupv2ControllersFile='/sys/fs/cgroup/cgroup.controllers'

if [ -s "$cgroupv2ControllersFile" ]; then

mapfile -t cgroupv2Controllers < "$cgroupv2ControllersFile"

wrap_good '- cgroupv2 controllers' "${cgroupv2Controllers[*]}"

else

wrap_warning "warning: missing '$cgroupv2ControllersFile'"

fi

My bad, I'll update the variable names to camelCase and create the warning if the file doesn't exist.

AkihiroSuda · 2021-01-20T04:18:19Z

contrib/check-config.sh

we should confirm that the file contains the following controllers:

cpu

cpuset

memory

io

pids

I'll update it to show if the file has cpu,cpuset,memory,io,pids enabled or missing similar to other checks.

Is this better?

AkihiroSuda · 2021-01-20T04:18:33Z

Can we also check existence of freezer?

gunadhya · 2021-01-20T12:00:03Z

Can we also check existence of freezer?

@AkihiroSuda
So a check for CONFIG_CGROUP_FREEZER flag already exists here

The doc mentions cgroup.freezer file https://www.kernel.org/doc/html/latest/admin-guide/cgroup-v2.html#core-interface-files.
And when I grep my kernel config is says yes

But I don't see the file

Do I have to enable this somehow?

thaJeztah · 2021-01-21T13:17:51Z

@AkihiroSuda ptal #41897 (comment)

AkihiroSuda · 2021-01-22T03:57:34Z

cgroup.freezer only exists in subgroup, not in the top group

gunadhya · 2021-01-24T19:34:03Z

I've made some changes and added a freezer check.
This is what the new output looks like.

AkihiroSuda

Thanks, but could you squash commits

gunadhya · 2021-01-25T05:22:54Z

Thanks, but could you squash commits

Done

thaJeztah · 2021-01-25T09:24:25Z

Thanks for updating! @tianon ptal

tianon · 2021-06-08T23:19:44Z

contrib/check-config.sh

Sorry for the delay -- this needs a rebase, but I think we can also simplify this a bit (and make sure it's still POSIX compatible, per #42410, which the mapfile-based implementation is not) with something like the following:

Suggested change

cgroupv2ControllerFile="/sys/fs/cgroup/cgroup.controllers"

# Prints available cgroupv2 controllers if the file exists

if test -f "$cgroupv2ControllerFile"; then

mapfile -t cgroupv2Controllers < $cgroupv2ControllerFile

echo "- cgroupv2 controllers:"

controllersv2=($cgroupv2Controllers)

[[ " ${controllersv2[@]} " =~ " io " ]] && wrap_good " - IO" "available" || wrap_bad " - IO" "missing"

[[ " ${controllersv2[@]} " =~ " cpu " ]] && wrap_good " - CPU" "available" || wrap_bad " - CPU" "missing"

[[ " ${controllersv2[@]} " =~ " cpuset " ]] && wrap_good " - CPUSET" "available" || wrap_bad " - CPUSET" "missing"

[[ " ${controllersv2[@]} " =~ " pids " ]] && wrap_good " - PIDs" "available" || wrap_bad " - PIDs" "missing"

[[ " ${controllersv2[@]} " =~ " memory " ]] && wrap_good " - MEMORY" "available" || wrap_bad " - MEMORY" "missing"

else

echo wrap_warning "warning: missing '$cgroupv2ControllerFile'"

fi

#check if cgroup.freeze exists in subdir

[ -n "$(find /sys/fs/cgroup -type f -name "cgroup.freeze" 2> /dev/null)" ] && wrap_good '- cgroupv2 freezer' 'available' || wrap_bad '- cgroupv2 freezer' 'missing'

cgroupv2ControllerFile='/sys/fs/cgroup/cgroup.controllers'

if [ -f "$cgroupv2ControllerFile" ]; then

echo ' Controllers:'

for controller in cpu cpuset io memory pids; do

if grep -qE '(^| )'"$controller"'($| )' "$cgroupv2ControllerFile"; then

echo " - $(wrap_good "$controller" 'available')"

else

echo " - $(wrap_bad "$controller" 'missing')"

fi

done

else

wrap_bad "$cgroupv2ControllerFile" 'nonexistent??'

fi

I'm also not wild about the unbounded find /sys/fs/cgroup -- I think if we can't find a better way to check whether the cgroup freezer is available then we should probably just leave a TODO comment instead (or not worry about it -- can any of these actually be usefully disabled?). 😕 😞

Yes, That makes sense. I'll rebase and update the checks for controllers in POSIX. As for the freezer check I'm not sure if we can disable it. I'll have to read about it. I can try to find a better way to do the check or I'll update it with aTODO. Is that okay?

thaJeztah · 2021-07-06T20:54:00Z

@gunadhya could you rebase, and have a look at @tianon's suggestion?

gunadhya · 2021-07-07T04:10:51Z

@thaJeztah @tianon Sorry for the late response. I'll get right on it.

thaJeztah · 2021-07-08T17:17:56Z

@tianon PR was updated, PTAL (thanks @gunadhya!)

tianon

Just two really minor nits but otherwise this LGTM! Thank you! 🎉

(I'm happy to push these changes myself if you'd rather or if that'd be helpful for you. 👍)

tianon · 2021-07-08T20:41:04Z

contrib/check-config.sh

Suggested change

#TODO check if cgroup.freeze exists in subdir

# TODO find an efficient way to check if cgroup.freeze exists in subdir

tianon · 2021-07-08T20:41:15Z

contrib/check-config.sh

Suggested change

echo "$(wrap_good 'cgroup hierarchy' 'cgroupv2')"

wrap_good 'cgroup hierarchy' 'cgroupv2'

Signed-off-by: gunadhya <6939749+gunadhya@users.noreply.github.com> Added cgroupv2 controller check Modified comment

gunadhya · 2021-07-09T03:57:29Z

@tianon I've updated the pr with new changes.

tianon

👍🥳

thaJeztah

LGTM, thanks!!

perhaps we should create a tracking issue for the todo (in case someone wants to work on that)

gunadhya force-pushed the f-Update_check_config branch 2 times, most recently from d06f3f6 to 8ba4858 Compare January 19, 2021 14:12

thaJeztah added area/contrib kind/enhancement Enhancements are not bugs or new features but can improve usability or performance. status/2-code-review labels Jan 19, 2021

tianon reviewed Jan 19, 2021

View reviewed changes

AkihiroSuda reviewed Jan 20, 2021

View reviewed changes

thaJeztah mentioned this pull request Jan 21, 2021

Rootless docker with --icc=false does not start #41913

Closed

AkihiroSuda added the area/cgroup2 cgroup v2 label Jan 23, 2021

AkihiroSuda approved these changes Jan 25, 2021

View reviewed changes

gunadhya force-pushed the f-Update_check_config branch from fb5643d to c155c9f Compare January 25, 2021 05:22

AkihiroSuda approved these changes Jan 25, 2021

View reviewed changes

AkihiroSuda added the process/cherry-pick label Feb 2, 2021

gunadhya requested a review from tianon February 8, 2021 16:19

tianon reviewed Jun 8, 2021

View reviewed changes

gunadhya force-pushed the f-Update_check_config branch from c155c9f to a3e3d37 Compare July 8, 2021 15:43

tianon reviewed Jul 8, 2021

View reviewed changes

Updated check_config

aff02db

Signed-off-by: gunadhya <6939749+gunadhya@users.noreply.github.com> Added cgroupv2 controller check Modified comment

gunadhya force-pushed the f-Update_check_config branch from a3e3d37 to aff02db Compare July 9, 2021 03:55

tianon approved these changes Jul 9, 2021

View reviewed changes

thaJeztah approved these changes Jul 9, 2021

View reviewed changes

thaJeztah added status/4-merge and removed status/2-code-review labels Jul 9, 2021

thaJeztah merged commit 39a9d03 into moby:master Jul 29, 2021

thaJeztah added this to the 21.xx milestone Oct 15, 2021

thaJeztah removed the process/cherry-pick label Apr 26, 2023

-	cgroupv2_controller_loc="/sys/fs/cgroup/cgroup.controllers"
-	# Prints available cgroupv2 controllers if the file exists
-	if test -f "$cgroupv2_controller_loc"; then
-		mapfile -t cgroupv2_controllers < $cgroupv2_controller_loc
-		wrap_good "- cgroupv2 controllers" "${cgroupv2_controllers[*]}"
-	else
-		echo "$cgroupv2_controller_loc not found."
-	fi
+	cgroupv2ControllersFile='/sys/fs/cgroup/cgroup.controllers'
+	if [ -s "$cgroupv2ControllersFile" ]; then
+		mapfile -t cgroupv2Controllers < "$cgroupv2ControllersFile"
+		wrap_good '- cgroupv2 controllers' "${cgroupv2Controllers[*]}"
+	else
+		wrap_warning "warning: missing '$cgroupv2ControllersFile'"
+	fi

-	cgroupv2ControllerFile="/sys/fs/cgroup/cgroup.controllers"
-	# Prints available cgroupv2 controllers if the file exists
-	if test -f "$cgroupv2ControllerFile"; then
-		mapfile -t cgroupv2Controllers < $cgroupv2ControllerFile
-		echo "- cgroupv2 controllers:"
-		controllersv2=($cgroupv2Controllers)
-		[[ " ${controllersv2[@]} " =~ " io " ]] && wrap_good "  - IO" "available" || wrap_bad "  - IO" "missing"
-		[[ " ${controllersv2[@]} " =~ " cpu " ]] && wrap_good "  - CPU" "available" || wrap_bad "  - CPU" "missing"
-		[[ " ${controllersv2[@]} " =~ " cpuset " ]] && wrap_good "  - CPUSET" "available" || wrap_bad "  - CPUSET" "missing"
-		[[ " ${controllersv2[@]} " =~ " pids " ]] && wrap_good "  - PIDs" "available" || wrap_bad "  - PIDs" "missing"
-		[[ " ${controllersv2[@]} " =~ " memory " ]] && wrap_good "  - MEMORY" "available" || wrap_bad "  - MEMORY" "missing"
-	else
-		echo wrap_warning "warning: missing '$cgroupv2ControllerFile'"
-	fi
-	#check if cgroup.freeze exists in subdir
-	[ -n "$(find /sys/fs/cgroup -type f -name "cgroup.freeze" 2> /dev/null)" ] && wrap_good '- cgroupv2 freezer' 'available' || wrap_bad '- cgroupv2 freezer' 'missing'
+	cgroupv2ControllerFile='/sys/fs/cgroup/cgroup.controllers'
+	if [ -f "$cgroupv2ControllerFile" ]; then
+		echo '  Controllers:'
+		for controller in cpu cpuset io memory pids; do
+			if grep -qE '(^| )'"$controller"'($| )' "$cgroupv2ControllerFile"; then
+				echo "  - $(wrap_good "$controller" 'available')"
+			else
+				echo "  - $(wrap_bad "$controller" 'missing')"
+			fi
+		done
+	else
+		wrap_bad "$cgroupv2ControllerFile" 'nonexistent??'
+	fi

	#TODO check if cgroup.freeze exists in subdir
	# TODO find an efficient way to check if cgroup.freeze exists in subdir

	echo "$(wrap_good 'cgroup hierarchy' 'cgroupv2')"
	wrap_good 'cgroup hierarchy' 'cgroupv2'

Conversation

gunadhya commented Jan 19, 2021

Uh oh!

thaJeztah commented Jan 19, 2021

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AkihiroSuda commented Jan 20, 2021

Uh oh!

gunadhya commented Jan 20, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

thaJeztah commented Jan 21, 2021

Uh oh!

AkihiroSuda commented Jan 22, 2021

Uh oh!

gunadhya commented Jan 24, 2021

Uh oh!

AkihiroSuda left a comment

Choose a reason for hiding this comment

Uh oh!

gunadhya commented Jan 25, 2021

Uh oh!

thaJeztah commented Jan 25, 2021

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

thaJeztah commented Jul 6, 2021

Uh oh!

gunadhya commented Jul 7, 2021

Uh oh!

thaJeztah commented Jul 8, 2021

Uh oh!

tianon left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gunadhya commented Jul 9, 2021

Uh oh!

tianon left a comment

Choose a reason for hiding this comment

Uh oh!

thaJeztah left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

gunadhya commented Jan 20, 2021 •

edited

Loading