Re: [Xen-devel] [PATCH v2 2/5] libxl: enable per-VCPU extratime flag for RTDS

[Meng Xu <xumengpanda@xxxxxxxxx>]

On Wed, Sep 13, 2017 at 8:16 PM, Dario Faggioli
<dario.faggioli@xxxxxxxxxx> wrote:
> On Fri, 2017-09-01 at 12:03 -0400, Meng Xu wrote:
>> On Fri, Sep 1, 2017 at 11:58 AM, Meng Xu <mengxu@xxxxxxxxxxxxx>
>> wrote:
>> > @@ -705,6 +717,12 @@ static int sched_rtds_domain_set(libxl__gc
>> > *gc, uint32_t domid,
>> >          sdom.period = scinfo->period;
>> >      if (scinfo->budget != LIBXL_DOMAIN_SCHED_PARAM_BUDGET_DEFAULT)
>> >          sdom.budget = scinfo->budget;
>> > +    if (scinfo->extratime !=
>> > LIBXL_DOMAIN_SCHED_PARAM_EXTRATIME_DEFAULT) {
>> > +        if (scinfo->extratime)
>> > +            sdom.flags |= XEN_DOMCTL_SCHEDRT_extra;
>> > +        else
>> > +            sdom.flags &= ~XEN_DOMCTL_SCHEDRT_extra;
>> > +    }
>> >      if (sched_rtds_validate_params(gc, sdom.period, sdom.budget))
>> >          return ERROR_INVAL;
>>
>>
>> As you mentioned in the comment to the xl patch v1, I used
>> LIBXL_DOMAIN_SCHED_PARAM_EXTRATIME_DEFAULT for extratime flag as what
>> we did for period and budget. But the way we handle flags is exactly
>> the same with the way we handle period and budget.
>>
> Mmm... and (since you say 'But') is that a problem?

Sorry, the sentence should be "But the way we handle flags is *not* exactly
the same with the way we handle period and budget".
I missed "not" in the previous sentence.

>
>> I'm ok with what it is in this patch, although I feel that we can
>> kill the
>>  if (scinfo->extratime != LIBXL_DOMAIN_SCHED_PARAM_EXTRATIME_DEFAULT)
>> because LIBXL_DOMAIN_SCHED_PARAM_EXTRATIME_DEFAULT is -1.
>>
> No, sorry, I don't understand what you mean here...

I was thinking about the following code:

    if (scinfo->extratime != LIBXL_DOMAIN_SCHED_PARAM_EXTRATIME_DEFAULT) {
        if (scinfo->extratime)
            sdom.flags |= XEN_DOMCTL_SCHEDRT_extra;
        else
            sdom.flags &= ~XEN_DOMCTL_SCHEDRT_extra;
    }

This code can be changed to
        if (scinfo->extratime)
            sdom.flags |= XEN_DOMCTL_SCHEDRT_extra;
        else
            sdom.flags &= ~XEN_DOMCTL_SCHEDRT_extra;

If the extratime uses default value (-1), we still set the extratime flag.

That's why I feel we may kill the
 if (scinfo->extratime != LIBXL_DOMAIN_SCHED_PARAM_EXTRATIME_DEFAULT)

Please correct me if I'm wrong.

For the next version, I plan to keep what it is right now. That is:
    if (scinfo->extratime != LIBXL_DOMAIN_SCHED_PARAM_EXTRATIME_DEFAULT) {
        if (scinfo->extratime)
            sdom.flags |= XEN_DOMCTL_SCHEDRT_extra;
        else
            sdom.flags &= ~XEN_DOMCTL_SCHEDRT_extra;
    }


Best,

Meng

-- 
Meng Xu
Ph.D. Candidate in Computer and Information Science
University of Pennsylvania
http://www.cis.upenn.edu/~mengxu/

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