Re: [PATCH 2/4] xen: credit2: allow to set and get utilization cap

[Dario Faggioli <dario.faggioli@citrix.com>]

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

On Wed, 2017-06-28 at 16:19 +0100, George Dunlap wrote:
> On Thu, Jun 8, 2017 at 1:08 PM, Dario Faggioli
> <dario.faggioli@citrix.com> wrote:
> > 
> > +            /*
> > +             * We are disabling the cap for this domain, which may
> > have vCPUs
> > +             * waiting for a replenishment, and we need to unpark
> > them all.
> > +             * Parked vcpus sit in the domain's parked_vcpus list,
> > which would
> > +             * require being manipulated with the budget_lock
> > held. However,
> > +             * we have already disabled budget accounting for all
> > the vCPUs of
> > +             * this domain in the loop above, and therefore, no
> > vCPU will run
> > +             * out of budget and need being added to the list.
> > +             *
> > +             * For this reason, it is safe, in this case, to just
> > go ahead and
> > +             * drain the list, without the need of taking the
> > budget_lock.
> > +             */
> > +            unpark_parked_vcpus(ops, &sdom->parked_vcpus);
> 
> I think it is safe currently.  But is there any reason not to just
> grab the lock anyway?  We don't expect cap adjustment actions to be
> that common, and it would mean less chance of error in the future.
> 
The code for doing that properly is a little more involved. That's
because unpark_parked_vcpus() takes runqueue locks, and hence can't be
called with the budget lock held anyway.

We'd therefore need to do the "trick" of the temporary list, as we do
already in repl_sdom_budget() and csched2_context_saved().

That's why I did not do that here... but, after all, if I actually do
things that way, I can kill the comment, so probably the number of
changed lines for this patch would be the same (if not smaller! :-D).

I'll think about it (with a bias toward taking the lock, as you
suggest).

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

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

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