From c3ffd2ef81aa67398e06c798d53d7f98cef39e9e Mon Sep 17 00:00:00 2001 From: Daniel Dao Date: Wed, 3 Feb 2021 11:26:19 +0000 Subject: [PATCH] Do not convert blkio weight value using blkio->io conversion scheme bfq weight controller (i.e. io.bfq.weight if present) is still using the same bfq weight scheme (i.e 1->1000, see [1].) Unfortunately the documentation for this was wrong, and only fixed recently [2]. Therefore, if we map blkio weight to io.bfq.weight, there's no need to do any conversion. Otherwise, we will try to write invalid value which results in error such as: ``` time="2021-02-03T14:55:30Z" level=error msg="container_linux.go:367: starting container process caused: process_linux.go:495: container init caused: process_linux.go:458: setting cgroup config for procHooks process caused: failed to write \"7475\": write /sys/fs/cgroup/runc-cgroups-integration-test/test-cgroup/io.bfq.weight: numerical result out of range" ``` [1] https://github.com/torvalds/linux/blob/master/Documentation/block/bfq-iosched.rst [2] https://github.com/torvalds/linux/commit/65752aef0a407e1ef17ec78a7fc31ba4e0b360f9 Signed-off-by: Daniel Dao --- libcontainer/cgroups/fs2/io.go | 2 +- libcontainer/cgroups/utils.go | 11 ----------- libcontainer/cgroups/utils_test.go | 14 -------------- tests/integration/cgroups.bats | 14 ++++++++++++++ 4 files changed, 15 insertions(+), 26 deletions(-) diff --git a/libcontainer/cgroups/fs2/io.go b/libcontainer/cgroups/fs2/io.go index e01ea001b35..4c4d656c10e 100644 --- a/libcontainer/cgroups/fs2/io.go +++ b/libcontainer/cgroups/fs2/io.go @@ -29,7 +29,7 @@ func setIo(dirPath string, cgroup *configs.Cgroup) error { if cgroup.Resources.BlkioWeight != 0 { filename := "io.bfq.weight" if err := fscommon.WriteFile(dirPath, filename, - strconv.FormatUint(cgroups.ConvertBlkIOToCgroupV2Value(cgroup.Resources.BlkioWeight), 10)); err != nil { + strconv.FormatUint(uint64(cgroup.Resources.BlkioWeight), 10)); err != nil { return err } } diff --git a/libcontainer/cgroups/utils.go b/libcontainer/cgroups/utils.go index 840817e398a..a542b538eef 100644 --- a/libcontainer/cgroups/utils.go +++ b/libcontainer/cgroups/utils.go @@ -400,17 +400,6 @@ func WriteCgroupProc(dir string, pid int) error { return err } -// Since the OCI spec is designed for cgroup v1, in some cases -// there is need to convert from the cgroup v1 configuration to cgroup v2 -// the formula for BlkIOWeight is y = (1 + (x - 10) * 9999 / 990) -// convert linearly from [10-1000] to [1-10000] -func ConvertBlkIOToCgroupV2Value(blkIoWeight uint16) uint64 { - if blkIoWeight == 0 { - return 0 - } - return uint64(1 + (uint64(blkIoWeight)-10)*9999/990) -} - // Since the OCI spec is designed for cgroup v1, in some cases // there is need to convert from the cgroup v1 configuration to cgroup v2 // the formula for cpuShares is y = (1 + ((x - 2) * 9999) / 262142) diff --git a/libcontainer/cgroups/utils_test.go b/libcontainer/cgroups/utils_test.go index 9e7e4ed9ebe..0f621f25ccf 100644 --- a/libcontainer/cgroups/utils_test.go +++ b/libcontainer/cgroups/utils_test.go @@ -548,20 +548,6 @@ func TestGetHugePageSizeImpl(t *testing.T) { } } -func TestConvertBlkIOToCgroupV2Value(t *testing.T) { - cases := map[uint16]uint64{ - 0: 0, - 10: 1, - 1000: 10000, - } - for i, expected := range cases { - got := ConvertBlkIOToCgroupV2Value(i) - if got != expected { - t.Errorf("expected ConvertBlkIOToCgroupV2Value(%d) to be %d, got %d", i, expected, got) - } - } -} - func TestConvertCPUSharesToCgroupV2Value(t *testing.T) { cases := map[uint64]uint64{ 0: 0, diff --git a/tests/integration/cgroups.bats b/tests/integration/cgroups.bats index a319ed04218..1cf2ead49b7 100644 --- a/tests/integration/cgroups.bats +++ b/tests/integration/cgroups.bats @@ -190,6 +190,20 @@ function setup() { [[ "$output" == *'invalid configuration'* ]] } +@test "runc run (blkio weight)" { + requires root cgroups_v2 + + set_cgroups_path "$BUSYBOX_BUNDLE" + update_config '.linux.resources.blockIO |= {"weight": 750}' "${BUSYBOX_BUNDLE}" + + runc run -d --console-socket "$CONSOLE_SOCKET" test_cgroups_unified + [ "$status" -eq 0 ] + + runc exec test_cgroups_unified sh -c 'cat /sys/fs/cgroup/io.bfq.weight' + [ "$status" -eq 0 ] + [ "$output" = 'default 750' ] +} + @test "runc run (cgroup v2 resources.unified only)" { requires root cgroups_v2