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

[Dario Faggioli <dario.faggioli@xxxxxxxxxx>]

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

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: signature.asc
Description: This is a digitally signed message part

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