Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Date:   Wed, 8 Jan 2020 09:51:08 +0000
From:   Quentin Perret <qperret@google.com>
To:     Dietmar Eggemann <dietmar.eggemann@arm.com>
Cc:     Qais Yousef <qais.yousef@arm.com>, Ingo Molnar <mingo@redhat.com>,
        Peter Zijlstra <peterz@infradead.org>,
        Steven Rostedt <rostedt@goodmis.org>,
        Luis Chamberlain <mcgrof@kernel.org>,
        Kees Cook <keescook@chromium.org>,
        Iurii Zaikin <yzaikin@google.com>,
        Juri Lelli <juri.lelli@redhat.com>,
        Vincent Guittot <vincent.guittot@linaro.org>,
        Ben Segall <bsegall@google.com>, Mel Gorman <mgorman@suse.de>,
        valentin.schneider@arm.com,
        Patrick Bellasi <patrick.bellasi@matbug.net>,
        linux-kernel@vger.kernel.org, linux-fsdevel@vger.kernel.org,
        kernel-team@android.com
Subject: Re: [PATCH] sched/rt: Add a new sysctl to control uclamp_util_min
Message-ID: <20200108095108.GA153171@google.com>
References: <20191220164838.31619-1-qais.yousef@arm.com>
 <20200107134234.GA158998@google.com>
 <8bb17e84-d43f-615f-d04d-c36bb6ede5e0@arm.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <8bb17e84-d43f-615f-d04d-c36bb6ede5e0@arm.com>
User-Agent: Mutt/1.10.1 (2018-07-13)
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Tuesday 07 Jan 2020 at 20:30:36 (+0100), Dietmar Eggemann wrote:
> On 07/01/2020 14:42, Quentin Perret wrote:
> > Hi Qais,
> > 
> > On Friday 20 Dec 2019 at 16:48:38 (+0000), Qais Yousef wrote:
> 
> [...]
> 
> >> diff --git a/kernel/sched/rt.c b/kernel/sched/rt.c
> >> index e591d40fd645..19572dfc175b 100644
> >> --- a/kernel/sched/rt.c
> >> +++ b/kernel/sched/rt.c
> >> @@ -2147,6 +2147,12 @@ static void pull_rt_task(struct rq *this_rq)
> >>   */
> >>  static void task_woken_rt(struct rq *rq, struct task_struct *p)
> >>  {
> >> +	/*
> >> +	 * When sysctl_sched_rt_uclamp_util_min value is changed by the user,
> >> +	 * we apply any new value on the next wakeup, which is here.
> >> +	 */
> >> +	uclamp_rt_sync_default_util_min(p);
> > 
> > The task has already been enqueued and sugov has been called by then I
> > think, so this is a bit late. You could do that in uclamp_rq_inc() maybe?
> 
> That's probably better.
> Just to be sure ...we want this feature (an existing rt task gets its
> UCLAMP_MIN value set when the sysctl changes) because there could be rt
> tasks running before the sysctl is set?

Yeah, I was wondering the same thing, but I'd expect sysadmin to want
this. We could change the min clamp of existing RT tasks in userspace
instead, but given how simple Qais' lazy update code is, the in-kernel
looks reasonable to me. No strong opinion, though.

Thanks,
Quentin