Re: [PATCH v2] Xen: credit2: avoid picking a spurious idle unit when caps are used

[Jan Beulich <jbeulich@xxxxxxxx>]

On 04.08.2021 19:55, Dario Faggioli wrote:
> Commit 07b0eb5d0ef0 ("credit2: make sure we pick a runnable unit from the
> runq if there is one") did not fix completely the problem of potentially
> selecting a scheduling unit that will then not be able to run.
> 
> In fact, in case caps are used and the unit we are currently looking
> at, during the runqueue scan, does not have enough budget for being run,
> we should continue looking instead than giving up and picking the idle
> unit.
> 
> Suggested-by: George Dunlap <george.dunlap@xxxxxxxxxx>
> Signed-off-by: Dario Faggioli <dfaggioli@xxxxxxxx>

In part based on your explanation in response to my v1 comments:
Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>
with one nit:

> @@ -3494,16 +3503,25 @@ runq_candidate(struct csched2_runqueue_data *rqd,
>          }
>  
>          /*
> -         * If the one in the runqueue has more credit than current (or idle,
> -         * if current is not runnable), or if current is yielding, and also
> -         * if the one in runqueue either is not capped, or is capped but has
> -         * some budget, then choose it.
> +         * If we are here, we are almost sure we want to pick the unit in
> +         * the runqueue. Last thing we need to check is that it either is
> +         * not capped or, if it is, it has some budget.
> +         *
> +         * Note that budget availability must be the very last check that
> +         * we, in this loop, due to the side effects that unit_grab_budget().
> +         * causes.

The sentence looks broken to me: I think you either want to delete "that
we," or add "do" before the 1st or after the 2nd comma. If you agree, or
if you have another suggestion, I'd be happy to apply the adjustment
(let me know of your preference, if any) while committing.

Jan