[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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



On Wed, Sep 13, 2017 at 8:51 PM, Dario Faggioli
<dario.faggioli@xxxxxxxxxx> wrote:
>
> On Fri, 2017-09-01 at 11:58 -0400, Meng Xu wrote:
> > diff --git a/tools/xl/xl_cmdtable.c b/tools/xl/xl_cmdtable.c
> > index ba0159d..1b03d44 100644
> > --- a/tools/xl/xl_cmdtable.c
> > +++ b/tools/xl/xl_cmdtable.c
> > @@ -272,12 +272,13 @@ struct cmd_spec cmd_table[] = {
> >      { "sched-rtds",
> >        &main_sched_rtds, 0, 1,
> >        "Get/set rtds scheduler parameters",
> > -      "[-d <Domain> [-v[=VCPUID/all]] [-p[=PERIOD]] [-b[=BUDGET]]]",
> > +      "[-d <Domain> [-v[=VCPUID/all]] [-p[=PERIOD]] [-b[=BUDGET]] [-
> > e[=EXTRATIME]]]",
> >        "-d DOMAIN, --domain=DOMAIN     Domain to modify\n"
> >        "-v VCPUID/all, --vcpuid=VCPUID/all    VCPU to modify or
> > output;\n"
> >        "               Using '-v all' to modify/output all vcpus\n"
> >        "-p PERIOD, --period=PERIOD     Period (us)\n"
> >        "-b BUDGET, --budget=BUDGET     Budget (us)\n"
> > +      "-e EXTRATIME, --extratime=EXTRATIME EXTRATIME (1=yes, 0=no)\n"
>                                               Extratime
> ?

We need to provide the option to configure the extratime flag for each
vcpu, right?

>
> >      },
> >      { "domid",
> >        &main_domid, 0, 0,
> > diff --git a/tools/xl/xl_sched.c b/tools/xl/xl_sched.c
> > index 85722fe..5138012 100644
> > --- a/tools/xl/xl_sched.c
> > +++ b/tools/xl/xl_sched.c
> > @@ -251,7 +251,7 @@ static int sched_rtds_domain_output(
> >      libxl_domain_sched_params scinfo;
> >
> >      if (domid < 0) {
> > -        printf("%-33s %4s %9s %9s\n", "Name", "ID", "Period",
> > "Budget");
> > +        printf("%-33s %4s %9s %9s %10s\n", "Name", "ID", "Period",
> > "Budget", "Extra time");
> >          return 0;
> >      }
> >
> Can you paste the output of:
>

Sure

> xl sched-rtds

Cpupool Pool-0: sched=RTDS
Name                                ID    Period    Budget Extra time
Domain-0                             0     10000      4000        yes

> xl sched-rtds -d 0

Name                                ID    Period    Budget Extra time
Domain-0                             0     10000      4000        yes

> xl sched-rtds -d 0 -v 1

Name                                ID VCPU    Period    Budget Extra time
Domain-0                             0    1     10000      4000        yes


> xl sched-rtds -d 0 -v all

Name                                ID VCPU    Period    Budget Extra time
Domain-0                             0    0     10000      4000        yes
Domain-0                             0    1     10000      4000        yes
Domain-0                             0    2     10000      4000        yes
Domain-0                             0    3     10000      4000        yes
Domain-0                             0    4     10000      4000        yes
Domain-0                             0    5     10000      4000        yes
Domain-0                             0    6     10000      4000        yes
Domain-0                             0    7     10000      4000        yes
Domain-0                             0    8     10000      4000        yes
Domain-0                             0    9     10000      4000        yes
Domain-0                             0   10     10000      4000        yes
Domain-0                             0   11     10000      4000        yes

>
> with the series applied?
>
> > @@ -785,8 +801,9 @@ int main_sched_rtds(int argc, char **argv)
> >          goto out;
> >      }
> >      if (((v_index > b_index) && opt_b) || ((v_index > p_index) &&
> > opt_p)
> > -        || p_index != b_index) {
> > -        fprintf(stderr, "Incorrect number of period and budget\n");
> > +         || ((v_index > e_index) && opt_e) || p_index != b_index
> > +         || p_index != e_index || b_index != e_index ) {
> >
> I don't think you need the `b_indes ! e_index` part. If p==b and p==e,
> it's automatically true that b==e.

Right.

>
> > @@ -820,7 +837,7 @@ int main_sched_rtds(int argc, char **argv)
> >                  r = EXIT_FAILURE;
> >                  goto out;
> >              }
> > -        } else if (!opt_p && !opt_b) {
> > +        } else if (!opt_p && !opt_b && !opt_e) {
> >              /* get per-vcpu rtds scheduling parameters */
> >              libxl_vcpu_sched_params scinfo;
> >              libxl_vcpu_sched_params_init(&scinfo);
> > @@ -852,6 +869,7 @@ int main_sched_rtds(int argc, char **argv)
> >                      scinfo.vcpus[i].vcpuid = vcpus[i];
> >                      scinfo.vcpus[i].period = periods[i];
> >                      scinfo.vcpus[i].budget = budgets[i];
> > +                    scinfo.vcpus[i].extratime = extratimes[i] ? 1 :
> > 0;
> >                  }
> >                  rc = sched_vcpu_set(domid, &scinfo);
> >              } else { /* set params for all vcpus */
> > @@ -860,6 +878,7 @@ int main_sched_rtds(int argc, char **argv)
> >                                 xmalloc(sizeof(libxl_sched_params));
> >                  scinfo.vcpus[0].period = periods[0];
> >                  scinfo.vcpus[0].budget = budgets[0];
> > +                scinfo.vcpus[0].extratime = extratimes[0] ? 1 : 0;
> >
> But does these two hunks mean that if I pass `-e 10`, that is
> considered a legal way to enable extratime? Shouldn't we enforce
> (either here in xl or in libxl) the value to be 0 or 1 ?

Yes, we should enforce the extratime to 0 or 1. How about checking the
value of extratime when we parse each extratime value?
The change of the code will be like the following in xl_sched.c:

757     case 'e':
758         if (e_index >= e_size) { /* extratime array is full */
759             e_size *= 2;
760             extratimes = xrealloc(extratimes, e_size);
761         }
762         extratimes[e_index++] = strtol(optarg, NULL, 10);
763         if ( extratimes[e_index-1] != 0 && extratimes[e_index-1] != 1)
764         {
765             fprintf(stderr, "Invalid extratime.\n");
766             r = EXIT_FAILURE;
767             goto out;
768         }
769         opt_e = 1;
770         break;

What do you think?

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

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.