2021-09-10 08:26:17

by Kailun Qin

Subject: [PATCH] sched/core: Fix wrong burst unit in cgroup2 cpu.max write

In cpu_max_write(), as the eventual tg_set_cfs_bandwidth() operates on
the burst in nsec which is input from tg_get_cfs_burst() in usec, it
should be converted into nsec accordingly.

If not, this may cause a write into cgroup2 cpu.max to unexpectedly
change an already set cpu.max.burst.

This patch addresses the above issue.

Signed-off-by: Kailun Qin <[email protected]>
---
kernel/sched/core.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index c4462c454ab9..fc9fcc56149f 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -10711,7 +10711,7 @@ static ssize_t cpu_max_write(struct kernfs_open_file *of,
{
struct task_group *tg = css_tg(of_css(of));
u64 period = tg_get_cfs_period(tg);
- u64 burst = tg_get_cfs_burst(tg);
+ u64 burst = (u64)tg_get_cfs_burst(tg) * NSEC_PER_USEC;
u64 quota;
int ret;

--
2.25.1

2021-09-10 20:46:07

by Benjamin Segall

Subject: Re: [PATCH] sched/core: Fix wrong burst unit in cgroup2 cpu.max write

Kailun Qin <[email protected]> writes:

> In cpu_max_write(), as the eventual tg_set_cfs_bandwidth() operates on
> the burst in nsec which is input from tg_get_cfs_burst() in usec, it
> should be converted into nsec accordingly.
>
> If not, this may cause a write into cgroup2 cpu.max to unexpectedly
> change an already set cpu.max.burst.
>
> This patch addresses the above issue.
>
> Signed-off-by: Kailun Qin <[email protected]>

Oh, huh, cpu_period_quota_parse is confusing and changes the units in
period. (It might make more sense to make all that interaction clearer
somehow, but this is probably fine)

It's probably also better to just use tg->cfs_bandwidth.burst directly
rather than do an unnecessary div+multiply.

For whichever version:

Reviewed-by: Ben Segall <[email protected]>

> ---
> kernel/sched/core.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/kernel/sched/core.c b/kernel/sched/core.c
> index c4462c454ab9..fc9fcc56149f 100644
> --- a/kernel/sched/core.c
> +++ b/kernel/sched/core.c
> @@ -10711,7 +10711,7 @@ static ssize_t cpu_max_write(struct kernfs_open_file *of,
> {
> struct task_group *tg = css_tg(of_css(of));
> u64 period = tg_get_cfs_period(tg);
> - u64 burst = tg_get_cfs_burst(tg);
> + u64 burst = (u64)tg_get_cfs_burst(tg) * NSEC_PER_USEC;
> u64 quota;
> int ret;