Re: [PATCH v6 05/16] sched/core: uclamp: Update CPU's refcount on clamp changes

Peter Zijlstra
2019-01-22T13:28:17Z

On Tue, Jan 22, 2019 at 10:43:05AM +0000, Patrick Bellasi wrote:
> On 22-Jan 10:37, Peter Zijlstra wrote:

> > Sure, I get that. What I don't get is why you're adding that (2) here.
> > Like said, __sched_setscheduler() already does a dequeue/enqueue under
> > rq->lock, which should already take care of that.
>
> Oh, ok... got it what you mean now.
>
> With:
>
> [PATCH v6 01/16] sched/core: Allow sched_setattr() to use the current policy
> <20190115101513.2822-2-patrick.bellasi@arm.com>
>
> we can call __sched_setscheduler() with:
>
> attr->sched_flags & SCHED_FLAG_KEEP_POLICY
>
> whenever we want just to change the clamp values of a task without
> changing its class. Thus, we can end up returning from
> __sched_setscheduler() without doing an actual dequeue/enqueue.

I don't see that happening.. when KEEP_POLICY we set attr.sched_policy =
SETPARAM_POLICY. That is then checked again in __setscheduler_param(),
which is in the middle of that dequeue/enqueue.

Also, and this might be 'broken', SETPARAM_POLICY _does_ reset all the
other attributes, it only preserves policy, but it will (re)set nice
level for example (see that same function).

So maybe we want to introduce another (few?) FLAG_KEEP flag(s) that
preserve the other bits; I'm thinking at least KEEP_PARAM and KEEP_UTIL
or something.