Re: [PATCH v2 3/3] sched/fair: Simplify some logic in update_sd_pick_busiest()

[David Vernet <void@manifault.com>]

[-- Attachment #1: Type: text/plain, Size: 1384 bytes --]

On Sun, Feb 04, 2024 at 12:48:11PM +0100, Vincent Guittot wrote:
> On Sun, 4 Feb 2024 at 05:46, David Vernet <void@manifault.com> wrote:
> >
> > When comparing the current struct sched_group with the yet-busiest
> > domain in update_sd_pick_busiest(), if the two groups have the same
> > group type, we're currently doing a bit of unnecessary work for any
> > group >= group_misfit_task. We're comparing the two groups, and then
> > returning only if false (the group in question is not the busiest).
> > Othewise, we break, do an extra unnecessary conditional check that's
> > vacuously false for any group type > group_fully_busy, and then always
> > return true.
> >
> > Let's just return directly in the switch statement instead. This doesn't
> > change the size of vmlinux with llvm 17 (not surprising given that all
> > of this is inlined in load_balance()), but it does shrink load_balance()
> > by 88 bytes on x86. Given that it also improves readability, this seems
> > worth doing.
> >
> > As a bonus, remove an unnecessary goto in update_sd_lb_stats().
> 
> The line above is not relevant to the content of the patch.

Ah, thanks for catching that.

Should I send a v3 of the patch set? Or should I just let whomever
applies remove that line?

> Other than that
> 
> Reviewed-by: Vincent Guittot <vincent.guittot@linaro.org>

Thanks,
David

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]