Re: [PATCH v2 1/4] riscv: defconfig: drop RT_GROUP_SCHED=y

[Celeste Liu]

On 2024-09-09 19:53, Conor Dooley wrote:

> On Fri, Aug 23, 2024 at 01:43:26AM +0800, Celeste Liu wrote:
>> Commit ba6cfef057e1 ("riscv: enable Docker requirements in defconfig")
>> introduced it because of Docker, but Docker has removed this requirement
>> since [1] (2023-04-19).
>>
>> For cgroup v1, if turned on, and there's any cgroup in the "cpu" hierarchy it
>> needs an RT budget assigned, otherwise the processes in it will not be able to
>> get RT at all. The problem with RT group scheduling is that it requires the
>> budget assigned but there's no way we could assign a default budget, since the
>> values to assign are both upper and lower time limits, are absolute, and need to
>> be sum up to < 1 for each individal cgroup. That means we cannot really come up
>> with values that would work by default in the general case.[2]
>>
>> For cgroup v2, it's almost unusable as well. If it turned on, the cpu controller
>> can only be enabled when all RT processes are in the root cgroup. But it will
>> lose the benefits of cgroup v2 if all RT process were placed in the same cgroup.
>>
>> Red Hat, Gentoo, Arch Linux and Debian all disable it. systemd also doesn't
>> support it.[3]
>>
>> [1]: https://github.com/moby/moby/commit/005150ed69c540fb0b5323e0f2208608c1204536
>> [2]: https://bugzilla.redhat.com/show_bug.cgi?id=1229700
>> [3]: https://github.com/systemd/systemd/issues/13781#issuecomment-549164383
>>
>> Fixes: ba6cfef057e1 ("riscv: enable Docker requirements in defconfig")
> 
> I don't think this fixes tag is suitable, the commit you cite in
> moby/docker is a year younger than the one in the fixes tag, so it was
> correct at the time it was written. I think the fixes tag should just be
> removed, since that commit was not wrong. Or am I missing something?

The Docker commit I cited introduce the fix, not the bug. So it should be later
than commit in fixes tag, otherwise Heinrich Schuchardt won't pick the wrong
config from upstream.

Timeline is:
1) 2015-06-19 Qiang Huang introduce RT_GROUP_SCHED check in Docker.[1]
2) 2022-06-08 Heinrich Schuchardt pick this to defconfig in commit
              in fixes tag.
3) 2023-04-19 Florian Schmaus remove this requirement from Docker in
              commit I cited.

[1]: https://github.com/moby/moby/commit/fb85a99f5899df7475ba6b1ac4b2e1d32974eca2

> 
>> Signed-off-by: Celeste Liu <CoelacanthusHex@xxxxxxxxx>
>> ---
>>  arch/riscv/configs/defconfig | 1 -
>>  1 file changed, 1 deletion(-)
>>
>> diff --git a/arch/riscv/configs/defconfig b/arch/riscv/configs/defconfig
>> index 12dc8c73a8ac..de85c3ab261e 100644
>> --- a/arch/riscv/configs/defconfig
>> +++ b/arch/riscv/configs/defconfig
>> @@ -9,7 +9,6 @@ CONFIG_CGROUPS=y
>>  CONFIG_MEMCG=y
>>  CONFIG_CGROUP_SCHED=y
>>  CONFIG_CFS_BANDWIDTH=y
>> -CONFIG_RT_GROUP_SCHED=y
>>  CONFIG_CGROUP_PIDS=y
>>  CONFIG_CGROUP_FREEZER=y
>>  CONFIG_CGROUP_HUGETLB=y
>>
>> -- 
>> 2.46.0
>>