-
Notifications
You must be signed in to change notification settings - Fork 2.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
script/check-config.sh: fixes for cgroupv2 and v5.x kernels #2729
Conversation
1fe94e3
to
85a687e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Generally LGTM, just one minor nit 👍 ❤️
script/check-config.sh
Outdated
cgroupDir="$(dirname "$cgroupSubsystemDir")" | ||
if [ -d "$cgroupDir/cpu" -o -d "$cgroupDir/cpuacct" -o -d "$cgroupDir/cpuset" -o -d "$cgroupDir/devices" -o -d "$cgroupDir/freezer" -o -d "$cgroupDir/memory" ]; then | ||
echo "$(wrap_good 'cgroup hierarchy' 'properly mounted') [$cgroupDir]" | ||
if [[ $(stat -f -c %t /sys/fs/cgroup 2>/dev/null) = "63677270" ]]; then |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[[
isn't necessary here; technically we should do $()
in a separate command to catch the failure, but in this case we don't actually care about the failure (and the else
should cover it) so it's fine:
if [[ $(stat -f -c %t /sys/fs/cgroup 2>/dev/null) = "63677270" ]]; then | |
if [ "$(stat -f -c %t /sys/fs/cgroup 2>/dev/null)" = '63677270' ]; then |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, makes sense (I am unsure why I used [[
in the first place).
Fixed.
(Sorry, saw |
Before: > Generally Necessary: > - cgroup hierarchy: nonexistent?? > (see https://github.com/tianon/cgroupfs-mount) After: > Generally Necessary: > - cgroup hierarchy: cgroupv2 Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
CONFIG_NF_NAT_IPV4 was removed in kernel commit 3bf195ae6037e310, which made its way into v5.1-rc1. The functionality is now under NF_NAT which we already check for. Make the check for NF_NAT_IPV4 conditional. Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
CONFIG_NF_NAT_NEEDED was removed in kernel commit 4806e975729f99c7, which made its way into v5.2-rc1. The functionality is now under NF_NAT which we already check for. Make the check for NF_NAT_NEEDED conditional. Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Kernel commit 2d1c498072de69e (which made its way into kernel v5.8-rc1) removed CONFIG_MEMCG_SWAP_ENABLED Kconfig option, making swap accounting always enabled (unless swapaccount=0 boot option is provided). Make the check conditional. Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
These config options are removed by kernel commit f382fb0bcef4, which made its way into kernel v5.0-rc1. Make the check conditional. Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
85a687e
to
b3cf483
Compare
if [ -d "$cgroupDir/cpu" -o -d "$cgroupDir/cpuacct" -o -d "$cgroupDir/cpuset" -o -d "$cgroupDir/devices" -o -d "$cgroupDir/freezer" -o -d "$cgroupDir/memory" ]; then | ||
echo "$(wrap_good 'cgroup hierarchy' 'properly mounted') [$cgroupDir]" | ||
if [ "$(stat -f -c %t /sys/fs/cgroup 2>/dev/null)" = "63677270" ]; then | ||
echo "$(wrap_good 'cgroup hierarchy' 'cgroupv2')" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we need more validations for cgroup2, but it can be worked out later, so LGTM
This fixes script/check-config.sh to
See individual commits for details.