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

[Meng Xu <xumengpanda@xxxxxxxxx>]

On Tue, Sep 19, 2017 at 5:23 AM, Dario Faggioli
<dario.faggioli@xxxxxxxxxx> wrote:
>
> On Fri, 2017-09-15 at 12:01 -0400, Meng Xu wrote:
> > On Wed, Sep 13, 2017 at 8:16 PM, Dario Faggioli
> > <dario.faggioli@xxxxxxxxxx> wrote:
> > >
> > > > 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)
> >
> Mmm... Ok, I see it now. Well, this is of course all up to the tools'
> maintainers.
>
> What I think it would be valauble to ask ourself here is, can, at this
> point, scinfo->extratime be equal to
> XL_DOMAIN_SCHED_PARAM_EXTRATIME_DEFAULT?
>
> And if it is, what does it mean, and what do we want to do?
>
> I mean, if extratime is -1, it means that we've been called, without it
> being touched by xl (although, remember that, as a library, libxl can
> be linked to and called by other programs too, e.g., libvirt).
>
> If you think that this is a serious programming bug, you can use
> XL_DOMAIN_SCHED_PARAM_EXTRATIME_DEFAULT to check that, and raise an
> assert.
>
> If you think it's an API misuse, you can use it to check for that, and
> return an error.
>
> If you think it's just fine, you can do whatever you want to do as
> default (which, AFAIUI, it's set the flag). In this case, it's probably
> fine to ignore XL_DOMAIN_SCHED_PARAM_EXTRATIME_DEFAULT in actual code.
> Although, I'd still put a reference to it in a comment, to explain
> what's going on, and why we're doing things differently from budget and
> period (since _their_ *_DEFAULT are checked).


I think it should be fine for API to call the function without setting
extratime parameter. We set the extratime by default.

I will go with the following code for the next version.
>         if (scinfo->extratime)
>             sdom.flags |= XEN_DOMCTL_SCHEDRT_extra;
>         else
>             sdom.flags &= ~XEN_DOMCTL_SCHEDRT_extra;
>

Thank you very much!

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