Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Fri, 2 Oct 2020 13:38:12 +0800
From:   Yun Hsiang <hsiang023167@gmail.com>
To:     Dietmar Eggemann <dietmar.eggemann@arm.com>
Cc:     peterz@infradead.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 1/1] sched/uclamp: release per-task uclamp control if
 user set to default value
Message-ID: <20201002053812.GA176142@ubuntu>
References: <20200928082643.133257-1-hsiang023167@gmail.com>
 <8272de8d-9868-d419-e2bb-d5e2c0614b63@arm.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <8272de8d-9868-d419-e2bb-d5e2c0614b63@arm.com>
Precedence: bulk

On Wed, Sep 30, 2020 at 03:12:51PM +0200, Dietmar Eggemann wrote:
Hi Dietmar,

> Hi Yun,
> 
> On 28/09/2020 10:26, Yun Hsiang wrote:
> > If the user wants to release the util clamp and let cgroup to control it,
> > we need a method to reset.
> > 
> > So if the user set the task uclamp to the default value (0 for UCLAMP_MIN
> > and 1024 for UCLAMP_MAX), reset the user_defined flag to release control.
> > 
> > Signed-off-by: Yun Hsiang <hsiang023167@gmail.com>
> 
> could you explain with a little bit more detail why you would need this
> feature?
> 
> Currently we assume that once the per-task uclamp (user-defined) values
> are set, you could only change the effective uclamp values of this task
> by (1) moving it into another taskgroup or (2) changing the system
> default uclamp values.
> 

Assume a module that controls group (such as top-app in android) uclamp and
task A in the group.
Once task A set uclamp, it will not be affected by the group setting.
If task A doesn't want to control itself anymore,
it can not go back to the initial state to let the module(group) control.
But the other tasks in the group will be affected by the group.

The policy might be
1) if the task wants to control it's uclamp, use task uclamp value
(but under group uclamp constraint)
2) if the task doesn't want to control it's uclamp, use group uclamp value.

If the policy is proper, we need a reset method for per-task uclamp.

> > ---
> >  kernel/sched/core.c | 7 +++++--
> >  1 file changed, 5 insertions(+), 2 deletions(-)
> > 
> > diff --git a/kernel/sched/core.c b/kernel/sched/core.c
> > index 9a2fbf98fd6f..fa63d70d783a 100644
> > --- a/kernel/sched/core.c
> > +++ b/kernel/sched/core.c
> > @@ -1187,6 +1187,7 @@ static void __setscheduler_uclamp(struct task_struct *p,
> >  				  const struct sched_attr *attr)
> >  {
> >  	enum uclamp_id clamp_id;
> > +	bool user_defined;
> >  
> >  	/*
> >  	 * On scheduling class change, reset to default clamps for tasks
> > @@ -1210,14 +1211,16 @@ static void __setscheduler_uclamp(struct task_struct *p,
> >  	if (likely(!(attr->sched_flags & SCHED_FLAG_UTIL_CLAMP)))
> >  		return;
> >  
> > +	user_defined = attr->sched_util_min == 0 ? false : true;
> >  	if (attr->sched_flags & SCHED_FLAG_UTIL_CLAMP_MIN) {
> >  		uclamp_se_set(&p->uclamp_req[UCLAMP_MIN],
> > -			      attr->sched_util_min, true);
> > +			      attr->sched_util_min, user_defined);
> >  	}
> >  
> > +	user_defined = attr->sched_util_max == 1024 ? false : true;
> >  	if (attr->sched_flags & SCHED_FLAG_UTIL_CLAMP_MAX) {
> >  		uclamp_se_set(&p->uclamp_req[UCLAMP_MAX],
> > -			      attr->sched_util_max, true);
> > +			      attr->sched_util_max, user_defined);
> >  	}
> >  }