In the cgroup cpu subsystem, when we remove the restriction on cfs
bandwidth, the burst feature is also turned off. At that time, we expect
that the value of burst is zero.
Patch 1 fixes it in cgroup v1 by Zhao Wenhui and patch 2 fixes it in
cgroup v2.
Cheng Yu (1):
sched/fair: set burst to zero when set max to cpu.max
Zhao Wenhui (1):
sched/fair: limit burst to zero when cfs bandwidth is turned off
kernel/sched/core.c | 15 +++++++++++++--
1 file changed, 13 insertions(+), 2 deletions(-)
--
2.25.1
In the cgroup v2 cpu subsystem, assuming we have a cgroup named 'test',
and we set cpu.max and cpu.max.burst:
# echo 1000000 > /sys/fs/cgroup/test/cpu.max
# echo 1000000 > /sys/fs/cgroup/test/cpu.max.burst
Next we remove the restriction on cfs bandwidth:
# echo max > /sys/fs/cgroup/test/cpu.max
# cat /sys/fs/cgroup/test/cpu.max
max 100000
# cat /sys/fs/cgroup/test/cpu.max.burst
1000000
Now we expect that the value of burst should be 0. When the burst is 0,
it means that the restriction on burst is cancelled.
Fixes: f4183717b370 ("sched/fair: Introduce the burstable CFS controller")
Reported-by: Qixin Liao <[email protected]>
Signed-off-by: Cheng Yu <[email protected]>
---
kernel/sched/core.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index e9198e30bb74..982d357b3983 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -11414,8 +11414,11 @@ static ssize_t cpu_max_write(struct kernfs_open_file *of,
int ret;
ret = cpu_period_quota_parse(buf, &period, "a);
- if (!ret)
+ if (!ret) {
+ if (quota == RUNTIME_INF)
+ burst = 0;
ret = tg_set_cfs_bandwidth(tg, period, quota, burst);
+ }
return ret ?: nbytes;
}
#endif
--
2.25.1
From: Zhao Wenhui <[email protected]>
When the quota value in CFS bandwidth is set to -1, that imples the
cfs bandwidth is turned off. So the burst feature is supposed to
be disable as well.
Currently, when quota is -1, burst will not be checked, so that it can be
set to any value. Examples:
mkdir /sys/fs/cgroup/cpu/test
echo -1 > /sys/fs/cgroup/cpu/test/cpu.cfs_quota_us
echo 10000000000000000 > /sys/fs/cgroup/cpu/test/cpu.cfs_burst_us
Moreover, after the burst modified by this way, quota can't be set
to any value:
echo 100000 > cpu.cfs_quota_us
-bash: echo: write error: Invalid argument
This patch can ensure the burst value being zero and unchangeable,
when quota is set to -1.
Fixes: f4183717b370 ("sched/fair: Introduce the burstable CFS controller")
Reported-by: Zhao Gongyi <[email protected]>
Signed-off-by: Zhao Wenhui <[email protected]>
Reviewed-by: Tianchen Ding <[email protected]>
Reviewed-by: Ben Segall <[email protected]>
---
v1: https://lore.kernel.org/all/[email protected]/
---
kernel/sched/core.c | 10 +++++++++-
1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index bcf2c4cc0522..e9198e30bb74 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -10840,6 +10840,12 @@ static int tg_set_cfs_bandwidth(struct task_group *tg, u64 period, u64 quota,
burst + quota > max_cfs_runtime))
return -EINVAL;
+ /*
+ * Ensure burst equals to zero when quota is -1.
+ */
+ if (quota == RUNTIME_INF && burst)
+ return -EINVAL;
+
/*
* Prevent race between setting of cfs_rq->runtime_enabled and
* unthrottle_offline_cfs_rqs().
@@ -10899,8 +10905,10 @@ static int tg_set_cfs_quota(struct task_group *tg, long cfs_quota_us)
period = ktime_to_ns(tg->cfs_bandwidth.period);
burst = tg->cfs_bandwidth.burst;
- if (cfs_quota_us < 0)
+ if (cfs_quota_us < 0) {
quota = RUNTIME_INF;
+ burst = 0;
+ }
else if ((u64)cfs_quota_us <= U64_MAX / NSEC_PER_USEC)
quota = (u64)cfs_quota_us * NSEC_PER_USEC;
else
--
2.25.1
On Wed, May 22, 2024 at 11:10:05AM +0800, Cheng Yu wrote:
> In the cgroup cpu subsystem, when we remove the restriction on cfs
> bandwidth, the burst feature is also turned off. At that time, we expect
> that the value of burst is zero.
>
> Patch 1 fixes it in cgroup v1 by Zhao Wenhui and patch 2 fixes it in
> cgroup v2.
>
> Cheng Yu (1):
> sched/fair: set burst to zero when set max to cpu.max
>
> Zhao Wenhui (1):
> sched/fair: limit burst to zero when cfs bandwidth is turned off
>
## Before patch
# uname -r
6.9.0-12124-g6d69b6c12fce-dirty
# mkdir test
# cd test/
# cat cpu.max cpu.max.burst
max 100000
0
# echo 10000000 > cpu.max.burst
# echo 1000000000000 > cpu.max.burst
# cat cpu.max cpu.max.burst
max 100000
1000000000000
# echo "1000 100000" > cpu.max
-bash: echo: write error: Invalid argument
# echo 1000 > cpu.max.burst
# echo "1000 100000" > cpu.max
# cat cpu.max cpu.max.burst
1000 100000
1000
## After patch
# uname -r
6.9.0-12126-g7eb1a247b675-dirty
# mkdir test
# cd test/
# cat cpu.max cpu.max.burst
max 100000
0
# echo 1134535435 > cpu.max.burst
-bash: echo: write error: Invalid argument
# echo 1 > cpu.max.burst
-bash: echo: write error: Invalid argument
# echo -1 > cpu.max.burst
-bash: echo: write error: Invalid argument
# echo "10000 100000" > cpu.max
# echo 1000 > cpu.max.burst
# cat cpu.max cpu.max.burst
10000 100000
1000
# echo "max 100000" > cpu.max
# cat cpu.max cpu.max.burst
max 100000
0
# git log --oneline
7eb1a247b6753 (HEAD) sched/fair: set burst to zero when set max to cpu.max
421647086da9e sched/fair: limit burst to zero when cfs bandwidth is turned off
6d69b6c12fce4 (origin/master, origin/HEAD, master) Merge tag 'nfs-for-6.10-1' o
Now, the burst value can only be set after setting
the quota. This change also prevents setting excessively
large burst values.
Thank you the fix.
Tested-by: Vishal Chourasia <[email protected]>
> kernel/sched/core.c | 15 +++++++++++++--
> 1 file changed, 13 insertions(+), 2 deletions(-)
>
> --
> 2.25.1
>
Cheng Yu <[email protected]> writes:
> In the cgroup v2 cpu subsystem, assuming we have a cgroup named 'test',
> and we set cpu.max and cpu.max.burst:
> # echo 1000000 > /sys/fs/cgroup/test/cpu.max
> # echo 1000000 > /sys/fs/cgroup/test/cpu.max.burst
>
> Next we remove the restriction on cfs bandwidth:
> # echo max > /sys/fs/cgroup/test/cpu.max
> # cat /sys/fs/cgroup/test/cpu.max
> max 100000
> # cat /sys/fs/cgroup/test/cpu.max.burst
> 1000000
>
> Now we expect that the value of burst should be 0. When the burst is 0,
> it means that the restriction on burst is cancelled.
>
> Fixes: f4183717b370 ("sched/fair: Introduce the burstable CFS controller")
> Reported-by: Qixin Liao <[email protected]>
> Signed-off-by: Cheng Yu <[email protected]>
Yeah, makes sense. My general assumption would be to put these in one
patch, but if there's a convention to separate v1 and v2 that I've
missed, I have no opinion.
Reviewed-by: Ben Segall <[email protected]>
> ---
> kernel/sched/core.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/kernel/sched/core.c b/kernel/sched/core.c
> index e9198e30bb74..982d357b3983 100644
> --- a/kernel/sched/core.c
> +++ b/kernel/sched/core.c
> @@ -11414,8 +11414,11 @@ static ssize_t cpu_max_write(struct kernfs_open_file *of,
> int ret;
>
> ret = cpu_period_quota_parse(buf, &period, "a);
> - if (!ret)
> + if (!ret) {
> + if (quota == RUNTIME_INF)
> + burst = 0;
> ret = tg_set_cfs_bandwidth(tg, period, quota, burst);
> + }
> return ret ?: nbytes;
> }
> #endif
On 2024/5/29 7:10, Benjamin Segall wrote:
> Cheng Yu <[email protected]> writes:
>
>> In the cgroup v2 cpu subsystem, assuming we have a cgroup named 'test',
>> and we set cpu.max and cpu.max.burst:
>> # echo 1000000 > /sys/fs/cgroup/test/cpu.max
>> # echo 1000000 > /sys/fs/cgroup/test/cpu.max.burst
>>
>> Next we remove the restriction on cfs bandwidth:
>> # echo max > /sys/fs/cgroup/test/cpu.max
>> # cat /sys/fs/cgroup/test/cpu.max
>> max 100000
>> # cat /sys/fs/cgroup/test/cpu.max.burst
>> 1000000
>>
>> Now we expect that the value of burst should be 0. When the burst is 0,
>> it means that the restriction on burst is cancelled.
>>
>> Fixes: f4183717b370 ("sched/fair: Introduce the burstable CFS controller")
>> Reported-by: Qixin Liao <[email protected]>
>> Signed-off-by: Cheng Yu <[email protected]>
>
> Yeah, makes sense. My general assumption would be to put these in one
> patch, but if there's a convention to separate v1 and v2 that I've
> missed, I have no opinion.
>
> Reviewed-by: Ben Segall <[email protected]>
>
Thanks for the advice. I will submit a v2 patch that includes both cgroup v1 and v2 modification.
>> ---
>> kernel/sched/core.c | 5 ++++-
>> 1 file changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/kernel/sched/core.c b/kernel/sched/core.c
>> index e9198e30bb74..982d357b3983 100644
>> --- a/kernel/sched/core.c
>> +++ b/kernel/sched/core.c
>> @@ -11414,8 +11414,11 @@ static ssize_t cpu_max_write(struct kernfs_open_file *of,
>> int ret;
>>
>> ret = cpu_period_quota_parse(buf, &period, "a);
>> - if (!ret)
>> + if (!ret) {
>> + if (quota == RUNTIME_INF)
>> + burst = 0;
>> ret = tg_set_cfs_bandwidth(tg, period, quota, burst);
>> + }
>> return ret ?: nbytes;
>> }
>> #endif