Received-SPF: pass (google.com: domain of linux-kernel+bounces-193221-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1;
From: Benjamin Segall <bsegall@google.com>
To: Cheng Yu <serein.chengyu@huawei.com>
Cc: <mingo@redhat.com>,  <peterz@infradead.org>,  <juri.lelli@redhat.com>,
  <vincent.guittot@linaro.org>,  <dietmar.eggemann@arm.com>,
  <rostedt@goodmis.org>,  <mgorman@suse.de>,  <bristot@redhat.com>,
  <vschneid@redhat.com>,  <changhuaixin@linux.alibaba.com>,
  <shanpeic@linux.alibaba.com>,  <dtcccc@linux.alibaba.com>,
  <tj@kernel.org>,  <linux-kernel@vger.kernel.org>,
  <zhangqiao22@huawei.com>,  <judy.chenhui@huawei.com>,
  <yusongping@huawei.com>,  <zhaowenhui8@huawei.com>,
  <liaoqixin@huawei.com>
Subject: Re: [PATCH 2/2] sched/fair: set burst to zero when set max to cpu.max
In-Reply-To: <20240522031007.643498-3-serein.chengyu@huawei.com> (Cheng Yu's
	message of "Wed, 22 May 2024 11:10:07 +0800")
References: <20240522031007.643498-1-serein.chengyu@huawei.com>
	<20240522031007.643498-3-serein.chengyu@huawei.com>
Date: Tue, 28 May 2024 16:10:42 -0700
Message-ID: <xm268qzt4jnx.fsf@google.com>
User-Agent: Gnus/5.13 (Gnus v5.13)
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain

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