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

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

[Kailun Qin]

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 <kailun.qin@intel.com>
---
 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

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

[Kailun Qin]

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 <kailun.qin-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
---
 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

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

[Benjamin Segall]

Kailun Qin <kailun.qin@intel.com> 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 <kailun.qin@intel.com>

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 <bsegall@google.com>

> ---
>  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;

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

[Benjamin Segall]

Kailun Qin <kailun.qin-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org> 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 <kailun.qin-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>

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 <bsegall-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>

> ---
>  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;

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

[Kailun Qin]

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 <kailun.qin-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
---
 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