Re: [Xen-devel] [PATCH 1/2] xen: credit2: avoid vCPUs to ever reach lower credits than idle

From: Dario Faggioli <dfaggioli@suse.com>
To: Jan Beulich <jbeulich@suse.com>
Cc: Juergen Gross <jgross@suse.com>,
	Charles Arnold <carnold@suse.com>, Glen <glenbarney@gmail.com>,
	George Dunlap <george.dunlap@citrix.com>,
	Tomas Mozes <hydrapolic@gmail.com>, Sarah Newman <srn@prgmr.com>,
	xen-devel@lists.xenproject.org
Subject: Re: [Xen-devel] [PATCH 1/2] xen: credit2: avoid vCPUs to ever reach lower credits than idle
Date: Thu, 12 Mar 2020 17:11:35 +0100	[thread overview]
Message-ID: <94b27cbc655806ea2853e5f3c099d2a4a93ee5ce.camel@suse.com> (raw)
In-Reply-To: <2e53ee4c-e2d9-f892-c8c3-62bff0c526ab@suse.com>

[-- Attachment #1.1: Type: text/plain, Size: 1556 bytes --]

On Thu, 2020-03-12 at 16:26 +0100, Jan Beulich wrote:
> On 12.03.2020 14:44, Dario Faggioli wrote:
> > --- a/xen/common/sched/credit2.c
> > +++ b/xen/common/sched/credit2.c
> > @@ -234,7 +234,7 @@
> >   * units does not consume credits, and it must be lower than
> > whatever
> >   * amount of credit 'regular' unit would end up with.
> >   */
> > -#define CSCHED2_IDLE_CREDIT          (-(1U<<30))
> > +#define CSCHED2_IDLE_CREDIT          INT_MIN
> 
> The title saying "lower than", is "equal" actually fine? Looking
> at e.g. runq_insert() I'm getting the impression it's not.
> 
In Credit2, we don't have the idle vCPUs in the runqueue. So we will
never compare non-idle with idle while inserting.

But this is a good point, in general, and I think we may need to turn
the ">"
in a ">=" in runq_candidate().

> Looking at t2c_update() I'm also getting the impression that
> there's UB when the subtraction underflows. After all, if
> -1 << 30 wasn't small enough a value, I don't see why -1 << 31
> would be.
> 
Mmm... not sure I am getting. Are you suggesting we should apply a cap
to val?

If yes, this looks like an issue independent from what the value of
CSCHED2_IDLE_CREDIT is, but yeah, we can do that. Or am I missing
something?

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 #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[-- Attachment #2: Type: text/plain, Size: 157 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel