Re: [Xen-devel] [PATCH v3 for Xen 4.6 3/4] libxl: enable per-VCPU parameter settings for RTDS scheduler

[Chong Li <lichong659@xxxxxxxxx>]

On Tue, Jun 30, 2015 at 7:26 AM, Ian Campbell <ian.campbell@xxxxxxxxxx> wrote:
> On Sun, 2015-06-28 at 21:44 -0500, Chong Li wrote:
>
>> +    GC_INIT(ctx);
>> +    libxl_scheduler sched = scinfo->sched;
>> +    int ret;
>> +
>> +    if (sched == LIBXL_SCHEDULER_UNKNOWN)
>> +        sched = libxl__domain_scheduler(gc, domid);
>> +
>> +    switch (sched) {
>> +    case LIBXL_SCHEDULER_RTDS:
>> +        ret = sched_rtds_vcpu_set(gc, domid, scinfo);
>> +        break;
>> +    default:
>> +        LOG(ERROR, "Unknown scheduler");
>
> There are several other known schedulers, I don't think the fact that
> they don't have any need for per-vcpu variables should result in this
> error message, they should get something more specific to that
> situation. Only truly unknown values for the scheduler should result in
> this message.

Right, I'll change it.
>
> [...]
>>  /*
>> + * libxl_vcpu_sched_params is used to get/set per-vcpu params
>> +*/
>> +#define LIBXL_HAVE_VCPU_SCHED_PARAMS 1
>> +
>> +/*
>> + * libxl_sched_params is used to store per-vcpu params
>> +*/
>> +#define LIBXL_SCHED_PARAMS 1
>
> Do we need both of these? In particular I'm not sure what the second one
> is indicating has changed.

Sorry for the vague description there, and there is an error there.
The right way should be:

/* libxl_sched_params is introduced to store per-vcpu params. */
#define LIBXL_HAVE_SCHED_PARAMS

/* libxl_vcpu_sched_params is introduced to store the array of
libxl_sched_params (per-vcpu params). */
#define LIBXL_HAVE_VCPU_SCHED_PARAMS

Actually, now I'm not sure if both them should be there, because I've
not found "LIBXL_HAVE_DOMAIN_SCHED_PARAMS" there.

>
>> +
>> +/*
>>   * libxl ABI compatibility
>>   *
>>   * The only guarantee which libxl makes regarding ABI compatibility
>> @@ -1554,10 +1564,17 @@ int libxl_sched_credit_params_set(libxl_ctx *ctx, 
>> uint32_t poolid,
>>  #define LIBXL_DOMAIN_SCHED_PARAM_EXTRATIME_DEFAULT -1
>>  #define LIBXL_DOMAIN_SCHED_PARAM_BUDGET_DEFAULT    -1
>>
>> +/* Per-VCPU parameters*/
>> +#define LIBXL_DOMAIN_SCHED_PARAM_VCPU_INDEX_DEFAULT -1
>
> What is the effect of passing vcpuid == -1 to this interface?

I think the right way should be:

#define LIBXL_SCHED_PARAM_VCPU_INDEX_DEFAULT -1

which is the default value for the vcpuid in libxl_sched_params. Even
though libxl_sched_params is used for per-vcpu settings, all the other
fields (except vcpuid) would still use
LIBXL_DOMAIN_SCHED_PARAM_*_DEFAULT values, just for convenience.

Chong

>
> Ian.
>



-- 
Chong Li
Department of Computer Science and Engineering
Washington University in St.louis

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