| Message ID | 1505177142-14864-3-git-send-email-anshulmakkar@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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> > --- > tools/libxl/libxl.h | 2 +- > tools/libxl/libxl_cpupool.c | 15 +++++++++++++-- > tools/libxl/libxl_types.idl | 46 ++++++++++++++++++++++++++++++++++----------- > tools/xl/xl_cpupool.c | 16 ++++++++++++++-- > 4 files changed, 63 insertions(+), 16 deletions(-) > > diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h > index 91408b4..6617c64 100644 > --- a/tools/libxl/libxl.h > +++ b/tools/libxl/libxl.h > @@ -2150,7 +2150,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_param); You are modifying an exported libxl function. This requires compatibility hooks (look e.g. how this is handled in libxl.h for functions like libxl_get_memory_target) > 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? > + } > + 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. > > - rc = xc_cpupool_create(ctx->xch, &xcpoolid, sched); > + rc = xc_cpupool_create(ctx->xch, &xcpoolid, sched, &xc_sched_param); > if (rc) { > LOGEV(ERROR, rc, "Could not create cpupool"); > GC_FREE; > diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl > index 173d70a..f25429d 100644 > --- a/tools/libxl/libxl_types.idl > +++ b/tools/libxl/libxl_types.idl > @@ -194,6 +194,16 @@ libxl_scheduler = Enumeration("scheduler", [ > (9, "null"), > ]) > > +# consistent with sched_credit2.c > +libxl_credit2_runqueue = Enumeration("credit2_runqueue", [ > + (0, "CPU"), > + (1, "CORE"), > + (2, "SOCKET"), > + (3, "NODE"), > + (4, "ALL"), > + (5, "DEFAULT"), > + ]) > + > # Consistent with SHUTDOWN_* in sched.h (apart from UNKNOWN) > libxl_shutdown_reason = Enumeration("shutdown_reason", [ > (-1, "unknown"), > @@ -326,15 +336,38 @@ libxl_dominfo = Struct("dominfo",[ > ("domain_type", libxl_domain_type), > ], dir=DIR_OUT) > > +libxl_sched_credit_params = Struct("sched_credit_params", [ > + ("tslice_ms", integer), > + ("ratelimit_us", integer), > + ], dispose_fn=None) > + > +libxl_sched_credit2_params = Struct("sched_credit2_params", [ > + ("ratelimit_us", integer), > + ("runqueue", libxl_credit2_runqueue), > + ], dispose_fn=None) > + > +libxl_scheduler_params = Struct("scheduler_params", [ > + ("u", KeyedUnion(None,libxl_scheduler_tpye "scheduler_type", > + [("credit2", libxl_sched_credit2_params), > + ("credit", libxl_sched_credit_params), > + ("null", None), > + ("arinc653", None), > + ("rtds", None), > + ("unknown", None), > + ("sedf", None), > + ])), > + ]) > + > libxl_cpupoolinfo = Struct("cpupoolinfo", [ > ("poolid", uint32), > ("pool_name", string), > ("sched", libxl_scheduler), > ("n_dom", uint32), > - ("cpumap", libxl_bitmap) > + ("cpumap", libxl_bitmap), > + ("sched_param", libxl_scheduler_params), You need a LIBXL_HAVE_* define in libxl.h to indicate presence of the new struct member. Juergen
[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
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@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. 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
diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h index 91408b4..6617c64 100644 --- a/tools/libxl/libxl.h +++ b/tools/libxl/libxl.h @@ -2150,7 +2150,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_param); 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; + } + else + xc_sched_param.u.sched_credit2.runq = LIBXL_CREDIT2_RUNQUEUE_DEFAULT; - rc = xc_cpupool_create(ctx->xch, &xcpoolid, sched); + rc = xc_cpupool_create(ctx->xch, &xcpoolid, sched, &xc_sched_param); if (rc) { LOGEV(ERROR, rc, "Could not create cpupool"); GC_FREE; diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl index 173d70a..f25429d 100644 --- a/tools/libxl/libxl_types.idl +++ b/tools/libxl/libxl_types.idl @@ -194,6 +194,16 @@ libxl_scheduler = Enumeration("scheduler", [ (9, "null"), ]) +# consistent with sched_credit2.c +libxl_credit2_runqueue = Enumeration("credit2_runqueue", [ + (0, "CPU"), + (1, "CORE"), + (2, "SOCKET"), + (3, "NODE"), + (4, "ALL"), + (5, "DEFAULT"), + ]) + # Consistent with SHUTDOWN_* in sched.h (apart from UNKNOWN) libxl_shutdown_reason = Enumeration("shutdown_reason", [ (-1, "unknown"), @@ -326,15 +336,38 @@ libxl_dominfo = Struct("dominfo",[ ("domain_type", libxl_domain_type), ], dir=DIR_OUT) +libxl_sched_credit_params = Struct("sched_credit_params", [ + ("tslice_ms", integer), + ("ratelimit_us", integer), + ], dispose_fn=None) + +libxl_sched_credit2_params = Struct("sched_credit2_params", [ + ("ratelimit_us", integer), + ("runqueue", libxl_credit2_runqueue), + ], dispose_fn=None) + +libxl_scheduler_params = Struct("scheduler_params", [ + ("u", KeyedUnion(None,libxl_scheduler_tpye "scheduler_type", + [("credit2", libxl_sched_credit2_params), + ("credit", libxl_sched_credit_params), + ("null", None), + ("arinc653", None), + ("rtds", None), + ("unknown", None), + ("sedf", None), + ])), + ]) + libxl_cpupoolinfo = Struct("cpupoolinfo", [ ("poolid", uint32), ("pool_name", string), ("sched", libxl_scheduler), ("n_dom", uint32), - ("cpumap", libxl_bitmap) + ("cpumap", libxl_bitmap), + ("sched_param", libxl_scheduler_params), ], dir=DIR_OUT) -libxl_channelinfo = Struct("channelinfo", [ +ibxl_channelinfo = Struct("channelinfo", [ ("backend", string), ("backend_id", uint32), ("frontend", string), @@ -910,15 +943,6 @@ libxl_pcitopology = Struct("pcitopology", [ ("node", uint32), ], dir=DIR_OUT) -libxl_sched_credit_params = Struct("sched_credit_params", [ - ("tslice_ms", integer), - ("ratelimit_us", integer), - ], dispose_fn=None) - -libxl_sched_credit2_params = Struct("sched_credit2_params", [ - ("ratelimit_us", integer), - ], dispose_fn=None) - libxl_domain_remus_info = Struct("domain_remus_info",[ ("interval", integer), ("allow_unsafe", libxl_defbool), diff --git a/tools/xl/xl_cpupool.c b/tools/xl/xl_cpupool.c index 273811b..dc419eb 100644 --- a/tools/xl/xl_cpupool.c +++ b/tools/xl/xl_cpupool.c @@ -43,6 +43,7 @@ int main_cpupoolcreate(int argc, char **argv) char *name = NULL; uint32_t poolid; libxl_scheduler sched = 0; + libxl_scheduler_params sched_params; XLU_ConfigList *cpus; XLU_ConfigList *nodes; int n_cpus, n_nodes, i, n; @@ -207,16 +208,27 @@ int main_cpupoolcreate(int argc, char **argv) } else n_cpus = 0; + sched_params.u.credit2.runqueue = LIBXL_CREDIT2_RUNQUEUE_CORE; + if (!xlu_cfg_get_string (config, "runqueue", &buf, 0) && + sched == LIBXL_SCHEDULER_CREDIT2) { + if ((libxl_credit2_runqueue_from_string(buf, &sched_params.u.credit2.runqueue)) < 0 ) { + fprintf(stderr, "Unknown runqueue option\n"); + sched_params.u.credit2.runqueue = LIBXL_CREDIT2_RUNQUEUE_CORE; /*default CORE */ + } + } + libxl_uuid_generate(&uuid); printf("Using config file \"%s\"\n", config_src); printf("cpupool name: %s\n", name); printf("scheduler: %s\n", libxl_scheduler_to_string(sched)); + if (sched == LIBXL_SCHEDULER_CREDIT2) + printf(" runq: %s\n", libxl_credit2_runqueue_to_string(sched_params.u.credit2.runqueue)); printf("number of cpus: %d\n", n_cpus); if (!dryrun_only) { poolid = LIBXL_CPUPOOL_POOLID_ANY; - if (libxl_cpupool_create(ctx, name, sched, cpumap, &uuid, &poolid)) { + if (libxl_cpupool_create(ctx, name, sched, cpumap, &uuid, &poolid, &sched_params)) { fprintf(stderr, "error on creating cpupool\n"); goto out_cfg; } @@ -587,7 +599,7 @@ int main_cpupoolnumasplit(int argc, char **argv) xasprintf(&name, "Pool-node%d", node); libxl_uuid_generate(&uuid); poolid = 0; - if (libxl_cpupool_create(ctx, name, sched, cpumap, &uuid, &poolid)) { + if (libxl_cpupool_create(ctx, name, sched, cpumap, &uuid, &poolid, NULL)) { fprintf(stderr, "error on creating cpupool\n"); goto out; }
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> --- tools/libxl/libxl.h | 2 +- tools/libxl/libxl_cpupool.c | 15 +++++++++++++-- tools/libxl/libxl_types.idl | 46 ++++++++++++++++++++++++++++++++++----------- tools/xl/xl_cpupool.c | 16 ++++++++++++++-- 4 files changed, 63 insertions(+), 16 deletions(-)