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

[George Dunlap <george.dunlap@citrix.com>]

On Wed, Jun 28, 2017 at 3:56 PM, Dario Faggioli
<dario.faggioli@citrix.com> wrote:
> 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@citrix.com> 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.

Ah, right -- I did notice that next_repl was being moved forward each
iteration too, but didn't connect that it would mean you'd end up
going for 2T without calling repl_sdom_budget() again.

So I'm convinced that this behavior won't break the credit system, but
I'm not sure why it's an advantage over just having the domain "sit
out" this time period.

Did you actually see this happen in practice on a regular basis during
your tests, or is this mostly a hypothetical situation we're talking
about here?

>> 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

Right, I think that's a natural interpretation given the difference in
understanding. :-)

Peace,
 -George

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