linux-kernel - Re: [PATCH 2/2] sched/fair: set burst to zero when set max to cpu.max

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <xm268qzt4jnx.fsf@google.com>
Date: Tue, 28 May 2024 16:10:42 -0700
From: Benjamin Segall <bsegall@...gle.com>
To: Cheng Yu <serein.chengyu@...wei.com>
Cc: <mingo@...hat.com>,  <peterz@...radead.org>,  <juri.lelli@...hat.com>,
  <vincent.guittot@...aro.org>,  <dietmar.eggemann@....com>,
  <rostedt@...dmis.org>,  <mgorman@...e.de>,  <bristot@...hat.com>,
  <vschneid@...hat.com>,  <changhuaixin@...ux.alibaba.com>,
  <shanpeic@...ux.alibaba.com>,  <dtcccc@...ux.alibaba.com>,
  <tj@...nel.org>,  <linux-kernel@...r.kernel.org>,
  <zhangqiao22@...wei.com>,  <judy.chenhui@...wei.com>,
  <yusongping@...wei.com>,  <zhaowenhui8@...wei.com>,
  <liaoqixin@...wei.com>
Subject: Re: [PATCH 2/2] sched/fair: set burst to zero when set max to cpu.max

Cheng Yu <serein.chengyu@...wei.com> 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 <liaoqixin@...wei.com>
> Signed-off-by: Cheng Yu <serein.chengyu@...wei.com>

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

> ---
>  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, &quota);
> -	if (!ret)
> +	if (!ret) {
> +		if (quota == RUNTIME_INF)
> +			burst = 0;
>  		ret = tg_set_cfs_bandwidth(tg, period, quota, burst);
> +	}
>  	return ret ?: nbytes;
>  }
>  #endif