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

[Meng Xu <xumengpanda@xxxxxxxxx>]

On Thu, Aug 3, 2017 at 12:03 PM, Dario Faggioli
<dario.faggioli@xxxxxxxxxx> wrote:
> 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

Ah, my bad..

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

Yes, bool is enough.
Is "workconserving" too long here?

I thought about alternative names, such as "wc", "workc", and
"extratime". None of them is good enough. The ideal one should be much
shorter and easy to link to "work conserving". :(
If we use "extratime", it may cause confusion with the "extratime" in
the depreciated SEDF. (That is my concern of reusing the EXTRATIME in
the libxl_type.idl.)

Maybe "workc" is better than "workconserving"?

Thanks,

Meng

-----------
Meng Xu
PhD 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