* [PATCH 0 of 3] support of setting scheduler parameters on domain creation
@ 2012-05-22 9:16 Juergen Gross
2012-05-22 9:16 ` [PATCH 1 of 3] Support of getting scheduler defaults Juergen Gross
` (2 more replies)
0 siblings, 3 replies; 24+ messages in thread
From: Juergen Gross @ 2012-05-22 9:16 UTC (permalink / raw)
To: xen-devel
Support setting scheduling parameters on domain creation.
Depending on the scheduler of the cpupool in which the domain is started,
the default parameters are obtained from the hypervisor. Any scheduling
parameters specified during domain creation will modify these defaults.
This patch series consists of 3 patches:
1: support of getting scheduler default parameters in the hypervisor
2: support in libxc
3: support in xl/libxl
14 files changed, 214 insertions(+), 28 deletions(-)
tools/libxc/xc_csched.c | 21 +++++++++++++
tools/libxc/xc_csched2.c | 21 +++++++++++++
tools/libxc/xc_sedf.c | 21 +++++++++++++
tools/libxc/xenctrl.h | 10 ++++++
tools/libxl/libxl.h | 2 +
tools/libxl/libxl_dom.c | 65 +++++++++++++++++++++++++++++++++++++++++--
tools/libxl/libxl_types.idl | 1
tools/libxl/xl_cmdimpl.c | 10 ++++--
xen/common/sched_credit.c | 5 +++
xen/common/sched_credit2.c | 17 +++++++++++
xen/common/sched_sedf.c | 20 +++++++++++++
xen/common/schedule.c | 5 +--
xen/include/public/domctl.h | 38 +++++++++++++------------
xen/include/public/sysctl.h | 6 ++-
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH 1 of 3] Support of getting scheduler defaults
2012-05-22 9:16 [PATCH 0 of 3] support of setting scheduler parameters on domain creation Juergen Gross
@ 2012-05-22 9:16 ` Juergen Gross
2012-05-22 12:22 ` Ian Campbell
2012-05-22 9:16 ` [PATCH 2 of 3] Support getting scheduler defaults in libxc Juergen Gross
2012-05-22 9:16 ` [PATCH 3 of 3] full support of setting scheduler parameters on domain creation Juergen Gross
2 siblings, 1 reply; 24+ messages in thread
From: Juergen Gross @ 2012-05-22 9:16 UTC (permalink / raw)
To: xen-devel
[-- Attachment #1: Type: text/plain, Size: 654 bytes --]
Support a new sysctl schedop sub-command to get the scheduling defaults of a
specific scheduler.
Additionally correct parameter checking of the sysctl handling in schedule.c
(checked wrong sub-commands: domctl instead of sysctl).
Signed-off-by: Juergen Gross <juergen.gross@ts.fujitsu.com>
6 files changed, 69 insertions(+), 22 deletions(-)
xen/common/sched_credit.c | 5 +++++
xen/common/sched_credit2.c | 17 +++++++++++++++++
xen/common/sched_sedf.c | 20 ++++++++++++++++++++
xen/common/schedule.c | 5 +++--
xen/include/public/domctl.h | 38 ++++++++++++++++++++------------------
xen/include/public/sysctl.h | 6 ++++--
[-- Attachment #2: xen-staging.hg-3.patch --]
[-- Type: text/x-patch, Size: 6493 bytes --]
# HG changeset patch
# User Juergen Gross <juergen.gross@ts.fujitsu.com>
# Date 1337674472 -7200
# Node ID 56c50b3f6cc3eb1de8b86024d0e41e65345d9a79
# Parent 238900a4ed227d04c164d4cd12dfc66f7a25b946
Support of getting scheduler defaults
Support a new sysctl schedop sub-command to get the scheduling defaults of a
specific scheduler.
Additionally correct parameter checking of the sysctl handling in schedule.c
(checked wrong sub-commands: domctl instead of sysctl).
Signed-off-by: Juergen Gross <juergen.gross@ts.fujitsu.com>
diff -r 238900a4ed22 -r 56c50b3f6cc3 xen/common/sched_credit.c
--- a/xen/common/sched_credit.c Mon May 21 12:03:32 2012 +0200
+++ b/xen/common/sched_credit.c Tue May 22 10:14:32 2012 +0200
@@ -858,6 +858,11 @@ csched_sys_cntl(const struct scheduler *
params->ratelimit_us = prv->ratelimit_us;
rc = 0;
break;
+ case XEN_SYSCTL_SCHEDOP_getdefaults:
+ sc->u.defaults.credit.weight = CSCHED_DEFAULT_WEIGHT;
+ sc->u.defaults.credit.cap = 0U;
+ rc = 0;
+ break;
}
out:
return rc;
diff -r 238900a4ed22 -r 56c50b3f6cc3 xen/common/sched_credit2.c
--- a/xen/common/sched_credit2.c Mon May 21 12:03:32 2012 +0200
+++ b/xen/common/sched_credit2.c Tue May 22 10:14:32 2012 +0200
@@ -1423,6 +1423,22 @@ csched_dom_cntl(
return 0;
}
+static int
+csched_sys_cntl(const struct scheduler *ops,
+ struct xen_sysctl_scheduler_op *sc)
+{
+ int rc = -EINVAL;
+
+ switch ( sc->cmd )
+ {
+ case XEN_SYSCTL_SCHEDOP_getdefaults:
+ sc->u.defaults.credit2.weight = CSCHED_DEFAULT_WEIGHT;
+ rc = 0;
+ break;
+ }
+ return rc;
+}
+
static void *
csched_alloc_domdata(const struct scheduler *ops, struct domain *dom)
{
@@ -2110,6 +2126,7 @@ const struct scheduler sched_credit2_def
.wake = csched_vcpu_wake,
.adjust = csched_dom_cntl,
+ .adjust_global = csched_sys_cntl,
.pick_cpu = csched_cpu_pick,
.migrate = csched_vcpu_migrate,
diff -r 238900a4ed22 -r 56c50b3f6cc3 xen/common/sched_sedf.c
--- a/xen/common/sched_sedf.c Mon May 21 12:03:32 2012 +0200
+++ b/xen/common/sched_sedf.c Tue May 22 10:14:32 2012 +0200
@@ -1502,6 +1502,25 @@ out:
return rc;
}
+static int sedf_adjust_global(const struct scheduler *ops,
+ struct xen_sysctl_scheduler_op *sc)
+{
+ int rc = -EINVAL;
+
+ switch ( sc->cmd )
+ {
+ case XEN_SYSCTL_SCHEDOP_getdefaults:
+ sc->u.defaults.sedf.period = WEIGHT_PERIOD;
+ sc->u.defaults.sedf.slice = 0;
+ sc->u.defaults.sedf.latency = 0;
+ sc->u.defaults.sedf.extratime = EXTRA_AWARE;
+ sc->u.defaults.sedf.weight = 0;
+ rc = 0;
+ break;
+ }
+ return rc;
+}
+
static struct sedf_priv_info _sedf_priv;
const struct scheduler sched_sedf_def = {
@@ -1531,6 +1550,7 @@ const struct scheduler sched_sedf_def =
.sleep = sedf_sleep,
.wake = sedf_wake,
.adjust = sedf_adjust,
+ .adjust_global = sedf_adjust_global,
};
/*
diff -r 238900a4ed22 -r 56c50b3f6cc3 xen/common/schedule.c
--- a/xen/common/schedule.c Mon May 21 12:03:32 2012 +0200
+++ b/xen/common/schedule.c Tue May 22 10:14:32 2012 +0200
@@ -1029,8 +1029,9 @@ long sched_adjust_global(struct xen_sysc
struct cpupool *pool;
int rc;
- if ( (op->cmd != XEN_DOMCTL_SCHEDOP_putinfo) &&
- (op->cmd != XEN_DOMCTL_SCHEDOP_getinfo) )
+ if ( (op->cmd != XEN_SYSCTL_SCHEDOP_putinfo) &&
+ (op->cmd != XEN_SYSCTL_SCHEDOP_getinfo) &&
+ (op->cmd != XEN_SYSCTL_SCHEDOP_getdefaults))
return -EINVAL;
pool = cpupool_get_by_id(op->cpupool_id);
diff -r 238900a4ed22 -r 56c50b3f6cc3 xen/include/public/domctl.h
--- a/xen/include/public/domctl.h Mon May 21 12:03:32 2012 +0200
+++ b/xen/include/public/domctl.h Tue May 22 10:14:32 2012 +0200
@@ -303,28 +303,30 @@ DEFINE_XEN_GUEST_HANDLE(xen_domctl_max_v
#define XEN_SCHEDULER_CREDIT 5
#define XEN_SCHEDULER_CREDIT2 6
#define XEN_SCHEDULER_ARINC653 7
+/* Scheduling parameters (used in sysctl.h, too) */
+union xen_sched_par {
+ struct xen_domctl_sched_sedf {
+ uint64_aligned_t period;
+ uint64_aligned_t slice;
+ uint64_aligned_t latency;
+ uint32_t extratime;
+ uint32_t weight;
+ } sedf;
+ struct xen_domctl_sched_credit {
+ uint16_t weight;
+ uint16_t cap;
+ } credit;
+ struct xen_domctl_sched_credit2 {
+ uint16_t weight;
+ } credit2;
+};
/* Set or get info? */
-#define XEN_DOMCTL_SCHEDOP_putinfo 0
-#define XEN_DOMCTL_SCHEDOP_getinfo 1
+#define XEN_DOMCTL_SCHEDOP_putinfo 0
+#define XEN_DOMCTL_SCHEDOP_getinfo 1
struct xen_domctl_scheduler_op {
uint32_t sched_id; /* XEN_SCHEDULER_* */
uint32_t cmd; /* XEN_DOMCTL_SCHEDOP_* */
- union {
- struct xen_domctl_sched_sedf {
- uint64_aligned_t period;
- uint64_aligned_t slice;
- uint64_aligned_t latency;
- uint32_t extratime;
- uint32_t weight;
- } sedf;
- struct xen_domctl_sched_credit {
- uint16_t weight;
- uint16_t cap;
- } credit;
- struct xen_domctl_sched_credit2 {
- uint16_t weight;
- } credit2;
- } u;
+ union xen_sched_par u;
};
typedef struct xen_domctl_scheduler_op xen_domctl_scheduler_op_t;
DEFINE_XEN_GUEST_HANDLE(xen_domctl_scheduler_op_t);
diff -r 238900a4ed22 -r 56c50b3f6cc3 xen/include/public/sysctl.h
--- a/xen/include/public/sysctl.h Mon May 21 12:03:32 2012 +0200
+++ b/xen/include/public/sysctl.h Tue May 22 10:14:32 2012 +0200
@@ -579,8 +579,9 @@ DEFINE_XEN_GUEST_HANDLE(xen_sysctl_credi
/* XEN_SYSCTL_scheduler_op */
/* Set or get info? */
-#define XEN_SYSCTL_SCHEDOP_putinfo 0
-#define XEN_SYSCTL_SCHEDOP_getinfo 1
+#define XEN_SYSCTL_SCHEDOP_putinfo 0
+#define XEN_SYSCTL_SCHEDOP_getinfo 1
+#define XEN_SYSCTL_SCHEDOP_getdefaults 2
struct xen_sysctl_scheduler_op {
uint32_t cpupool_id; /* Cpupool whose scheduler is to be targetted. */
uint32_t sched_id; /* XEN_SCHEDULER_* (domctl.h) */
@@ -590,6 +591,7 @@ struct xen_sysctl_scheduler_op {
XEN_GUEST_HANDLE_64(xen_sysctl_arinc653_schedule_t) schedule;
} sched_arinc653;
struct xen_sysctl_credit_schedule sched_credit;
+ union xen_sched_par defaults;
} u;
};
typedef struct xen_sysctl_scheduler_op xen_sysctl_scheduler_op_t;
[-- Attachment #3: Type: text/plain, Size: 126 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH 2 of 3] Support getting scheduler defaults in libxc
2012-05-22 9:16 [PATCH 0 of 3] support of setting scheduler parameters on domain creation Juergen Gross
2012-05-22 9:16 ` [PATCH 1 of 3] Support of getting scheduler defaults Juergen Gross
@ 2012-05-22 9:16 ` Juergen Gross
2012-05-22 9:16 ` [PATCH 3 of 3] full support of setting scheduler parameters on domain creation Juergen Gross
2 siblings, 0 replies; 24+ messages in thread
From: Juergen Gross @ 2012-05-22 9:16 UTC (permalink / raw)
To: xen-devel
[-- Attachment #1: Type: text/plain, Size: 386 bytes --]
Add scheduler specific interfaces to get scheduling defaults from the
hypervisor.
Signed-off-by: Juergen Gross <juergen.gross@ts.fujitsu.com>
4 files changed, 73 insertions(+)
tools/libxc/xc_csched.c | 21 +++++++++++++++++++++
tools/libxc/xc_csched2.c | 21 +++++++++++++++++++++
tools/libxc/xc_sedf.c | 21 +++++++++++++++++++++
tools/libxc/xenctrl.h | 10 ++++++++++
[-- Attachment #2: xen-staging.hg-3.patch --]
[-- Type: text/x-patch, Size: 4372 bytes --]
# HG changeset patch
# User Juergen Gross <juergen.gross@ts.fujitsu.com>
# Date 1337675490 -7200
# Node ID 19aaa30d7fdce2f1b56cb13399d603d955a61fb8
# Parent 56c50b3f6cc3eb1de8b86024d0e41e65345d9a79
Support getting scheduler defaults in libxc
Add scheduler specific interfaces to get scheduling defaults from the
hypervisor.
Signed-off-by: Juergen Gross <juergen.gross@ts.fujitsu.com>
diff -r 56c50b3f6cc3 -r 19aaa30d7fdc tools/libxc/xc_csched.c
--- a/tools/libxc/xc_csched.c Tue May 22 10:14:32 2012 +0200
+++ b/tools/libxc/xc_csched.c Tue May 22 10:31:30 2012 +0200
@@ -105,3 +105,24 @@ xc_sched_credit_params_get(
return rc;
}
+
+int
+xc_sched_credit_defaults_get(
+ xc_interface *xch,
+ uint32_t cpupool_id,
+ struct xen_domctl_sched_credit *sdom)
+{
+ int rc;
+ DECLARE_SYSCTL;
+
+ sysctl.cmd = XEN_SYSCTL_scheduler_op;
+ sysctl.u.scheduler_op.cpupool_id = cpupool_id;
+ sysctl.u.scheduler_op.sched_id = XEN_SCHEDULER_CREDIT;
+ sysctl.u.scheduler_op.cmd = XEN_SYSCTL_SCHEDOP_getdefaults;
+
+ rc = do_sysctl(xch, &sysctl);
+ if ( rc == 0 )
+ *sdom = sysctl.u.scheduler_op.u.defaults.credit;
+
+ return rc;
+}
diff -r 56c50b3f6cc3 -r 19aaa30d7fdc tools/libxc/xc_csched2.c
--- a/tools/libxc/xc_csched2.c Tue May 22 10:14:32 2012 +0200
+++ b/tools/libxc/xc_csched2.c Tue May 22 10:31:30 2012 +0200
@@ -61,3 +61,24 @@ xc_sched_credit2_domain_get(
return err;
}
+
+int
+xc_sched_credit2_defaults_get(
+ xc_interface *xch,
+ uint32_t cpupool_id,
+ struct xen_domctl_sched_credit2 *sdom)
+{
+ int rc;
+ DECLARE_SYSCTL;
+
+ sysctl.cmd = XEN_SYSCTL_scheduler_op;
+ sysctl.u.scheduler_op.cpupool_id = cpupool_id;
+ sysctl.u.scheduler_op.sched_id = XEN_SCHEDULER_CREDIT2;
+ sysctl.u.scheduler_op.cmd = XEN_SYSCTL_SCHEDOP_getdefaults;
+
+ rc = do_sysctl(xch, &sysctl);
+ if ( rc == 0 )
+ *sdom = sysctl.u.scheduler_op.u.defaults.credit2;
+
+ return rc;
+}
diff -r 56c50b3f6cc3 -r 19aaa30d7fdc tools/libxc/xc_sedf.c
--- a/tools/libxc/xc_sedf.c Tue May 22 10:14:32 2012 +0200
+++ b/tools/libxc/xc_sedf.c Tue May 22 10:31:30 2012 +0200
@@ -76,3 +76,24 @@ int xc_sedf_domain_get(
*weight = p->weight;
return ret;
}
+
+int
+xc_sched_sedf_defaults_get(
+ xc_interface *xch,
+ uint32_t cpupool_id,
+ struct xen_domctl_sched_sedf *sdom)
+{
+ int rc;
+ DECLARE_SYSCTL;
+
+ sysctl.cmd = XEN_SYSCTL_scheduler_op;
+ sysctl.u.scheduler_op.cpupool_id = cpupool_id;
+ sysctl.u.scheduler_op.sched_id = XEN_SCHEDULER_SEDF;
+ sysctl.u.scheduler_op.cmd = XEN_SYSCTL_SCHEDOP_getdefaults;
+
+ rc = do_sysctl(xch, &sysctl);
+ if ( rc == 0 )
+ *sdom = sysctl.u.scheduler_op.u.defaults.sedf;
+
+ return rc;
+}
diff -r 56c50b3f6cc3 -r 19aaa30d7fdc tools/libxc/xenctrl.h
--- a/tools/libxc/xenctrl.h Tue May 22 10:14:32 2012 +0200
+++ b/tools/libxc/xenctrl.h Tue May 22 10:31:30 2012 +0200
@@ -664,6 +664,9 @@ int xc_sedf_domain_get(xc_interface *xch
uint64_t* period, uint64_t *slice,
uint64_t *latency, uint16_t *extratime,
uint16_t *weight);
+int xc_sched_sedf_defaults_get(xc_interface *xch,
+ uint32_t cpupool_id,
+ struct xen_domctl_sched_sedf *sdom);
int xc_sched_credit_domain_set(xc_interface *xch,
uint32_t domid,
@@ -678,12 +681,19 @@ int xc_sched_credit_params_get(xc_interf
int xc_sched_credit_params_get(xc_interface *xch,
uint32_t cpupool_id,
struct xen_sysctl_credit_schedule *schedule);
+int xc_sched_credit_defaults_get(xc_interface *xch,
+ uint32_t cpupool_id,
+ struct xen_domctl_sched_credit *sdom);
+
int xc_sched_credit2_domain_set(xc_interface *xch,
uint32_t domid,
struct xen_domctl_sched_credit2 *sdom);
int xc_sched_credit2_domain_get(xc_interface *xch,
uint32_t domid,
+ struct xen_domctl_sched_credit2 *sdom);
+int xc_sched_credit2_defaults_get(xc_interface *xch,
+ uint32_t cpupool_id,
struct xen_domctl_sched_credit2 *sdom);
int
[-- Attachment #3: Type: text/plain, Size: 126 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH 3 of 3] full support of setting scheduler parameters on domain creation
2012-05-22 9:16 [PATCH 0 of 3] support of setting scheduler parameters on domain creation Juergen Gross
2012-05-22 9:16 ` [PATCH 1 of 3] Support of getting scheduler defaults Juergen Gross
2012-05-22 9:16 ` [PATCH 2 of 3] Support getting scheduler defaults in libxc Juergen Gross
@ 2012-05-22 9:16 ` Juergen Gross
2012-05-22 12:30 ` Ian Campbell
2 siblings, 1 reply; 24+ messages in thread
From: Juergen Gross @ 2012-05-22 9:16 UTC (permalink / raw)
To: xen-devel
[-- Attachment #1: Type: text/plain, Size: 487 bytes --]
Obtains default scheduler parameters before modifying any settings specified
via domain config.
Corrected an error for setting sedf parameters (setting .period multiple
times).
Signed-off-by: Juergen Gross <juergen.gross@ts.fujitsu.com>
4 files changed, 72 insertions(+), 6 deletions(-)
tools/libxl/libxl.h | 2 +
tools/libxl/libxl_dom.c | 65 +++++++++++++++++++++++++++++++++++++++++--
tools/libxl/libxl_types.idl | 1
tools/libxl/xl_cmdimpl.c | 10 ++++--
[-- Attachment #2: xen-staging.hg-3.patch --]
[-- Type: text/x-patch, Size: 6064 bytes --]
# HG changeset patch
# User Juergen Gross <juergen.gross@ts.fujitsu.com>
# Date 1337677423 -7200
# Node ID 953383741ff44d97587c2e75da79b092523d6e83
# Parent 19aaa30d7fdce2f1b56cb13399d603d955a61fb8
full support of setting scheduler parameters on domain creation
Obtains default scheduler parameters before modifying any settings specified
via domain config.
Corrected an error for setting sedf parameters (setting .period multiple
times).
Signed-off-by: Juergen Gross <juergen.gross@ts.fujitsu.com>
diff -r 19aaa30d7fdc -r 953383741ff4 tools/libxl/libxl.h
--- a/tools/libxl/libxl.h Tue May 22 10:31:30 2012 +0200
+++ b/tools/libxl/libxl.h Tue May 22 11:03:43 2012 +0200
@@ -605,6 +605,8 @@ int libxl_primary_console_exec(libxl_ctx
/* May be called with info_r == NULL to check for domain's existance */
int libxl_domain_info(libxl_ctx*, libxl_dominfo *info_r,
uint32_t domid);
+int libxl_sched_set_defaults(libxl_ctx*, uint32_t poolid,
+ libxl_sched_params *scparams);
libxl_dominfo * libxl_list_domain(libxl_ctx*, int *nb_domain);
void libxl_dominfo_list_free(libxl_dominfo *list, int nr);
libxl_cpupoolinfo * libxl_list_cpupool(libxl_ctx*, int *nb_pool);
diff -r 19aaa30d7fdc -r 953383741ff4 tools/libxl/libxl_dom.c
--- a/tools/libxl/libxl_dom.c Tue May 22 10:31:30 2012 +0200
+++ b/tools/libxl/libxl_dom.c Tue May 22 11:03:43 2012 +0200
@@ -42,17 +42,76 @@ libxl_domain_type libxl__domain_type(lib
return LIBXL_DOMAIN_TYPE_PV;
}
+int libxl_sched_set_defaults(libxl_ctx *ctx,
+ uint32_t poolid, libxl_sched_params *scparams)
+{
+ libxl_cpupoolinfo *poolinfo;
+ struct xen_domctl_sched_credit credit_info;
+ struct xen_domctl_sched_credit2 credit2_info;
+ struct xen_domctl_sched_sedf sedf_info;
+ int n_pools, p;
+ int ret;
+
+ poolinfo = libxl_list_cpupool(ctx, &n_pools);
+ if (!poolinfo)
+ return ERROR_NOMEM;
+
+ ret = ERROR_INVAL;
+ for (p = 0; p < n_pools; p++) {
+ if (poolinfo[p].poolid == poolid) {
+ scparams->sched = poolinfo[p].sched;
+ ret = 0;
+ }
+ libxl_cpupoolinfo_dispose(poolinfo + p);
+ }
+
+ if (!ret) {
+ switch (scparams->sched) {
+ case LIBXL_SCHEDULER_SEDF:
+ if (xc_sched_sedf_defaults_get(ctx->xch, poolid,
+ &sedf_info))
+ ret = ERROR_FAIL;
+ else {
+ scparams->period = sedf_info.period;
+ scparams->slice = sedf_info.slice;
+ scparams->latency = sedf_info.latency;
+ scparams->extratime = sedf_info.extratime;
+ scparams->weight = sedf_info.weight;
+ }
+ break;
+ case LIBXL_SCHEDULER_CREDIT:
+ if (xc_sched_credit_defaults_get(ctx->xch, poolid,
+ &credit_info))
+ ret = ERROR_FAIL;
+ else {
+ scparams->weight = credit_info.weight;
+ scparams->cap = credit_info.cap;
+ }
+ break;
+ case LIBXL_SCHEDULER_CREDIT2:
+ if (xc_sched_credit2_defaults_get(ctx->xch, poolid,
+ &credit2_info))
+ ret = ERROR_FAIL;
+ else {
+ scparams->weight = credit2_info.weight;
+ }
+ break;
+ default: ret = ERROR_INVAL;
+ }
+ }
+
+ return ret;
+}
+
int libxl__sched_set_params(libxl__gc *gc, uint32_t domid, libxl_sched_params *scparams)
{
libxl_ctx *ctx = libxl__gc_owner(gc);
- libxl_scheduler sched;
libxl_sched_sedf_domain sedf_info;
libxl_sched_credit_domain credit_info;
libxl_sched_credit2_domain credit2_info;
int ret;
- sched = libxl_get_scheduler (ctx);
- switch (sched) {
+ switch (scparams->sched) {
case LIBXL_SCHEDULER_SEDF:
sedf_info.period = scparams->period;
sedf_info.slice = scparams->slice;
diff -r 19aaa30d7fdc -r 953383741ff4 tools/libxl/libxl_types.idl
--- a/tools/libxl/libxl_types.idl Tue May 22 10:31:30 2012 +0200
+++ b/tools/libxl/libxl_types.idl Tue May 22 11:03:43 2012 +0200
@@ -225,6 +225,7 @@ MemKB = UInt(64, init_val = "LIBXL_MEMKB
MemKB = UInt(64, init_val = "LIBXL_MEMKB_DEFAULT")
libxl_sched_params = Struct("sched_params",[
+ ("sched", libxl_scheduler),
("weight", integer),
("cap", integer),
("tslice_ms", integer),
diff -r 19aaa30d7fdc -r 953383741ff4 tools/libxl/xl_cmdimpl.c
--- a/tools/libxl/xl_cmdimpl.c Tue May 22 10:31:30 2012 +0200
+++ b/tools/libxl/xl_cmdimpl.c Tue May 22 11:03:43 2012 +0200
@@ -628,6 +628,10 @@ static void parse_config_data(const char
libxl_domain_build_info_init_type(b_info, c_info->type);
/* the following is the actual config parsing with overriding values in the structures */
+ if (libxl_sched_set_defaults(ctx, c_info->poolid, &b_info->sched_params)) {
+ fprintf(stderr, "Failed to set default scheduling parameters\n");
+ exit(1);
+ }
if (!xlu_cfg_get_long (config, "cpu_weight", &l, 0))
b_info->sched_params.weight = l;
if (!xlu_cfg_get_long (config, "cap", &l, 0))
@@ -639,11 +643,11 @@ static void parse_config_data(const char
if (!xlu_cfg_get_long (config, "period", &l, 0))
b_info->sched_params.period = l;
if (!xlu_cfg_get_long (config, "slice", &l, 0))
- b_info->sched_params.period = l;
+ b_info->sched_params.slice = l;
if (!xlu_cfg_get_long (config, "latency", &l, 0))
- b_info->sched_params.period = l;
+ b_info->sched_params.latency = l;
if (!xlu_cfg_get_long (config, "extratime", &l, 0))
- b_info->sched_params.period = l;
+ b_info->sched_params.extratime = l;
if (!xlu_cfg_get_long (config, "vcpus", &l, 0)) {
b_info->max_vcpus = l;
[-- Attachment #3: Type: text/plain, Size: 126 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 1 of 3] Support of getting scheduler defaults
2012-05-22 9:16 ` [PATCH 1 of 3] Support of getting scheduler defaults Juergen Gross
@ 2012-05-22 12:22 ` Ian Campbell
2012-05-22 12:29 ` Juergen Gross
0 siblings, 1 reply; 24+ messages in thread
From: Ian Campbell @ 2012-05-22 12:22 UTC (permalink / raw)
To: Juergen Gross; +Cc: xen-devel
On Tue, 2012-05-22 at 10:16 +0100, Juergen Gross wrote:
> Support a new sysctl schedop sub-command to get the scheduling defaults of a
> specific scheduler.
Why is XEN_DOMCTL_SCHEDOP_getinfo not sufficient here?
Isn't it actually more useful/meaningful to get the current actual
setting rather than a default?
Ian.
>
> Additionally correct parameter checking of the sysctl handling in schedule.c
> (checked wrong sub-commands: domctl instead of sysctl).
>
> Signed-off-by: Juergen Gross <juergen.gross@ts.fujitsu.com>
>
>
> 6 files changed, 69 insertions(+), 22 deletions(-)
> xen/common/sched_credit.c | 5 +++++
> xen/common/sched_credit2.c | 17 +++++++++++++++++
> xen/common/sched_sedf.c | 20 ++++++++++++++++++++
> xen/common/schedule.c | 5 +++--
> xen/include/public/domctl.h | 38 ++++++++++++++++++++------------------
> xen/include/public/sysctl.h | 6 ++++--
>
>
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 1 of 3] Support of getting scheduler defaults
2012-05-22 12:22 ` Ian Campbell
@ 2012-05-22 12:29 ` Juergen Gross
2012-05-22 12:32 ` Ian Campbell
0 siblings, 1 reply; 24+ messages in thread
From: Juergen Gross @ 2012-05-22 12:29 UTC (permalink / raw)
To: Ian Campbell; +Cc: xen-devel
On 05/22/2012 02:22 PM, Ian Campbell wrote:
> On Tue, 2012-05-22 at 10:16 +0100, Juergen Gross wrote:
>> Support a new sysctl schedop sub-command to get the scheduling defaults of a
>> specific scheduler.
> Why is XEN_DOMCTL_SCHEDOP_getinfo not sufficient here?
>
> Isn't it actually more useful/meaningful to get the current actual
> setting rather than a default?
When setting the parameters from the config file we have no domain yet which
we would have to specify for XEN_DOMCTL_SCHEDOP_getinfo. I didn't want to
parse part of the config only after domain creation.
A default should be okay, as this is what we want to modify. :-)
The scheduler should initialize a new domain with this default, of course.
Juergen
--
Juergen Gross Principal Developer Operating Systems
PDG ES&S SWE OS6 Telephone: +49 (0) 89 3222 2967
Fujitsu Technology Solutions e-mail: juergen.gross@ts.fujitsu.com
Domagkstr. 28 Internet: ts.fujitsu.com
D-80807 Muenchen Company details: ts.fujitsu.com/imprint.html
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 3 of 3] full support of setting scheduler parameters on domain creation
2012-05-22 9:16 ` [PATCH 3 of 3] full support of setting scheduler parameters on domain creation Juergen Gross
@ 2012-05-22 12:30 ` Ian Campbell
2012-05-22 12:39 ` Juergen Gross
0 siblings, 1 reply; 24+ messages in thread
From: Ian Campbell @ 2012-05-22 12:30 UTC (permalink / raw)
To: Juergen Gross; +Cc: xen-devel
> # HG changeset patch
> # User Juergen Gross <juergen.gross@ts.fujitsu.com>
> # Date 1337677423 -7200
> # Node ID 953383741ff44d97587c2e75da79b092523d6e83
> # Parent 19aaa30d7fdce2f1b56cb13399d603d955a61fb8
> full support of setting scheduler parameters on domain creation
>
> Obtains default scheduler parameters before modifying any settings
> specified
> via domain config.
>
> Corrected an error for setting sedf parameters (setting .period
> multiple
> times).
>
> Signed-off-by: Juergen Gross <juergen.gross@ts.fujitsu.com>
>
> diff -r 19aaa30d7fdc -r 953383741ff4 tools/libxl/libxl.h
> --- a/tools/libxl/libxl.h Tue May 22 10:31:30 2012 +0200
> +++ b/tools/libxl/libxl.h Tue May 22 11:03:43 2012 +0200
> @@ -605,6 +605,8 @@ int libxl_primary_console_exec(libxl_ctx
> /* May be called with info_r == NULL to check for domain's existance
> */
> int libxl_domain_info(libxl_ctx*, libxl_dominfo *info_r,
> uint32_t domid);
> +int libxl_sched_set_defaults(libxl_ctx*, uint32_t poolid,
> + libxl_sched_params *scparams);
This interface really makes libxl_sched_params differ from all the other
libxl structs (which have a public _init function and an internal
setdefaults function). I'm not really sure its justified either, I was
under the impression that you'd found that there were useful
discriminating values?
If this function was called libxl_sched_init (replacing the
autogenerated one) then it might be ok. Although I'm still not really
sure what the issue is with having a discriminating value meaning
default is, doing that keeps the _init function cheap too.
> int libxl__sched_set_params(libxl__gc *gc, uint32_t domid,
> libxl_sched_params *scparams)
> {
> libxl_ctx *ctx = libxl__gc_owner(gc);
> - libxl_scheduler sched;
> libxl_sched_sedf_domain sedf_info;
> libxl_sched_credit_domain credit_info;
> libxl_sched_credit2_domain credit2_info;
> int ret;
>
> - sched = libxl_get_scheduler (ctx);
> - switch (sched) {
> + switch (scparams->sched) {
What happens if scparams->sched is not the scheduler used for this
domain? Should it either be checked or set somewhere?
>
> Ian.
>
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 1 of 3] Support of getting scheduler defaults
2012-05-22 12:29 ` Juergen Gross
@ 2012-05-22 12:32 ` Ian Campbell
2012-05-22 12:58 ` Juergen Gross
0 siblings, 1 reply; 24+ messages in thread
From: Ian Campbell @ 2012-05-22 12:32 UTC (permalink / raw)
To: Juergen Gross; +Cc: xen-devel
On Tue, 2012-05-22 at 13:29 +0100, Juergen Gross wrote:
> On 05/22/2012 02:22 PM, Ian Campbell wrote:
> > On Tue, 2012-05-22 at 10:16 +0100, Juergen Gross wrote:
> >> Support a new sysctl schedop sub-command to get the scheduling defaults of a
> >> specific scheduler.
> > Why is XEN_DOMCTL_SCHEDOP_getinfo not sufficient here?
> >
> > Isn't it actually more useful/meaningful to get the current actual
> > setting rather than a default?
>
> When setting the parameters from the config file we have no domain yet which
> we would have to specify for XEN_DOMCTL_SCHEDOP_getinfo. I didn't want to
> parse part of the config only after domain creation.
That seems like another problem with doing this up front instead of
doing read-modify-write when we come to set the values for the domain.
> A default should be okay, as this is what we want to modify. :-)
> The scheduler should initialize a new domain with this default, of course.
So I can't use this interface to change the current one of the current
settings for running a domain, while leaving the others at their current
value?
Which interface can I use for that and why isn't it the same as this
one?
Ian.
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 3 of 3] full support of setting scheduler parameters on domain creation
2012-05-22 12:30 ` Ian Campbell
@ 2012-05-22 12:39 ` Juergen Gross
2012-05-22 12:51 ` Ian Campbell
2012-05-22 22:15 ` Dario Faggioli
0 siblings, 2 replies; 24+ messages in thread
From: Juergen Gross @ 2012-05-22 12:39 UTC (permalink / raw)
To: Ian Campbell; +Cc: xen-devel
On 05/22/2012 02:30 PM, Ian Campbell wrote:
>> # HG changeset patch
>> # User Juergen Gross<juergen.gross@ts.fujitsu.com>
>> # Date 1337677423 -7200
>> # Node ID 953383741ff44d97587c2e75da79b092523d6e83
>> # Parent 19aaa30d7fdce2f1b56cb13399d603d955a61fb8
>> full support of setting scheduler parameters on domain creation
>>
>> Obtains default scheduler parameters before modifying any settings
>> specified
>> via domain config.
>>
>> Corrected an error for setting sedf parameters (setting .period
>> multiple
>> times).
>>
>> Signed-off-by: Juergen Gross<juergen.gross@ts.fujitsu.com>
>>
>> diff -r 19aaa30d7fdc -r 953383741ff4 tools/libxl/libxl.h
>> --- a/tools/libxl/libxl.h Tue May 22 10:31:30 2012 +0200
>> +++ b/tools/libxl/libxl.h Tue May 22 11:03:43 2012 +0200
>> @@ -605,6 +605,8 @@ int libxl_primary_console_exec(libxl_ctx
>> /* May be called with info_r == NULL to check for domain's existance
>> */
>> int libxl_domain_info(libxl_ctx*, libxl_dominfo *info_r,
>> uint32_t domid);
>> +int libxl_sched_set_defaults(libxl_ctx*, uint32_t poolid,
>> + libxl_sched_params *scparams);
> This interface really makes libxl_sched_params differ from all the other
> libxl structs (which have a public _init function and an internal
> setdefaults function). I'm not really sure its justified either, I was
> under the impression that you'd found that there were useful
> discriminating values?
Dario opted for this solution, so I proposed a patch implementing it.
I prefer this solution, too, as it isn't exporting scheduler internals to
the tools.
> If this function was called libxl_sched_init (replacing the
> autogenerated one) then it might be ok. Although I'm still not really
> sure what the issue is with having a discriminating value meaning
> default is, doing that keeps the _init function cheap too.
Okay, I can rename it.
>
>> int libxl__sched_set_params(libxl__gc *gc, uint32_t domid,
>> libxl_sched_params *scparams)
>> {
>> libxl_ctx *ctx = libxl__gc_owner(gc);
>> - libxl_scheduler sched;
>> libxl_sched_sedf_domain sedf_info;
>> libxl_sched_credit_domain credit_info;
>> libxl_sched_credit2_domain credit2_info;
>> int ret;
>>
>> - sched = libxl_get_scheduler (ctx);
>> - switch (sched) {
>> + switch (scparams->sched) {
> What happens if scparams->sched is not the scheduler used for this
> domain? Should it either be checked or set somewhere?
The check would be the same as the original setting of scparams->sched.
Setting of the scheduler parameters will be rejected by the hypervisor if the
scheduler does not match.
Juergen
--
Juergen Gross Principal Developer Operating Systems
PDG ES&S SWE OS6 Telephone: +49 (0) 89 3222 2967
Fujitsu Technology Solutions e-mail: juergen.gross@ts.fujitsu.com
Domagkstr. 28 Internet: ts.fujitsu.com
D-80807 Muenchen Company details: ts.fujitsu.com/imprint.html
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 3 of 3] full support of setting scheduler parameters on domain creation
2012-05-22 12:39 ` Juergen Gross
@ 2012-05-22 12:51 ` Ian Campbell
2012-05-22 22:15 ` Dario Faggioli
1 sibling, 0 replies; 24+ messages in thread
From: Ian Campbell @ 2012-05-22 12:51 UTC (permalink / raw)
To: Juergen Gross; +Cc: xen-devel
On Tue, 2012-05-22 at 13:39 +0100, Juergen Gross wrote:
> On 05/22/2012 02:30 PM, Ian Campbell wrote:
> >> # HG changeset patch
> >> # User Juergen Gross<juergen.gross@ts.fujitsu.com>
> >> # Date 1337677423 -7200
> >> # Node ID 953383741ff44d97587c2e75da79b092523d6e83
> >> # Parent 19aaa30d7fdce2f1b56cb13399d603d955a61fb8
> >> full support of setting scheduler parameters on domain creation
> >>
> >> Obtains default scheduler parameters before modifying any settings
> >> specified
> >> via domain config.
> >>
> >> Corrected an error for setting sedf parameters (setting .period
> >> multiple
> >> times).
> >>
> >> Signed-off-by: Juergen Gross<juergen.gross@ts.fujitsu.com>
> >>
> >> diff -r 19aaa30d7fdc -r 953383741ff4 tools/libxl/libxl.h
> >> --- a/tools/libxl/libxl.h Tue May 22 10:31:30 2012 +0200
> >> +++ b/tools/libxl/libxl.h Tue May 22 11:03:43 2012 +0200
> >> @@ -605,6 +605,8 @@ int libxl_primary_console_exec(libxl_ctx
> >> /* May be called with info_r == NULL to check for domain's existance
> >> */
> >> int libxl_domain_info(libxl_ctx*, libxl_dominfo *info_r,
> >> uint32_t domid);
> >> +int libxl_sched_set_defaults(libxl_ctx*, uint32_t poolid,
> >> + libxl_sched_params *scparams);
> > This interface really makes libxl_sched_params differ from all the other
> > libxl structs (which have a public _init function and an internal
> > setdefaults function). I'm not really sure its justified either, I was
> > under the impression that you'd found that there were useful
> > discriminating values?
>
> Dario opted for this solution, so I proposed a patch implementing it.
> I prefer this solution, too, as it isn't exporting scheduler internals to
> the tools.
I don't think "-1 is not a valid weight" is really "exporting scheduler
internals".
Anyway, I am far more concerned that the libxl API is useful to users
than I am about the API between libxl/libxc and the hypervisor (which is
not set in stone and can be fixed etc). The libxl API is the one we want
to expose and keep stable going forward etc.
>
> > If this function was called libxl_sched_init (replacing the
> > autogenerated one) then it might be ok. Although I'm still not really
> > sure what the issue is with having a discriminating value meaning
> > default is, doing that keeps the _init function cheap too.
>
> Okay, I can rename it.
>
> >
> >> int libxl__sched_set_params(libxl__gc *gc, uint32_t domid,
> >> libxl_sched_params *scparams)
> >> {
> >> libxl_ctx *ctx = libxl__gc_owner(gc);
> >> - libxl_scheduler sched;
> >> libxl_sched_sedf_domain sedf_info;
> >> libxl_sched_credit_domain credit_info;
> >> libxl_sched_credit2_domain credit2_info;
> >> int ret;
> >>
> >> - sched = libxl_get_scheduler (ctx);
> >> - switch (sched) {
> >> + switch (scparams->sched) {
> > What happens if scparams->sched is not the scheduler used for this
> > domain? Should it either be checked or set somewhere?
>
> The check would be the same as the original setting of scparams->sched.
> Setting of the scheduler parameters will be rejected by the hypervisor if the
> scheduler does not match.
>
>
> Juergen
>
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 1 of 3] Support of getting scheduler defaults
2012-05-22 12:32 ` Ian Campbell
@ 2012-05-22 12:58 ` Juergen Gross
2012-05-22 13:05 ` George Dunlap
0 siblings, 1 reply; 24+ messages in thread
From: Juergen Gross @ 2012-05-22 12:58 UTC (permalink / raw)
To: Ian Campbell; +Cc: xen-devel
On 05/22/2012 02:32 PM, Ian Campbell wrote:
> On Tue, 2012-05-22 at 13:29 +0100, Juergen Gross wrote:
>> On 05/22/2012 02:22 PM, Ian Campbell wrote:
>>> On Tue, 2012-05-22 at 10:16 +0100, Juergen Gross wrote:
>>>> Support a new sysctl schedop sub-command to get the scheduling defaults of a
>>>> specific scheduler.
>>> Why is XEN_DOMCTL_SCHEDOP_getinfo not sufficient here?
>>>
>>> Isn't it actually more useful/meaningful to get the current actual
>>> setting rather than a default?
>> When setting the parameters from the config file we have no domain yet which
>> we would have to specify for XEN_DOMCTL_SCHEDOP_getinfo. I didn't want to
>> parse part of the config only after domain creation.
> That seems like another problem with doing this up front instead of
> doing read-modify-write when we come to set the values for the domain.
Do you think it would be okay to parse the scheduler config data _after_
domain creation? If yes, the read-modify-write approach is simple and always
correct. If no, you will have to initialize the parameters to something
scheduler specific, and not domain specific.
>> A default should be okay, as this is what we want to modify. :-)
>> The scheduler should initialize a new domain with this default, of course.
> So I can't use this interface to change the current one of the current
> settings for running a domain, while leaving the others at their current
> value?
I don't understand this sentence :-) ...
> Which interface can I use for that and why isn't it the same as this
> one?
... making it hard to answer to this one. :-)
Nevertheless trying: you can change the scheduling parameters of a running
domain with xl sched-credit/sched-sedf/...
Juergen
--
Juergen Gross Principal Developer Operating Systems
PDG ES&S SWE OS6 Telephone: +49 (0) 89 3222 2967
Fujitsu Technology Solutions e-mail: juergen.gross@ts.fujitsu.com
Domagkstr. 28 Internet: ts.fujitsu.com
D-80807 Muenchen Company details: ts.fujitsu.com/imprint.html
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 1 of 3] Support of getting scheduler defaults
2012-05-22 12:58 ` Juergen Gross
@ 2012-05-22 13:05 ` George Dunlap
2012-05-22 13:16 ` Ian Campbell
2012-05-22 13:40 ` Ian Campbell
0 siblings, 2 replies; 24+ messages in thread
From: George Dunlap @ 2012-05-22 13:05 UTC (permalink / raw)
To: Juergen Gross; +Cc: xen-devel, Ian Campbell
On Tue, May 22, 2012 at 1:58 PM, Juergen Gross
<juergen.gross@ts.fujitsu.com> wrote:
>>> When setting the parameters from the config file we have no domain yet
>>> which
>>> we would have to specify for XEN_DOMCTL_SCHEDOP_getinfo. I didn't want to
>>> parse part of the config only after domain creation.
>>
>> That seems like another problem with doing this up front instead of
>> doing read-modify-write when we come to set the values for the domain.
>
>
> Do you think it would be okay to parse the scheduler config data _after_
> domain creation? If yes, the read-modify-write approach is simple and always
> correct. If no, you will have to initialize the parameters to something
> scheduler specific, and not domain specific.
>
>
>>> A default should be okay, as this is what we want to modify. :-)
>>> The scheduler should initialize a new domain with this default, of
>>> course.
>>
>> So I can't use this interface to change the current one of the current
>> settings for running a domain, while leaving the others at their current
>> value?
>
>
> I don't understand this sentence :-) ...
I think he means this: Suppose after the domain is created, you want
to set the weight. With the proposed interface, you have to manually
read the values and change them yourself. With an interface that
allows -1 to be default, libxl can do that for you (making programming
easier).
>
>
>> Which interface can I use for that and why isn't it the same as this
>> one?
>
>
> ... making it hard to answer to this one. :-)
> Nevertheless trying: you can change the scheduling parameters of a running
> domain with xl sched-credit/sched-sedf/...
I think Ian has convinced me that just using -1 for default values
would be the best option:
* It doesn't require a new special 'what are the defaults" hypercall
* The same interface can be used to set only a subset of the options
(leaving the other ones alone) *after* the domain is created.
Thanks for doing the work to code it up, though Juergen.
-George
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 1 of 3] Support of getting scheduler defaults
2012-05-22 13:05 ` George Dunlap
@ 2012-05-22 13:16 ` Ian Campbell
2012-05-22 13:40 ` Ian Campbell
1 sibling, 0 replies; 24+ messages in thread
From: Ian Campbell @ 2012-05-22 13:16 UTC (permalink / raw)
To: George Dunlap; +Cc: Juergen Gross, xen-devel
On Tue, 2012-05-22 at 14:05 +0100, George Dunlap wrote:
> On Tue, May 22, 2012 at 1:58 PM, Juergen Gross
> <juergen.gross@ts.fujitsu.com> wrote:
> >>> When setting the parameters from the config file we have no domain yet
> >>> which
> >>> we would have to specify for XEN_DOMCTL_SCHEDOP_getinfo. I didn't want to
> >>> parse part of the config only after domain creation.
> >>
> >> That seems like another problem with doing this up front instead of
> >> doing read-modify-write when we come to set the values for the domain.
> >
> >
> > Do you think it would be okay to parse the scheduler config data _after_
> > domain creation?
No, I don't think there's any sensible way to make this work at the
libxl interface, you'd need to call back out to xl half way through
domain build to ask it to parse the sched params for you.
> If yes, the read-modify-write approach is simple and always
> > correct. If no, you will have to initialize the parameters to something
> > scheduler specific, and not domain specific.
> >
> >
> >>> A default should be okay, as this is what we want to modify. :-)
> >>> The scheduler should initialize a new domain with this default, of
> >>> course.
> >>
> >> So I can't use this interface to change the current one of the current
> >> settings for running a domain, while leaving the others at their current
> >> value?
> >
> >
> > I don't understand this sentence :-) ...
>
> I think he means this: Suppose after the domain is created, you want
> to set the weight. With the proposed interface, you have to manually
> read the values and change them yourself. With an interface that
> allows -1 to be default, libxl can do that for you (making programming
> easier).
Right. And furthermore with this patch the interface for setting the
initial values is different to the one for updating them, since this
patch gets you the defaults which you change.
>
> >
> >
> >> Which interface can I use for that and why isn't it the same as this
> >> one?
> >
> >
> > ... making it hard to answer to this one. :-)
> > Nevertheless trying: you can change the scheduling parameters of a running
> > domain with xl sched-credit/sched-sedf/...
Right, my question above was -- why I can't I use the same libxl
interface as these commands at build time (perhaps internally inside
libxl)
> I think Ian has convinced me that just using -1 for default values
> would be the best option:
> * It doesn't require a new special 'what are the defaults" hypercall
> * The same interface can be used to set only a subset of the options
> (leaving the other ones alone) *after* the domain is created.
>
> Thanks for doing the work to code it up, though Juergen.
>
> -George
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 1 of 3] Support of getting scheduler defaults
2012-05-22 13:05 ` George Dunlap
2012-05-22 13:16 ` Ian Campbell
@ 2012-05-22 13:40 ` Ian Campbell
2012-05-22 14:59 ` Ian Campbell
1 sibling, 1 reply; 24+ messages in thread
From: Ian Campbell @ 2012-05-22 13:40 UTC (permalink / raw)
To: George Dunlap; +Cc: Juergen Gross, xen-devel
On Tue, 2012-05-22 at 14:05 +0100, George Dunlap wrote:
>
> I think Ian has convinced me that just using -1 for default values
> would be the best option:
Perhaps I should code up what I'm talking about for comparison.
Ian.
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 1 of 3] Support of getting scheduler defaults
2012-05-22 13:40 ` Ian Campbell
@ 2012-05-22 14:59 ` Ian Campbell
2012-05-22 23:46 ` Dario Faggioli
0 siblings, 1 reply; 24+ messages in thread
From: Ian Campbell @ 2012-05-22 14:59 UTC (permalink / raw)
To: George Dunlap; +Cc: Juergen Gross, xen-devel
On Tue, 2012-05-22 at 14:40 +0100, Ian Campbell wrote:
> On Tue, 2012-05-22 at 14:05 +0100, George Dunlap wrote:
> >
> > I think Ian has convinced me that just using -1 for default values
> > would be the best option:
>
> Perhaps I should code up what I'm talking about for comparison.
Like the below. Lightly tested with the credit scheduler.
I think CAP is the only one for which 0 is a real valid value, but I'm
not sure (especially with the SEDF ones which didn't have existing
limits checks in the set function I could crib from...).
I'm pretty sure that libxl__sched_set_params needs to get the correct
scheduler for the particular domain, but I've no idea how to get that...
8<---------------------------
# HG changeset patch
# User Ian Campbell <ian.campbell@citrix.com>
# Date 1337698727 -3600
# Node ID 355030f95eb313605a0e43aa7328e731b28a28b3
# Parent 426bbf58cea4559464b6e5d3ff0f65324a5f5926
libxl: make it possible to explicitly specify default sched params
To do so we define a descriminating value which is never a valid real value for
each parameter.
While there:
- remove tslice_ms and ratelimit_us from libxl_sched_params and from the xl
domain config parser. These are global scheduler properties, not per-domain
ones (and were never read in any case).
- removed libxl_sched_*_domain in favour of libxl_sched_params.
- rename libxl_sched_params to libxl_sched_domain_params for clarity.
- use this new functionality for the various xl commands which set sched
parameters, which saves an explicit read-modify-write in xl.
- removed call of xc_domain_getinfolist from a few functions which weren't
actually using the result (looks like a cut and paste error)
- fix xl which was setting period for a variety of different config keys.
Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
diff -r 426bbf58cea4 -r 355030f95eb3 tools/libxl/libxl.c
--- a/tools/libxl/libxl.c Tue May 22 14:19:07 2012 +0100
+++ b/tools/libxl/libxl.c Tue May 22 15:58:47 2012 +0100
@@ -3168,19 +3168,19 @@ libxl_scheduler libxl_get_scheduler(libx
}
int libxl_sched_credit_domain_get(libxl_ctx *ctx, uint32_t domid,
- libxl_sched_credit_domain *scinfo)
+ libxl_sched_domain_params *scinfo)
{
struct xen_domctl_sched_credit sdom;
int rc;
- libxl_sched_credit_domain_init(scinfo);
-
rc = xc_sched_credit_domain_get(ctx->xch, domid, &sdom);
if (rc != 0) {
LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "getting domain sched credit");
return ERROR_FAIL;
}
+ libxl_sched_domain_params_init(scinfo);
+ scinfo->sched = LIBXL_SCHEDULER_CREDIT;
scinfo->weight = sdom.weight;
scinfo->cap = sdom.cap;
@@ -3188,7 +3188,7 @@ int libxl_sched_credit_domain_get(libxl_
}
int libxl_sched_credit_domain_set(libxl_ctx *ctx, uint32_t domid,
- libxl_sched_credit_domain *scinfo)
+ libxl_sched_domain_params *scinfo)
{
struct xen_domctl_sched_credit sdom;
xc_domaininfo_t domaininfo;
@@ -3202,22 +3202,33 @@ int libxl_sched_credit_domain_set(libxl_
if (rc != 1 || domaininfo.domain != domid)
return ERROR_INVAL;
-
- if (scinfo->weight < 1 || scinfo->weight > 65535) {
- LIBXL__LOG(ctx, LIBXL__LOG_ERROR,
- "Cpu weight out of range, valid values are within range from 1 to 65535");
- return ERROR_INVAL;
+ rc = xc_sched_credit_domain_get(ctx->xch, domid, &sdom);
+ if (rc != 0) {
+ LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "getting domain sched credit");
+ return ERROR_FAIL;
}
- if (scinfo->cap < 0 || scinfo->cap > (domaininfo.max_vcpu_id + 1) * 100) {
- LIBXL__LOG(ctx, LIBXL__LOG_ERROR,
- "Cpu cap out of range, valid range is from 0 to %d for specified number of vcpus",
- ((domaininfo.max_vcpu_id + 1) * 100));
- return ERROR_INVAL;
+ if (scinfo->weight != LIBXL_SCHED_DOMAIN_PARAM_WEIGHT_DEFAULT) {
+ if (scinfo->weight < 1 || scinfo->weight > 65535) {
+ LIBXL__LOG(ctx, LIBXL__LOG_ERROR,
+ "Cpu weight out of range, "
+ "valid values are within range from 1 to 65535");
+ return ERROR_INVAL;
+ }
+ sdom.weight = scinfo->weight;
}
- sdom.weight = scinfo->weight;
- sdom.cap = scinfo->cap;
+ if (scinfo->cap != LIBXL_SCHED_DOMAIN_PARAM_CAP_DEFAULT) {
+ if (scinfo->cap < 0
+ || scinfo->cap > (domaininfo.max_vcpu_id + 1) * 100) {
+ LIBXL__LOG(ctx, LIBXL__LOG_ERROR,
+ "Cpu cap out of range, "
+ "valid range is from 0 to %d for specified number of vcpus",
+ ((domaininfo.max_vcpu_id + 1) * 100));
+ return ERROR_INVAL;
+ }
+ sdom.cap = scinfo->cap;
+ }
rc = xc_sched_credit_domain_set(ctx->xch, domid, &sdom);
if ( rc < 0 ) {
@@ -3290,13 +3301,11 @@ int libxl_sched_credit_params_set(libxl_
}
int libxl_sched_credit2_domain_get(libxl_ctx *ctx, uint32_t domid,
- libxl_sched_credit2_domain *scinfo)
+ libxl_sched_domain_params *scinfo)
{
struct xen_domctl_sched_credit2 sdom;
int rc;
- libxl_sched_credit2_domain_init(scinfo);
-
rc = xc_sched_credit2_domain_get(ctx->xch, domid, &sdom);
if (rc != 0) {
LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR,
@@ -3304,36 +3313,37 @@ int libxl_sched_credit2_domain_get(libxl
return ERROR_FAIL;
}
+ libxl_sched_domain_params_init(scinfo);
+ scinfo->sched = LIBXL_SCHEDULER_CREDIT2;
scinfo->weight = sdom.weight;
return 0;
}
int libxl_sched_credit2_domain_set(libxl_ctx *ctx, uint32_t domid,
- libxl_sched_credit2_domain *scinfo)
+ libxl_sched_domain_params *scinfo)
{
struct xen_domctl_sched_credit2 sdom;
- xc_domaininfo_t domaininfo;
int rc;
- rc = xc_domain_getinfolist(ctx->xch, domid, 1, &domaininfo);
- if (rc < 0) {
- LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "getting domain info list");
+ rc = xc_sched_credit2_domain_get(ctx->xch, domid, &sdom);
+ if (rc != 0) {
+ LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR,
+ "getting domain sched credit2");
return ERROR_FAIL;
}
- if (rc != 1 || domaininfo.domain != domid)
- return ERROR_INVAL;
-
-
- if (scinfo->weight < 1 || scinfo->weight > 65535) {
- LIBXL__LOG_ERRNOVAL(ctx, LIBXL__LOG_ERROR, rc,
- "Cpu weight out of range, valid values are within range from "
- "1 to 65535");
- return ERROR_INVAL;
+
+ if (scinfo->weight != LIBXL_SCHED_DOMAIN_PARAM_WEIGHT_DEFAULT) {
+ if (scinfo->weight < 1 || scinfo->weight > 65535) {
+ LIBXL__LOG(ctx, LIBXL__LOG_ERROR,
+ "Cpu weight out of range, "
+ "valid values are within range from "
+ "1 to 65535");
+ return ERROR_INVAL;
+ }
+ sdom.weight = scinfo->weight;
}
- sdom.weight = scinfo->weight;
-
rc = xc_sched_credit2_domain_set(ctx->xch, domid, &sdom);
if ( rc < 0 ) {
LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR,
@@ -3345,7 +3355,7 @@ int libxl_sched_credit2_domain_set(libxl
}
int libxl_sched_sedf_domain_get(libxl_ctx *ctx, uint32_t domid,
- libxl_sched_sedf_domain *scinfo)
+ libxl_sched_domain_params *scinfo)
{
uint64_t period;
uint64_t slice;
@@ -3354,8 +3364,6 @@ int libxl_sched_sedf_domain_get(libxl_ct
uint16_t weight;
int rc;
- libxl_sched_sedf_domain_init(scinfo);
-
rc = xc_sedf_domain_get(ctx->xch, domid, &period, &slice, &latency,
&extratime, &weight);
if (rc != 0) {
@@ -3363,6 +3371,8 @@ int libxl_sched_sedf_domain_get(libxl_ct
return ERROR_FAIL;
}
+ libxl_sched_domain_params_init(scinfo);
+ scinfo->sched = LIBXL_SCHEDULER_SEDF;
scinfo->period = period / 1000000;
scinfo->slice = slice / 1000000;
scinfo->latency = latency / 1000000;
@@ -3373,24 +3383,37 @@ int libxl_sched_sedf_domain_get(libxl_ct
}
int libxl_sched_sedf_domain_set(libxl_ctx *ctx, uint32_t domid,
- libxl_sched_sedf_domain *scinfo)
+ libxl_sched_domain_params *scinfo)
{
- xc_domaininfo_t domaininfo;
- int rc;
-
- rc = xc_domain_getinfolist(ctx->xch, domid, 1, &domaininfo);
- if (rc < 0) {
- LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "getting domain info list");
+ uint64_t period;
+ uint64_t slice;
+ uint64_t latency;
+ uint16_t extratime;
+ uint16_t weight;
+
+ int ret;
+
+ ret = xc_sedf_domain_get(ctx->xch, domid, &period, &slice, &latency,
+ &extratime, &weight);
+ if (ret != 0) {
+ LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "getting domain sched sedf");
return ERROR_FAIL;
}
- if (rc != 1 || domaininfo.domain != domid)
- return ERROR_INVAL;
-
-
- rc = xc_sedf_domain_set(ctx->xch, domid, scinfo->period * 1000000,
- scinfo->slice * 1000000, scinfo->latency * 1000000,
- scinfo->extratime, scinfo->weight);
- if ( rc < 0 ) {
+
+ if (scinfo->period != LIBXL_SCHED_DOMAIN_PARAM_PERIOD_DEFAULT)
+ period = scinfo->period * 1000000;
+ if (scinfo->slice != LIBXL_SCHED_DOMAIN_PARAM_SLICE_DEFAULT)
+ period = scinfo->slice * 1000000;
+ if (scinfo->latency != LIBXL_SCHED_DOMAIN_PARAM_LATENCY_DEFAULT)
+ period = scinfo->latency * 1000000;
+ if (scinfo->extratime != LIBXL_SCHED_DOMAIN_PARAM_EXTRATIME_DEFAULT)
+ period = scinfo->extratime;
+ if (scinfo->weight != LIBXL_SCHED_DOMAIN_PARAM_WEIGHT_DEFAULT)
+ period = scinfo->weight;
+
+ ret = xc_sedf_domain_set(ctx->xch, domid, period, slice, latency,
+ extratime, weight);
+ if ( ret < 0 ) {
LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "setting domain sched sedf");
return ERROR_FAIL;
}
diff -r 426bbf58cea4 -r 355030f95eb3 tools/libxl/libxl.h
--- a/tools/libxl/libxl.h Tue May 22 14:19:07 2012 +0100
+++ b/tools/libxl/libxl.h Tue May 22 15:58:47 2012 +0100
@@ -805,23 +805,33 @@ int libxl_set_vcpuonline(libxl_ctx *ctx,
libxl_scheduler libxl_get_scheduler(libxl_ctx *ctx);
-
-int libxl_sched_credit_domain_get(libxl_ctx *ctx, uint32_t domid,
- libxl_sched_credit_domain *scinfo);
-int libxl_sched_credit_domain_set(libxl_ctx *ctx, uint32_t domid,
- libxl_sched_credit_domain *scinfo);
+/* Per-scheduler parameters */
int libxl_sched_credit_params_get(libxl_ctx *ctx, uint32_t poolid,
libxl_sched_credit_params *scinfo);
int libxl_sched_credit_params_set(libxl_ctx *ctx, uint32_t poolid,
libxl_sched_credit_params *scinfo);
+
+/* Scheduler Per-domain parameters */
+
+#define LIBXL_SCHED_DOMAIN_PARAM_WEIGHT_DEFAULT 0
+#define LIBXL_SCHED_DOMAIN_PARAM_CAP_DEFAULT -1
+#define LIBXL_SCHED_DOMAIN_PARAM_PERIOD_DEFAULT 0
+#define LIBXL_SCHED_DOMAIN_PARAM_SLICE_DEFAULT 0
+#define LIBXL_SCHED_DOMAIN_PARAM_LATENCY_DEFAULT 0
+#define LIBXL_SCHED_DOMAIN_PARAM_EXTRATIME_DEFAULT 0
+
+int libxl_sched_credit_domain_get(libxl_ctx *ctx, uint32_t domid,
+ libxl_sched_domain_params *scinfo);
+int libxl_sched_credit_domain_set(libxl_ctx *ctx, uint32_t domid,
+ libxl_sched_domain_params *scinfo);
int libxl_sched_credit2_domain_get(libxl_ctx *ctx, uint32_t domid,
- libxl_sched_credit2_domain *scinfo);
+ libxl_sched_domain_params *scinfo);
int libxl_sched_credit2_domain_set(libxl_ctx *ctx, uint32_t domid,
- libxl_sched_credit2_domain *scinfo);
+ libxl_sched_domain_params *scinfo);
int libxl_sched_sedf_domain_get(libxl_ctx *ctx, uint32_t domid,
- libxl_sched_sedf_domain *scinfo);
+ libxl_sched_domain_params *scinfo);
int libxl_sched_sedf_domain_set(libxl_ctx *ctx, uint32_t domid,
- libxl_sched_sedf_domain *scinfo);
+ libxl_sched_domain_params *scinfo);
int libxl_send_trigger(libxl_ctx *ctx, uint32_t domid,
libxl_trigger trigger, uint32_t vcpuid);
int libxl_send_sysrq(libxl_ctx *ctx, uint32_t domid, char sysrq);
diff -r 426bbf58cea4 -r 355030f95eb3 tools/libxl/libxl_dom.c
--- a/tools/libxl/libxl_dom.c Tue May 22 14:19:07 2012 +0100
+++ b/tools/libxl/libxl_dom.c Tue May 22 15:58:47 2012 +0100
@@ -42,36 +42,30 @@ libxl_domain_type libxl__domain_type(lib
return LIBXL_DOMAIN_TYPE_PV;
}
-int libxl__sched_set_params(libxl__gc *gc, uint32_t domid, libxl_sched_params *scparams)
+int libxl__sched_set_params(libxl__gc *gc, uint32_t domid,
+ libxl_sched_domain_params *scparams)
{
libxl_ctx *ctx = libxl__gc_owner(gc);
- libxl_scheduler sched;
- libxl_sched_sedf_domain sedf_info;
- libxl_sched_credit_domain credit_info;
- libxl_sched_credit2_domain credit2_info;
+ libxl_scheduler sched = scparams->sched;
int ret;
- sched = libxl_get_scheduler (ctx);
+ if (sched == LIBXL_SCHEDULER_UNKNOWN)
+ sched = libxl_get_scheduler(ctx);
+
switch (sched) {
case LIBXL_SCHEDULER_SEDF:
- sedf_info.period = scparams->period;
- sedf_info.slice = scparams->slice;
- sedf_info.latency = scparams->latency;
- sedf_info.extratime = scparams->extratime;
- sedf_info.weight = scparams->weight;
- ret=libxl_sched_sedf_domain_set(ctx, domid, &sedf_info);
- break;
+ ret=libxl_sched_sedf_domain_set(ctx, domid, scparams);
+ break;
case LIBXL_SCHEDULER_CREDIT:
- credit_info.weight = scparams->weight;
- credit_info.cap = scparams->cap;
- ret=libxl_sched_credit_domain_set(ctx, domid, &credit_info);
- break;
+ ret=libxl_sched_credit_domain_set(ctx, domid, scparams);
+ break;
case LIBXL_SCHEDULER_CREDIT2:
- credit2_info.weight = scparams->weight;
- ret=libxl_sched_credit2_domain_set(ctx, domid, &credit2_info);
- break;
+ ret=libxl_sched_credit2_domain_set(ctx, domid, scparams);
+ break;
default:
- ret=-1;
+ LOG(ERROR, "Unknown scheduler");
+ ret=ERROR_INVAL;
+ break;
}
return ret;
}
diff -r 426bbf58cea4 -r 355030f95eb3 tools/libxl/libxl_internal.h
--- a/tools/libxl/libxl_internal.h Tue May 22 14:19:07 2012 +0100
+++ b/tools/libxl/libxl_internal.h Tue May 22 15:58:47 2012 +0100
@@ -716,7 +716,7 @@ _hidden int libxl__atfork_init(libxl_ctx
/* from xl_dom */
_hidden libxl_domain_type libxl__domain_type(libxl__gc *gc, uint32_t domid);
_hidden int libxl__domain_shutdown_reason(libxl__gc *gc, uint32_t domid);
-_hidden int libxl__sched_set_params(libxl__gc *gc, uint32_t domid, libxl_sched_params *scparams);
+_hidden int libxl__sched_set_params(libxl__gc *gc, uint32_t domid, libxl_sched_domain_params *scparams);
#define LIBXL__DOMAIN_IS_TYPE(gc, domid, type) \
libxl__domain_type((gc), (domid)) == LIBXL_DOMAIN_TYPE_##type
typedef struct {
diff -r 426bbf58cea4 -r 355030f95eb3 tools/libxl/libxl_types.idl
--- a/tools/libxl/libxl_types.idl Tue May 22 14:19:07 2012 +0100
+++ b/tools/libxl/libxl_types.idl Tue May 22 15:58:47 2012 +0100
@@ -109,7 +109,9 @@ libxl_bios_type = Enumeration("bios_type
])
# Consistent with values defined in domctl.h
+# Except unknown which is special.
libxl_scheduler = Enumeration("scheduler", [
+ (0, "unknown"),
(4, "sedf"),
(5, "credit"),
(6, "credit2"),
@@ -224,15 +226,14 @@ libxl_domain_create_info = Struct("domai
MemKB = UInt(64, init_val = "LIBXL_MEMKB_DEFAULT")
-libxl_sched_params = Struct("sched_params",[
- ("weight", integer),
- ("cap", integer),
- ("tslice_ms", integer),
- ("ratelimit_us", integer),
- ("period", integer),
- ("slice", integer),
- ("latency", integer),
- ("extratime", integer),
+libxl_sched_domain_params = Struct("sched_domain_params",[
+ ("sched", libxl_scheduler),
+ ("weight", integer, {'init_val': 'LIBXL_SCHED_DOMAIN_PARAM_WEIGHT_DEFAULT'}),
+ ("cap", integer, {'init_val': 'LIBXL_SCHED_DOMAIN_PARAM_CAP_DEFAULT'}),
+ ("period", integer, {'init_val': 'LIBXL_SCHED_DOMAIN_PARAM_PERIOD_DEFAULT'}),
+ ("slice", integer, {'init_val': 'LIBXL_SCHED_DOMAIN_PARAM_SLICE_DEFAULT'}),
+ ("latency", integer, {'init_val': 'LIBXL_SCHED_DOMAIN_PARAM_LATENCY_DEFAULT'}),
+ ("extratime", integer, {'init_val': 'LIBXL_SCHED_DOMAIN_PARAM_EXTRATIME_DEFAULT'}),
], dir=DIR_IN)
# Instances of libxl_file_reference contained in this struct which
@@ -267,7 +268,7 @@ libxl_domain_build_info = Struct("domain
# extra parameters pass directly to qemu for HVM guest, NULL terminated
("extra_hvm", libxl_string_list),
# parameters for all type of scheduler
- ("sched_params", libxl_sched_params),
+ ("sched_params", libxl_sched_domain_params),
("u", KeyedUnion(None, libxl_domain_type, "type",
[("hvm", Struct(None, [("firmware", string),
@@ -432,28 +433,11 @@ libxl_cputopology = Struct("cputopology"
("node", uint32),
], dir=DIR_OUT)
-libxl_sched_credit_domain = Struct("sched_credit_domain", [
- ("weight", integer),
- ("cap", integer),
- ])
-
libxl_sched_credit_params = Struct("sched_credit_params", [
("tslice_ms", integer),
("ratelimit_us", integer),
], dispose_fn=None)
-libxl_sched_credit2_domain = Struct("sched_credit2_domain", [
- ("weight", integer),
- ])
-
-libxl_sched_sedf_domain = Struct("sched_sedf_domain", [
- ("period", integer),
- ("slice", integer),
- ("latency", integer),
- ("extratime", integer),
- ("weight", integer),
- ])
-
libxl_domain_remus_info = Struct("domain_remus_info",[
("interval", integer),
("blackhole", bool),
diff -r 426bbf58cea4 -r 355030f95eb3 tools/libxl/xl_cmdimpl.c
--- a/tools/libxl/xl_cmdimpl.c Tue May 22 14:19:07 2012 +0100
+++ b/tools/libxl/xl_cmdimpl.c Tue May 22 15:58:47 2012 +0100
@@ -627,23 +627,20 @@ static void parse_config_data(const char
libxl_domain_build_info_init_type(b_info, c_info->type);
- /* the following is the actual config parsing with overriding values in the structures */
+ /* the following is the actual config parsing with overriding
+ * values in the structures */
if (!xlu_cfg_get_long (config, "cpu_weight", &l, 0))
b_info->sched_params.weight = l;
if (!xlu_cfg_get_long (config, "cap", &l, 0))
b_info->sched_params.cap = l;
- if (!xlu_cfg_get_long (config, "tslice_ms", &l, 0))
- b_info->sched_params.tslice_ms = l;
- if (!xlu_cfg_get_long (config, "ratelimit_us", &l, 0))
- b_info->sched_params.ratelimit_us = l;
if (!xlu_cfg_get_long (config, "period", &l, 0))
b_info->sched_params.period = l;
if (!xlu_cfg_get_long (config, "slice", &l, 0))
- b_info->sched_params.period = l;
+ b_info->sched_params.slice = l;
if (!xlu_cfg_get_long (config, "latency", &l, 0))
- b_info->sched_params.period = l;
+ b_info->sched_params.latency = l;
if (!xlu_cfg_get_long (config, "extratime", &l, 0))
- b_info->sched_params.period = l;
+ b_info->sched_params.extratime = l;
if (!xlu_cfg_get_long (config, "vcpus", &l, 0)) {
b_info->max_vcpus = l;
@@ -4355,7 +4352,7 @@ int main_sharing(int argc, char **argv)
return 0;
}
-static int sched_credit_domain_get(int domid, libxl_sched_credit_domain *scinfo)
+static int sched_credit_domain_get(int domid, libxl_sched_domain_params *scinfo)
{
int rc;
@@ -4366,7 +4363,7 @@ static int sched_credit_domain_get(int d
return rc;
}
-static int sched_credit_domain_set(int domid, libxl_sched_credit_domain *scinfo)
+static int sched_credit_domain_set(int domid, libxl_sched_domain_params *scinfo)
{
int rc;
@@ -4402,7 +4399,7 @@ static int sched_credit_params_get(int p
static int sched_credit_domain_output(int domid)
{
char *domname;
- libxl_sched_credit_domain scinfo;
+ libxl_sched_domain_params scinfo;
int rc;
if (domid < 0) {
@@ -4419,7 +4416,7 @@ static int sched_credit_domain_output(in
scinfo.weight,
scinfo.cap);
free(domname);
- libxl_sched_credit_domain_dispose(&scinfo);
+ libxl_sched_domain_params_dispose(&scinfo);
return 0;
}
@@ -4445,7 +4442,7 @@ static int sched_credit_pool_output(uint
}
static int sched_credit2_domain_get(
- int domid, libxl_sched_credit2_domain *scinfo)
+ int domid, libxl_sched_domain_params *scinfo)
{
int rc;
@@ -4457,7 +4454,7 @@ static int sched_credit2_domain_get(
}
static int sched_credit2_domain_set(
- int domid, libxl_sched_credit2_domain *scinfo)
+ int domid, libxl_sched_domain_params *scinfo)
{
int rc;
@@ -4472,7 +4469,7 @@ static int sched_credit2_domain_output(
int domid)
{
char *domname;
- libxl_sched_credit2_domain scinfo;
+ libxl_sched_domain_params scinfo;
int rc;
if (domid < 0) {
@@ -4488,12 +4485,12 @@ static int sched_credit2_domain_output(
domid,
scinfo.weight);
free(domname);
- libxl_sched_credit2_domain_dispose(&scinfo);
+ libxl_sched_domain_params_dispose(&scinfo);
return 0;
}
static int sched_sedf_domain_get(
- int domid, libxl_sched_sedf_domain *scinfo)
+ int domid, libxl_sched_domain_params *scinfo)
{
int rc;
@@ -4505,7 +4502,7 @@ static int sched_sedf_domain_get(
}
static int sched_sedf_domain_set(
- int domid, libxl_sched_sedf_domain *scinfo)
+ int domid, libxl_sched_domain_params *scinfo)
{
int rc;
@@ -4519,7 +4516,7 @@ static int sched_sedf_domain_output(
int domid)
{
char *domname;
- libxl_sched_sedf_domain scinfo;
+ libxl_sched_domain_params scinfo;
int rc;
if (domid < 0) {
@@ -4540,7 +4537,7 @@ static int sched_sedf_domain_output(
scinfo.extratime,
scinfo.weight);
free(domname);
- libxl_sched_sedf_domain_dispose(&scinfo);
+ libxl_sched_domain_params_dispose(&scinfo);
return 0;
}
@@ -4620,7 +4617,6 @@ static int sched_domain_output(libxl_sch
*/
int main_sched_credit(int argc, char **argv)
{
- libxl_sched_credit_domain scinfo;
const char *dom = NULL;
const char *cpupool = NULL;
int weight = 256, cap = 0, opt_w = 0, opt_c = 0;
@@ -4695,7 +4691,7 @@ int main_sched_credit(int argc, char **a
}
if (opt_s) {
- libxl_sched_credit_params scparam;
+ libxl_sched_credit_params scparam;
uint32_t poolid = 0;
if (cpupool) {
@@ -4731,20 +4727,19 @@ int main_sched_credit(int argc, char **a
} else {
find_domain(dom);
- rc = sched_credit_domain_get(domid, &scinfo);
- if (rc)
- return -rc;
-
if (!opt_w && !opt_c) { /* output credit scheduler info */
sched_credit_domain_output(-1);
return -sched_credit_domain_output(domid);
} else { /* set credit scheduler paramaters */
+ libxl_sched_domain_params scinfo;
+ libxl_sched_domain_params_init(&scinfo);
+ scinfo.sched = LIBXL_SCHEDULER_CREDIT;
if (opt_w)
scinfo.weight = weight;
if (opt_c)
scinfo.cap = cap;
rc = sched_credit_domain_set(domid, &scinfo);
- libxl_sched_credit_domain_dispose(&scinfo);
+ libxl_sched_domain_params_dispose(&scinfo);
if (rc)
return -rc;
}
@@ -4755,7 +4750,6 @@ int main_sched_credit(int argc, char **a
int main_sched_credit2(int argc, char **argv)
{
- libxl_sched_credit2_domain scinfo;
const char *dom = NULL;
const char *cpupool = NULL;
int weight = 256, opt_w = 0;
@@ -4810,18 +4804,17 @@ int main_sched_credit2(int argc, char **
} else {
find_domain(dom);
- rc = sched_credit2_domain_get(domid, &scinfo);
- if (rc)
- return -rc;
-
if (!opt_w) { /* output credit2 scheduler info */
sched_credit2_domain_output(-1);
return -sched_credit2_domain_output(domid);
} else { /* set credit2 scheduler paramaters */
+ libxl_sched_domain_params scinfo;
+ libxl_sched_domain_params_init(&scinfo);
+ scinfo.sched = LIBXL_SCHEDULER_CREDIT2;
if (opt_w)
scinfo.weight = weight;
rc = sched_credit2_domain_set(domid, &scinfo);
- libxl_sched_credit2_domain_dispose(&scinfo);
+ libxl_sched_domain_params_dispose(&scinfo);
if (rc)
return -rc;
}
@@ -4832,7 +4825,6 @@ int main_sched_credit2(int argc, char **
int main_sched_sedf(int argc, char **argv)
{
- libxl_sched_sedf_domain scinfo;
const char *dom = NULL;
const char *cpupool = NULL;
int period = 0, opt_p = 0;
@@ -4915,15 +4907,15 @@ int main_sched_sedf(int argc, char **arg
} else {
find_domain(dom);
- rc = sched_sedf_domain_get(domid, &scinfo);
- if (rc)
- return -rc;
-
if (!opt_p && !opt_s && !opt_l && !opt_e && !opt_w) {
/* output sedf scheduler info */
sched_sedf_domain_output(-1);
return -sched_sedf_domain_output(domid);
} else { /* set sedf scheduler paramaters */
+ libxl_sched_domain_params scinfo;
+ libxl_sched_domain_params_init(&scinfo);
+ scinfo.sched = LIBXL_SCHEDULER_SEDF;
+
if (opt_p) {
scinfo.period = period;
scinfo.weight = 0;
@@ -4942,7 +4934,7 @@ int main_sched_sedf(int argc, char **arg
scinfo.slice = 0;
}
rc = sched_sedf_domain_set(domid, &scinfo);
- libxl_sched_sedf_domain_dispose(&scinfo);
+ libxl_sched_domain_params_dispose(&scinfo);
if (rc)
return -rc;
}
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 3 of 3] full support of setting scheduler parameters on domain creation
2012-05-22 12:39 ` Juergen Gross
2012-05-22 12:51 ` Ian Campbell
@ 2012-05-22 22:15 ` Dario Faggioli
1 sibling, 0 replies; 24+ messages in thread
From: Dario Faggioli @ 2012-05-22 22:15 UTC (permalink / raw)
To: Juergen Gross; +Cc: xen-devel, Ian Campbell
[-- Attachment #1.1: Type: text/plain, Size: 2430 bytes --]
On Tue, 2012-05-22 at 14:39 +0200, Juergen Gross wrote:
> > This interface really makes libxl_sched_params differ from all the other
> > libxl structs (which have a public _init function and an internal
> > setdefaults function). I'm not really sure its justified either, I was
> > under the impression that you'd found that there were useful
> > discriminating values?
>
> Dario opted for this solution, so I proposed a patch implementing it.
> I prefer this solution, too, as it isn't exporting scheduler internals to
> the tools.
>
Yep. I agree with Ian that having two different procedures for the first
as compared to the subsequent operations on the scheduling parameters is
bad, but still, as a matter of my personal taste, I'd prefer no to have
things belonging to the hypervisor/scheduler replicated in the
toolstack, although they're just simple default values. It's still
something you need to always remember to check for consistency, or bad
things will happen! :-/
However, Ian's approach seems clean and nice as well, and I'm not really
sure which one I like most, so don't count me when deciding, I'm fine
with both.
> >> int libxl__sched_set_params(libxl__gc *gc, uint32_t domid,
> >> libxl_sched_params *scparams)
> >> {
> >> libxl_ctx *ctx = libxl__gc_owner(gc);
> >> - libxl_scheduler sched;
> >> libxl_sched_sedf_domain sedf_info;
> >> libxl_sched_credit_domain credit_info;
> >> libxl_sched_credit2_domain credit2_info;
> >> int ret;
> >>
> >> - sched = libxl_get_scheduler (ctx);
> >> - switch (sched) {
> >> + switch (scparams->sched) {
> > What happens if scparams->sched is not the scheduler used for this
> > domain? Should it either be checked or set somewhere?
>
> The check would be the same as the original setting of scparams->sched.
> Setting of the scheduler parameters will be rejected by the hypervisor if the
> scheduler does not match.
>
I was thinking this should go through finding out what domid's cpupool
is and then checking that scheduler, as it is being done somewhere
else... Isn't this the case?
Regards,
Dario
--
<<This happens because I choose it to happen!>> (Raistlin Majere)
-----------------------------------------------------------------
Dario Faggioli, Ph.D, http://retis.sssup.it/people/faggioli
Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)
[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 198 bytes --]
[-- Attachment #2: Type: text/plain, Size: 126 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 1 of 3] Support of getting scheduler defaults
2012-05-22 14:59 ` Ian Campbell
@ 2012-05-22 23:46 ` Dario Faggioli
2012-05-23 5:34 ` Juergen Gross
0 siblings, 1 reply; 24+ messages in thread
From: Dario Faggioli @ 2012-05-22 23:46 UTC (permalink / raw)
To: Ian Campbell; +Cc: George Dunlap, xen-devel, Juergen Gross
[-- Attachment #1.1: Type: text/plain, Size: 6052 bytes --]
On Tue, 2012-05-22 at 15:59 +0100, Ian Campbell wrote:
> Like the below. Lightly tested with the credit scheduler.
>
> I think CAP is the only one for which 0 is a real valid value, but I'm
> not sure (especially with the SEDF ones which didn't have existing
> limits checks in the set function I could crib from...).
>
Yep, that's because they're mostly time values. xen/common/sched_sedf.c
hosts some ranges for some of them, but I'm not sure we want to
propagate those:
#define PERIOD_MAX MILLISECS(10000) /* 10s */
#define PERIOD_MIN (MICROSECS(10)) /* 10us */
#define SLICE_MIN (MICROSECS(5)) /* 5us */
Also, extratime is a flag, so I think 0 and 1 are both meaningful
values, maybe we can go for -1 as for cap (I'll try and let you know).
> I'm pretty sure that libxl__sched_set_params needs to get the correct
> scheduler for the particular domain, but I've no idea how to get that...
>
Again, I was thinking something like what Juergen did here could help
(go getting the scheduler of the cpupool the domain belongs to)... O am
I misunderstanding the issue?
+ poolinfo = libxl_list_cpupool(ctx, &n_pools);
+ if (!poolinfo)
+ return ERROR_NOMEM;
+
+ ret = ERROR_INVAL;
+ for (p = 0; p < n_pools; p++) {
+ if (poolinfo[p].poolid == poolid) {
+ scparams->sched = poolinfo[p].sched;
+ ret = 0;
+ }
+ libxl_cpupoolinfo_dispose(poolinfo + p);
+ }
+
> 8<---------------------------
>
> # HG changeset patch
> # User Ian Campbell <ian.campbell@citrix.com>
> # Date 1337698727 -3600
> # Node ID 355030f95eb313605a0e43aa7328e731b28a28b3
> # Parent 426bbf58cea4559464b6e5d3ff0f65324a5f5926
> libxl: make it possible to explicitly specify default sched params
>
> To do so we define a descriminating value which is never a valid real value for
> each parameter.
>
> While there:
>
> - remove tslice_ms and ratelimit_us from libxl_sched_params and from the xl
> domain config parser. These are global scheduler properties, not per-domain
> ones (and were never read in any case).
> - removed libxl_sched_*_domain in favour of libxl_sched_params.
> - rename libxl_sched_params to libxl_sched_domain_params for clarity.
> - use this new functionality for the various xl commands which set sched
> parameters, which saves an explicit read-modify-write in xl.
> - removed call of xc_domain_getinfolist from a few functions which weren't
> actually using the result (looks like a cut and paste error)
> - fix xl which was setting period for a variety of different config keys.
>
> Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
>
> diff -r 426bbf58cea4 -r 355030f95eb3 tools/libxl/libxl.c
> --- a/tools/libxl/libxl.c Tue May 22 14:19:07 2012 +0100
> +++ b/tools/libxl/libxl.c Tue May 22 15:58:47 2012 +0100
> @@ -3168,19 +3168,19 @@ libxl_scheduler libxl_get_scheduler(libx
> }
>
<snip>
>
> int libxl_sched_sedf_domain_set(libxl_ctx *ctx, uint32_t domid,
> - libxl_sched_sedf_domain *scinfo)
> + libxl_sched_domain_params *scinfo)
> {
> - xc_domaininfo_t domaininfo;
> - int rc;
> -
> - rc = xc_domain_getinfolist(ctx->xch, domid, 1, &domaininfo);
> - if (rc < 0) {
> - LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "getting domain info list");
> + uint64_t period;
> + uint64_t slice;
> + uint64_t latency;
> + uint16_t extratime;
> + uint16_t weight;
> +
> + int ret;
> +
> + ret = xc_sedf_domain_get(ctx->xch, domid, &period, &slice, &latency,
> + &extratime, &weight);
> + if (ret != 0) {
> + LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "getting domain sched sedf");
> return ERROR_FAIL;
> }
> - if (rc != 1 || domaininfo.domain != domid)
> - return ERROR_INVAL;
> -
> -
> - rc = xc_sedf_domain_set(ctx->xch, domid, scinfo->period * 1000000,
> - scinfo->slice * 1000000, scinfo->latency * 1000000,
> - scinfo->extratime, scinfo->weight);
> - if ( rc < 0 ) {
> +
> + if (scinfo->period != LIBXL_SCHED_DOMAIN_PARAM_PERIOD_DEFAULT)
> + period = scinfo->period * 1000000;
> + if (scinfo->slice != LIBXL_SCHED_DOMAIN_PARAM_SLICE_DEFAULT)
> + period = scinfo->slice * 1000000;
> + if (scinfo->latency != LIBXL_SCHED_DOMAIN_PARAM_LATENCY_DEFAULT)
> + period = scinfo->latency * 1000000;
> + if (scinfo->extratime != LIBXL_SCHED_DOMAIN_PARAM_EXTRATIME_DEFAULT)
> + period = scinfo->extratime;
> + if (scinfo->weight != LIBXL_SCHED_DOMAIN_PARAM_WEIGHT_DEFAULT)
> + period = scinfo->weight;
> +
> + ret = xc_sedf_domain_set(ctx->xch, domid, period, slice, latency,
> + extratime, weight);
> + if ( ret < 0 ) {
> LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "setting domain sched sedf");
> return ERROR_FAIL;
> }
>
# xl create vm1.cfg
Parsing config from vm1.cfg
libxl: error: libxl.c:3417:libxl_sched_sedf_domain_set: setting domain sched sedf: Invalid argument
And I'm getting the above independently on what I put in the config file
(valid params, no params at all, etc.). I also can't change the
scheduling parameters of a domain on-line anymore:
# xl sched-sedf -d 3 -p 100 -s 50
libxl: error: libxl.c:3417:libxl_sched_sedf_domain_set: setting domain sched sedf: Invalid argument
libxl_sched_sedf_domain_set failed.
I'll try digging a bit more into this ASAP.
On more thing, are we ok with the _set command failing and he domain
being created anyway? Or perhaps the whole process should just abort?
Regards,
Dario
--
<<This happens because I choose it to happen!>> (Raistlin Majere)
-----------------------------------------------------------------
Dario Faggioli, Ph.D, http://retis.sssup.it/people/faggioli
Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)
[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 198 bytes --]
[-- Attachment #2: Type: text/plain, Size: 126 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 1 of 3] Support of getting scheduler defaults
2012-05-22 23:46 ` Dario Faggioli
@ 2012-05-23 5:34 ` Juergen Gross
2012-05-23 7:22 ` Dario Faggioli
0 siblings, 1 reply; 24+ messages in thread
From: Juergen Gross @ 2012-05-23 5:34 UTC (permalink / raw)
To: Dario Faggioli; +Cc: George Dunlap, xen-devel, Ian Campbell
On 05/23/2012 01:46 AM, Dario Faggioli wrote:
> On Tue, 2012-05-22 at 15:59 +0100, Ian Campbell wrote:
>> Like the below. Lightly tested with the credit scheduler.
>>
>> I think CAP is the only one for which 0 is a real valid value, but I'm
>> not sure (especially with the SEDF ones which didn't have existing
>> limits checks in the set function I could crib from...).
>>
> Yep, that's because they're mostly time values. xen/common/sched_sedf.c
> hosts some ranges for some of them, but I'm not sure we want to
> propagate those:
>
> #define PERIOD_MAX MILLISECS(10000) /* 10s */
> #define PERIOD_MIN (MICROSECS(10)) /* 10us */
> #define SLICE_MIN (MICROSECS(5)) /* 5us */
I think this should remain in the hypervisor only.
> Also, extratime is a flag, so I think 0 and 1 are both meaningful
> values, maybe we can go for -1 as for cap (I'll try and let you know).
Why not -1 for all values?
>> I'm pretty sure that libxl__sched_set_params needs to get the correct
>> scheduler for the particular domain, but I've no idea how to get that...
>>
> Again, I was thinking something like what Juergen did here could help
> (go getting the scheduler of the cpupool the domain belongs to)... O am
> I misunderstanding the issue?
>
> + poolinfo = libxl_list_cpupool(ctx,&n_pools);
> + if (!poolinfo)
> + return ERROR_NOMEM;
> +
> + ret = ERROR_INVAL;
> + for (p = 0; p< n_pools; p++) {
> + if (poolinfo[p].poolid == poolid) {
> + scparams->sched = poolinfo[p].sched;
> + ret = 0;
> + }
> + libxl_cpupoolinfo_dispose(poolinfo + p);
> + }
> +
This was exactly the purpose of the sniplet.
>> 8<---------------------------
>>
>> # HG changeset patch
>> # User Ian Campbell<ian.campbell@citrix.com>
>> # Date 1337698727 -3600
>> # Node ID 355030f95eb313605a0e43aa7328e731b28a28b3
>> # Parent 426bbf58cea4559464b6e5d3ff0f65324a5f5926
>> libxl: make it possible to explicitly specify default sched params
>>
>> To do so we define a descriminating value which is never a valid real value for
>> each parameter.
>>
>> While there:
>>
>> - remove tslice_ms and ratelimit_us from libxl_sched_params and from the xl
>> domain config parser. These are global scheduler properties, not per-domain
>> ones (and were never read in any case).
>> - removed libxl_sched_*_domain in favour of libxl_sched_params.
>> - rename libxl_sched_params to libxl_sched_domain_params for clarity.
>> - use this new functionality for the various xl commands which set sched
>> parameters, which saves an explicit read-modify-write in xl.
>> - removed call of xc_domain_getinfolist from a few functions which weren't
>> actually using the result (looks like a cut and paste error)
>> - fix xl which was setting period for a variety of different config keys.
>>
>> Signed-off-by: Ian Campbell<ian.campbell@citrix.com>
>>
>> diff -r 426bbf58cea4 -r 355030f95eb3 tools/libxl/libxl.c
>> --- a/tools/libxl/libxl.c Tue May 22 14:19:07 2012 +0100
>> +++ b/tools/libxl/libxl.c Tue May 22 15:58:47 2012 +0100
>> @@ -3168,19 +3168,19 @@ libxl_scheduler libxl_get_scheduler(libx
>> }
>>
> <snip>
>>
>> int libxl_sched_sedf_domain_set(libxl_ctx *ctx, uint32_t domid,
>> - libxl_sched_sedf_domain *scinfo)
>> + libxl_sched_domain_params *scinfo)
>> {
>> - xc_domaininfo_t domaininfo;
>> - int rc;
>> -
>> - rc = xc_domain_getinfolist(ctx->xch, domid, 1,&domaininfo);
>> - if (rc< 0) {
>> - LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "getting domain info list");
>> + uint64_t period;
>> + uint64_t slice;
>> + uint64_t latency;
>> + uint16_t extratime;
>> + uint16_t weight;
>> +
>> + int ret;
>> +
>> + ret = xc_sedf_domain_get(ctx->xch, domid,&period,&slice,&latency,
>> +&extratime,&weight);
>> + if (ret != 0) {
>> + LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "getting domain sched sedf");
>> return ERROR_FAIL;
>> }
>> - if (rc != 1 || domaininfo.domain != domid)
>> - return ERROR_INVAL;
>> -
>> -
>> - rc = xc_sedf_domain_set(ctx->xch, domid, scinfo->period * 1000000,
>> - scinfo->slice * 1000000, scinfo->latency * 1000000,
>> - scinfo->extratime, scinfo->weight);
>> - if ( rc< 0 ) {
>> +
>> + if (scinfo->period != LIBXL_SCHED_DOMAIN_PARAM_PERIOD_DEFAULT)
>> + period = scinfo->period * 1000000;
>> + if (scinfo->slice != LIBXL_SCHED_DOMAIN_PARAM_SLICE_DEFAULT)
>> + period = scinfo->slice * 1000000;
>> + if (scinfo->latency != LIBXL_SCHED_DOMAIN_PARAM_LATENCY_DEFAULT)
>> + period = scinfo->latency * 1000000;
>> + if (scinfo->extratime != LIBXL_SCHED_DOMAIN_PARAM_EXTRATIME_DEFAULT)
>> + period = scinfo->extratime;
>> + if (scinfo->weight != LIBXL_SCHED_DOMAIN_PARAM_WEIGHT_DEFAULT)
>> + period = scinfo->weight;
>> +
>> + ret = xc_sedf_domain_set(ctx->xch, domid, period, slice, latency,
>> + extratime, weight);
>> + if ( ret< 0 ) {
>> LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "setting domain sched sedf");
>> return ERROR_FAIL;
>> }
>>
> # xl create vm1.cfg
> Parsing config from vm1.cfg
> libxl: error: libxl.c:3417:libxl_sched_sedf_domain_set: setting domain sched sedf: Invalid argument
>
> And I'm getting the above independently on what I put in the config file
> (valid params, no params at all, etc.). I also can't change the
> scheduling parameters of a domain on-line anymore:
>
> # xl sched-sedf -d 3 -p 100 -s 50
> libxl: error: libxl.c:3417:libxl_sched_sedf_domain_set: setting domain sched sedf: Invalid argument
> libxl_sched_sedf_domain_set failed.
>
> I'll try digging a bit more into this ASAP.
It's easy: Ian repeated an error he (and I) corrected elsewhere:
He's always setting period, regardless of the changed parameter...
> On more thing, are we ok with the _set command failing and he domain
> being created anyway? Or perhaps the whole process should just abort?
Perhaps a warning might be okay.
Juergen
--
Juergen Gross Principal Developer Operating Systems
PDG ES&S SWE OS6 Telephone: +49 (0) 89 3222 2967
Fujitsu Technology Solutions e-mail: juergen.gross@ts.fujitsu.com
Domagkstr. 28 Internet: ts.fujitsu.com
D-80807 Muenchen Company details: ts.fujitsu.com/imprint.html
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 1 of 3] Support of getting scheduler defaults
2012-05-23 5:34 ` Juergen Gross
@ 2012-05-23 7:22 ` Dario Faggioli
2012-05-23 7:41 ` Ian Campbell
2012-05-23 8:48 ` Juergen Gross
0 siblings, 2 replies; 24+ messages in thread
From: Dario Faggioli @ 2012-05-23 7:22 UTC (permalink / raw)
To: Juergen Gross; +Cc: George Dunlap, xen-devel, Ian Campbell
[-- Attachment #1.1: Type: text/plain, Size: 4052 bytes --]
On Wed, 2012-05-23 at 07:34 +0200, Juergen Gross wrote:
> > #define PERIOD_MAX MILLISECS(10000) /* 10s */
> > #define PERIOD_MIN (MICROSECS(10)) /* 10us */
> > #define SLICE_MIN (MICROSECS(5)) /* 5us */
>
> I think this should remain in the hypervisor only.
>
Me too.
> > Also, extratime is a flag, so I think 0 and 1 are both meaningful
> > values, maybe we can go for -1 as for cap (I'll try and let you know).
>
> Why not -1 for all values?
>
Would work, I guess, unless there's some collision with big weights
represented on short unsigned value (not sure it's like that, and I've
always been bad at this kind of math! :-P).
> >> I'm pretty sure that libxl__sched_set_params needs to get the correct
> >> scheduler for the particular domain, but I've no idea how to get that...
> >>
> > Again, I was thinking something like what Juergen did here could help
> > (go getting the scheduler of the cpupool the domain belongs to)... O am
> > I misunderstanding the issue?
> >
> > + poolinfo = libxl_list_cpupool(ctx,&n_pools);
> > + if (!poolinfo)
> > + return ERROR_NOMEM;
> > +
> > + ret = ERROR_INVAL;
> > + for (p = 0; p< n_pools; p++) {
> > + if (poolinfo[p].poolid == poolid) {
> > + scparams->sched = poolinfo[p].sched;
> > + ret = 0;
> > + }
> > + libxl_cpupoolinfo_dispose(poolinfo + p);
> > + }
> > +
>
> This was exactly the purpose of the sniplet.
>
Good to hear that. :-)
> > # xl create vm1.cfg
> > Parsing config from vm1.cfg
> > libxl: error: libxl.c:3417:libxl_sched_sedf_domain_set: setting domain sched sedf: Invalid argument
> >
> > And I'm getting the above independently on what I put in the config file
> > (valid params, no params at all, etc.). I also can't change the
> > scheduling parameters of a domain on-line anymore:
> >
> > # xl sched-sedf -d 3 -p 100 -s 50
> > libxl: error: libxl.c:3417:libxl_sched_sedf_domain_set: setting domain sched sedf: Invalid argument
> > libxl_sched_sedf_domain_set failed.
> >
> > I'll try digging a bit more into this ASAP.
>
> It's easy: Ian repeated an error he (and I) corrected elsewhere:
> He's always setting period, regardless of the changed parameter...
>
Not sure, I think Ian's patch fixes that:
--- a/tools/libxl/xl_cmdimpl.c Tue May 22 14:19:07 2012 +0100
+++ b/tools/libxl/xl_cmdimpl.c Tue May 22 15:58:47 2012 +0100
@@ -627,23 +627,20 @@ static void parse_config_data(const char
libxl_domain_build_info_init_type(b_info, c_info->type);
- /* the following is the actual config parsing with overriding values in the structures */
+ /* the following is the actual config parsing with overriding
+ * values in the structures */
if (!xlu_cfg_get_long (config, "cpu_weight", &l, 0))
b_info->sched_params.weight = l;
if (!xlu_cfg_get_long (config, "cap", &l, 0))
b_info->sched_params.cap = l;
- if (!xlu_cfg_get_long (config, "tslice_ms", &l, 0))
- b_info->sched_params.tslice_ms = l;
- if (!xlu_cfg_get_long (config, "ratelimit_us", &l, 0))
- b_info->sched_params.ratelimit_us = l;
if (!xlu_cfg_get_long (config, "period", &l, 0))
b_info->sched_params.period = l;
if (!xlu_cfg_get_long (config, "slice", &l, 0))
- b_info->sched_params.period = l;
+ b_info->sched_params.slice = l;
if (!xlu_cfg_get_long (config, "latency", &l, 0))
- b_info->sched_params.period = l;
+ b_info->sched_params.latency = l;
if (!xlu_cfg_get_long (config, "extratime", &l, 0))
- b_info->sched_params.period = l;
+ b_info->sched_params.extratime = l;
So it has to be something else...
Thanks and Regards,
Dario
--
<<This happens because I choose it to happen!>> (Raistlin Majere)
-----------------------------------------------------------------
Dario Faggioli, Ph.D, http://retis.sssup.it/people/faggioli
Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)
[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 198 bytes --]
[-- Attachment #2: Type: text/plain, Size: 126 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 1 of 3] Support of getting scheduler defaults
2012-05-23 7:22 ` Dario Faggioli
@ 2012-05-23 7:41 ` Ian Campbell
2012-05-23 8:45 ` Juergen Gross
2012-05-23 8:48 ` Juergen Gross
1 sibling, 1 reply; 24+ messages in thread
From: Ian Campbell @ 2012-05-23 7:41 UTC (permalink / raw)
To: Dario Faggioli; +Cc: Juergen Gross, xen-devel, George Dunlap
On Wed, 2012-05-23 at 08:22 +0100, Dario Faggioli wrote:
> On Wed, 2012-05-23 at 07:34 +0200, Juergen Gross wrote:
> > > #define PERIOD_MAX MILLISECS(10000) /* 10s */
> > > #define PERIOD_MIN (MICROSECS(10)) /* 10us */
> > > #define SLICE_MIN (MICROSECS(5)) /* 5us */
> >
> > I think this should remain in the hypervisor only.
> >
> Me too.
Agreed.
> > > Also, extratime is a flag, so I think 0 and 1 are both meaningful
> > > values, maybe we can go for -1 as for cap (I'll try and let you know).
> >
> > Why not -1 for all values?
> >
> Would work, I guess, unless there's some collision with big weights
> represented on short unsigned value (not sure it's like that, and I've
> always been bad at this kind of math! :-P).
In general in libxl where we can use 0 for the default we do so. It's
not a strong preference though and it's mainly historical from before
the IDL supported init_val.
> > >> I'm pretty sure that libxl__sched_set_params needs to get the correct
> > >> scheduler for the particular domain, but I've no idea how to get that...
> > >>
> > > Again, I was thinking something like what Juergen did here could help
> > > (go getting the scheduler of the cpupool the domain belongs to)... O am
> > > I misunderstanding the issue?
> > >
> > > + poolinfo = libxl_list_cpupool(ctx,&n_pools);
> > > + if (!poolinfo)
> > > + return ERROR_NOMEM;
> > > +
> > > + ret = ERROR_INVAL;
> > > + for (p = 0; p< n_pools; p++) {
> > > + if (poolinfo[p].poolid == poolid) {
> > > + scparams->sched = poolinfo[p].sched;
> > > + ret = 0;
> > > + }
> > > + libxl_cpupoolinfo_dispose(poolinfo + p);
> > > + }
> > > +
> >
> > This was exactly the purpose of the sniplet.
> >
> Good to hear that. :-)
Great, I'll make a suitably named function out of it.
> > > # xl create vm1.cfg
> > > Parsing config from vm1.cfg
> > > libxl: error: libxl.c:3417:libxl_sched_sedf_domain_set: setting domain sched sedf: Invalid argument
> > >
> > > And I'm getting the above independently on what I put in the config file
> > > (valid params, no params at all, etc.). I also can't change the
> > > scheduling parameters of a domain on-line anymore:
> > >
> > > # xl sched-sedf -d 3 -p 100 -s 50
> > > libxl: error: libxl.c:3417:libxl_sched_sedf_domain_set: setting domain sched sedf: Invalid argument
> > > libxl_sched_sedf_domain_set failed.
> > >
> > > I'll try digging a bit more into this ASAP.
> >
> > It's easy: Ian repeated an error he (and I) corrected elsewhere:
> > He's always setting period, regardless of the changed parameter...
> >
> Not sure, I think Ian's patch fixes that:
I thought so to!
> --- a/tools/libxl/xl_cmdimpl.c Tue May 22 14:19:07 2012 +0100
> +++ b/tools/libxl/xl_cmdimpl.c Tue May 22 15:58:47 2012 +0100
> @@ -627,23 +627,20 @@ static void parse_config_data(const char
>
> libxl_domain_build_info_init_type(b_info, c_info->type);
>
> - /* the following is the actual config parsing with overriding values in the structures */
> + /* the following is the actual config parsing with overriding
> + * values in the structures */
> if (!xlu_cfg_get_long (config, "cpu_weight", &l, 0))
> b_info->sched_params.weight = l;
> if (!xlu_cfg_get_long (config, "cap", &l, 0))
> b_info->sched_params.cap = l;
> - if (!xlu_cfg_get_long (config, "tslice_ms", &l, 0))
> - b_info->sched_params.tslice_ms = l;
> - if (!xlu_cfg_get_long (config, "ratelimit_us", &l, 0))
> - b_info->sched_params.ratelimit_us = l;
> if (!xlu_cfg_get_long (config, "period", &l, 0))
> b_info->sched_params.period = l;
> if (!xlu_cfg_get_long (config, "slice", &l, 0))
> - b_info->sched_params.period = l;
> + b_info->sched_params.slice = l;
> if (!xlu_cfg_get_long (config, "latency", &l, 0))
> - b_info->sched_params.period = l;
> + b_info->sched_params.latency = l;
> if (!xlu_cfg_get_long (config, "extratime", &l, 0))
> - b_info->sched_params.period = l;
> + b_info->sched_params.extratime = l;
>
>
> So it has to be something else...
>
> Thanks and Regards,
> Dario
>
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 1 of 3] Support of getting scheduler defaults
2012-05-23 7:41 ` Ian Campbell
@ 2012-05-23 8:45 ` Juergen Gross
2012-05-23 9:17 ` Ian Campbell
2012-05-23 10:18 ` Dario Faggioli
0 siblings, 2 replies; 24+ messages in thread
From: Juergen Gross @ 2012-05-23 8:45 UTC (permalink / raw)
To: Ian Campbell; +Cc: George Dunlap, xen-devel, Dario Faggioli
On 05/23/2012 09:41 AM, Ian Campbell wrote:
> # xl create vm1.cfg
> Parsing config from vm1.cfg
> libxl: error: libxl.c:3417:libxl_sched_sedf_domain_set: setting domain sched sedf: Invalid argument
>
> And I'm getting the above independently on what I put in the config file
> (valid params, no params at all, etc.). I also can't change the
> scheduling parameters of a domain on-line anymore:
>
> # xl sched-sedf -d 3 -p 100 -s 50
> libxl: error: libxl.c:3417:libxl_sched_sedf_domain_set: setting domain sched sedf: Invalid argument
> libxl_sched_sedf_domain_set failed.
>
> I'll try digging a bit more into this ASAP.
>>> It's easy: Ian repeated an error he (and I) corrected elsewhere:
>>> He's always setting period, regardless of the changed parameter...
>>>
>> Not sure, I think Ian's patch fixes that:
> I thought so to!
It fixes the original error.
>> --- a/tools/libxl/xl_cmdimpl.c Tue May 22 14:19:07 2012 +0100
>> +++ b/tools/libxl/xl_cmdimpl.c Tue May 22 15:58:47 2012 +0100
>> @@ -627,23 +627,20 @@ static void parse_config_data(const char
>>
>> libxl_domain_build_info_init_type(b_info, c_info->type);
>>
>> - /* the following is the actual config parsing with overriding values in the structures */
>> + /* the following is the actual config parsing with overriding
>> + * values in the structures */
>> if (!xlu_cfg_get_long (config, "cpu_weight",&l, 0))
>> b_info->sched_params.weight = l;
>> if (!xlu_cfg_get_long (config, "cap",&l, 0))
>> b_info->sched_params.cap = l;
>> - if (!xlu_cfg_get_long (config, "tslice_ms",&l, 0))
>> - b_info->sched_params.tslice_ms = l;
>> - if (!xlu_cfg_get_long (config, "ratelimit_us",&l, 0))
>> - b_info->sched_params.ratelimit_us = l;
>> if (!xlu_cfg_get_long (config, "period",&l, 0))
>> b_info->sched_params.period = l;
>> if (!xlu_cfg_get_long (config, "slice",&l, 0))
>> - b_info->sched_params.period = l;
>> + b_info->sched_params.slice = l;
>> if (!xlu_cfg_get_long (config, "latency",&l, 0))
>> - b_info->sched_params.period = l;
>> + b_info->sched_params.latency = l;
>> if (!xlu_cfg_get_long (config, "extratime",&l, 0))
>> - b_info->sched_params.period = l;
>> + b_info->sched_params.extratime = l;
>>
>>
>> So it has to be something else...
Oh yes:
+
+ if (scinfo->period != LIBXL_SCHED_DOMAIN_PARAM_PERIOD_DEFAULT)
+ period = scinfo->period * 1000000;
+ if (scinfo->slice != LIBXL_SCHED_DOMAIN_PARAM_SLICE_DEFAULT)
+ period = scinfo->slice * 1000000;
+ if (scinfo->latency != LIBXL_SCHED_DOMAIN_PARAM_LATENCY_DEFAULT)
+ period = scinfo->latency * 1000000;
+ if (scinfo->extratime != LIBXL_SCHED_DOMAIN_PARAM_EXTRATIME_DEFAULT)
+ period = scinfo->extratime;
+ if (scinfo->weight != LIBXL_SCHED_DOMAIN_PARAM_WEIGHT_DEFAULT)
+ period = scinfo->weight;
Regardless which parameter was changed, it is always 'period' which is set
to the changed value.
Juergen
--
Juergen Gross Principal Developer Operating Systems
PDG ES&S SWE OS6 Telephone: +49 (0) 89 3222 2967
Fujitsu Technology Solutions e-mail: juergen.gross@ts.fujitsu.com
Domagkstr. 28 Internet: ts.fujitsu.com
D-80807 Muenchen Company details: ts.fujitsu.com/imprint.html
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 1 of 3] Support of getting scheduler defaults
2012-05-23 7:22 ` Dario Faggioli
2012-05-23 7:41 ` Ian Campbell
@ 2012-05-23 8:48 ` Juergen Gross
1 sibling, 0 replies; 24+ messages in thread
From: Juergen Gross @ 2012-05-23 8:48 UTC (permalink / raw)
To: Dario Faggioli; +Cc: George Dunlap, xen-devel, Ian Campbell
On 05/23/2012 09:22 AM, Dario Faggioli wrote:
> On Wed, 2012-05-23 at 07:34 +0200, Juergen Gross wrote:
>>> #define PERIOD_MAX MILLISECS(10000) /* 10s */
>>> #define PERIOD_MIN (MICROSECS(10)) /* 10us */
>>> #define SLICE_MIN (MICROSECS(5)) /* 5us */
>> I think this should remain in the hypervisor only.
>>
> Me too.
>
>>> Also, extratime is a flag, so I think 0 and 1 are both meaningful
>>> values, maybe we can go for -1 as for cap (I'll try and let you know).
>> Why not -1 for all values?
>>
> Would work, I guess, unless there's some collision with big weights
> represented on short unsigned value (not sure it's like that, and I've
> always been bad at this kind of math! :-P).
The -1 would be set in scparams only, and there it is an integer. It should
never be written to the "real" scheduler parameters passed to the hypervisor.
Juergen
--
Juergen Gross Principal Developer Operating Systems
PDG ES&S SWE OS6 Telephone: +49 (0) 89 3222 2967
Fujitsu Technology Solutions e-mail: juergen.gross@ts.fujitsu.com
Domagkstr. 28 Internet: ts.fujitsu.com
D-80807 Muenchen Company details: ts.fujitsu.com/imprint.html
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 1 of 3] Support of getting scheduler defaults
2012-05-23 8:45 ` Juergen Gross
@ 2012-05-23 9:17 ` Ian Campbell
2012-05-23 10:18 ` Dario Faggioli
1 sibling, 0 replies; 24+ messages in thread
From: Ian Campbell @ 2012-05-23 9:17 UTC (permalink / raw)
To: Juergen Gross; +Cc: George Dunlap, xen-devel, Dario Faggioli
On Wed, 2012-05-23 at 09:45 +0100, Juergen Gross wrote:
> Oh yes:
>
> +
> + if (scinfo->period != LIBXL_SCHED_DOMAIN_PARAM_PERIOD_DEFAULT)
> + period = scinfo->period * 1000000;
> + if (scinfo->slice != LIBXL_SCHED_DOMAIN_PARAM_SLICE_DEFAULT)
> + period = scinfo->slice * 1000000;
> + if (scinfo->latency != LIBXL_SCHED_DOMAIN_PARAM_LATENCY_DEFAULT)
> + period = scinfo->latency * 1000000;
> + if (scinfo->extratime != LIBXL_SCHED_DOMAIN_PARAM_EXTRATIME_DEFAULT)
> + period = scinfo->extratime;
> + if (scinfo->weight != LIBXL_SCHED_DOMAIN_PARAM_WEIGHT_DEFAULT)
> + period = scinfo->weight;
>
> Regardless which parameter was changed, it is always 'period' which is set
> to the changed value.
Oh balls! Thanks for noticing, I'll fix this and resend...
Ian.
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 1 of 3] Support of getting scheduler defaults
2012-05-23 8:45 ` Juergen Gross
2012-05-23 9:17 ` Ian Campbell
@ 2012-05-23 10:18 ` Dario Faggioli
1 sibling, 0 replies; 24+ messages in thread
From: Dario Faggioli @ 2012-05-23 10:18 UTC (permalink / raw)
To: Juergen Gross; +Cc: George Dunlap, xen-devel, Ian Campbell
[-- Attachment #1.1: Type: text/plain, Size: 1453 bytes --]
On Wed, 2012-05-23 at 10:45 +0200, Juergen Gross wrote:
> >> Not sure, I think Ian's patch fixes that:
> > I thought so to!
>
> It fixes the original error.
>
Yep, that was what I was looking at ...
> >> So it has to be something else...
>
> Oh yes:
>
> +
> + if (scinfo->period != LIBXL_SCHED_DOMAIN_PARAM_PERIOD_DEFAULT)
> + period = scinfo->period * 1000000;
> + if (scinfo->slice != LIBXL_SCHED_DOMAIN_PARAM_SLICE_DEFAULT)
> + period = scinfo->slice * 1000000;
> + if (scinfo->latency != LIBXL_SCHED_DOMAIN_PARAM_LATENCY_DEFAULT)
> + period = scinfo->latency * 1000000;
> + if (scinfo->extratime != LIBXL_SCHED_DOMAIN_PARAM_EXTRATIME_DEFAULT)
> + period = scinfo->extratime;
> + if (scinfo->weight != LIBXL_SCHED_DOMAIN_PARAM_WEIGHT_DEFAULT)
> + period = scinfo->weight;
>
> Regardless which parameter was changed, it is always 'period' which is set
> to the changed value.
>
... And completely ignored this! :-P
However, I'm still having some issues (even after fixing this)!
I'll keep looking at this, testing the series Ian just sent and let you
know.
Thanks and Regards,
Dario
--
<<This happens because I choose it to happen!>> (Raistlin Majere)
-----------------------------------------------------------------
Dario Faggioli, Ph.D, http://retis.sssup.it/people/faggioli
Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)
[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 198 bytes --]
[-- Attachment #2: Type: text/plain, Size: 126 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 24+ messages in thread
end of thread, other threads:[~2012-05-23 10:18 UTC | newest]
Thread overview: 24+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-05-22 9:16 [PATCH 0 of 3] support of setting scheduler parameters on domain creation Juergen Gross
2012-05-22 9:16 ` [PATCH 1 of 3] Support of getting scheduler defaults Juergen Gross
2012-05-22 12:22 ` Ian Campbell
2012-05-22 12:29 ` Juergen Gross
2012-05-22 12:32 ` Ian Campbell
2012-05-22 12:58 ` Juergen Gross
2012-05-22 13:05 ` George Dunlap
2012-05-22 13:16 ` Ian Campbell
2012-05-22 13:40 ` Ian Campbell
2012-05-22 14:59 ` Ian Campbell
2012-05-22 23:46 ` Dario Faggioli
2012-05-23 5:34 ` Juergen Gross
2012-05-23 7:22 ` Dario Faggioli
2012-05-23 7:41 ` Ian Campbell
2012-05-23 8:45 ` Juergen Gross
2012-05-23 9:17 ` Ian Campbell
2012-05-23 10:18 ` Dario Faggioli
2012-05-23 8:48 ` Juergen Gross
2012-05-22 9:16 ` [PATCH 2 of 3] Support getting scheduler defaults in libxc Juergen Gross
2012-05-22 9:16 ` [PATCH 3 of 3] full support of setting scheduler parameters on domain creation Juergen Gross
2012-05-22 12:30 ` Ian Campbell
2012-05-22 12:39 ` Juergen Gross
2012-05-22 12:51 ` Ian Campbell
2012-05-22 22:15 ` Dario Faggioli
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.