Re: [PATCH 1 of 5] libxl: validate scheduler parameters

[Dario Faggioli <raistlin@linux.it>]

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

On Fri, 2012-06-22 at 12:23 +0100, Ian Campbell wrote:
> > sched_params_valid is moved and gains a gc+domid parameters and
> > s/ctx/CTX/. The call is placed after
> > libxl__domain_build_info_setdefault in the create path, because
> > set_defaults doesn't have access to the domid and there are other
> > callers which don't even have a domid to give it.
> > 
> > Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
> 
> After consultation with Ian J and Dario I have committed this one in
> order to get a test pass ASAP. I'll leave the remainder of this series
> to get properly reviewed.
> 
This looked good at inspection, and it indeed works for the credit
scheduler. Unfortunately, sedf seems to require the domain's vcpus to be
properly setup (in the hypervisor), which is not true at this point (it
is still too early!).

In fact, what happens is it fails right here:

> > diff -r 5fb3c536b5a8 -r 998d48ccb890 tools/libxl/libxl_create.c
> > --- a/tools/libxl/libxl_create.c	Fri Jun 22 10:14:46 2012 +0100
> > +++ b/tools/libxl/libxl_create.c	Fri Jun 22 11:41:43 2012 +0100
> > @@ -72,6 +72,49 @@ int libxl__domain_create_info_setdefault
> >      return 0;
> >  }
> >  
> > +static int sched_params_valid(libxl__gc *gc,
> > +                              uint32_t domid, libxl_domain_sched_params *scp)
> > +{
> > +    int has_weight = scp->weight != LIBXL_DOMAIN_SCHED_PARAM_WEIGHT_DEFAULT;
> > +    int has_period = scp->period != LIBXL_DOMAIN_SCHED_PARAM_PERIOD_DEFAULT;
> > +    int has_slice = scp->slice != LIBXL_DOMAIN_SCHED_PARAM_SLICE_DEFAULT;
> > +    int has_extratime =
> > +                scp->extratime != LIBXL_DOMAIN_SCHED_PARAM_EXTRATIME_DEFAULT;
> > +    libxl_domain_sched_params sci;
> > +
> > +    libxl_domain_sched_params_get(CTX, domid, &sci);
> > +
With a splat like this:
(XEN) ----[ Xen-4.2-unstable  x86_64  debug=y  Not tainted ]----
(XEN) CPU:    0
(XEN) RIP:    e008:[<ffff82c480120d57>] sedf_adjust+0x8bd/0x9ad
(XEN) RFLAGS: 0000000000010046   CONTEXT: hypervisor
(XEN) rax: 0000000000000000   rbx: 0000000000000000   rcx: 0000000000000001
(XEN) rdx: ffff82c4802b7e48   rsi: ffff83023e401000   rdi: ffff83031b3e34e4

Because of this code in xen/common/sched_sedf.c:

        if ( p->vcpu[0] == NULL )
        {
            rc = -EINVAL;
            goto out;
        }

What I expect to happen, then, is the win7 HVM tests to restart being
successful, but the sedf-s ones restart failing. :-(

New patch against this patch coming. This time I tested it on both sedf
and credit and tested rebooting a domain as well... Let's hope this is
going to be the end of this!

Thanks and Regards,
Dario

-- 
<<This happens because I choose it to happen!>> (Raistlin Majere)
-----------------------------------------------------------------
Dario Faggioli, Ph.D, http://retis.sssup.it/people/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: 198 bytes --]

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

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