Re: [Xen-devel] [PATCH 1/4] xen: credit2: implement utilization cap

[Dario Faggioli <dario.faggioli@xxxxxxxxxx>]

On Wed, 2017-06-28 at 15:28 +0100, George Dunlap wrote:
> On Fri, Jun 23, 2017 at 5:19 PM, Dario Faggioli
> <dario.faggioli@xxxxxxxxxx> wrote:
> > > > +{
> > > > +    struct csched2_dom *sdom = data;
> > > > +    unsigned long flags;
> > > > +    s_time_t now;
> > > > +    LIST_HEAD(parked);
> > > > +
> > > > +    spin_lock_irqsave(&sdom->budget_lock, flags);
> > > > +
> > > > +    /*
> > > > +     * It is possible that the domain overrun, and that the
> > > > budget
> > > > hence went
> > > > +     * below 0 (reasons may be system overbooking, issues in
> > > > or
> > > > too coarse
> > > > +     * runtime accounting, etc.). In particular, if we overrun
> > > > by
> > > > more than
> > > > +     * tot_budget, then budget+tot_budget would still be < 0,
> > > > which in turn
> > > > +     * means that, despite replenishment, there's still no
> > > > budget
> > > > for unarking
> > > > +     * and running vCPUs.
> > > > +     *
> > > > +     * It is also possible that we are handling the
> > > > replenishment
> > > > much later
> > > > +     * than expected (reasons may again be overbooking, or
> > > > issues
> > > > with timers).
> > > > +     * If we are more than CSCHED2_BDGT_REPL_PERIOD late, this
> > > > means we have
> > > > +     * basically skipped (at least) one replenishment.
> > > > +     *
> > > > +     * We deal with both the issues here, by, basically, doing
> > > > more than just
> > > > +     * one replenishment. Note, however, that every time we
> > > > add
> > > > tot_budget
> > > > +     * to the budget, we also move next_repl away by
> > > > CSCHED2_BDGT_REPL_PERIOD.
> > > > +     * This guarantees we always respect the cap.
> > > > +     */
> > > > +    now = NOW();
> > > > +    do
> > > > +    {
> > > > +        sdom->next_repl += CSCHED2_BDGT_REPL_PERIOD;
> > > > +        sdom->budget += sdom->tot_budget;
> > > > +    }
> > > > +    while ( sdom->next_repl <= now || sdom->budget <= 0 );
>
> > In presence of an accounting/enforcing error, it may happen that it
> > executes for C between 0 and T, for 2C between T and 2T, for 0
> > between
> > 2T and 3T, etc. So, after 3T, it will also have executed for 3C, as
> > above.
> 
> Right, but is that what the loop actually does?
> 
It should be. Well, actually, it does not really do that, but it does
something that I think is equivalent.

I probably should have described it better... Sorry!

> It looks like now if it executes for 2C between T and 2T, then when
> the replenishment happens at 2T, the budget will be -C.  So the first
> round of the loop will bring the budget to 0; since budget <= 0, then
> it will add *another* C to the budget.  So then it will be allowed to
> execute for C again between 2T and 3T, giving it a total of 4C
> executed over 3T, in violation of the cap.
> 
> Am I reading that wrong?
> 
So, the loop body indeed adds C, but it also moves the next
replenishment time ahead by T.

In the case you describe, at 2T, with budget -C, the first round of the
loop will make the budget 0, and set the next replenishment to 3T. As
you say, since budget is 0, and 0 is <= than 0, we stay in the loop for
another round, which sets the budget to C, and the next replenishment
to 4T.

So, in summary, the domain have executed for 3C between 0 and 2T, and
will be given only another C, until 4T, summing up to a total of 4C/4T,
which should be fine.

Or at least, that's what I had in mind... if the code does actually do
something different, then it's a bug. ;-P

> I thought you had intentionally decided to allow that to happen, to
> avoid making the capped domain have to sit out for an entire budget
> period.
> 
Ah, so I completely misunderstood your point, when replying. I thought
*you* were arguing in favour of avoiding the capped domain have to sit
out for an entire budget period! :-D

Thanks and Regards,
Dario
-- 
<<This happens because I choose it to happen!>> (Raistlin Majere)
-----------------------------------------------------------------
Dario Faggioli, Ph.D, http://about.me/dario.faggioli
Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)

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

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel