2021-11-18 14:16:17

by Vincent Donnefort

[permalink] [raw]
Subject: [PATCH v2 1/2] sched/fair: Fix asym_fits_capacity() task_util type

task_util is an unsigned long value, compared with a CPU capacity which is
unsigned long as well. There's no need for an intermediate implicit int
cast.

Fixes: b4c9c9f15649 ("sched/fair: Prefer prev cpu in asymmetric wakeup path")
Signed-off-by: Vincent Donnefort <[email protected]>
Reviewed-by: Vincent Guittot <[email protected]>

diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index 945d987246c5..8fde6e10e24b 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -6346,7 +6346,7 @@ select_idle_capacity(struct task_struct *p, struct sched_domain *sd, int target)
return best_cpu;
}

-static inline bool asym_fits_capacity(int task_util, int cpu)
+static inline bool asym_fits_capacity(unsigned long task_util, int cpu)
{
if (static_branch_unlikely(&sched_asym_cpucapacity))
return fits_capacity(task_util, capacity_of(cpu));
--
2.25.1



2021-12-06 15:49:12

by Peter Zijlstra

[permalink] [raw]
Subject: Re: [PATCH v2 1/2] sched/fair: Fix asym_fits_capacity() task_util type

On Thu, Nov 18, 2021 at 02:14:10PM +0000, Vincent Donnefort wrote:
> task_util is an unsigned long value, compared with a CPU capacity which is
> unsigned long as well. There's no need for an intermediate implicit int
> cast.
>
> Fixes: b4c9c9f15649 ("sched/fair: Prefer prev cpu in asymmetric wakeup path")

Do either of these patches actually *fix* anything? Afaict they're an
absolute no-op, even in terms of code-gen due to the promotion rules.

Yes, its arguably nicer to not rely on those implicit promotions etc..
but I don't think this warrants a Fixes tag or even being split in two
patches.

Hmm?

2021-12-07 09:48:36

by Vincent Donnefort

[permalink] [raw]
Subject: Re: [PATCH v2 1/2] sched/fair: Fix asym_fits_capacity() task_util type

On Mon, Dec 06, 2021 at 04:20:04PM +0100, Peter Zijlstra wrote:
> On Thu, Nov 18, 2021 at 02:14:10PM +0000, Vincent Donnefort wrote:
> > task_util is an unsigned long value, compared with a CPU capacity which is
> > unsigned long as well. There's no need for an intermediate implicit int
> > cast.
> >
> > Fixes: b4c9c9f15649 ("sched/fair: Prefer prev cpu in asymmetric wakeup path")
>
> Do either of these patches actually *fix* anything? Afaict they're an
> absolute no-op, even in terms of code-gen due to the promotion rules.
>
> Yes, its arguably nicer to not rely on those implicit promotions etc..
> but I don't think this warrants a Fixes tag or even being split in two
> patches.
>
> Hmm?

As far as I know it doesn't change anything functionally speaking. So yeah,
let's consider it as a cleanup. I'll merge the changes and drop the Fix tag.