Re: [PATCH v6 for Xen 4.7 1/4] xen: enable per-VCPU parameter settings for RTDS scheduler

[Chong Li <lichong659@gmail.com>]

On Wed, Mar 16, 2016 at 9:37 AM, Meng Xu <mengxu@cis.upenn.edu> wrote:
> On Wed, Mar 16, 2016 at 4:23 AM, Dario Faggioli
> <dario.faggioli@citrix.com> wrote:
>> On Tue, 2016-03-15 at 23:43 -0400, Meng Xu wrote:
>>> On Tue, Mar 15, 2016 at 11:32 PM, Chong Li <lichong659@gmail.com>
>>> wrote:
>>> > > > How about:
>>> > > >
>>> > > > We create a global variable in sched_rt.c:
>>> > > >     /* This variable holds its value through hyerpcall re-
>>> > > > issueing.
>>> > > >      * When finding vcpu settings with too low budget or period
>>> > > > (e.g,
>>> > > > 100 us), we print a warning
>>> > > >      * and set this variable "true". No more warnings are
>>> > > > printed
>>> > > > until this variable
>>> > > >      * becomes false.
>>> > > >      */
>>> > > >     static bool warned;
>>> > > > Initialize it as "false" in rt_init().
>>> > > > In your example,
>>> > > > we "warned = true" when we find the first vcpu has budget less
>>> > > > than
>>> > > > 100 us. Outside
>>> > > > of the while loop, we do:
>>> > > >     if ( index == op->u.v.nr_vcpus ) /* no more hypercall re-
>>> > > > issueing */
>>> > > >         warned = false;
>>> > > >
>>> > >
>>> > >
>>> > If we define
>>> >
>>> >    static bool warned;
>>> >
>>> > at the beginning of rt_dom_cntl(), do we need to initialize it? If
>>> > without
>>> > initialization, I think its default value is "false", which is just
>>> > what we need.
>>> >
>>> We need initializing any variable we are going to use, of course. We
>>> should not reply on the compiler to give an initialized value. :-)
>>>
>> We need to initialize any variable that would be used uninitialized, if
>> we don't initialize it. :-)
>>
>> However, something along the line of a static variable was also what I
>> was thinking to, but I don't think it works sufficiently well for
>> justifying it being introduced. And making things work well is proving
>> to be too hard to keep bothering.
>>
>> Reasons why I'm saying I don't think it works well are that: (a) there
>> may be more than one CPU executing this hypercall, and they'd race on
>> the value of the static flag; (b) what if the hypercall finishes
>> processing the first lot of 64 vCPUs with the flag set to false, are we
>> sure it can't be anything than "still false", when the new hypercal,
>> for the next lot of vCPUs of the same domain, is re-issued?
>>
>> I continue to think that it could be useful to have this logged, but
>> I'm leaning toward just killing it for now (and maybe finding another
>> way to check and warn about the same thing or one of the effects it
>> produces, later).
>>
>> Meng, what do you think?
>
> I'm thinking about if it may not be worthwhile *for now only* to
> provide such information with so much effort and the danger of
> introducing more serious issues.
>
> Right, race condition occurs on the global variable and I believe we
> don't want to encounter this race condition.
> So let's just not use the global variable.
>
> We should definitely put a large warning in the wiki for the RTDS
> scheduler about the parameter settings. Incorrect setting should never
> crash system but may lead to poor real-time performance users want.
>
> Once this patch is done, I will modify the wiki then. (Chong, could
> you remind me if I happen to forget?)
>
Sure, I'll.

Chong



-- 
Chong Li
Department of Computer Science and Engineering
Washington University in St.louis

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