linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Quentin Perret <qperret@google.com>
To: Xuewen Yan <xuewen.yan94@gmail.com>
Cc: mingo@redhat.com, peterz@infradead.org, juri.lelli@redhat.com,
	vincent.guittot@linaro.org, dietmar.eggemann@arm.com,
	rostedt@goodmis.org, bsegall@google.com, mgorman@suse.de,
	bristot@redhat.com, linux-kernel@vger.kernel.org,
	zhang.lyra@gmail.com, xuewyan@foxmail.com,
	patrick.bellasi@matbug.net, tj@kernel.org
Subject: Re: [PATCH] sched/uclamp: Avoid setting cpu.uclamp.min bigger than cpu.uclamp.max
Date: Wed, 2 Jun 2021 13:22:07 +0000	[thread overview]
Message-ID: <YLeF/556Wbvx1Ssc@google.com> (raw)
In-Reply-To: <20210602123803.15738-1-xuewen.yan94@gmail.com>

+CC Patrick and Tejun

On Wednesday 02 Jun 2021 at 20:38:03 (+0800), Xuewen Yan wrote:
> From: Xuewen Yan <xuewen.yan@unisoc.com>
> 
> When setting cpu.uclamp.min/max in cgroup, there is no validating
> like uclamp_validate() in __sched_setscheduler(). It may cause the
> cpu.uclamp.min is bigger than cpu.uclamp.max.

ISTR this was intentional. We also allow child groups to ask for
whatever clamps they want, but that is always limited by the parent, and
reflected in the 'effective' values, as per the cgroup delegation model.

> Although there is protection in cpu_util_update_eff():
> “eff[UCLAMP_MIN] = min(eff[UCLAMP_MIN], eff[UCLAMP_MAX])”, it's better
> not to let it happen.
> 
> Judging the uclamp value before setting uclamp_min/max, avoid
> the cpu.uclamp.min is bigger than cpu.uclamp.max.
> 
> Signed-off-by: Xuewen Yan <xuewen.yan@unisoc.com>
> ---
>  kernel/sched/core.c | 26 +++++++++++++++++++++++++-
>  1 file changed, 25 insertions(+), 1 deletion(-)
> 
> diff --git a/kernel/sched/core.c b/kernel/sched/core.c
> index 5226cc26a095..520a2da40dc9 100644
> --- a/kernel/sched/core.c
> +++ b/kernel/sched/core.c
> @@ -8867,6 +8867,30 @@ static ssize_t cpu_uclamp_write(struct kernfs_open_file *of, char *buf,
>  	rcu_read_lock();
>  
>  	tg = css_tg(of_css(of));
> +
> +	switch (clamp_id) {
> +	case UCLAMP_MIN: {
> +		unsigned int uc_req_max = tg->uclamp_req[UCLAMP_MAX].value;
> +
> +		if (req.util > uc_req_max) {
> +			nbytes = -EINVAL;
> +			goto unlock;
> +		}
> +		break;
> +	}
> +	case UCLAMP_MAX: {
> +		unsigned int uc_req_min = tg->uclamp_req[UCLAMP_MIN].value;
> +
> +		if (req.util < uc_req_min) {
> +			nbytes = -EINVAL;
> +			goto unlock;
> +		}
> +		break;
> +	}
> +	default:
> +		nbytes = -EINVAL;
> +		goto unlock;
> +	}
>  	if (tg->uclamp_req[clamp_id].value != req.util)
>  		uclamp_se_set(&tg->uclamp_req[clamp_id], req.util, false);
>  
> @@ -8878,7 +8902,7 @@ static ssize_t cpu_uclamp_write(struct kernfs_open_file *of, char *buf,
>  
>  	/* Update effective clamps to track the most restrictive value */
>  	cpu_util_update_eff(of_css(of));
> -
> +unlock:
>  	rcu_read_unlock();
>  	mutex_unlock(&uclamp_mutex);
>  
> -- 
> 2.25.1
> 

  reply	other threads:[~2021-06-02 13:23 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-06-02 12:38 Xuewen Yan
2021-06-02 13:22 ` Quentin Perret [this message]
2021-06-03  2:24   ` Xuewen Yan
2021-06-04 16:08     ` Qais Yousef
2021-06-04 16:22       ` Dietmar Eggemann
2021-06-05  2:14         ` Xuewen Yan
2021-06-05  2:12       ` Xuewen Yan
2021-06-05 11:49         ` Qais Yousef
2021-06-05 13:24           ` Xuewen Yan
2021-06-05 14:14             ` Qais Yousef
2021-06-07 13:49             ` Qais Yousef
2021-06-08 11:45               ` Xuewen Yan
2021-06-08 14:25                 ` Qais Yousef
2021-06-08 15:01                   ` Xuewen Yan
2021-06-08 18:21                     ` Qais Yousef

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=YLeF/556Wbvx1Ssc@google.com \
    --to=qperret@google.com \
    --cc=bristot@redhat.com \
    --cc=bsegall@google.com \
    --cc=dietmar.eggemann@arm.com \
    --cc=juri.lelli@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mgorman@suse.de \
    --cc=mingo@redhat.com \
    --cc=patrick.bellasi@matbug.net \
    --cc=peterz@infradead.org \
    --cc=rostedt@goodmis.org \
    --cc=tj@kernel.org \
    --cc=vincent.guittot@linaro.org \
    --cc=xuewen.yan94@gmail.com \
    --cc=xuewyan@foxmail.com \
    --cc=zhang.lyra@gmail.com \
    --subject='Re: [PATCH] sched/uclamp: Avoid setting cpu.uclamp.min bigger than cpu.uclamp.max' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
on how to clone and mirror all data and code used for this inbox