Re: [PATCH v5 2/2] sched/uclamp: Protect uclamp fast path code with static key
On 06/30/20 19:44, Patrick Bellasi wrote: [...] > > I am sorry there's no written rule that says one should do it in a specific > > way. And AFAIK both way are implemented in the kernel. I appreciate your > > suggestion but as the person who did all the hard work, I think my > > preference > > matters here too. > > You sure know that sometime reviewing code can be an "hard work" too, so I > would not go down that way at all with the discussion. Quite likely I > have a different "subjective" view on how Open Source development works. > > > And actually with my approach when uclamp is not compiled in there's no > > need to > > define an extra variable; and since uclamp_is_used() is defined as false for > > !CONFIG_UCLAMP_TASK, it'll help with DCE, so less likely to end up with dead > > code that'll never run in the final binary. > > Good, this is the simple and small reply I've politely asked for. I am sorry if I offended you. I took all your comments seriously and answered them to the best of my ability. All of your comments and suggestions were highly appreciated too. If the wrong message reached across, rest assured it wasn't the intended one. Thanks -- Qais Yousef
Re: [PATCH v5 2/2] sched/uclamp: Protect uclamp fast path code with static key
On Tue, Jun 30, 2020 at 17:40:34 +0200, Qais Yousef wrote... > Hi Patrick > > On 06/30/20 16:55, Patrick Bellasi wrote: >> >> Hi Qais, >> sorry for commenting on v5 with a v6 already posted, but... >> ... I cannot keep up with your re-spinning rate ;) > > I classified that as a nit really and doesn't affect correctness. We have > different subjective view on what is better here. I did all the work in the > past 2 weeks and I think as the author of this patch I have the right to keep > my preference on subjective matters. I did consider your feedback and didn't > ignore it and improved the naming and added a comment to make sure there's no > confusion. > > We could nitpick the best name forever, but is it really that important? Which leans toward confirming the impression I had while reading your previous response, i.e. you stopped reading at the name change observation, which would be _just_ a nit-picking, although still worth IMHO. Instead, I went further and asked you to consider a different approach: not adding a new kernel symbol to represent a concept already there. > I really don't see any added value for one approach or another here to start > a long debate about it. Then you could have just called out that instead of silently ignoring the comment/proposal. > The comments were small enough that I didn't see any controversy that > warrants holding the patches longer. I agreed with your proposal to use > uc_se->active and clarified why your other suggestions don't hold. > > You pointed that uclamp_is_enabled() confused you; and I responded that I'll > change the name. Perhaps it would not confuse only me having 'something_enabled()' referring to 'something_used'. > Sorry for not being explicit about answering the below, but > I thought my answer implied that I don't prefer it. Your answer was about a name change, don't see correlation with a different approach... but should be just me. >> >> Thus, perhaps we can just use the same pattern used by the >> >> sched_numa_balancing static key: >> >> >> >> $ git grep sched_numa_balancing >> >> kernel/sched/core.c:DEFINE_STATIC_KEY_FALSE(sched_numa_balancing); >> >> kernel/sched/core.c: >> >> static_branch_enable(_numa_balancing); >> >> kernel/sched/core.c: >> >> static_branch_disable(_numa_balancing); >> >> kernel/sched/core.c:int state = >> >> static_branch_likely(_numa_balancing); >> >> kernel/sched/fair.c:if >> >> (!static_branch_likely(_numa_balancing)) >> >> kernel/sched/fair.c:if >> >> (!static_branch_likely(_numa_balancing)) >> >> kernel/sched/fair.c:if >> >> (!static_branch_likely(_numa_balancing)) >> >> kernel/sched/fair.c:if >> >> (static_branch_unlikely(_numa_balancing)) >> >> kernel/sched/sched.h:extern struct static_key_false >> >> sched_numa_balancing; >> >> >> >> IOW: unconditionally define sched_uclamp_used as non static in core.c, >> >> and use it directly on schedutil too. >> >> So, what about this instead of adding the (renamed) method above? > > I am sorry there's no written rule that says one should do it in a specific > way. And AFAIK both way are implemented in the kernel. I appreciate your > suggestion but as the person who did all the hard work, I think my preference > matters here too. You sure know that sometime reviewing code can be an "hard work" too, so I would not go down that way at all with the discussion. Quite likely I have a different "subjective" view on how Open Source development works. > And actually with my approach when uclamp is not compiled in there's no need > to > define an extra variable; and since uclamp_is_used() is defined as false for > !CONFIG_UCLAMP_TASK, it'll help with DCE, so less likely to end up with dead > code that'll never run in the final binary. Good, this is the simple and small reply I've politely asked for. Best, Patrick
Re: [PATCH v5 2/2] sched/uclamp: Protect uclamp fast path code with static key
Hi Patrick On 06/30/20 16:55, Patrick Bellasi wrote: > > Hi Qais, > sorry for commenting on v5 with a v6 already posted, but... > ... I cannot keep up with your re-spinning rate ;) I classified that as a nit really and doesn't affect correctness. We have different subjective view on what is better here. I did all the work in the past 2 weeks and I think as the author of this patch I have the right to keep my preference on subjective matters. I did consider your feedback and didn't ignore it and improved the naming and added a comment to make sure there's no confusion. We could nitpick the best name forever, but is it really that important? I really don't see any added value for one approach or another here to start a long debate about it. The comments were small enough that I didn't see any controversy that warrants holding the patches longer. I agreed with your proposal to use uc_se->active and clarified why your other suggestions don't hold. You pointed that uclamp_is_enabled() confused you; and I responded that I'll change the name. Sorry for not being explicit about answering the below, but I thought my answer implied that I don't prefer it. > > >> Thus, perhaps we can just use the same pattern used by the > >> sched_numa_balancing static key: > >> > >> $ git grep sched_numa_balancing > >> kernel/sched/core.c:DEFINE_STATIC_KEY_FALSE(sched_numa_balancing); > >> kernel/sched/core.c: > >> static_branch_enable(_numa_balancing); > >> kernel/sched/core.c: > >> static_branch_disable(_numa_balancing); > >> kernel/sched/core.c:int state = > >> static_branch_likely(_numa_balancing); > >> kernel/sched/fair.c:if (!static_branch_likely(_numa_balancing)) > >> kernel/sched/fair.c:if (!static_branch_likely(_numa_balancing)) > >> kernel/sched/fair.c:if (!static_branch_likely(_numa_balancing)) > >> kernel/sched/fair.c:if > >> (static_branch_unlikely(_numa_balancing)) > >> kernel/sched/sched.h:extern struct static_key_false sched_numa_balancing; > >> > >> IOW: unconditionally define sched_uclamp_used as non static in core.c, > >> and use it directly on schedutil too. > > So, what about this instead of adding the (renamed) method above? I am sorry there's no written rule that says one should do it in a specific way. And AFAIK both way are implemented in the kernel. I appreciate your suggestion but as the person who did all the hard work, I think my preference matters here too. And actually with my approach when uclamp is not compiled in there's no need to define an extra variable; and since uclamp_is_used() is defined as false for !CONFIG_UCLAMP_TASK, it'll help with DCE, so less likely to end up with dead code that'll never run in the final binary. Thanks a lot for all of your comments and feedback anyway! -- Qais Yousef
Re: [PATCH v5 2/2] sched/uclamp: Protect uclamp fast path code with static key
Hi Qais,
sorry for commenting on v5 with a v6 already posted, but...
... I cannot keep up with your re-spinning rate ;)
More importantly, perhaps you missed to comment on one of my previous
points.
Will have a better look at the rest of v6 later today.
Cheers,
Patrick
On Tue, Jun 30, 2020 at 11:46:24 +0200, Qais Yousef
wrote...
> On 06/30/20 10:11, Patrick Bellasi wrote:
>> On Mon, Jun 29, 2020 at 18:26:33 +0200, Qais Yousef
>> wrote...
[...]
>> > +
>> > +static inline bool uclamp_is_enabled(void)
>> > +{
>> > + return static_branch_likely(_uclamp_used);
>> > +}
>>
>> Looks like here we mix up terms, which can be confusing.
>> AFAIKS, we use:
>> - *_enabled for the sched class flags (compile time)
>> - *_usedfor the user-space opting in (run time)
>
> I wanted to add a comment here.
>
> I can rename it to uclamp_is_used() if you want.
In my previous message I was mostly asking about this:
>> Thus, perhaps we can just use the same pattern used by the
>> sched_numa_balancing static key:
>>
>> $ git grep sched_numa_balancing
>> kernel/sched/core.c:DEFINE_STATIC_KEY_FALSE(sched_numa_balancing);
>> kernel/sched/core.c:
>> static_branch_enable(_numa_balancing);
>> kernel/sched/core.c:
>> static_branch_disable(_numa_balancing);
>> kernel/sched/core.c:int state =
>> static_branch_likely(_numa_balancing);
>> kernel/sched/fair.c:if (!static_branch_likely(_numa_balancing))
>> kernel/sched/fair.c:if (!static_branch_likely(_numa_balancing))
>> kernel/sched/fair.c:if (!static_branch_likely(_numa_balancing))
>> kernel/sched/fair.c:if (static_branch_unlikely(_numa_balancing))
>> kernel/sched/sched.h:extern struct static_key_false sched_numa_balancing;
>>
>> IOW: unconditionally define sched_uclamp_used as non static in core.c,
>> and use it directly on schedutil too.
So, what about this instead of adding the (renamed) method above?
Re: [PATCH v5 2/2] sched/uclamp: Protect uclamp fast path code with static key
Hi Patrick
On 06/30/20 10:11, Patrick Bellasi wrote:
>
> Hi Qais,
> here are some more 2c from me...
>
> On Mon, Jun 29, 2020 at 18:26:33 +0200, Qais Yousef
> wrote...
>
> [...]
>
> > diff --git a/kernel/sched/core.c b/kernel/sched/core.c
> > index 235b2cae00a0..8d80d6091d86 100644
> > --- a/kernel/sched/core.c
> > +++ b/kernel/sched/core.c
> > @@ -794,6 +794,26 @@ unsigned int sysctl_sched_uclamp_util_max =
> > SCHED_CAPACITY_SCALE;
> > /* All clamps are required to be less or equal than these values */
> > static struct uclamp_se uclamp_default[UCLAMP_CNT];
> >
> > +/*
> > + * This static key is used to reduce the uclamp overhead in the fast path.
> > It
> > + * primarily disables the call to uclamp_rq_{inc, dec}() in
> > + * enqueue/dequeue_task().
> > + *
> > + * This allows users to continue to enable uclamp in their kernel config
> > with
> > + * minimum uclamp overhead in the fast path.
> > + *
> > + * As soon as userspace modifies any of the uclamp knobs, the static key is
> > + * enabled, since we have an actual users that make use of uclamp
> > + * functionality.
> > + *
> > + * The knobs that would enable this static key are:
> > + *
> > + * * A task modifying its uclamp value with sched_setattr().
> > + * * An admin modifying the sysctl_sched_uclamp_{min, max} via procfs.
> > + * * An admin modifying the cgroup cpu.uclamp.{min, max}
>
> I guess this list can be obtained with a grep or git changelog, moreover
> this text will require maintenance.
>
> What about replacing this full comment with something shorted like:
>
> ---8<---
> Static key to reduce uclamp overhead in the fast path by disabling
> calls to uclamp_rq_{inc, dec}().
> ---8<---
If you don't mind, I rather more verbose info. As a relatively new comer, lack
of comments about expectation of some functions is still a challenge.
>
> > + */
> > +DEFINE_STATIC_KEY_FALSE(sched_uclamp_used);
> > +
> > /* Integer rounded range for each bucket */
> > #define UCLAMP_BUCKET_DELTA DIV_ROUND_CLOSEST(SCHED_CAPACITY_SCALE,
> > UCLAMP_BUCKETS)
> >
> > @@ -994,9 +1014,30 @@ static inline void uclamp_rq_dec_id(struct rq *rq,
> > struct task_struct *p,
> > lockdep_assert_held(>lock);
> >
> > bucket = _rq->bucket[uc_se->bucket_id];
> > - SCHED_WARN_ON(!bucket->tasks);
> > +
> > + /*
> > +* bucket->tasks could be zero if sched_uclamp_used was enabled while
> > +* the current task was running, hence we could end up with unbalanced
> > +* call to uclamp_rq_dec_id().
> > +*
> > +* Need to be careful of the following enqeueue/dequeue order
> > +* problem too
> > +*
> > +* enqueue(taskA)
> > +* // sched_uclamp_used gets enabled
> > +* enqueue(taskB)
> > +* dequeue(taskA)
> > +* // bucket->tasks is now 0
> > +* dequeue(taskB)
> > +*
> > +* where we could end up with uc_se->active of the task set to true and
> > +* the wrong bucket[uc_se->bucket_id].value.
> > +*
> > +* Hence always make sure we reset things properly.
> > +*/
> > if (likely(bucket->tasks))
> > bucket->tasks--;
> > +
> > uc_se->active = false;
>
> Better than v4, what about just using this active flag?
>
> ---8<---
> diff --git a/kernel/sched/core.c b/kernel/sched/core.c
> index 8f360326861e..465a7645713b 100644
> --- a/kernel/sched/core.c
> +++ b/kernel/sched/core.c
> @@ -990,6 +990,13 @@ static inline void uclamp_rq_dec_id(struct rq *rq,
> struct task_struct *p,
>
> lockdep_assert_held(>lock);
>
> + /*
> +* If a task was already enqueue at uclamp enable time
> +* nothing has been accounted for it.
> +*/
> + if (unlikely(!uc_se->active))
> + return;
> +
> bucket = _rq->bucket[uc_se->bucket_id];
> SCHED_WARN_ON(!bucket->tasks);
> if (likely(bucket->tasks))
> ---8<---
>
> This will allow also to keep in all the ref count checks we have,
> e.g. the SChed_WARN_ON().
Works for me. Though I'd like to expand on the comment more just because there
were few things that were caught out and worth documenting IMO.
>
>
> > /*
> > @@ -1032,6 +1073,13 @@ static inline void uclamp_rq_inc(struct rq *rq,
> > struct task_struct *p)
> > {
> > enum uclamp_id clamp_id;
> >
> > + /*
> > +* Avoid any overhead until uclamp is actually used by the userspace.
> > +* Including the branch if we use static_branch_likely()
>
> I still find this last sentence hard to parse, but perhaps it's just me
> still missing a breakfast :)
It used to be
* Including the JMP if we use static_branch_likely()
Note s/branch/JMP/
Effectively the condition is written such that we produce a NOP when uclamp is
not used. I'll rephrase.
>
> > +*/
> > + if (!static_branch_unlikely(_uclamp_used))
> > + return;
>
> I'm also still wondering if the optimization is still working when we
> have that ! in front.
It does. I
Re: [PATCH v5 2/2] sched/uclamp: Protect uclamp fast path code with static key
On 30/06/20 09:11, Patrick Bellasi wrote:
> Hi Qais,
> here are some more 2c from me...
>
> On Mon, Jun 29, 2020 at 18:26:33 +0200, Qais Yousef
> wrote...
>
> [...]
>
>> diff --git a/kernel/sched/core.c b/kernel/sched/core.c
>> index 235b2cae00a0..8d80d6091d86 100644
>> --- a/kernel/sched/core.c
>> +++ b/kernel/sched/core.c
>> @@ -794,6 +794,26 @@ unsigned int sysctl_sched_uclamp_util_max =
>> SCHED_CAPACITY_SCALE;
>> /* All clamps are required to be less or equal than these values */
>> static struct uclamp_se uclamp_default[UCLAMP_CNT];
>>
>> +/*
>> + * This static key is used to reduce the uclamp overhead in the fast path.
>> It
>> + * primarily disables the call to uclamp_rq_{inc, dec}() in
>> + * enqueue/dequeue_task().
>> + *
>> + * This allows users to continue to enable uclamp in their kernel config
>> with
>> + * minimum uclamp overhead in the fast path.
>> + *
>> + * As soon as userspace modifies any of the uclamp knobs, the static key is
>> + * enabled, since we have an actual users that make use of uclamp
>> + * functionality.
>> + *
>> + * The knobs that would enable this static key are:
>> + *
>> + * * A task modifying its uclamp value with sched_setattr().
>> + * * An admin modifying the sysctl_sched_uclamp_{min, max} via procfs.
>> + * * An admin modifying the cgroup cpu.uclamp.{min, max}
>
> I guess this list can be obtained with a grep or git changelog, moreover
> this text will require maintenance.
>
> What about replacing this full comment with something shorted like:
>
> ---8<---
> Static key to reduce uclamp overhead in the fast path by disabling
> calls to uclamp_rq_{inc, dec}().
> ---8<---
>
Having some sense of when that key gets flipped is worthwhile IMO; though
it may not have to be exhaustive list.
>> + */
>> +DEFINE_STATIC_KEY_FALSE(sched_uclamp_used);
>> +
>> /* Integer rounded range for each bucket */
>> #define UCLAMP_BUCKET_DELTA DIV_ROUND_CLOSEST(SCHED_CAPACITY_SCALE,
>> UCLAMP_BUCKETS)
>>
[...]
>> @@ -1032,6 +1073,13 @@ static inline void uclamp_rq_inc(struct rq *rq,
>> struct task_struct *p)
>> {
>> enum uclamp_id clamp_id;
>>
>> +/*
>> + * Avoid any overhead until uclamp is actually used by the userspace.
>> + * Including the branch if we use static_branch_likely()
>
> I still find this last sentence hard to parse, but perhaps it's just me
> still missing a breakfast :)
>
AIUI this tries to explain why we use 'unlikely' rather than 'likely' -
it's to prevent !uclamp users from having the branch overhead (see
include/linux/jump_label.h, there's a nice fat comment just above
static_branch_likely()).
IMO that point is already covered by the first sentence, as I blabbered on
some previous version.
>> + */
>> +if (!static_branch_unlikely(_uclamp_used))
>> +return;
>
> I'm also still wondering if the optimization is still working when we
> have that ! in front.
>
> Had a check at:
>
>
> https://elixir.bootlin.com/linux/latest/source/include/linux/jump_label.h#L399
>
> and AFAIU, it all boils down to cook a __branch_check()'s compiler hint,
> and ISTR that those are "anti-patterns"?
>
> That said we do have some usages for this pattern too:
>
> $ git grep '!static_branch_unlikely' | wc -l 36
> $ git grep 'static_branch_unlikely' | wc -l 220
>
> ?
>
We use it for e.g. the sched_asym_cpucapacity key, and that works (and I've
been down into the asm a few times).
>> +
>> if (unlikely(!p->sched_class->uclamp_enabled))
>> return;
>>
>
> [...]
>
>> +/**
>> + * uclamp_rq_util_with - clamp @util with @rq and @p effective uclamp
>> values.
>> + * @rq: The rq to clamp against. Must not be NULL.
>> + * @util: The util value to clamp.
>> + * @p: The task to clamp against. Can be NULL if you want to
>> clamp
>> + * against @rq only.
>> + *
>> + * Clamps the passed @util to the max(@rq, @p) effective uclamp values.
>> + *
>> + * If sched_uclamp_used static key is disabled, then just return the util
>> + * without any clamping since uclamp aggregation at the rq level in the fast
>> + * path is disabled, rendering this operation a NOP.
>> + *
>> + * Use uclamp_eff_value() if you don't care about uclamp values at rq
>> level. It
>> + * will return the correct effective uclamp value of the task even if the
>> + * static key is disabled.
>
> Well, if you don't care about rq, you don't call a uclamp_rq_* method.
>
> I would say that the above paragraph is redundant, moreover it adds some
> cross-reference to a different method (name) which required maintenance.
>
> What about removing it?
>
>> + */
>> static __always_inline
>> unsigned long uclamp_rq_util_with(struct rq *rq, unsigned long util,
>>struct task_struct *p)
>> {
>> -unsigned long min_util = READ_ONCE(rq->uclamp[UCLAMP_MIN].value);
>> -unsigned long max_util = READ_ONCE(rq->uclamp[UCLAMP_MAX].value);
>> +unsigned long min_util;
>> +
Re: [PATCH v5 2/2] sched/uclamp: Protect uclamp fast path code with static key
Hi Qais,
here are some more 2c from me...
On Mon, Jun 29, 2020 at 18:26:33 +0200, Qais Yousef
wrote...
[...]
> diff --git a/kernel/sched/core.c b/kernel/sched/core.c
> index 235b2cae00a0..8d80d6091d86 100644
> --- a/kernel/sched/core.c
> +++ b/kernel/sched/core.c
> @@ -794,6 +794,26 @@ unsigned int sysctl_sched_uclamp_util_max =
> SCHED_CAPACITY_SCALE;
> /* All clamps are required to be less or equal than these values */
> static struct uclamp_se uclamp_default[UCLAMP_CNT];
>
> +/*
> + * This static key is used to reduce the uclamp overhead in the fast path. It
> + * primarily disables the call to uclamp_rq_{inc, dec}() in
> + * enqueue/dequeue_task().
> + *
> + * This allows users to continue to enable uclamp in their kernel config with
> + * minimum uclamp overhead in the fast path.
> + *
> + * As soon as userspace modifies any of the uclamp knobs, the static key is
> + * enabled, since we have an actual users that make use of uclamp
> + * functionality.
> + *
> + * The knobs that would enable this static key are:
> + *
> + * * A task modifying its uclamp value with sched_setattr().
> + * * An admin modifying the sysctl_sched_uclamp_{min, max} via procfs.
> + * * An admin modifying the cgroup cpu.uclamp.{min, max}
I guess this list can be obtained with a grep or git changelog, moreover
this text will require maintenance.
What about replacing this full comment with something shorted like:
---8<---
Static key to reduce uclamp overhead in the fast path by disabling
calls to uclamp_rq_{inc, dec}().
---8<---
> + */
> +DEFINE_STATIC_KEY_FALSE(sched_uclamp_used);
> +
> /* Integer rounded range for each bucket */
> #define UCLAMP_BUCKET_DELTA DIV_ROUND_CLOSEST(SCHED_CAPACITY_SCALE,
> UCLAMP_BUCKETS)
>
> @@ -994,9 +1014,30 @@ static inline void uclamp_rq_dec_id(struct rq *rq,
> struct task_struct *p,
> lockdep_assert_held(>lock);
>
> bucket = _rq->bucket[uc_se->bucket_id];
> - SCHED_WARN_ON(!bucket->tasks);
> +
> + /*
> + * bucket->tasks could be zero if sched_uclamp_used was enabled while
> + * the current task was running, hence we could end up with unbalanced
> + * call to uclamp_rq_dec_id().
> + *
> + * Need to be careful of the following enqeueue/dequeue order
> + * problem too
> + *
> + * enqueue(taskA)
> + * // sched_uclamp_used gets enabled
> + * enqueue(taskB)
> + * dequeue(taskA)
> + * // bucket->tasks is now 0
> + * dequeue(taskB)
> + *
> + * where we could end up with uc_se->active of the task set to true and
> + * the wrong bucket[uc_se->bucket_id].value.
> + *
> + * Hence always make sure we reset things properly.
> + */
> if (likely(bucket->tasks))
> bucket->tasks--;
> +
> uc_se->active = false;
Better than v4, what about just using this active flag?
---8<---
diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index 8f360326861e..465a7645713b 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -990,6 +990,13 @@ static inline void uclamp_rq_dec_id(struct rq *rq, struct
task_struct *p,
lockdep_assert_held(>lock);
+ /*
+* If a task was already enqueue at uclamp enable time
+* nothing has been accounted for it.
+*/
+ if (unlikely(!uc_se->active))
+ return;
+
bucket = _rq->bucket[uc_se->bucket_id];
SCHED_WARN_ON(!bucket->tasks);
if (likely(bucket->tasks))
---8<---
This will allow also to keep in all the ref count checks we have,
e.g. the SChed_WARN_ON().
> /*
> @@ -1032,6 +1073,13 @@ static inline void uclamp_rq_inc(struct rq *rq, struct
> task_struct *p)
> {
> enum uclamp_id clamp_id;
>
> + /*
> + * Avoid any overhead until uclamp is actually used by the userspace.
> + * Including the branch if we use static_branch_likely()
I still find this last sentence hard to parse, but perhaps it's just me
still missing a breakfast :)
> + */
> + if (!static_branch_unlikely(_uclamp_used))
> + return;
I'm also still wondering if the optimization is still working when we
have that ! in front.
Had a check at:
https://elixir.bootlin.com/linux/latest/source/include/linux/jump_label.h#L399
and AFAIU, it all boils down to cook a __branch_check()'s compiler hint,
and ISTR that those are "anti-patterns"?
That said we do have some usages for this pattern too:
$ git grep '!static_branch_unlikely' | wc -l 36
$ git grep 'static_branch_unlikely' | wc -l 220
?
> +
> if (unlikely(!p->sched_class->uclamp_enabled))
> return;
>
[...]
> +/**
> + * uclamp_rq_util_with - clamp @util with @rq and @p effective uclamp values.
> + * @rq: The rq to clamp against. Must not be NULL.
> + * @util:The util value to clamp.
> + * @p: The task to clamp against. Can be NULL if you want to
> clamp
> + *
