Re: [PATCH v2 2/2] xen: credit2: fix credit reset happening too few times

[Dario Faggioli <dfaggioli@xxxxxxxx>]

On Thu, 2020-04-02 at 18:32 +0000, George Dunlap wrote:
> > On Mar 19, 2020, at 12:12 AM, Dario Faggioli <dfaggioli@xxxxxxxx>
> > wrote:
> > 
> > There is a bug in commit 5e4b4199667b9 ("xen: credit2: only reset
> > credit on reset condition"). In fact, the aim of that commit was to
> > make sure that we do not perform too many credit reset operations
> > (which are not super cheap, and in an hot-path). But the check used
> > to determine whether a reset is necessary was the wrong one.
> > 
> > In fact, knowing just that some vCPUs have been skipped, while
> > traversing the runqueue (in runq_candidate()), is not enough. We
> > need to check explicitly whether the first vCPU in the runqueue
> > has a negative amount of credit.
> 
> Oh, so if the top of the runqueue has negative credit, but it’s not
> chosen, then the one we *do* run has even lower credit.  Still not
> quite sure how that leads to a situation where credit resets don’t
> happen for long periods of time.  But anyway...
> 
Fact is, without this patch, we wouldn't call reset_credit() if we
"skipped" a vcpu/unit.

That means we skip all the times we did not pick the head of the
runqueue, even if the one we picked also have negative credits (as the
check for 'skipped_units == 0' was the first condition of the '&&').

That's what was making us skip resets. :-)

> Acked-by: George Dunlap <george.dunlap@xxxxxxxxxx>
> 
Thanks and Regards
-- 
Dario Faggioli, Ph.D
http://about.me/dario.faggioli
Virtualization Software Engineer
SUSE Labs, SUSE https://www.suse.com/
-------------------------------------------------------------------
<<This happens because _I_ choose it to happen!>> (Raistlin Majere)

Attachment: signature.asc
Description: This is a digitally signed message part