Re: [PATCH 2/3] credit2: libxl related changes to add support for runqueue per cpupool.

[Anshul Makkar <anshulmakkar@gmail.com>]

[Trimming the Cc-list a bit]


On 9/14/17 7:37 AM, Juergen Gross wrote:
> On 12/09/17 02:45, anshulmakkar wrote:
>> Introduces scheduler specific parameter at libxl level which are
>> passed on to libxc. eg runqueue for credit2
>>
>> Signed-off-by: Anshul Makkar <anshulmakkar@gmail.com>
>>
>>   int libxl_cpupool_destroy(libxl_ctx *ctx, uint32_t poolid);
>>   int libxl_cpupool_rename(libxl_ctx *ctx, const char *name, uint32_t poolid);
>>   int libxl_cpupool_cpuadd(libxl_ctx *ctx, uint32_t poolid, int cpu);
>> diff --git a/tools/libxl/libxl_cpupool.c b/tools/libxl/libxl_cpupool.c
>> index 85b0688..e3ce7b3 100644
>> --- a/tools/libxl/libxl_cpupool.c
>> +++ b/tools/libxl/libxl_cpupool.c
>> @@ -130,7 +130,7 @@ int libxl_get_freecpus(libxl_ctx *ctx, libxl_bitmap *cpumap)
>>   int libxl_cpupool_create(libxl_ctx *ctx, const char *name,
>>                            libxl_scheduler sched,
>>                            libxl_bitmap cpumap, libxl_uuid *uuid,
>> -                         uint32_t *poolid)
>> +                         uint32_t *poolid, const libxl_scheduler_params *sched_params)
>>   {
>>       GC_INIT(ctx);
>>       int rc;
>> @@ -138,6 +138,7 @@ int libxl_cpupool_create(libxl_ctx *ctx, const char *name,
>>       xs_transaction_t t;
>>       char *uuid_string;
>>       uint32_t xcpoolid;
>> +    xc_schedparam_t xc_sched_param;
>>   
>>       /* Accept '0' as 'any poolid' for backwards compatibility */
>>       if ( *poolid == LIBXL_CPUPOOL_POOLID_ANY
>> @@ -151,8 +152,18 @@ int libxl_cpupool_create(libxl_ctx *ctx, const char *name,
>>           GC_FREE;
>>           return ERROR_NOMEM;
>>       }
>> +    if (sched_params)
>> +    {
>> +        xc_sched_param.u.sched_credit2.ratelimit_us =
>> +                                                    sched_params->u.credit2.ratelimit_us;
>> +        xc_sched_param.u.sched_credit2.runq = sched_params->u.credit2.runqueue;
>> +        xc_sched_param.u.sched_credit.tslice_ms = sched_params->u.credit.tslice_ms;
>> +        xc_sched_param.u.sched_credit.ratelimit_us = sched_params->u.credit.ratelimit_us;
> Don't you need some input parameter validation here?
Agree. Will perform validation.
>> +    }
>> +    else
>> +        xc_sched_param.u.sched_credit2.runq = LIBXL_CREDIT2_RUNQUEUE_DEFAULT;
> So you are passing the LIBXL defines down to the hypervisor expecting
> they match. I think this is a major layering violation.
I need to pass the DEFAULT runq arrangement if the user has not selected
any option and I want to do it near to the top level (libxc) so that 
consistency
can be maintained at the lower scheduler layer.
Please can you suggest alternative that will maintain layering consistency.
>
>
>
> Juergen
anshul


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