Re: [Xen-devel] [PATCH RFC v1 3/3] xl: enable per-VCPU work conserving flag for RTDS

[Dario Faggioli <dario.faggioli@xxxxxxxxxx>]

On Tue, 2017-08-01 at 14:33 -0400, Meng Xu wrote:
> --- 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]]]
> [-w[=WORKCONSERVING]]",
>        "-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"
> +      "-w WORKCONSERVING, --
> workconserving=WORKCONSERVING    WORKCONSERVING (1=yes,0=no)\n"
>
Does this really need to accept a 1 or 0 parameter? Can't it be that,
if -w is provided, the vCPU is marked as work-conserving, if it's not,
it's considered reservation only.

> --- a/tools/xl/xl_sched.c
> +++ b/tools/xl/xl_sched.c
> 
> @@ -279,8 +280,8 @@ static int sched_rtds_vcpu_output(int domid,
> libxl_vcpu_sched_params *scinfo)
>      int i;
>  
>      if (domid < 0) {
> -        printf("%-33s %4s %4s %9s %9s\n", "Name", "ID",
> -               "VCPU", "Period", "Budget");
> +        printf("%-33s %4s %4s %9s %9s %15s\n", "Name", "ID",
> +               "VCPU", "Period", "Budget", "Work conserving");
>          return 0;
>      }
>  
> @@ -290,12 +291,13 @@ static int sched_rtds_vcpu_output(int domid,
> libxl_vcpu_sched_params *scinfo)
>  
>      domname = libxl_domid_to_name(ctx, domid);
>      for ( i = 0; i < scinfo->num_vcpus; i++ ) {
> -        printf("%-33s %4d %4d %9"PRIu32" %9"PRIu32"\n",
> +        printf("%-33s %4d %4d %9"PRIu32" %9"PRIu32" %15d\n",
>
As far as printing it goes, OTOH, I would indeed print a string, i.e.,
"yes", if the field is found to be 1 (true), or "no", if the field is
found to be 0 (false).

> @@ -702,14 +705,18 @@ int main_sched_rtds(int argc, char **argv)
>      int *vcpus = (int *)xmalloc(sizeof(int)); /* IDs of VCPUs that
> change */
>      int *periods = (int *)xmalloc(sizeof(int)); /* period is in
> microsecond */
>      int *budgets = (int *)xmalloc(sizeof(int)); /* budget is in
> microsecond */
> +    int *workconservings = (int *)xmalloc(sizeof(int)); /* budget is
> in microsecond */
>
Yeah, budget is in microseconds. But this is not budget! :-P

In fact (jokes apart), it can be just a bool, can't it?

Regadrs,
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