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

[Juergen Gross <jgross@xxxxxxxx>]

On 16/11/17 22:10, Anshul Makkar wrote:
> [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@xxxxxxxxx>
>>>
>>>   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.

So either have some glue code translating the LIBXL defines to the
hypervisor ones, or add some statements triggering a build failure in
case they don't match.


Juergen


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