* [PATCH v3 for Xen 4.6 0/4] Enable per-VCPU parameter settings for RTDS scheduler
@ 2015-06-29 2:44 Chong Li
2015-06-29 2:44 ` [PATCH v3 for Xen 4.6 1/4] xen: enable " Chong Li
` (4 more replies)
0 siblings, 5 replies; 39+ messages in thread
From: Chong Li @ 2015-06-29 2:44 UTC (permalink / raw)
To: xen-devel
Cc: Chong Li, wei.liu2, george.dunlap, dario.faggioli, ian.jackson,
ian.campbell, mengxu, jbeulich, lichong659, dgolomb
[Goal]
The current xl sched-rtds tool can only set the VCPUs of a domain to the same parameter
although the scheduler supports VCPUs with different parameters. This patchset is to
enable xl sched-rtds tool to configure the VCPUs of a domain with different parameters.
This per-VCPU settings can be used in many scenarios. For example, based on Dario's statement in our pervious discussion(http://lists.xen.org/archives/html/xen-devel/2014-09/msg00423.html), if there are two real-time applications, which have different timing requirements, running in a multi-VCPU guest domain, it is beneficial to pin these two applications to two seperate VCPUs with different scheduling parameters.
What this patchset includes is a wanted and planned feature for RTDS scheudler(http://wiki.xenproject.org/wiki/RTDS-Based-Scheduler) in Xen 4.6. The interface design of the xl sched-rtds tool is based on Meng's previous discussion with Dario, George and Wei(http://lists.xen.org/archives/html/xen-devel/2015-02/msg02606.html). Basically, there are two main changes:
1) in xl, we create an array that records all VCPUs whose parameters are about to modify or output.
2) in libxl, we receive the array and call different xc functions to handle it.
3) in xen and libxc, we use XEN_DOMCTL_SCHEDOP_getvcpuinfo/putvcpuinfo(introduced by this patchset) as the hypercall for per-VCPU operations(get/set method).
[Usage]
With this patchset in use, xl sched-rtds tool can:
1) show the budget and period of each VCPU of each domain, by using "xl sched-rtds -v all" command. An example would be like:
# xl sched-rtds -v all
Cpupool Pool-0: sched=RTDS
Name ID VCPU Period Budget
Domain-0 0 0 10000 4000
vm1 1 0 300 150
vm1 1 1 400 200
vm1 1 2 10000 4000
vm1 1 3 1000 500
vm2 2 0 10000 4000
vm2 2 1 10000 4000
2) show the budget and period of each VCPU of a specific domain, by using,
e.g., "xl sched-rtds -d vm1 -v all" command. The output would be like:
# xl sched-rtds -d vm1 -v all
Name ID VCPU Period Budget
vm1 1 0 300 150
vm1 1 1 400 200
vm1 1 2 10000 4000
vm1 1 3 1000 500
To show a subset of the parameters of the VCPUs of a specific domain, please use, e.g.,"xl sched-rtds -d vm1 -v 0 -v 3" command. The output would be:
# xl sched-rtds -d vm1 -v 0 -v 3
Name ID VCPU Period Budget
vm1 1 0 300 150
vm1 1 3 1000 500
3) Users can set the budget and period of multiple VCPUs of a specific domain
with only one command, e.g., "xl sched-rtds -d vm1 -v 0 -p 100 -b 50 -v 3 -p 300 -b 150".
Users can set all VCPUs with the same parameters, by one command.
e.g., "xl sched-rtds -d vm1 -v all -p 500 -b 250".
---
CC: <dario.faggioli@citrix.com>
CC: <george.dunlap@eu.citrix.com>
CC: <dgolomb@seas.upenn.edu>
CC: <mengxu@cis.upenn.edu>
CC: <jbeulich@suse.com>
CC: <wei.liu2@citrix.com>
CC: <lichong659@gmail.com>
CC: <ian.jackson@eu.citrix.com>
CC: <ian.campbell@eu.citrix.com>
Chong Li (4):
xen: enable per-VCPU parameter settings for RTDS scheduler
libxc: enable per-VCPU parameter settings for RTDS scheduler
libxl: enable per-VCPU parameter settings for RTDS scheduler
xl: enable per-VCPU parameter settings for RTDS scheduler
docs/man/xl.pod.1 | 4 +
tools/libxc/include/xenctrl.h | 9 ++
tools/libxc/xc_csched.c | 4 +-
tools/libxc/xc_csched2.c | 4 +-
tools/libxc/xc_rt.c | 64 +++++++++-
tools/libxc/xc_sedf.c | 4 +-
tools/libxl/libxl.c | 209 +++++++++++++++++++++++++++----
tools/libxl/libxl.h | 17 +++
tools/libxl/libxl_types.idl | 16 +++
tools/libxl/xl_cmdimpl.c | 284 +++++++++++++++++++++++++++++++++++-------
tools/libxl/xl_cmdtable.c | 10 +-
xen/common/Makefile | 1 -
xen/common/domctl.c | 3 +
xen/common/sched_credit.c | 14 +--
xen/common/sched_credit2.c | 6 +-
xen/common/sched_rt.c | 80 +++++++++++-
xen/common/schedule.c | 5 +-
xen/include/public/domctl.h | 64 +++++++---
18 files changed, 680 insertions(+), 118 deletions(-)
--
1.9.1
^ permalink raw reply [flat|nested] 39+ messages in thread
* [PATCH v3 for Xen 4.6 1/4] xen: enable per-VCPU parameter settings for RTDS scheduler
2015-06-29 2:44 [PATCH v3 for Xen 4.6 0/4] Enable per-VCPU parameter settings for RTDS scheduler Chong Li
@ 2015-06-29 2:44 ` Chong Li
2015-07-07 8:59 ` Jan Beulich
2015-07-07 14:51 ` Dario Faggioli
2015-06-29 2:44 ` [PATCH v3 for Xen 4.6 2/4] libxc: " Chong Li
` (3 subsequent siblings)
4 siblings, 2 replies; 39+ messages in thread
From: Chong Li @ 2015-06-29 2:44 UTC (permalink / raw)
To: xen-devel
Cc: Chong Li, Sisu Xi, george.dunlap, dario.faggioli, Meng Xu,
jbeulich, lichong659, dgolomb
Add two hypercalls(XEN_DOMCTL_SCHEDOP_getvcpuinfo/putvcpuinfo) to get/set a domain's
per-VCPU parameters.
Changes on PATCH v2:
1) Change struct xen_domctl_scheduler_op, for transferring per-vcpu parameters
between libxc and hypervisor.
2) Handler of XEN_DOMCTL_SCHEDOP_getinfo now just returns the default budget and period values of RTDS scheduler.
3) Handler of XEN_DOMCTL_SCHEDOP_getvcpuinfo now can return a random subset of the parameters of the VCPUs of a specific domain
Signed-off-by: Chong Li <chong.li@wustl.edu>
Signed-off-by: Meng Xu <mengxu@cis.upenn.edu>
Signed-off-by: Sisu Xi <xisisu@gmail.com>
---
CC: <dario.faggioli@citrix.com>
CC: <george.dunlap@eu.citrix.com>
CC: <dgolomb@seas.upenn.edu>
CC: <mengxu@cis.upenn.edu>
CC: <jbeulich@suse.com>
CC: <lichong659@gmail.com>
---
xen/common/Makefile | 1 -
xen/common/domctl.c | 3 ++
xen/common/sched_credit.c | 14 ++++----
xen/common/sched_credit2.c | 6 ++--
xen/common/sched_rt.c | 80 +++++++++++++++++++++++++++++++++++++++++----
xen/common/schedule.c | 5 +--
xen/include/public/domctl.h | 64 ++++++++++++++++++++++++++----------
7 files changed, 136 insertions(+), 37 deletions(-)
diff --git a/xen/common/Makefile b/xen/common/Makefile
index 1cddebc..3fdf931 100644
--- a/xen/common/Makefile
+++ b/xen/common/Makefile
@@ -31,7 +31,6 @@ obj-y += rbtree.o
obj-y += rcupdate.o
obj-y += sched_credit.o
obj-y += sched_credit2.o
-obj-y += sched_sedf.o
obj-y += sched_arinc653.o
obj-y += sched_rt.o
obj-y += schedule.o
diff --git a/xen/common/domctl.c b/xen/common/domctl.c
index 2a2d203..349f68b 100644
--- a/xen/common/domctl.c
+++ b/xen/common/domctl.c
@@ -839,6 +839,9 @@ long do_domctl(XEN_GUEST_HANDLE_PARAM(xen_domctl_t) u_domctl)
case XEN_DOMCTL_scheduler_op:
ret = sched_adjust(d, &op->u.scheduler_op);
+ if ( ret == -ERESTART )
+ ret = hypercall_create_continuation(
+ __HYPERVISOR_domctl, "h", u_domctl);
copyback = 1;
break;
diff --git a/xen/common/sched_credit.c b/xen/common/sched_credit.c
index 953ecb0..43b086b 100644
--- a/xen/common/sched_credit.c
+++ b/xen/common/sched_credit.c
@@ -1039,25 +1039,25 @@ csched_dom_cntl(
if ( op->cmd == XEN_DOMCTL_SCHEDOP_getinfo )
{
- op->u.credit.weight = sdom->weight;
- op->u.credit.cap = sdom->cap;
+ op->u.d.credit.weight = sdom->weight;
+ op->u.d.credit.cap = sdom->cap;
}
else
{
ASSERT(op->cmd == XEN_DOMCTL_SCHEDOP_putinfo);
- if ( op->u.credit.weight != 0 )
+ if ( op->u.d.credit.weight != 0 )
{
if ( !list_empty(&sdom->active_sdom_elem) )
{
prv->weight -= sdom->weight * sdom->active_vcpu_count;
- prv->weight += op->u.credit.weight * sdom->active_vcpu_count;
+ prv->weight += op->u.d.credit.weight * sdom->active_vcpu_count;
}
- sdom->weight = op->u.credit.weight;
+ sdom->weight = op->u.d.credit.weight;
}
- if ( op->u.credit.cap != (uint16_t)~0U )
- sdom->cap = op->u.credit.cap;
+ if ( op->u.d.credit.cap != (uint16_t)~0U )
+ sdom->cap = op->u.d.credit.cap;
}
diff --git a/xen/common/sched_credit2.c b/xen/common/sched_credit2.c
index 75e0321..8992423 100644
--- a/xen/common/sched_credit2.c
+++ b/xen/common/sched_credit2.c
@@ -1438,20 +1438,20 @@ csched2_dom_cntl(
if ( op->cmd == XEN_DOMCTL_SCHEDOP_getinfo )
{
- op->u.credit2.weight = sdom->weight;
+ op->u.d.credit2.weight = sdom->weight;
}
else
{
ASSERT(op->cmd == XEN_DOMCTL_SCHEDOP_putinfo);
- if ( op->u.credit2.weight != 0 )
+ if ( op->u.d.credit2.weight != 0 )
{
struct list_head *iter;
int old_weight;
old_weight = sdom->weight;
- sdom->weight = op->u.credit2.weight;
+ sdom->weight = op->u.d.credit2.weight;
/* Update weights for vcpus, and max_weight for runqueues on which they reside */
list_for_each ( iter, &sdom->vcpu )
diff --git a/xen/common/sched_rt.c b/xen/common/sched_rt.c
index 4372486..8d1740d 100644
--- a/xen/common/sched_rt.c
+++ b/xen/common/sched_rt.c
@@ -1137,18 +1137,19 @@ rt_dom_cntl(
struct list_head *iter;
unsigned long flags;
int rc = 0;
+ xen_domctl_schedparam_vcpu_t local_sched;
+ unsigned int index;
switch ( op->cmd )
{
case XEN_DOMCTL_SCHEDOP_getinfo:
spin_lock_irqsave(&prv->lock, flags);
- svc = list_entry(sdom->vcpu.next, struct rt_vcpu, sdom_elem);
- op->u.rtds.period = svc->period / MICROSECS(1); /* transfer to us */
- op->u.rtds.budget = svc->budget / MICROSECS(1);
+ op->u.d.rtds.period = RTDS_DEFAULT_PERIOD / MICROSECS(1); /* transfer to us */
+ op->u.d.rtds.budget = RTDS_DEFAULT_BUDGET / MICROSECS(1);
spin_unlock_irqrestore(&prv->lock, flags);
break;
case XEN_DOMCTL_SCHEDOP_putinfo:
- if ( op->u.rtds.period == 0 || op->u.rtds.budget == 0 )
+ if ( op->u.d.rtds.period == 0 || op->u.d.rtds.budget == 0 )
{
rc = -EINVAL;
break;
@@ -1157,8 +1158,75 @@ rt_dom_cntl(
list_for_each( iter, &sdom->vcpu )
{
struct rt_vcpu * svc = list_entry(iter, struct rt_vcpu, sdom_elem);
- svc->period = MICROSECS(op->u.rtds.period); /* transfer to nanosec */
- svc->budget = MICROSECS(op->u.rtds.budget);
+ svc->period = MICROSECS(op->u.d.rtds.period); /* transfer to nanosec */
+ svc->budget = MICROSECS(op->u.d.rtds.budget);
+ }
+ spin_unlock_irqrestore(&prv->lock, flags);
+ break;
+ case XEN_DOMCTL_SCHEDOP_getvcpuinfo:
+ spin_lock_irqsave(&prv->lock, flags);
+ for( index = 0; index < op->u.v.nr_vcpus; index++ )
+ {
+ if ( copy_from_guest_offset(&local_sched,
+ op->u.v.vcpus, index, 1) )
+ {
+ rc = -EFAULT;
+ break;
+ }
+ if ( local_sched.vcpuid >= d->max_vcpus
+ || d->vcpu[local_sched.vcpuid] == NULL )
+ {
+ rc = -EINVAL;
+ break;
+ }
+ svc = rt_vcpu(d->vcpu[local_sched.vcpuid]);
+
+ local_sched.vcpuid = svc->vcpu->vcpu_id;
+ local_sched.s.rtds.budget = svc->budget / MICROSECS(1);
+ local_sched.s.rtds.period = svc->period / MICROSECS(1);
+ if( index >= op->u.v.nr_vcpus ) /* not enough guest buffer*/
+ {
+ rc = -ENOBUFS;
+ break;
+ }
+ if ( copy_to_guest_offset(op->u.v.vcpus, index,
+ &local_sched, 1) )
+ {
+ rc = -EFAULT;
+ break;
+ }
+ if( hypercall_preempt_check() )
+ {
+ rc = -ERESTART;
+ break;
+ }
+ }
+ spin_unlock_irqrestore(&prv->lock, flags);
+ break;
+ case XEN_DOMCTL_SCHEDOP_putvcpuinfo:
+ spin_lock_irqsave(&prv->lock, flags);
+ for( index = 0; index < op->u.v.nr_vcpus; index++ )
+ {
+ if ( copy_from_guest_offset(&local_sched,
+ op->u.v.vcpus, index, 1) )
+ {
+ rc = -EFAULT;
+ break;
+ }
+ if ( local_sched.vcpuid >= d->max_vcpus
+ || d->vcpu[local_sched.vcpuid] == NULL )
+ {
+ rc = -EINVAL;
+ break;
+ }
+ svc = rt_vcpu(d->vcpu[local_sched.vcpuid]);
+ svc->period = MICROSECS(local_sched.s.rtds.period);
+ svc->budget = MICROSECS(local_sched.s.rtds.budget);
+ if( hypercall_preempt_check() )
+ {
+ rc = -ERESTART;
+ break;
+ }
}
spin_unlock_irqrestore(&prv->lock, flags);
break;
diff --git a/xen/common/schedule.c b/xen/common/schedule.c
index ecf1545..159425e 100644
--- a/xen/common/schedule.c
+++ b/xen/common/schedule.c
@@ -65,7 +65,6 @@ DEFINE_PER_CPU(struct schedule_data, schedule_data);
DEFINE_PER_CPU(struct scheduler *, scheduler);
static const struct scheduler *schedulers[] = {
- &sched_sedf_def,
&sched_credit_def,
&sched_credit2_def,
&sched_arinc653_def,
@@ -1054,7 +1053,9 @@ long sched_adjust(struct domain *d, struct xen_domctl_scheduler_op *op)
if ( (op->sched_id != DOM2OP(d)->sched_id) ||
((op->cmd != XEN_DOMCTL_SCHEDOP_putinfo) &&
- (op->cmd != XEN_DOMCTL_SCHEDOP_getinfo)) )
+ (op->cmd != XEN_DOMCTL_SCHEDOP_getinfo) &&
+ (op->cmd != XEN_DOMCTL_SCHEDOP_putvcpuinfo) &&
+ (op->cmd != XEN_DOMCTL_SCHEDOP_getvcpuinfo)) )
return -EINVAL;
/* NB: the pluggable scheduler code needs to take care
diff --git a/xen/include/public/domctl.h b/xen/include/public/domctl.h
index bc45ea5..67a5626 100644
--- a/xen/include/public/domctl.h
+++ b/xen/include/public/domctl.h
@@ -330,31 +330,59 @@ DEFINE_XEN_GUEST_HANDLE(xen_domctl_max_vcpus_t);
#define XEN_SCHEDULER_ARINC653 7
#define XEN_SCHEDULER_RTDS 8
+typedef struct xen_domctl_sched_sedf {
+ uint64_aligned_t period;
+ uint64_aligned_t slice;
+ uint64_aligned_t latency;
+ uint32_t extratime;
+ uint32_t weight;
+} xen_domctl_sched_sedf_t;
+
+typedef struct xen_domctl_sched_credit {
+ uint16_t weight;
+ uint16_t cap;
+} xen_domctl_sched_credit_t;
+
+typedef struct xen_domctl_sched_credit2 {
+ uint16_t weight;
+} xen_domctl_sched_credit2_t;
+
+typedef struct xen_domctl_sched_rtds {
+ uint32_t period;
+ uint32_t budget;
+} xen_domctl_sched_rtds_t;
+
+typedef union xen_domctl_schedparam {
+ xen_domctl_sched_sedf_t sedf;
+ xen_domctl_sched_credit_t credit;
+ xen_domctl_sched_credit2_t credit2;
+ xen_domctl_sched_rtds_t rtds;
+} xen_domctl_schedparam_t;
+
+typedef struct xen_domctl_schedparam_vcpu {
+ union {
+ xen_domctl_sched_credit_t credit;
+ xen_domctl_sched_credit2_t credit2;
+ xen_domctl_sched_rtds_t rtds;
+ } s;
+ uint16_t vcpuid;
+} xen_domctl_schedparam_vcpu_t;
+DEFINE_XEN_GUEST_HANDLE(xen_domctl_schedparam_vcpu_t);
+
/* Set or get info? */
#define XEN_DOMCTL_SCHEDOP_putinfo 0
#define XEN_DOMCTL_SCHEDOP_getinfo 1
+#define XEN_DOMCTL_SCHEDOP_putvcpuinfo 2
+#define XEN_DOMCTL_SCHEDOP_getvcpuinfo 3
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;
- struct xen_domctl_sched_rtds {
- uint32_t period;
- uint32_t budget;
- } rtds;
+ xen_domctl_schedparam_t d;
+ struct {
+ XEN_GUEST_HANDLE_64(xen_domctl_schedparam_vcpu_t) vcpus;
+ uint16_t nr_vcpus;
+ } v;
} u;
};
typedef struct xen_domctl_scheduler_op xen_domctl_scheduler_op_t;
--
1.9.1
^ permalink raw reply related [flat|nested] 39+ messages in thread
* [PATCH v3 for Xen 4.6 2/4] libxc: enable per-VCPU parameter settings for RTDS scheduler
2015-06-29 2:44 [PATCH v3 for Xen 4.6 0/4] Enable per-VCPU parameter settings for RTDS scheduler Chong Li
2015-06-29 2:44 ` [PATCH v3 for Xen 4.6 1/4] xen: enable " Chong Li
@ 2015-06-29 2:44 ` Chong Li
2015-06-30 12:22 ` Ian Campbell
2015-06-29 2:44 ` [PATCH v3 for Xen 4.6 3/4] libxl: " Chong Li
` (2 subsequent siblings)
4 siblings, 1 reply; 39+ messages in thread
From: Chong Li @ 2015-06-29 2:44 UTC (permalink / raw)
To: xen-devel
Cc: Chong Li, wei.liu2, Sisu Xi, george.dunlap, dario.faggioli,
Meng Xu, lichong659, dgolomb
Add xc_sched_rtds_vcpu_get/set functions to interact with Xen to get/set a domain's
per-VCPU parameters.
Changes on PATCH v2:
1) Minor modifications due to the change of struct xen_domctl_scheduler_op.
Signed-off-by: Chong Li <chong.li@wustl.edu>
Signed-off-by: Meng Xu <mengxu@cis.upenn.edu>
Signed-off-by: Sisu Xi <xisisu@gmail.com>
---
CC: <dario.faggioli@citrix.com>
CC: <george.dunlap@eu.citrix.com>
CC: <dgolomb@seas.upenn.edu>
CC: <mengxu@cis.upenn.edu>
CC: <wei.liu2@citrix.com>
CC: <lichong659@gmail.com>
---
tools/libxc/include/xenctrl.h | 9 ++++++
tools/libxc/xc_csched.c | 4 +--
tools/libxc/xc_csched2.c | 4 +--
tools/libxc/xc_rt.c | 64 +++++++++++++++++++++++++++++++++++++++----
tools/libxc/xc_sedf.c | 4 +--
5 files changed, 74 insertions(+), 11 deletions(-)
diff --git a/tools/libxc/include/xenctrl.h b/tools/libxc/include/xenctrl.h
index d1d2ab3..58f1a7a 100644
--- a/tools/libxc/include/xenctrl.h
+++ b/tools/libxc/include/xenctrl.h
@@ -915,6 +915,15 @@ int xc_sched_rtds_domain_get(xc_interface *xch,
uint32_t domid,
struct xen_domctl_sched_rtds *sdom);
+int xc_sched_rtds_vcpu_set(xc_interface *xch,
+ uint32_t domid,
+ xen_domctl_schedparam_vcpu_t *vcpus,
+ uint16_t num_vcpus);
+int xc_sched_rtds_vcpu_get(xc_interface *xch,
+ uint32_t domid,
+ xen_domctl_schedparam_vcpu_t *vcpus,
+ uint16_t num_vcpus);
+
int
xc_sched_arinc653_schedule_set(
xc_interface *xch,
diff --git a/tools/libxc/xc_csched.c b/tools/libxc/xc_csched.c
index 390c645..5a2bdf4 100644
--- a/tools/libxc/xc_csched.c
+++ b/tools/libxc/xc_csched.c
@@ -36,7 +36,7 @@ xc_sched_credit_domain_set(
domctl.domain = (domid_t) domid;
domctl.u.scheduler_op.sched_id = XEN_SCHEDULER_CREDIT;
domctl.u.scheduler_op.cmd = XEN_DOMCTL_SCHEDOP_putinfo;
- domctl.u.scheduler_op.u.credit = *sdom;
+ domctl.u.scheduler_op.u.d.credit = *sdom;
return do_domctl(xch, &domctl);
}
@@ -57,7 +57,7 @@ xc_sched_credit_domain_get(
err = do_domctl(xch, &domctl);
if ( err == 0 )
- *sdom = domctl.u.scheduler_op.u.credit;
+ *sdom = domctl.u.scheduler_op.u.d.credit;
return err;
}
diff --git a/tools/libxc/xc_csched2.c b/tools/libxc/xc_csched2.c
index 6da6a46..a604278 100644
--- a/tools/libxc/xc_csched2.c
+++ b/tools/libxc/xc_csched2.c
@@ -36,7 +36,7 @@ xc_sched_credit2_domain_set(
domctl.domain = (domid_t) domid;
domctl.u.scheduler_op.sched_id = XEN_SCHEDULER_CREDIT2;
domctl.u.scheduler_op.cmd = XEN_DOMCTL_SCHEDOP_putinfo;
- domctl.u.scheduler_op.u.credit2 = *sdom;
+ domctl.u.scheduler_op.u.d.credit2 = *sdom;
return do_domctl(xch, &domctl);
}
@@ -57,7 +57,7 @@ xc_sched_credit2_domain_get(
err = do_domctl(xch, &domctl);
if ( err == 0 )
- *sdom = domctl.u.scheduler_op.u.credit2;
+ *sdom = domctl.u.scheduler_op.u.d.credit2;
return err;
}
diff --git a/tools/libxc/xc_rt.c b/tools/libxc/xc_rt.c
index b2d1cc5..ee139b1 100644
--- a/tools/libxc/xc_rt.c
+++ b/tools/libxc/xc_rt.c
@@ -27,7 +27,7 @@
int xc_sched_rtds_domain_set(xc_interface *xch,
uint32_t domid,
- struct xen_domctl_sched_rtds *sdom)
+ xen_domctl_sched_rtds_t *sdom)
{
int rc;
DECLARE_DOMCTL;
@@ -36,8 +36,8 @@ int xc_sched_rtds_domain_set(xc_interface *xch,
domctl.domain = (domid_t) domid;
domctl.u.scheduler_op.sched_id = XEN_SCHEDULER_RTDS;
domctl.u.scheduler_op.cmd = XEN_DOMCTL_SCHEDOP_putinfo;
- domctl.u.scheduler_op.u.rtds.period = sdom->period;
- domctl.u.scheduler_op.u.rtds.budget = sdom->budget;
+ domctl.u.scheduler_op.u.d.rtds.period = sdom->period;
+ domctl.u.scheduler_op.u.d.rtds.budget = sdom->budget;
rc = do_domctl(xch, &domctl);
@@ -46,7 +46,7 @@ int xc_sched_rtds_domain_set(xc_interface *xch,
int xc_sched_rtds_domain_get(xc_interface *xch,
uint32_t domid,
- struct xen_domctl_sched_rtds *sdom)
+ xen_domctl_sched_rtds_t *sdom)
{
int rc;
DECLARE_DOMCTL;
@@ -59,7 +59,61 @@ int xc_sched_rtds_domain_get(xc_interface *xch,
rc = do_domctl(xch, &domctl);
if ( rc == 0 )
- *sdom = domctl.u.scheduler_op.u.rtds;
+ *sdom = domctl.u.scheduler_op.u.d.rtds;
+
+ return rc;
+}
+
+int xc_sched_rtds_vcpu_set(xc_interface *xch,
+ uint32_t domid,
+ xen_domctl_schedparam_vcpu_t *vcpus,
+ uint16_t num_vcpus)
+{
+ int rc;
+ DECLARE_DOMCTL;
+ DECLARE_HYPERCALL_BOUNCE(vcpus, sizeof(*vcpus) * num_vcpus,
+ XC_HYPERCALL_BUFFER_BOUNCE_IN);
+
+ if ( xc_hypercall_bounce_pre(xch, vcpus) )
+ return -1;
+
+ domctl.cmd = XEN_DOMCTL_scheduler_op;
+ domctl.domain = (domid_t) domid;
+ domctl.u.scheduler_op.sched_id = XEN_SCHEDULER_RTDS;
+ domctl.u.scheduler_op.cmd = XEN_DOMCTL_SCHEDOP_putvcpuinfo;
+ domctl.u.scheduler_op.u.v.nr_vcpus = num_vcpus;
+ set_xen_guest_handle(domctl.u.scheduler_op.u.v.vcpus, vcpus);
+
+ rc = do_domctl(xch, &domctl);
+
+ xc_hypercall_bounce_post(xch, vcpus);
+
+ return rc;
+}
+
+int xc_sched_rtds_vcpu_get(xc_interface *xch,
+ uint32_t domid,
+ xen_domctl_schedparam_vcpu_t *vcpus,
+ uint16_t num_vcpus)
+{
+ int rc;
+ DECLARE_DOMCTL;
+ DECLARE_HYPERCALL_BOUNCE(vcpus, sizeof(*vcpus) * num_vcpus,
+ XC_HYPERCALL_BUFFER_BOUNCE_BOTH);
+
+ if ( xc_hypercall_bounce_pre(xch, vcpus) )
+ return -1;
+
+ domctl.cmd = XEN_DOMCTL_scheduler_op;
+ domctl.domain = (domid_t) domid;
+ domctl.u.scheduler_op.sched_id = XEN_SCHEDULER_RTDS;
+ domctl.u.scheduler_op.cmd = XEN_DOMCTL_SCHEDOP_getvcpuinfo;
+ domctl.u.scheduler_op.u.v.nr_vcpus = num_vcpus;
+ set_xen_guest_handle(domctl.u.scheduler_op.u.v.vcpus, vcpus);
+
+ rc = do_domctl(xch, &domctl);
+
+ xc_hypercall_bounce_post(xch, vcpus);
return rc;
}
diff --git a/tools/libxc/xc_sedf.c b/tools/libxc/xc_sedf.c
index db372ca..a1cf5f0 100644
--- a/tools/libxc/xc_sedf.c
+++ b/tools/libxc/xc_sedf.c
@@ -34,7 +34,7 @@ int xc_sedf_domain_set(
uint16_t weight)
{
DECLARE_DOMCTL;
- struct xen_domctl_sched_sedf *p = &domctl.u.scheduler_op.u.sedf;
+ struct xen_domctl_sched_sedf *p = &domctl.u.scheduler_op.u.d.sedf;
domctl.cmd = XEN_DOMCTL_scheduler_op;
domctl.domain = (domid_t)domid;
@@ -60,7 +60,7 @@ int xc_sedf_domain_get(
{
DECLARE_DOMCTL;
int ret;
- struct xen_domctl_sched_sedf *p = &domctl.u.scheduler_op.u.sedf;
+ struct xen_domctl_sched_sedf *p = &domctl.u.scheduler_op.u.d.sedf;
domctl.cmd = XEN_DOMCTL_scheduler_op;
domctl.domain = (domid_t)domid;
--
1.9.1
^ permalink raw reply related [flat|nested] 39+ messages in thread
* [PATCH v3 for Xen 4.6 3/4] libxl: enable per-VCPU parameter settings for RTDS scheduler
2015-06-29 2:44 [PATCH v3 for Xen 4.6 0/4] Enable per-VCPU parameter settings for RTDS scheduler Chong Li
2015-06-29 2:44 ` [PATCH v3 for Xen 4.6 1/4] xen: enable " Chong Li
2015-06-29 2:44 ` [PATCH v3 for Xen 4.6 2/4] libxc: " Chong Li
@ 2015-06-29 2:44 ` Chong Li
2015-06-30 12:26 ` Ian Campbell
2015-07-07 16:23 ` Dario Faggioli
2015-06-29 2:44 ` [PATCH v3 for Xen 4.6 4/4] xl: " Chong Li
2015-07-07 15:16 ` [PATCH v3 for Xen 4.6 0/4] Enable " Dario Faggioli
4 siblings, 2 replies; 39+ messages in thread
From: Chong Li @ 2015-06-29 2:44 UTC (permalink / raw)
To: xen-devel
Cc: Chong Li, wei.liu2, Sisu Xi, george.dunlap, dario.faggioli,
ian.jackson, ian.campbell, Meng Xu, lichong659, dgolomb
Add libxl_vcpu_sched_params_get/set and sched_rtds_vcpu_get/set functions to support
per-VCPU settings.
Changes on PATCH v2:
1) New data structure (libxl_vcpu_sched_params and libxl_sched_params) to help per-VCPU settings.
2) sched_rtds_vcpu_get now can return a random subset of the parameters of the VCPUs of a specific domain.
Signed-off-by: Chong Li <chong.li@wustl.edu>
Signed-off-by: Meng Xu <mengxu@cis.upenn.edu>
Signed-off-by: Sisu Xi <xisisu@gmail.com>
---
CC: <dario.faggioli@citrix.com>
CC: <george.dunlap@eu.citrix.com>
CC: <dgolomb@seas.upenn.edu>
CC: <mengxu@cis.upenn.edu>
CC: <wei.liu2@citrix.com>
CC: <lichong659@gmail.com>
CC: <ian.jackson@eu.citrix.com>
CC: <ian.campbell@eu.citrix.com>
---
tools/libxl/libxl.c | 209 +++++++++++++++++++++++++++++++++++++++-----
tools/libxl/libxl.h | 17 ++++
tools/libxl/libxl_types.idl | 16 ++++
3 files changed, 220 insertions(+), 22 deletions(-)
diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index e9a2d26..9a9c59b 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -5854,6 +5854,144 @@ static int sched_sedf_domain_set(libxl__gc *gc, uint32_t domid,
return 0;
}
+static int sched_rtds_validate_params(libxl__gc *gc, int period,
+ int budget, uint32_t *sdom_period,
+ uint32_t *sdom_budget)
+{
+ if (period != LIBXL_DOMAIN_SCHED_PARAM_PERIOD_DEFAULT) {
+ if (period < 1) {
+ LOG(ERROR, "VCPU period is not set or out of range, "
+ "valid values are larger than or equal to 1");
+ return 1;
+ }
+ *sdom_period = period;
+ }
+
+ if (budget != LIBXL_DOMAIN_SCHED_PARAM_BUDGET_DEFAULT) {
+ if (budget < 1) {
+ LOG(ERROR, "VCPU budget is not set or out of range, "
+ "valid values are larger than or equal to 1");
+ return 1;
+ }
+ *sdom_budget = budget;
+ }
+
+ if (budget > period) {
+ LOG(ERROR, "VCPU budget is larger than VCPU period");
+ return 1;
+ }
+
+ return 0;
+}
+
+static int sched_rtds_vcpu_get(libxl__gc *gc, uint32_t domid,
+ libxl_vcpu_sched_params *scinfo)
+{
+ uint16_t num_vcpus;
+ int rc, i;
+ xc_dominfo_t info;
+ xen_domctl_schedparam_vcpu_t *vcpus;
+
+ if (scinfo->num_vcpus == 0) {
+ rc = xc_domain_getinfo(CTX->xch, domid, 1, &info);
+ if (rc < 0) {
+ LOGE(ERROR, "getting domain info");
+ return ERROR_FAIL;
+ }
+ num_vcpus = info.max_vcpu_id + 1;
+ } else
+ num_vcpus = scinfo->num_vcpus;
+
+ GCNEW_ARRAY(vcpus, num_vcpus);
+
+ if (scinfo->num_vcpus > 0)
+ for(i=0; i < num_vcpus; i++)
+ vcpus[i].vcpuid = scinfo->vcpus[i].vcpuid;
+ else
+ for(i=0; i < num_vcpus; i++)
+ vcpus[i].vcpuid = i;
+
+ rc = xc_sched_rtds_vcpu_get(CTX->xch, domid, vcpus, num_vcpus);
+ if (rc != 0) {
+ LOGE(ERROR, "getting vcpu sched rtds");
+ return ERROR_FAIL;
+ }
+
+ scinfo->sched = LIBXL_SCHEDULER_RTDS;
+ if (scinfo->num_vcpus == 0) {
+ scinfo->num_vcpus = num_vcpus;
+ scinfo->vcpus = libxl__calloc(NOGC, num_vcpus, sizeof(libxl_sched_params));
+ }
+ for(i = 0; i < num_vcpus; i++) {
+ scinfo->vcpus[i].period = vcpus[i].s.rtds.period;
+ scinfo->vcpus[i].budget = vcpus[i].s.rtds.budget;
+ scinfo->vcpus[i].vcpuid = vcpus[i].vcpuid;
+ }
+
+ return 0;
+}
+
+static int sched_rtds_vcpu_set(libxl__gc *gc, uint32_t domid,
+ const libxl_vcpu_sched_params *scinfo)
+{
+ int rc;
+ int i;
+ uint16_t max_vcpuid;
+ xc_dominfo_t info;
+ xen_domctl_schedparam_vcpu_t *vcpus;
+ int num_vcpus;
+
+ rc = xc_domain_getinfo(CTX->xch, domid, 1, &info);
+ if (rc < 0) {
+ LOGE(ERROR, "getting domain info");
+ return ERROR_FAIL;
+ }
+ max_vcpuid = info.max_vcpu_id;
+
+ if (scinfo->num_vcpus > 0) {
+ num_vcpus = scinfo->num_vcpus;
+ GCNEW_ARRAY(vcpus, num_vcpus);
+ for (i = 0; i < num_vcpus; i++) {
+ if (scinfo->vcpus[i].vcpuid < 0 ||
+ scinfo->vcpus[i].vcpuid > max_vcpuid) {
+ LOG(ERROR, "VCPU index is out of range, "
+ "valid values are within range from 0 to %d",
+ max_vcpuid);
+ return ERROR_INVAL;
+ }
+ vcpus[i].vcpuid = scinfo->vcpus[i].vcpuid;
+
+ rc = sched_rtds_validate_params(gc,
+ scinfo->vcpus[i].period, scinfo->vcpus[i].budget,
+ &vcpus[i].s.rtds.period, &vcpus[i].s.rtds.budget);
+ if (rc)
+ return ERROR_INVAL;
+ }
+ } else {
+ num_vcpus = max_vcpuid + 1;
+ GCNEW_ARRAY(vcpus, num_vcpus);
+ rc = sched_rtds_validate_params(gc,
+ scinfo->vcpus[0].period, scinfo->vcpus[0].budget,
+ &vcpus[0].s.rtds.period, &vcpus[0].s.rtds.budget);
+ if (rc)
+ return ERROR_INVAL;
+ for (i = 0; i < num_vcpus; i++) {
+ vcpus[i].vcpuid = i;
+ vcpus[i].s.rtds.period = scinfo->vcpus[0].period;
+ vcpus[i].s.rtds.budget = scinfo->vcpus[0].budget;
+ }
+ }
+
+ rc = xc_sched_rtds_vcpu_set(CTX->xch, domid,
+ vcpus, num_vcpus);
+ if (rc != 0) {
+ LOGE(ERROR, "setting vcpu sched rtds");
+ return ERROR_FAIL;
+ }
+
+ return rc;
+}
+
static int sched_rtds_domain_get(libxl__gc *gc, uint32_t domid,
libxl_domain_sched_params *scinfo)
{
@@ -5887,29 +6025,10 @@ static int sched_rtds_domain_set(libxl__gc *gc, uint32_t domid,
return ERROR_FAIL;
}
- if (scinfo->period != LIBXL_DOMAIN_SCHED_PARAM_PERIOD_DEFAULT) {
- if (scinfo->period < 1) {
- LOG(ERROR, "VCPU period is not set or out of range, "
- "valid values are larger than 1");
- return ERROR_INVAL;
- }
- sdom.period = scinfo->period;
- }
-
- if (scinfo->budget != LIBXL_DOMAIN_SCHED_PARAM_BUDGET_DEFAULT) {
- if (scinfo->budget < 1) {
- LOG(ERROR, "VCPU budget is not set or out of range, "
- "valid values are larger than 1");
- return ERROR_INVAL;
- }
- sdom.budget = scinfo->budget;
- }
-
- if (sdom.budget > sdom.period) {
- LOG(ERROR, "VCPU budget is larger than VCPU period, "
- "VCPU budget should be no larger than VCPU period");
+ rc = sched_rtds_validate_params(gc, scinfo->period, scinfo->budget,
+ &sdom.period, &sdom.budget);
+ if (rc)
return ERROR_INVAL;
- }
rc = xc_sched_rtds_domain_set(CTX->xch, domid, &sdom);
if (rc < 0) {
@@ -5956,6 +6075,30 @@ int libxl_domain_sched_params_set(libxl_ctx *ctx, uint32_t domid,
return ret;
}
+int libxl_vcpu_sched_params_set(libxl_ctx *ctx, uint32_t domid,
+ const libxl_vcpu_sched_params *scinfo)
+{
+ GC_INIT(ctx);
+ libxl_scheduler sched = scinfo->sched;
+ int ret;
+
+ if (sched == LIBXL_SCHEDULER_UNKNOWN)
+ sched = libxl__domain_scheduler(gc, domid);
+
+ switch (sched) {
+ case LIBXL_SCHEDULER_RTDS:
+ ret = sched_rtds_vcpu_set(gc, domid, scinfo);
+ break;
+ default:
+ LOG(ERROR, "Unknown scheduler");
+ ret = ERROR_INVAL;
+ break;
+ }
+
+ GC_FREE;
+ return ret;
+}
+
int libxl_domain_sched_params_get(libxl_ctx *ctx, uint32_t domid,
libxl_domain_sched_params *scinfo)
{
@@ -5989,6 +6132,28 @@ int libxl_domain_sched_params_get(libxl_ctx *ctx, uint32_t domid,
return ret;
}
+int libxl_vcpu_sched_params_get(libxl_ctx *ctx, uint32_t domid,
+ libxl_vcpu_sched_params *scinfo)
+{
+ GC_INIT(ctx);
+ int ret;
+
+ scinfo->sched = libxl__domain_scheduler(gc, domid);
+
+ switch (scinfo->sched) {
+ case LIBXL_SCHEDULER_RTDS:
+ ret = sched_rtds_vcpu_get(gc, domid, scinfo);
+ break;
+ default:
+ LOG(ERROR, "Unknown scheduler");
+ ret = ERROR_INVAL;
+ break;
+ }
+
+ GC_FREE;
+ return ret;
+}
+
static int libxl__domain_s3_resume(libxl__gc *gc, int domid)
{
int rc = 0;
diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
index a1c5d15..6d7763c 100644
--- a/tools/libxl/libxl.h
+++ b/tools/libxl/libxl.h
@@ -200,6 +200,16 @@
#define LIBXL_HAVE_DEVICETREE_PASSTHROUGH 1
/*
+ * libxl_vcpu_sched_params is used to get/set per-vcpu params
+*/
+#define LIBXL_HAVE_VCPU_SCHED_PARAMS 1
+
+/*
+ * libxl_sched_params is used to store per-vcpu params
+*/
+#define LIBXL_SCHED_PARAMS 1
+
+/*
* libxl ABI compatibility
*
* The only guarantee which libxl makes regarding ABI compatibility
@@ -1554,10 +1564,17 @@ int libxl_sched_credit_params_set(libxl_ctx *ctx, uint32_t poolid,
#define LIBXL_DOMAIN_SCHED_PARAM_EXTRATIME_DEFAULT -1
#define LIBXL_DOMAIN_SCHED_PARAM_BUDGET_DEFAULT -1
+/* Per-VCPU parameters*/
+#define LIBXL_DOMAIN_SCHED_PARAM_VCPU_INDEX_DEFAULT -1
+
int libxl_domain_sched_params_get(libxl_ctx *ctx, uint32_t domid,
libxl_domain_sched_params *params);
int libxl_domain_sched_params_set(libxl_ctx *ctx, uint32_t domid,
const libxl_domain_sched_params *params);
+int libxl_vcpu_sched_params_get(libxl_ctx *ctx, uint32_t domid,
+ libxl_vcpu_sched_params *params);
+int libxl_vcpu_sched_params_set(libxl_ctx *ctx, uint32_t domid,
+ const libxl_vcpu_sched_params *params);
int libxl_send_trigger(libxl_ctx *ctx, uint32_t domid,
libxl_trigger trigger, uint32_t vcpuid);
diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
index e1632fa..b527aa3 100644
--- a/tools/libxl/libxl_types.idl
+++ b/tools/libxl/libxl_types.idl
@@ -351,6 +351,22 @@ libxl_domain_restore_params = Struct("domain_restore_params", [
("checkpointed_stream", integer),
])
+libxl_sched_params = Struct("sched_params",[
+ ("vcpuid", integer, {'init_val': 'LIBXL_DOMAIN_SCHED_PARAM_VCPU_INDEX_DEFAULT'}),
+ ("weight", integer, {'init_val': 'LIBXL_DOMAIN_SCHED_PARAM_WEIGHT_DEFAULT'}),
+ ("cap", integer, {'init_val': 'LIBXL_DOMAIN_SCHED_PARAM_CAP_DEFAULT'}),
+ ("period", integer, {'init_val': 'LIBXL_DOMAIN_SCHED_PARAM_PERIOD_DEFAULT'}),
+ ("slice", integer, {'init_val': 'LIBXL_DOMAIN_SCHED_PARAM_SLICE_DEFAULT'}),
+ ("latency", integer, {'init_val': 'LIBXL_DOMAIN_SCHED_PARAM_LATENCY_DEFAULT'}),
+ ("extratime", integer, {'init_val': 'LIBXL_DOMAIN_SCHED_PARAM_EXTRATIME_DEFAULT'}),
+ ("budget", integer, {'init_val': 'LIBXL_DOMAIN_SCHED_PARAM_BUDGET_DEFAULT'}),
+ ])
+
+libxl_vcpu_sched_params = Struct("vcpu_sched_params",[
+ ("sched", libxl_scheduler),
+ ("vcpus", Array(libxl_sched_params, "num_vcpus")),
+ ])
+
libxl_domain_sched_params = Struct("domain_sched_params",[
("sched", libxl_scheduler),
("weight", integer, {'init_val': 'LIBXL_DOMAIN_SCHED_PARAM_WEIGHT_DEFAULT'}),
--
1.9.1
^ permalink raw reply related [flat|nested] 39+ messages in thread
* [PATCH v3 for Xen 4.6 4/4] xl: enable per-VCPU parameter settings for RTDS scheduler
2015-06-29 2:44 [PATCH v3 for Xen 4.6 0/4] Enable per-VCPU parameter settings for RTDS scheduler Chong Li
` (2 preceding siblings ...)
2015-06-29 2:44 ` [PATCH v3 for Xen 4.6 3/4] libxl: " Chong Li
@ 2015-06-29 2:44 ` Chong Li
2015-07-07 15:34 ` Dario Faggioli
2015-07-07 15:16 ` [PATCH v3 for Xen 4.6 0/4] Enable " Dario Faggioli
4 siblings, 1 reply; 39+ messages in thread
From: Chong Li @ 2015-06-29 2:44 UTC (permalink / raw)
To: xen-devel
Cc: Chong Li, wei.liu2, Sisu Xi, george.dunlap, dario.faggioli,
Meng Xu, lichong659, dgolomb
Change main_sched_rtds and related output functions to support per-VCPU settings.
Changes on PATCH v2:
1) Remove per-domain output functions for RTDS scheduler.
2) Users now use '-v all' to specify all VCPUs.
3) Support outputting a subset of the parameters of the VCPUs of a specific domain.
4) When setting all VCPUs with the same parameters (by only one command), no per-domain function is invoked.
Signed-off-by: Chong Li <chong.li@wustl.edu>
Signed-off-by: Meng Xu <mengxu@cis.upenn.edu>
Signed-off-by: Sisu Xi <xisisu@gmail.com>
---
CC: <dario.faggioli@citrix.com>
CC: <george.dunlap@eu.citrix.com>
CC: <dgolomb@seas.upenn.edu>
CC: <mengxu@cis.upenn.edu>
CC: <wei.liu2@citrix.com>
CC: <lichong659@gmail.com>
---
docs/man/xl.pod.1 | 4 +
tools/libxl/xl_cmdimpl.c | 284 +++++++++++++++++++++++++++++++++++++++-------
tools/libxl/xl_cmdtable.c | 10 +-
3 files changed, 250 insertions(+), 48 deletions(-)
diff --git a/docs/man/xl.pod.1 b/docs/man/xl.pod.1
index 4eb929d..d35e169 100644
--- a/docs/man/xl.pod.1
+++ b/docs/man/xl.pod.1
@@ -1093,6 +1093,10 @@ B<OPTIONS>
Specify domain for which scheduler parameters are to be modified or retrieved.
Mandatory for modifying scheduler parameters.
+=item B<-v VCPUID/all>, B<--vcpuid=VCPUID/all>
+
+Specify vcpu for which scheduler parameters are to be modified or retrieved.
+
=item B<-p PERIOD>, B<--period=PERIOD>
Period of time, in microseconds, over which to replenish the budget.
diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index c858068..7f120af 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -5630,6 +5630,37 @@ static int sched_domain_set(int domid, const libxl_domain_sched_params *scinfo)
return rc;
}
+static int sched_vcpu_get(libxl_scheduler sched, int domid,
+ libxl_vcpu_sched_params *scinfo)
+{
+ int rc;
+
+ rc = libxl_vcpu_sched_params_get(ctx, domid, scinfo);
+ if (rc) {
+ fprintf(stderr, "libxl_vcpu_sched_params_get failed.\n");
+ return rc;
+ }
+ if (scinfo->sched != sched) {
+ fprintf(stderr, "libxl_vcpu_sched_params_get returned %s not %s.\n",
+ libxl_scheduler_to_string(scinfo->sched),
+ libxl_scheduler_to_string(sched));
+ return ERROR_INVAL;
+ }
+
+ return 0;
+}
+
+static int sched_vcpu_set(int domid, const libxl_vcpu_sched_params *scinfo)
+{
+ int rc;
+
+ rc = libxl_vcpu_sched_params_set(ctx, domid, scinfo);
+ if (rc)
+ fprintf(stderr, "libxl_vcpu_sched_params_set failed.\n");
+
+ return rc;
+}
+
static int sched_credit_params_set(int poolid, libxl_sched_credit_params *scinfo)
{
int rc;
@@ -5756,33 +5787,35 @@ static int sched_sedf_domain_output(
return 0;
}
-static int sched_rtds_domain_output(
- int domid)
+static int sched_rtds_vcpu_output(
+ int domid, libxl_vcpu_sched_params *scinfo)
{
char *domname;
- libxl_domain_sched_params scinfo;
int rc = 0;
+ int i;
- if (domid < 0) {
- printf("%-33s %4s %9s %9s\n", "Name", "ID", "Period", "Budget");
+ if (domid < 0) {
+ printf("%-33s %4s %4s %9s %9s\n", "Name", "ID",
+ "VCPU", "Period", "Budget");
return 0;
}
- libxl_domain_sched_params_init(&scinfo);
- rc = sched_domain_get(LIBXL_SCHEDULER_RTDS, domid, &scinfo);
+ rc = sched_vcpu_get(LIBXL_SCHEDULER_RTDS, domid, scinfo);
if (rc)
goto out;
domname = libxl_domid_to_name(ctx, domid);
- printf("%-33s %4d %9d %9d\n",
- domname,
- domid,
- scinfo.period,
- scinfo.budget);
+ for( i = 0; i < scinfo->num_vcpus; i++ ) {
+ printf("%-33s %4d %4d %9"PRIu32" %9"PRIu32"\n",
+ domname,
+ domid,
+ scinfo->vcpus[i].vcpuid,
+ scinfo->vcpus[i].period,
+ scinfo->vcpus[i].budget);
+ }
free(domname);
out:
- libxl_domain_sched_params_dispose(&scinfo);
return rc;
}
@@ -5859,6 +5892,65 @@ static int sched_domain_output(libxl_scheduler sched, int (*output)(int),
return 0;
}
+static int sched_vcpu_output(libxl_scheduler sched,
+ int (*output)(int, libxl_vcpu_sched_params *),
+ int (*pooloutput)(uint32_t), const char *cpupool)
+{
+ libxl_dominfo *info;
+ libxl_cpupoolinfo *poolinfo = NULL;
+ uint32_t poolid;
+ int nb_domain, n_pools = 0, i, p;
+ int rc = 0;
+
+ if (cpupool) {
+ if (libxl_cpupool_qualifier_to_cpupoolid(ctx, cpupool, &poolid, NULL) ||
+ !libxl_cpupoolid_is_valid(ctx, poolid)) {
+ fprintf(stderr, "unknown cpupool \'%s\'\n", cpupool);
+ return -ERROR_FAIL;
+ }
+ }
+
+ info = libxl_list_domain(ctx, &nb_domain);
+ if (!info) {
+ fprintf(stderr, "libxl_list_domain failed.\n");
+ return 1;
+ }
+ poolinfo = libxl_list_cpupool(ctx, &n_pools);
+ if (!poolinfo) {
+ fprintf(stderr, "error getting cpupool info\n");
+ libxl_dominfo_list_free(info, nb_domain);
+ return -ERROR_NOMEM;
+ }
+
+ for (p = 0; !rc && (p < n_pools); p++) {
+ if ((poolinfo[p].sched != sched) ||
+ (cpupool && (poolid != poolinfo[p].poolid)))
+ continue;
+
+ pooloutput(poolinfo[p].poolid);
+
+ libxl_vcpu_sched_params scinfo_out;
+ libxl_vcpu_sched_params_init(&scinfo_out);
+ output(-1, &scinfo_out);
+ libxl_vcpu_sched_params_dispose(&scinfo_out);
+ for (i = 0; i < nb_domain; i++) {
+ if (info[i].cpupool != poolinfo[p].poolid)
+ continue;
+ libxl_vcpu_sched_params scinfo;
+ libxl_vcpu_sched_params_init(&scinfo);
+ scinfo.num_vcpus = 0;
+ rc = output(info[i].domid, &scinfo);
+ libxl_vcpu_sched_params_dispose(&scinfo);
+ if (rc)
+ break;
+ }
+ }
+
+ libxl_cpupoolinfo_list_free(poolinfo, n_pools);
+ libxl_dominfo_list_free(info, nb_domain);
+ return 0;
+}
+
/*
* <nothing> : List all domain params and sched params from all pools
* -d [domid] : List domain params for domain
@@ -6173,76 +6265,180 @@ int main_sched_rtds(int argc, char **argv)
{
const char *dom = NULL;
const char *cpupool = NULL;
- int period = 0; /* period is in microsecond */
- int budget = 0; /* budget is in microsecond */
+
+ int *vcpus = (int *)xmalloc(sizeof(int)); /* IDs of VCPUs that change */
+ int *periods = (int *)xmalloc(sizeof(int)); /* period is in microsecond */
+ int *budgets = (int *)xmalloc(sizeof(int)); /* budget is in microsecond */
+ int v_size = 1; /* size of vcpus array */
+ int p_size = 1; /* size of periods array */
+ int b_size = 1; /* size of budgets array */
+ int v_index = 0; /* index in vcpus array */
+ int p_index =0; /* index in periods array */
+ int b_index =0; /* index for in budgets array */
bool opt_p = false;
bool opt_b = false;
- int opt, rc;
+ bool opt_v = false;
+ bool opt_all = false; /* output per-dom parameters */
+ int opt, i;
+ int rc = 0;
static struct option opts[] = {
{"domain", 1, 0, 'd'},
{"period", 1, 0, 'p'},
{"budget", 1, 0, 'b'},
+ {"vcpuid",1, 0, 'v'},
{"cpupool", 1, 0, 'c'},
COMMON_LONG_OPTS,
{0, 0, 0, 0}
};
- SWITCH_FOREACH_OPT(opt, "d:p:b:c:h", opts, "sched-rtds", 0) {
+ SWITCH_FOREACH_OPT(opt, "d:p:b:v:c", opts, "sched-rtds", 0) {
case 'd':
dom = optarg;
break;
case 'p':
- period = strtol(optarg, NULL, 10);
+ if (p_index > b_index || p_index > v_index) {
+ /* budget or vcpuID is missed */
+ fprintf(stderr, "Must specify period, budget and vcpuID\n");
+ rc = 1;
+ goto out;
+ }
+ if (p_index >= p_size) {
+ p_size *= 2;
+ periods = xrealloc(periods, p_size);
+ if (!periods) {
+ fprintf(stderr, "Failed to realloc periods\n");
+ rc = 1;
+ goto out;
+ }
+ }
+ periods[p_index++] = strtol(optarg, NULL, 10);
opt_p = 1;
break;
case 'b':
- budget = strtol(optarg, NULL, 10);
+ if (b_index > p_index || b_index > v_index) {
+ /* period or vcpuID is missed */
+ fprintf(stderr, "Must specify period, budget and vcpuID\n");
+ rc = 1;
+ goto out;
+ }
+ if (b_index >= b_size) {
+ b_size *= 2;
+ budgets = xrealloc(budgets, b_size);
+ if (!budgets) {
+ fprintf(stderr, "Failed to realloc budgets\n");
+ rc = 1;
+ goto out;
+ }
+ }
+ budgets[b_index++] = strtol(optarg, NULL, 10);
opt_b = 1;
break;
+ case 'v':
+ if (!strcmp(optarg, "all")) { /* output per-dom parameters*/
+ opt_all = 1;
+ break;
+ }
+ if (v_index >= v_size) {
+ v_size *= 2;
+ vcpus = xrealloc(vcpus, v_size);
+ if (!vcpus) {
+ fprintf(stderr, "Failed to realloc vcpus\n");
+ rc = 1;
+ goto out;
+ }
+ }
+ vcpus[v_index++] = strtol(optarg, NULL, 10);
+ opt_v = 1;
+ break;
case 'c':
cpupool = optarg;
break;
}
- if (cpupool && (dom || opt_p || opt_b)) {
+ if (cpupool && (dom || opt_p || opt_b || opt_v || opt_all)) {
fprintf(stderr, "Specifying a cpupool is not allowed with "
"other options.\n");
- return 1;
+ rc = 1;
+ goto out;
}
- if (!dom && (opt_p || opt_b)) {
- fprintf(stderr, "Must specify a domain.\n");
- return 1;
+ if (!dom && ((opt_p || opt_b || opt_v) ||
+ (!opt_p && !opt_b && !opt_v && !opt_all))) {
+ fprintf(stderr, "Missing parameters.\n");
+ rc = 1;
+ goto out;
}
- if (opt_p != opt_b) {
- fprintf(stderr, "Must specify period and budget\n");
- return 1;
+ if (dom && !opt_v && !opt_all) {
+ fprintf(stderr, "Must specify VCPU.\n");
+ rc = 1;
+ goto out;
+ }
+ if (opt_v && opt_all) {
+ fprintf(stderr, "Incorrect VCPU IDs.\n");
+ rc = 1;
+ goto out;
+ }
+ if (((v_index > b_index) && opt_b) || ((v_index > p_index) && opt_p)
+ || p_index != b_index) {
+ fprintf(stderr, "Incorrect number of period and budget\n");
+ rc = 1;
+ goto out;
}
- if (!dom) { /* list all domain's rt scheduler info */
- return -sched_domain_output(LIBXL_SCHEDULER_RTDS,
- sched_rtds_domain_output,
+ if ((!dom) && opt_all) { /* list all domain's rtds scheduler info */
+ rc = -sched_vcpu_output(LIBXL_SCHEDULER_RTDS,
+ sched_rtds_vcpu_output,
sched_rtds_pool_output,
cpupool);
+ goto out;
} else {
uint32_t domid = find_domain(dom);
- if (!opt_p && !opt_b) { /* output rt scheduler info */
- sched_rtds_domain_output(-1);
- return -sched_rtds_domain_output(domid);
- } else { /* set rt scheduler paramaters */
- libxl_domain_sched_params scinfo;
- libxl_domain_sched_params_init(&scinfo);
+ if (!opt_p && !opt_b) { /* output rtds scheduler info */
+ libxl_vcpu_sched_params scinfo;
+ libxl_vcpu_sched_params_init(&scinfo);
+ sched_rtds_vcpu_output(-1, &scinfo);
+ scinfo.num_vcpus = v_index;
+ if (v_index > 0)
+ scinfo.vcpus = (libxl_sched_params *)
+ xmalloc(sizeof(libxl_sched_params) * (v_index));
+ for (i = 0; i < v_index; i++)
+ scinfo.vcpus[i].vcpuid = vcpus[i];
+ rc = -sched_rtds_vcpu_output(domid, &scinfo);
+ libxl_vcpu_sched_params_dispose(&scinfo);
+ goto out;
+ } else if (opt_v || opt_all) { /* set per-vcpu rtds scheduler parameters */
+ libxl_vcpu_sched_params scinfo;
+ libxl_vcpu_sched_params_init(&scinfo);
scinfo.sched = LIBXL_SCHEDULER_RTDS;
- scinfo.period = period;
- scinfo.budget = budget;
+ scinfo.num_vcpus = v_index;
+ if (v_index > 0) {
+ scinfo.vcpus = (libxl_sched_params *)
+ xmalloc(sizeof(libxl_sched_params) * (v_index));
+ for (i = 0; i < v_index; i++) {
+ scinfo.vcpus[i].vcpuid = vcpus[i];
+ scinfo.vcpus[i].period = periods[i];
+ scinfo.vcpus[i].budget = budgets[i];
+ }
+ } else { /* set params for all vcpus*/
+ scinfo.vcpus = (libxl_sched_params *)
+ xmalloc(sizeof(libxl_sched_params));
+ scinfo.vcpus[0].period = periods[0];
+ scinfo.vcpus[0].budget = budgets[0];
+ }
- rc = sched_domain_set(domid, &scinfo);
- libxl_domain_sched_params_dispose(&scinfo);
- if (rc)
- return -rc;
- }
+ rc = sched_vcpu_set(domid, &scinfo);
+ libxl_vcpu_sched_params_dispose(&scinfo);
+ if (rc) {
+ rc = -rc;
+ goto out;
+ }
+ }
}
- return 0;
+out:
+ free(vcpus);
+ free(periods);
+ free(budgets);
+ return rc;
}
int main_domid(int argc, char **argv)
diff --git a/tools/libxl/xl_cmdtable.c b/tools/libxl/xl_cmdtable.c
index 7f4759b..c38da55 100644
--- a/tools/libxl/xl_cmdtable.c
+++ b/tools/libxl/xl_cmdtable.c
@@ -284,10 +284,12 @@ struct cmd_spec cmd_table[] = {
{ "sched-rtds",
&main_sched_rtds, 0, 1,
"Get/set rtds scheduler parameters",
- "[-d <Domain> [-p[=PERIOD]] [-b[=BUDGET]]]",
- "-d DOMAIN, --domain=DOMAIN Domain to modify\n"
- "-p PERIOD, --period=PERIOD Period (us)\n"
- "-b BUDGET, --budget=BUDGET Budget (us)\n"
+ "[-d <Domain> [-v[=VCPUID]] [-p[=PERIOD]] [-b[=BUDGET]]]",
+ "-d DOMAIN, --domain=DOMAIN Domain to modify\n"
+ "-v VCPUID/all, --vcpuid=VCPUID/all VCPU to modify or output;\n"
+ " Using '-v all' to modify/output all vcpus\n"
+ "-p PERIOD, --period=PERIOD Period (us)\n"
+ "-b BUDGET, --budget=BUDGET Budget (us)\n"
},
{ "domid",
&main_domid, 0, 0,
--
1.9.1
^ permalink raw reply related [flat|nested] 39+ messages in thread
* Re: [PATCH v3 for Xen 4.6 2/4] libxc: enable per-VCPU parameter settings for RTDS scheduler
2015-06-29 2:44 ` [PATCH v3 for Xen 4.6 2/4] libxc: " Chong Li
@ 2015-06-30 12:22 ` Ian Campbell
2015-06-30 15:18 ` Chong Li
0 siblings, 1 reply; 39+ messages in thread
From: Ian Campbell @ 2015-06-30 12:22 UTC (permalink / raw)
To: Chong Li
Cc: Chong Li, wei.liu2, Sisu Xi, george.dunlap, dario.faggioli,
xen-devel, Meng Xu, dgolomb
On Sun, 2015-06-28 at 21:44 -0500, Chong Li wrote:
> Add xc_sched_rtds_vcpu_get/set functions to interact with Xen to get/set a domain's
> per-VCPU parameters.
>
> Changes on PATCH v2:
>
> 1) Minor modifications due to the change of struct xen_domctl_scheduler_op.
This intra-version changelog should go after the -- so it doesn't appear
in the final commit message.
> diff --git a/tools/libxc/xc_csched.c b/tools/libxc/xc_csched.c
> index 390c645..5a2bdf4 100644
> --- a/tools/libxc/xc_csched.c
> +++ b/tools/libxc/xc_csched.c
> @@ -36,7 +36,7 @@ xc_sched_credit_domain_set(
> domctl.domain = (domid_t) domid;
> domctl.u.scheduler_op.sched_id = XEN_SCHEDULER_CREDIT;
> domctl.u.scheduler_op.cmd = XEN_DOMCTL_SCHEDOP_putinfo;
> - domctl.u.scheduler_op.u.credit = *sdom;
> + domctl.u.scheduler_op.u.d.credit = *sdom;
I don't see any change to any types in this series, which makes me
suspect it is in another patch and that bisectability is therefore not
maintained through the series. Each patch in the series needs to build
when they are applied one by one.
This probably means that at to at least some degree hypercall interface
changes need to be done in the same time as the libxc adjustments. You
can still keep the _new_ functionality in a separate patch of course (if
that makes sense in this case).
Ian.
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v3 for Xen 4.6 3/4] libxl: enable per-VCPU parameter settings for RTDS scheduler
2015-06-29 2:44 ` [PATCH v3 for Xen 4.6 3/4] libxl: " Chong Li
@ 2015-06-30 12:26 ` Ian Campbell
2015-06-30 15:42 ` Chong Li
2015-07-07 16:23 ` Dario Faggioli
1 sibling, 1 reply; 39+ messages in thread
From: Ian Campbell @ 2015-06-30 12:26 UTC (permalink / raw)
To: Chong Li
Cc: Chong Li, wei.liu2, Sisu Xi, george.dunlap, dario.faggioli,
ian.jackson, xen-devel, Meng Xu, dgolomb
On Sun, 2015-06-28 at 21:44 -0500, Chong Li wrote:
> + GC_INIT(ctx);
> + libxl_scheduler sched = scinfo->sched;
> + int ret;
> +
> + if (sched == LIBXL_SCHEDULER_UNKNOWN)
> + sched = libxl__domain_scheduler(gc, domid);
> +
> + switch (sched) {
> + case LIBXL_SCHEDULER_RTDS:
> + ret = sched_rtds_vcpu_set(gc, domid, scinfo);
> + break;
> + default:
> + LOG(ERROR, "Unknown scheduler");
There are several other known schedulers, I don't think the fact that
they don't have any need for per-vcpu variables should result in this
error message, they should get something more specific to that
situation. Only truly unknown values for the scheduler should result in
this message.
[...]
> /*
> + * libxl_vcpu_sched_params is used to get/set per-vcpu params
> +*/
> +#define LIBXL_HAVE_VCPU_SCHED_PARAMS 1
> +
> +/*
> + * libxl_sched_params is used to store per-vcpu params
> +*/
> +#define LIBXL_SCHED_PARAMS 1
Do we need both of these? In particular I'm not sure what the second one
is indicating has changed.
> +
> +/*
> * libxl ABI compatibility
> *
> * The only guarantee which libxl makes regarding ABI compatibility
> @@ -1554,10 +1564,17 @@ int libxl_sched_credit_params_set(libxl_ctx *ctx, uint32_t poolid,
> #define LIBXL_DOMAIN_SCHED_PARAM_EXTRATIME_DEFAULT -1
> #define LIBXL_DOMAIN_SCHED_PARAM_BUDGET_DEFAULT -1
>
> +/* Per-VCPU parameters*/
> +#define LIBXL_DOMAIN_SCHED_PARAM_VCPU_INDEX_DEFAULT -1
What is the effect of passing vcpuid == -1 to this interface?
Ian.
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v3 for Xen 4.6 2/4] libxc: enable per-VCPU parameter settings for RTDS scheduler
2015-06-30 12:22 ` Ian Campbell
@ 2015-06-30 15:18 ` Chong Li
2015-06-30 15:32 ` Ian Campbell
0 siblings, 1 reply; 39+ messages in thread
From: Chong Li @ 2015-06-30 15:18 UTC (permalink / raw)
To: Ian Campbell
Cc: Chong Li, Wei Liu, Sisu Xi, George Dunlap, dario.faggioli,
xen-devel, Meng Xu, Dagaen Golomb
On Tue, Jun 30, 2015 at 7:22 AM, Ian Campbell <ian.campbell@citrix.com> wrote:
> On Sun, 2015-06-28 at 21:44 -0500, Chong Li wrote:
>> Add xc_sched_rtds_vcpu_get/set functions to interact with Xen to get/set a domain's
>> per-VCPU parameters.
>>
>> Changes on PATCH v2:
>>
>> 1) Minor modifications due to the change of struct xen_domctl_scheduler_op.
>
> This intra-version changelog should go after the -- so it doesn't appear
> in the final commit message.
Ok, I see.
>
>> diff --git a/tools/libxc/xc_csched.c b/tools/libxc/xc_csched.c
>> index 390c645..5a2bdf4 100644
>> --- a/tools/libxc/xc_csched.c
>> +++ b/tools/libxc/xc_csched.c
>> @@ -36,7 +36,7 @@ xc_sched_credit_domain_set(
>> domctl.domain = (domid_t) domid;
>> domctl.u.scheduler_op.sched_id = XEN_SCHEDULER_CREDIT;
>> domctl.u.scheduler_op.cmd = XEN_DOMCTL_SCHEDOP_putinfo;
>> - domctl.u.scheduler_op.u.credit = *sdom;
>> + domctl.u.scheduler_op.u.d.credit = *sdom;
>
> I don't see any change to any types in this series, which makes me
> suspect it is in another patch and that bisectability is therefore not
> maintained through the series. Each patch in the series needs to build
> when they are applied one by one.
>
> This probably means that at to at least some degree hypercall interface
> changes need to be done in the same time as the libxc adjustments. You
> can still keep the _new_ functionality in a separate patch of course (if
> that makes sense in this case).
Because both per-domain and per-vcpu parameters are now unioned in
struct xen_domctl_scheduler_op, we need to specify per-domain or
per-vcpu when using domctl.u.scheduler_op.u (.d means per-domain, .v
means per-vcpu). This change also happens to the XEN_DOMCTL_SCHEDOP_*
handlers (of all schedulers), where the domctl.u.scheduler_op.u.* is
passed to.
Chong
>
> Ian.
>
--
Chong Li
Department of Computer Science and Engineering
Washington University in St.louis
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v3 for Xen 4.6 2/4] libxc: enable per-VCPU parameter settings for RTDS scheduler
2015-06-30 15:18 ` Chong Li
@ 2015-06-30 15:32 ` Ian Campbell
2015-06-30 15:57 ` Chong Li
0 siblings, 1 reply; 39+ messages in thread
From: Ian Campbell @ 2015-06-30 15:32 UTC (permalink / raw)
To: Chong Li
Cc: Chong Li, Wei Liu, Sisu Xi, George Dunlap, dario.faggioli,
xen-devel, Meng Xu, Dagaen Golomb
On Tue, 2015-06-30 at 10:18 -0500, Chong Li wrote:
> On Tue, Jun 30, 2015 at 7:22 AM, Ian Campbell <ian.campbell@citrix.com> wrote:
> > On Sun, 2015-06-28 at 21:44 -0500, Chong Li wrote:
> >> Add xc_sched_rtds_vcpu_get/set functions to interact with Xen to get/set a domain's
> >> per-VCPU parameters.
> >>
> >> Changes on PATCH v2:
> >>
> >> 1) Minor modifications due to the change of struct xen_domctl_scheduler_op.
> >
> > This intra-version changelog should go after the -- so it doesn't appear
> > in the final commit message.
>
> Ok, I see.
> >
> >> diff --git a/tools/libxc/xc_csched.c b/tools/libxc/xc_csched.c
> >> index 390c645..5a2bdf4 100644
> >> --- a/tools/libxc/xc_csched.c
> >> +++ b/tools/libxc/xc_csched.c
> >> @@ -36,7 +36,7 @@ xc_sched_credit_domain_set(
> >> domctl.domain = (domid_t) domid;
> >> domctl.u.scheduler_op.sched_id = XEN_SCHEDULER_CREDIT;
> >> domctl.u.scheduler_op.cmd = XEN_DOMCTL_SCHEDOP_putinfo;
> >> - domctl.u.scheduler_op.u.credit = *sdom;
> >> + domctl.u.scheduler_op.u.d.credit = *sdom;
> >
> > I don't see any change to any types in this series, which makes me
> > suspect it is in another patch and that bisectability is therefore not
> > maintained through the series. Each patch in the series needs to build
> > when they are applied one by one.
> >
> > This probably means that at to at least some degree hypercall interface
> > changes need to be done in the same time as the libxc adjustments. You
> > can still keep the _new_ functionality in a separate patch of course (if
> > that makes sense in this case).
>
> Because both per-domain and per-vcpu parameters are now unioned in
> struct xen_domctl_scheduler_op, we need to specify per-domain or
> per-vcpu when using domctl.u.scheduler_op.u (.d means per-domain, .v
> means per-vcpu). This change also happens to the XEN_DOMCTL_SCHEDOP_*
> handlers (of all schedulers), where the domctl.u.scheduler_op.u.* is
> passed to.
I understand the interface change, but the problem is that the interface
change and the change ot the users of that interface seem to be in
different patches, which breaks bisectability.
Does this series compile and work at every step?
i.e. the following combinations of patches from this 4 patch series all
need to build and work successfully:
Patch #1
Patch #1+#2
Patch #1+#2+#3
Patch #1+#2+#3+#4
Is that the case?
Ian.
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v3 for Xen 4.6 3/4] libxl: enable per-VCPU parameter settings for RTDS scheduler
2015-06-30 12:26 ` Ian Campbell
@ 2015-06-30 15:42 ` Chong Li
2015-06-30 15:57 ` Ian Campbell
0 siblings, 1 reply; 39+ messages in thread
From: Chong Li @ 2015-06-30 15:42 UTC (permalink / raw)
To: Ian Campbell
Cc: Chong Li, Wei Liu, Sisu Xi, George Dunlap, dario.faggioli,
Ian Jackson, xen-devel, Meng Xu, Dagaen Golomb
On Tue, Jun 30, 2015 at 7:26 AM, Ian Campbell <ian.campbell@citrix.com> wrote:
> On Sun, 2015-06-28 at 21:44 -0500, Chong Li wrote:
>
>> + GC_INIT(ctx);
>> + libxl_scheduler sched = scinfo->sched;
>> + int ret;
>> +
>> + if (sched == LIBXL_SCHEDULER_UNKNOWN)
>> + sched = libxl__domain_scheduler(gc, domid);
>> +
>> + switch (sched) {
>> + case LIBXL_SCHEDULER_RTDS:
>> + ret = sched_rtds_vcpu_set(gc, domid, scinfo);
>> + break;
>> + default:
>> + LOG(ERROR, "Unknown scheduler");
>
> There are several other known schedulers, I don't think the fact that
> they don't have any need for per-vcpu variables should result in this
> error message, they should get something more specific to that
> situation. Only truly unknown values for the scheduler should result in
> this message.
Right, I'll change it.
>
> [...]
>> /*
>> + * libxl_vcpu_sched_params is used to get/set per-vcpu params
>> +*/
>> +#define LIBXL_HAVE_VCPU_SCHED_PARAMS 1
>> +
>> +/*
>> + * libxl_sched_params is used to store per-vcpu params
>> +*/
>> +#define LIBXL_SCHED_PARAMS 1
>
> Do we need both of these? In particular I'm not sure what the second one
> is indicating has changed.
Sorry for the vague description there, and there is an error there.
The right way should be:
/* libxl_sched_params is introduced to store per-vcpu params. */
#define LIBXL_HAVE_SCHED_PARAMS
/* libxl_vcpu_sched_params is introduced to store the array of
libxl_sched_params (per-vcpu params). */
#define LIBXL_HAVE_VCPU_SCHED_PARAMS
Actually, now I'm not sure if both them should be there, because I've
not found "LIBXL_HAVE_DOMAIN_SCHED_PARAMS" there.
>
>> +
>> +/*
>> * libxl ABI compatibility
>> *
>> * The only guarantee which libxl makes regarding ABI compatibility
>> @@ -1554,10 +1564,17 @@ int libxl_sched_credit_params_set(libxl_ctx *ctx, uint32_t poolid,
>> #define LIBXL_DOMAIN_SCHED_PARAM_EXTRATIME_DEFAULT -1
>> #define LIBXL_DOMAIN_SCHED_PARAM_BUDGET_DEFAULT -1
>>
>> +/* Per-VCPU parameters*/
>> +#define LIBXL_DOMAIN_SCHED_PARAM_VCPU_INDEX_DEFAULT -1
>
> What is the effect of passing vcpuid == -1 to this interface?
I think the right way should be:
#define LIBXL_SCHED_PARAM_VCPU_INDEX_DEFAULT -1
which is the default value for the vcpuid in libxl_sched_params. Even
though libxl_sched_params is used for per-vcpu settings, all the other
fields (except vcpuid) would still use
LIBXL_DOMAIN_SCHED_PARAM_*_DEFAULT values, just for convenience.
Chong
>
> Ian.
>
--
Chong Li
Department of Computer Science and Engineering
Washington University in St.louis
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v3 for Xen 4.6 2/4] libxc: enable per-VCPU parameter settings for RTDS scheduler
2015-06-30 15:32 ` Ian Campbell
@ 2015-06-30 15:57 ` Chong Li
2015-06-30 16:04 ` Ian Campbell
0 siblings, 1 reply; 39+ messages in thread
From: Chong Li @ 2015-06-30 15:57 UTC (permalink / raw)
To: Ian Campbell
Cc: Chong Li, Wei Liu, Sisu Xi, George Dunlap, dario.faggioli,
xen-devel, Meng Xu, Dagaen Golomb
On Tue, Jun 30, 2015 at 10:32 AM, Ian Campbell <ian.campbell@citrix.com> wrote:
> On Tue, 2015-06-30 at 10:18 -0500, Chong Li wrote:
>> On Tue, Jun 30, 2015 at 7:22 AM, Ian Campbell <ian.campbell@citrix.com> wrote:
>> > On Sun, 2015-06-28 at 21:44 -0500, Chong Li wrote:
>> >
>> >> diff --git a/tools/libxc/xc_csched.c b/tools/libxc/xc_csched.c
>> >> index 390c645..5a2bdf4 100644
>> >> --- a/tools/libxc/xc_csched.c
>> >> +++ b/tools/libxc/xc_csched.c
>> >> @@ -36,7 +36,7 @@ xc_sched_credit_domain_set(
>> >> domctl.domain = (domid_t) domid;
>> >> domctl.u.scheduler_op.sched_id = XEN_SCHEDULER_CREDIT;
>> >> domctl.u.scheduler_op.cmd = XEN_DOMCTL_SCHEDOP_putinfo;
>> >> - domctl.u.scheduler_op.u.credit = *sdom;
>> >> + domctl.u.scheduler_op.u.d.credit = *sdom;
>> >
>> > I don't see any change to any types in this series, which makes me
>> > suspect it is in another patch and that bisectability is therefore not
>> > maintained through the series. Each patch in the series needs to build
>> > when they are applied one by one.
>> >
>> > This probably means that at to at least some degree hypercall interface
>> > changes need to be done in the same time as the libxc adjustments. You
>> > can still keep the _new_ functionality in a separate patch of course (if
>> > that makes sense in this case).
>>
>> Because both per-domain and per-vcpu parameters are now unioned in
>> struct xen_domctl_scheduler_op, we need to specify per-domain or
>> per-vcpu when using domctl.u.scheduler_op.u (.d means per-domain, .v
>> means per-vcpu). This change also happens to the XEN_DOMCTL_SCHEDOP_*
>> handlers (of all schedulers), where the domctl.u.scheduler_op.u.* is
>> passed to.
>
> I understand the interface change, but the problem is that the interface
> change and the change ot the users of that interface seem to be in
> different patches, which breaks bisectability.
>
> Does this series compile and work at every step?
>
> i.e. the following combinations of patches from this 4 patch series all
> need to build and work successfully:
>
> Patch #1
> Patch #1+#2
> Patch #1+#2+#3
> Patch #1+#2+#3+#4
>
> Is that the case?
I see your point. Do you mean the change of struct
xen_domctl_scheduler_op, the change of XEN_DOMCTL_SCHEDOP_* handlers
(in xen) and the change of libxc (just shown above) should be in the
same patch? Should the title of that patch be something like
"xen+libxc: enable per-vcpu parameter settings for RTDS scheduler" ?
Chong
>
> Ian.
>
--
Chong Li
Department of Computer Science and Engineering
Washington University in St.louis
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v3 for Xen 4.6 3/4] libxl: enable per-VCPU parameter settings for RTDS scheduler
2015-06-30 15:42 ` Chong Li
@ 2015-06-30 15:57 ` Ian Campbell
2015-06-30 16:10 ` Chong Li
0 siblings, 1 reply; 39+ messages in thread
From: Ian Campbell @ 2015-06-30 15:57 UTC (permalink / raw)
To: Chong Li
Cc: Chong Li, Wei Liu, Sisu Xi, George Dunlap, dario.faggioli,
Ian Jackson, xen-devel, Meng Xu, Dagaen Golomb
On Tue, 2015-06-30 at 10:42 -0500, Chong Li wrote:
> >> @@ -1554,10 +1564,17 @@ int libxl_sched_credit_params_set(libxl_ctx *ctx, uint32_t poolid,
> >> #define LIBXL_DOMAIN_SCHED_PARAM_EXTRATIME_DEFAULT -1
> >> #define LIBXL_DOMAIN_SCHED_PARAM_BUDGET_DEFAULT -1
> >>
> >> +/* Per-VCPU parameters*/
> >> +#define LIBXL_DOMAIN_SCHED_PARAM_VCPU_INDEX_DEFAULT -1
> >
> > What is the effect of passing vcpuid == -1 to this interface?
>
> I think the right way should be:
>
> #define LIBXL_SCHED_PARAM_VCPU_INDEX_DEFAULT -1
>
> which is the default value for the vcpuid in libxl_sched_params. Even
> though libxl_sched_params is used for per-vcpu settings, all the other
> fields (except vcpuid) would still use
> LIBXL_DOMAIN_SCHED_PARAM_*_DEFAULT values, just for convenience.
My question is what does the concept of vcpu==-1 mean in this interface?
Ian.
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v3 for Xen 4.6 2/4] libxc: enable per-VCPU parameter settings for RTDS scheduler
2015-06-30 15:57 ` Chong Li
@ 2015-06-30 16:04 ` Ian Campbell
0 siblings, 0 replies; 39+ messages in thread
From: Ian Campbell @ 2015-06-30 16:04 UTC (permalink / raw)
To: Chong Li
Cc: Chong Li, Wei Liu, Sisu Xi, George Dunlap, dario.faggioli,
xen-devel, Meng Xu, Dagaen Golomb
On Tue, 2015-06-30 at 10:57 -0500, Chong Li wrote:
> On Tue, Jun 30, 2015 at 10:32 AM, Ian Campbell <ian.campbell@citrix.com> wrote:
> > On Tue, 2015-06-30 at 10:18 -0500, Chong Li wrote:
> >> On Tue, Jun 30, 2015 at 7:22 AM, Ian Campbell <ian.campbell@citrix.com> wrote:
> >> > On Sun, 2015-06-28 at 21:44 -0500, Chong Li wrote:
> >> >
> >> >> diff --git a/tools/libxc/xc_csched.c b/tools/libxc/xc_csched.c
> >> >> index 390c645..5a2bdf4 100644
> >> >> --- a/tools/libxc/xc_csched.c
> >> >> +++ b/tools/libxc/xc_csched.c
> >> >> @@ -36,7 +36,7 @@ xc_sched_credit_domain_set(
> >> >> domctl.domain = (domid_t) domid;
> >> >> domctl.u.scheduler_op.sched_id = XEN_SCHEDULER_CREDIT;
> >> >> domctl.u.scheduler_op.cmd = XEN_DOMCTL_SCHEDOP_putinfo;
> >> >> - domctl.u.scheduler_op.u.credit = *sdom;
> >> >> + domctl.u.scheduler_op.u.d.credit = *sdom;
> >> >
> >> > I don't see any change to any types in this series, which makes me
> >> > suspect it is in another patch and that bisectability is therefore not
> >> > maintained through the series. Each patch in the series needs to build
> >> > when they are applied one by one.
> >> >
> >> > This probably means that at to at least some degree hypercall interface
> >> > changes need to be done in the same time as the libxc adjustments. You
> >> > can still keep the _new_ functionality in a separate patch of course (if
> >> > that makes sense in this case).
> >>
> >> Because both per-domain and per-vcpu parameters are now unioned in
> >> struct xen_domctl_scheduler_op, we need to specify per-domain or
> >> per-vcpu when using domctl.u.scheduler_op.u (.d means per-domain, .v
> >> means per-vcpu). This change also happens to the XEN_DOMCTL_SCHEDOP_*
> >> handlers (of all schedulers), where the domctl.u.scheduler_op.u.* is
> >> passed to.
> >
> > I understand the interface change, but the problem is that the interface
> > change and the change ot the users of that interface seem to be in
> > different patches, which breaks bisectability.
> >
> > Does this series compile and work at every step?
> >
> > i.e. the following combinations of patches from this 4 patch series all
> > need to build and work successfully:
> >
> > Patch #1
> > Patch #1+#2
> > Patch #1+#2+#3
> > Patch #1+#2+#3+#4
> >
> > Is that the case?
>
> I see your point. Do you mean the change of struct
> xen_domctl_scheduler_op, the change of XEN_DOMCTL_SCHEDOP_* handlers
> (in xen) and the change of libxc (just shown above) should be in the
> same patch? Should the title of that patch be something like
> "xen+libxc: enable per-vcpu parameter settings for RTDS scheduler" ?
Ah I said, you should be able to do the nop interface change in one
patch and then add the bindings for the new scheduler in a separate one.
Ian.
>
> Chong
> >
> > Ian.
> >
>
>
>
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v3 for Xen 4.6 3/4] libxl: enable per-VCPU parameter settings for RTDS scheduler
2015-06-30 15:57 ` Ian Campbell
@ 2015-06-30 16:10 ` Chong Li
2015-06-30 16:19 ` Ian Campbell
0 siblings, 1 reply; 39+ messages in thread
From: Chong Li @ 2015-06-30 16:10 UTC (permalink / raw)
To: Ian Campbell
Cc: Chong Li, Wei Liu, Sisu Xi, George Dunlap, dario.faggioli,
Ian Jackson, xen-devel, Meng Xu, Dagaen Golomb
On Tue, Jun 30, 2015 at 10:57 AM, Ian Campbell <ian.campbell@citrix.com> wrote:
> On Tue, 2015-06-30 at 10:42 -0500, Chong Li wrote:
>> >> @@ -1554,10 +1564,17 @@ int libxl_sched_credit_params_set(libxl_ctx *ctx, uint32_t poolid,
>> >> #define LIBXL_DOMAIN_SCHED_PARAM_EXTRATIME_DEFAULT -1
>> >> #define LIBXL_DOMAIN_SCHED_PARAM_BUDGET_DEFAULT -1
>> >>
>> >> +/* Per-VCPU parameters*/
>> >> +#define LIBXL_DOMAIN_SCHED_PARAM_VCPU_INDEX_DEFAULT -1
>> >
>> > What is the effect of passing vcpuid == -1 to this interface?
>>
>> I think the right way should be:
>>
>> #define LIBXL_SCHED_PARAM_VCPU_INDEX_DEFAULT -1
>>
>> which is the default value for the vcpuid in libxl_sched_params. Even
>> though libxl_sched_params is used for per-vcpu settings, all the other
>> fields (except vcpuid) would still use
>> LIBXL_DOMAIN_SCHED_PARAM_*_DEFAULT values, just for convenience.
>
> My question is what does the concept of vcpu==-1 mean in this interface?
We need a default value for vcpuid when initiating libxl_sched_params,
just like other fields (e.g., budget, weight ...) in the struct. But
the default value could be others, maybe "0". I'm not sure.
>
> Ian.
>
--
Chong Li
Department of Computer Science and Engineering
Washington University in St.louis
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v3 for Xen 4.6 3/4] libxl: enable per-VCPU parameter settings for RTDS scheduler
2015-06-30 16:10 ` Chong Li
@ 2015-06-30 16:19 ` Ian Campbell
2015-06-30 16:53 ` Chong Li
2015-07-01 0:54 ` Meng Xu
0 siblings, 2 replies; 39+ messages in thread
From: Ian Campbell @ 2015-06-30 16:19 UTC (permalink / raw)
To: Chong Li
Cc: Chong Li, Wei Liu, Sisu Xi, George Dunlap, dario.faggioli,
Ian Jackson, xen-devel, Meng Xu, Dagaen Golomb
On Tue, 2015-06-30 at 11:10 -0500, Chong Li wrote:
> On Tue, Jun 30, 2015 at 10:57 AM, Ian Campbell <ian.campbell@citrix.com> wrote:
> > On Tue, 2015-06-30 at 10:42 -0500, Chong Li wrote:
> >> >> @@ -1554,10 +1564,17 @@ int libxl_sched_credit_params_set(libxl_ctx *ctx, uint32_t poolid,
> >> >> #define LIBXL_DOMAIN_SCHED_PARAM_EXTRATIME_DEFAULT -1
> >> >> #define LIBXL_DOMAIN_SCHED_PARAM_BUDGET_DEFAULT -1
> >> >>
> >> >> +/* Per-VCPU parameters*/
> >> >> +#define LIBXL_DOMAIN_SCHED_PARAM_VCPU_INDEX_DEFAULT -1
> >> >
> >> > What is the effect of passing vcpuid == -1 to this interface?
> >>
> >> I think the right way should be:
> >>
> >> #define LIBXL_SCHED_PARAM_VCPU_INDEX_DEFAULT -1
> >>
> >> which is the default value for the vcpuid in libxl_sched_params. Even
> >> though libxl_sched_params is used for per-vcpu settings, all the other
> >> fields (except vcpuid) would still use
> >> LIBXL_DOMAIN_SCHED_PARAM_*_DEFAULT values, just for convenience.
> >
> > My question is what does the concept of vcpu==-1 mean in this interface?
>
> We need a default value for vcpuid when initiating libxl_sched_params,
> just like other fields (e.g., budget, weight ...) in the struct. But
> the default value could be others, maybe "0". I'm not sure.
What will you do if you come across a -1 in this field? What are the
semantics of the interface WRT this field.
Note that this field is not the same as the others in this struct, it is
in effect part of the "key" while the others are the "values".
Ian.
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v3 for Xen 4.6 3/4] libxl: enable per-VCPU parameter settings for RTDS scheduler
2015-06-30 16:19 ` Ian Campbell
@ 2015-06-30 16:53 ` Chong Li
2015-07-01 0:54 ` Meng Xu
1 sibling, 0 replies; 39+ messages in thread
From: Chong Li @ 2015-06-30 16:53 UTC (permalink / raw)
To: Ian Campbell
Cc: Chong Li, Wei Liu, Sisu Xi, George Dunlap, dario.faggioli,
Ian Jackson, xen-devel, Meng Xu, Dagaen Golomb
On Tue, Jun 30, 2015 at 11:19 AM, Ian Campbell <ian.campbell@citrix.com> wrote:
> On Tue, 2015-06-30 at 11:10 -0500, Chong Li wrote:
>> On Tue, Jun 30, 2015 at 10:57 AM, Ian Campbell <ian.campbell@citrix.com> wrote:
>> > On Tue, 2015-06-30 at 10:42 -0500, Chong Li wrote:
>> >> >> @@ -1554,10 +1564,17 @@ int libxl_sched_credit_params_set(libxl_ctx *ctx, uint32_t poolid,
>> >> >> #define LIBXL_DOMAIN_SCHED_PARAM_EXTRATIME_DEFAULT -1
>> >> >> #define LIBXL_DOMAIN_SCHED_PARAM_BUDGET_DEFAULT -1
>> >> >>
>> >> >> +/* Per-VCPU parameters*/
>> >> >> +#define LIBXL_DOMAIN_SCHED_PARAM_VCPU_INDEX_DEFAULT -1
>> >> >
>> >> > What is the effect of passing vcpuid == -1 to this interface?
>> >>
>> >> I think the right way should be:
>> >>
>> >> #define LIBXL_SCHED_PARAM_VCPU_INDEX_DEFAULT -1
>> >>
>> >> which is the default value for the vcpuid in libxl_sched_params. Even
>> >> though libxl_sched_params is used for per-vcpu settings, all the other
>> >> fields (except vcpuid) would still use
>> >> LIBXL_DOMAIN_SCHED_PARAM_*_DEFAULT values, just for convenience.
>> >
>> > My question is what does the concept of vcpu==-1 mean in this interface?
>>
>> We need a default value for vcpuid when initiating libxl_sched_params,
>> just like other fields (e.g., budget, weight ...) in the struct. But
>> the default value could be others, maybe "0". I'm not sure.
>
> What will you do if you come across a -1 in this field? What are the
> semantics of the interface WRT this field.
>
> Note that this field is not the same as the others in this struct, it is
> in effect part of the "key" while the others are the "values".
>
For example, if vm1 has 4 VCPUs, and the user types in:
"xl rtds -d vm1 -v 1 -v 3"
to get (output) the per-vcpu information of VCPU 1 and VCPU 3. In
libxl.c, we create an array (say vcpus), which has two elements (each
one is a libxl_sched_params). Then we do:
vcpus[0].vcpuid = 1; vcpus[1].vcpuid = 3;
and all the other fields in each libxl_sched_params keep empty, but
will be filled in by the hypercall handler.
So vcpuid == -1 only exists during initiation, and it will be
definitely given a valid value later.
Chong
> Ian.
>
--
Chong Li
Department of Computer Science and Engineering
Washington University in St.louis
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v3 for Xen 4.6 3/4] libxl: enable per-VCPU parameter settings for RTDS scheduler
2015-06-30 16:19 ` Ian Campbell
2015-06-30 16:53 ` Chong Li
@ 2015-07-01 0:54 ` Meng Xu
2015-07-01 8:48 ` Ian Campbell
1 sibling, 1 reply; 39+ messages in thread
From: Meng Xu @ 2015-07-01 0:54 UTC (permalink / raw)
To: Ian Campbell
Cc: Chong Li, Wei Liu, Sisu Xi, George Dunlap, dario.faggioli,
Ian Jackson, xen-devel, Meng Xu, Chong Li, Dagaen Golomb
Hi Ian,
2015-06-30 9:19 GMT-07:00 Ian Campbell <ian.campbell@citrix.com>:
> On Tue, 2015-06-30 at 11:10 -0500, Chong Li wrote:
>> On Tue, Jun 30, 2015 at 10:57 AM, Ian Campbell <ian.campbell@citrix.com> wrote:
>> > On Tue, 2015-06-30 at 10:42 -0500, Chong Li wrote:
>> >> >> @@ -1554,10 +1564,17 @@ int libxl_sched_credit_params_set(libxl_ctx *ctx, uint32_t poolid,
>> >> >> #define LIBXL_DOMAIN_SCHED_PARAM_EXTRATIME_DEFAULT -1
>> >> >> #define LIBXL_DOMAIN_SCHED_PARAM_BUDGET_DEFAULT -1
>> >> >>
>> >> >> +/* Per-VCPU parameters*/
>> >> >> +#define LIBXL_DOMAIN_SCHED_PARAM_VCPU_INDEX_DEFAULT -1
>> >> >
>> >> > What is the effect of passing vcpuid == -1 to this interface?
>> >>
>> >> I think the right way should be:
>> >>
>> >> #define LIBXL_SCHED_PARAM_VCPU_INDEX_DEFAULT -1
>> >>
>> >> which is the default value for the vcpuid in libxl_sched_params. Even
>> >> though libxl_sched_params is used for per-vcpu settings, all the other
>> >> fields (except vcpuid) would still use
>> >> LIBXL_DOMAIN_SCHED_PARAM_*_DEFAULT values, just for convenience.
>> >
>> > My question is what does the concept of vcpu==-1 mean in this interface?
>>
>> We need a default value for vcpuid when initiating libxl_sched_params,
>> just like other fields (e.g., budget, weight ...) in the struct. But
>> the default value could be others, maybe "0". I'm not sure.
>
> What will you do if you come across a -1 in this field? What are the
> semantics of the interface WRT this field.
>
> Note that this field is not the same as the others in this struct, it is
> in effect part of the "key" while the others are the "values".
>
vcpuid in the libxl is not the key but the value.
When it is passed into hypervisor, vcpuid acts as the key to identify
which VCPU. vcpuid should be be negative when it is passed to
hypervisor.
Do you have any concerns about assigning the initial value of vcpuid as -1?
Thanks,
Meng
-----------
Meng Xu
PhD Student in Computer and Information Science
University of Pennsylvania
http://www.cis.upenn.edu/~mengxu/
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v3 for Xen 4.6 3/4] libxl: enable per-VCPU parameter settings for RTDS scheduler
2015-07-01 0:54 ` Meng Xu
@ 2015-07-01 8:48 ` Ian Campbell
2015-07-01 12:50 ` Dario Faggioli
0 siblings, 1 reply; 39+ messages in thread
From: Ian Campbell @ 2015-07-01 8:48 UTC (permalink / raw)
To: Meng Xu
Cc: Chong Li, Wei Liu, Sisu Xi, George Dunlap, dario.faggioli,
Ian Jackson, xen-devel, Meng Xu, Chong Li, Dagaen Golomb
On Tue, 2015-06-30 at 17:54 -0700, Meng Xu wrote:
> Hi Ian,
>
> 2015-06-30 9:19 GMT-07:00 Ian Campbell <ian.campbell@citrix.com>:
> > On Tue, 2015-06-30 at 11:10 -0500, Chong Li wrote:
> >> On Tue, Jun 30, 2015 at 10:57 AM, Ian Campbell <ian.campbell@citrix.com> wrote:
> >> > On Tue, 2015-06-30 at 10:42 -0500, Chong Li wrote:
> >> >> >> @@ -1554,10 +1564,17 @@ int libxl_sched_credit_params_set(libxl_ctx *ctx, uint32_t poolid,
> >> >> >> #define LIBXL_DOMAIN_SCHED_PARAM_EXTRATIME_DEFAULT -1
> >> >> >> #define LIBXL_DOMAIN_SCHED_PARAM_BUDGET_DEFAULT -1
> >> >> >>
> >> >> >> +/* Per-VCPU parameters*/
> >> >> >> +#define LIBXL_DOMAIN_SCHED_PARAM_VCPU_INDEX_DEFAULT -1
> >> >> >
> >> >> > What is the effect of passing vcpuid == -1 to this interface?
> >> >>
> >> >> I think the right way should be:
> >> >>
> >> >> #define LIBXL_SCHED_PARAM_VCPU_INDEX_DEFAULT -1
> >> >>
> >> >> which is the default value for the vcpuid in libxl_sched_params. Even
> >> >> though libxl_sched_params is used for per-vcpu settings, all the other
> >> >> fields (except vcpuid) would still use
> >> >> LIBXL_DOMAIN_SCHED_PARAM_*_DEFAULT values, just for convenience.
> >> >
> >> > My question is what does the concept of vcpu==-1 mean in this interface?
> >>
> >> We need a default value for vcpuid when initiating libxl_sched_params,
> >> just like other fields (e.g., budget, weight ...) in the struct. But
> >> the default value could be others, maybe "0". I'm not sure.
> >
> > What will you do if you come across a -1 in this field? What are the
> > semantics of the interface WRT this field.
> >
> > Note that this field is not the same as the others in this struct, it is
> > in effect part of the "key" while the others are the "values".
> >
>
> vcpuid in the libxl is not the key but the value.
>
> When it is passed into hypervisor, vcpuid acts as the key to identify
> which VCPU. vcpuid should be be negative when it is passed to
> hypervisor.
>
> Do you have any concerns about assigning the initial value of vcpuid as -1?
libxl needs to do _something_ if it is passed a vcpuid=-1. What is that
something?
Ian.
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v3 for Xen 4.6 3/4] libxl: enable per-VCPU parameter settings for RTDS scheduler
2015-07-01 8:48 ` Ian Campbell
@ 2015-07-01 12:50 ` Dario Faggioli
2015-07-01 16:59 ` Chong Li
0 siblings, 1 reply; 39+ messages in thread
From: Dario Faggioli @ 2015-07-01 12:50 UTC (permalink / raw)
To: Meng Xu, Chong Li
Cc: Wei Liu, Ian Campbell, Sisu Xi, George Dunlap, Ian Jackson,
xen-devel, Meng Xu, Chong Li, Dagaen Golomb
[-- Attachment #1.1: Type: text/plain, Size: 2039 bytes --]
On Wed, 2015-07-01 at 09:48 +0100, Ian Campbell wrote:
> On Tue, 2015-06-30 at 17:54 -0700, Meng Xu wrote:
> > 2015-06-30 9:19 GMT-07:00 Ian Campbell <ian.campbell@citrix.com>:
> > > Note that this field is not the same as the others in this struct, it is
> > > in effect part of the "key" while the others are the "values".
> > >
> >
> > vcpuid in the libxl is not the key but the value.
> >
> > When it is passed into hypervisor, vcpuid acts as the key to identify
> > which VCPU. vcpuid should be be negative when it is passed to
> > hypervisor.
> >
> > Do you have any concerns about assigning the initial value of vcpuid as -1?
>
> libxl needs to do _something_ if it is passed a vcpuid=-1. What is that
> something?
>
Exactly.
Meng, Chong, maybe what you are missing from Ian's point is the fact
that libxl is a library on top of which to build more advanced
toolstack, it is not just something that the xl command line tool links
to.
Therefore, it has to work even when it is not xl that makes the API
calls. Actually, I better say that --not only it has to work-- it has to
be easy and comfortable to use from high level components different than
xl.
The outcome of this reasoning is that, you can't just assume that a
default value does not matter much, because every code path in _xl_
overrides it. Everything that is in libxl must make sense in libxl
per-se, even without bringing xl into the picture.
So, in this case, you need to think whether it is convenient
_in_general_, not only wrt xl, to have -1 as default value for vcpuid
and, if yes, what happens if such default is not overridden and libxl
gets to deal with it!
Hope this helps... If this was not the issue, sorry for the noise. :-)
Regards,
Dario
--
<<This happens because I choose it to happen!>> (Raistlin Majere)
-----------------------------------------------------------------
Dario Faggioli, Ph.D, http://about.me/dario.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: 181 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] 39+ messages in thread
* Re: [PATCH v3 for Xen 4.6 3/4] libxl: enable per-VCPU parameter settings for RTDS scheduler
2015-07-01 12:50 ` Dario Faggioli
@ 2015-07-01 16:59 ` Chong Li
0 siblings, 0 replies; 39+ messages in thread
From: Chong Li @ 2015-07-01 16:59 UTC (permalink / raw)
To: Dario Faggioli
Cc: Chong Li, Wei Liu, Ian Campbell, Sisu Xi, George Dunlap,
Ian Jackson, xen-devel, Meng Xu, Meng Xu, Dagaen Golomb
On Wed, Jul 1, 2015 at 7:50 AM, Dario Faggioli
<dario.faggioli@citrix.com> wrote:
> On Wed, 2015-07-01 at 09:48 +0100, Ian Campbell wrote:
>> On Tue, 2015-06-30 at 17:54 -0700, Meng Xu wrote:
>> > 2015-06-30 9:19 GMT-07:00 Ian Campbell <ian.campbell@citrix.com>:
>> > > Note that this field is not the same as the others in this struct, it is
>> > > in effect part of the "key" while the others are the "values".
>> > >
>> >
>> > vcpuid in the libxl is not the key but the value.
>> >
>> > When it is passed into hypervisor, vcpuid acts as the key to identify
>> > which VCPU. vcpuid should be be negative when it is passed to
>> > hypervisor.
>> >
>> > Do you have any concerns about assigning the initial value of vcpuid as -1?
>>
>> libxl needs to do _something_ if it is passed a vcpuid=-1. What is that
>> something?
>>
> Exactly.
>
> Meng, Chong, maybe what you are missing from Ian's point is the fact
> that libxl is a library on top of which to build more advanced
> toolstack, it is not just something that the xl command line tool links
> to.
>
> Therefore, it has to work even when it is not xl that makes the API
> calls. Actually, I better say that --not only it has to work-- it has to
> be easy and comfortable to use from high level components different than
> xl.
>
> The outcome of this reasoning is that, you can't just assume that a
> default value does not matter much, because every code path in _xl_
> overrides it. Everything that is in libxl must make sense in libxl
> per-se, even without bringing xl into the picture.
>
> So, in this case, you need to think whether it is convenient
> _in_general_, not only wrt xl, to have -1 as default value for vcpuid
> and, if yes, what happens if such default is not overridden and libxl
> gets to deal with it!
>
> Hope this helps... If this was not the issue, sorry for the noise. :-)
I did some sanity check about the vcpuid in hypervisor. Maybe that's
not enough. I'll add other check on vcpuid in libxl
(sched_rtds_vcpu_get/set) to ensure vcpuid has a valid value.
Chong
>
> Regards,
> Dario
>
> --
> <<This happens because I choose it to happen!>> (Raistlin Majere)
> -----------------------------------------------------------------
> Dario Faggioli, Ph.D, http://about.me/dario.faggioli
> Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)
--
Chong Li
Department of Computer Science and Engineering
Washington University in St.louis
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v3 for Xen 4.6 1/4] xen: enable per-VCPU parameter settings for RTDS scheduler
2015-06-29 2:44 ` [PATCH v3 for Xen 4.6 1/4] xen: enable " Chong Li
@ 2015-07-07 8:59 ` Jan Beulich
2015-07-07 14:39 ` Dario Faggioli
` (2 more replies)
2015-07-07 14:51 ` Dario Faggioli
1 sibling, 3 replies; 39+ messages in thread
From: Jan Beulich @ 2015-07-07 8:59 UTC (permalink / raw)
To: Chong Li
Cc: Chong Li, Sisu Xi, george.dunlap, dario.faggioli, xen-devel,
Meng Xu, dgolomb
>>> On 29.06.15 at 04:44, <lichong659@gmail.com> wrote:
> --- a/xen/common/Makefile
> +++ b/xen/common/Makefile
> @@ -31,7 +31,6 @@ obj-y += rbtree.o
> obj-y += rcupdate.o
> obj-y += sched_credit.o
> obj-y += sched_credit2.o
> -obj-y += sched_sedf.o
> obj-y += sched_arinc653.o
> obj-y += sched_rt.o
> obj-y += schedule.o
Stray change. Or perhaps the file doesn't build anymore, in which case
you should instead have stated that the patch is dependent upon the
series removing SEDF.
> @@ -1157,8 +1158,75 @@ rt_dom_cntl(
> list_for_each( iter, &sdom->vcpu )
> {
> struct rt_vcpu * svc = list_entry(iter, struct rt_vcpu, sdom_elem);
> - svc->period = MICROSECS(op->u.rtds.period); /* transfer to nanosec */
> - svc->budget = MICROSECS(op->u.rtds.budget);
> + svc->period = MICROSECS(op->u.d.rtds.period); /* transfer to nanosec */
> + svc->budget = MICROSECS(op->u.d.rtds.budget);
> + }
> + spin_unlock_irqrestore(&prv->lock, flags);
> + break;
> + case XEN_DOMCTL_SCHEDOP_getvcpuinfo:
> + spin_lock_irqsave(&prv->lock, flags);
> + for( index = 0; index < op->u.v.nr_vcpus; index++ )
Coding style (more further down).
> + {
> + if ( copy_from_guest_offset(&local_sched,
> + op->u.v.vcpus, index, 1) )
Indentation.
> + {
> + rc = -EFAULT;
> + break;
> + }
> + if ( local_sched.vcpuid >= d->max_vcpus
> + || d->vcpu[local_sched.vcpuid] == NULL )
|| belongs at the end of the first line. Indentation.
> + {
> + rc = -EINVAL;
> + break;
> + }
> + svc = rt_vcpu(d->vcpu[local_sched.vcpuid]);
> +
> + local_sched.vcpuid = svc->vcpu->vcpu_id;
Why? If at all, this should be an ASSERT().
> + local_sched.s.rtds.budget = svc->budget / MICROSECS(1);
> + local_sched.s.rtds.period = svc->period / MICROSECS(1);
> + if( index >= op->u.v.nr_vcpus ) /* not enough guest buffer*/
Impossible due to the containing loop's condition.
> + {
> + rc = -ENOBUFS;
> + break;
> + }
> + if ( copy_to_guest_offset(op->u.v.vcpus, index,
__copy_to_guest_offset()
> + case XEN_DOMCTL_SCHEDOP_putvcpuinfo:
> + spin_lock_irqsave(&prv->lock, flags);
> + for( index = 0; index < op->u.v.nr_vcpus; index++ )
> + {
> + if ( copy_from_guest_offset(&local_sched,
> + op->u.v.vcpus, index, 1) )
> + {
> + rc = -EFAULT;
> + break;
> + }
> + if ( local_sched.vcpuid >= d->max_vcpus
> + || d->vcpu[local_sched.vcpuid] == NULL )
> + {
> + rc = -EINVAL;
> + break;
> + }
> + svc = rt_vcpu(d->vcpu[local_sched.vcpuid]);
> + svc->period = MICROSECS(local_sched.s.rtds.period);
> + svc->budget = MICROSECS(local_sched.s.rtds.budget);
Are all input values valid here?
> --- a/xen/common/schedule.c
> +++ b/xen/common/schedule.c
> @@ -65,7 +65,6 @@ DEFINE_PER_CPU(struct schedule_data, schedule_data);
> DEFINE_PER_CPU(struct scheduler *, scheduler);
>
> static const struct scheduler *schedulers[] = {
> - &sched_sedf_def,
> &sched_credit_def,
> &sched_credit2_def,
> &sched_arinc653_def,
See above.
> @@ -1054,7 +1053,9 @@ long sched_adjust(struct domain *d, struct xen_domctl_scheduler_op *op)
>
> if ( (op->sched_id != DOM2OP(d)->sched_id) ||
> ((op->cmd != XEN_DOMCTL_SCHEDOP_putinfo) &&
> - (op->cmd != XEN_DOMCTL_SCHEDOP_getinfo)) )
> + (op->cmd != XEN_DOMCTL_SCHEDOP_getinfo) &&
> + (op->cmd != XEN_DOMCTL_SCHEDOP_putvcpuinfo) &&
> + (op->cmd != XEN_DOMCTL_SCHEDOP_getvcpuinfo)) )
> return -EINVAL;
Convert to switch() please.
> --- a/xen/include/public/domctl.h
> +++ b/xen/include/public/domctl.h
> @@ -330,31 +330,59 @@ DEFINE_XEN_GUEST_HANDLE(xen_domctl_max_vcpus_t);
> #define XEN_SCHEDULER_ARINC653 7
> #define XEN_SCHEDULER_RTDS 8
>
> +typedef struct xen_domctl_sched_sedf {
> + uint64_aligned_t period;
> + uint64_aligned_t slice;
> + uint64_aligned_t latency;
> + uint32_t extratime;
> + uint32_t weight;
> +} xen_domctl_sched_sedf_t;
Indentation.
> +typedef union xen_domctl_schedparam {
> + xen_domctl_sched_sedf_t sedf;
> + xen_domctl_sched_credit_t credit;
> + xen_domctl_sched_credit2_t credit2;
> + xen_domctl_sched_rtds_t rtds;
> +} xen_domctl_schedparam_t;
I don't see the need for this extra wrapper type. Nor do I see the
need for the typedef here and above - they're generally only
created if you want to also define a matching guest handle type.
> +typedef struct xen_domctl_schedparam_vcpu {
> + union {
> + xen_domctl_sched_credit_t credit;
> + xen_domctl_sched_credit2_t credit2;
> + xen_domctl_sched_rtds_t rtds;
> + } s;
> + uint16_t vcpuid;
Explicit padding please.
> +} xen_domctl_schedparam_vcpu_t;
> +DEFINE_XEN_GUEST_HANDLE(xen_domctl_schedparam_vcpu_t);
> +
> /* Set or get info? */
> #define XEN_DOMCTL_SCHEDOP_putinfo 0
> #define XEN_DOMCTL_SCHEDOP_getinfo 1
> +#define XEN_DOMCTL_SCHEDOP_putvcpuinfo 2
> +#define XEN_DOMCTL_SCHEDOP_getvcpuinfo 3
> 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;
> - struct xen_domctl_sched_rtds {
> - uint32_t period;
> - uint32_t budget;
> - } rtds;
> + xen_domctl_schedparam_t d;
With this type gone I'm not even sure we need to wrap this in
another union; not doing so would eliminate some of the other
changes in this patch.
Jan
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v3 for Xen 4.6 1/4] xen: enable per-VCPU parameter settings for RTDS scheduler
2015-07-07 8:59 ` Jan Beulich
@ 2015-07-07 14:39 ` Dario Faggioli
2015-07-08 6:06 ` Meng Xu
2015-07-07 14:55 ` Dario Faggioli
2015-07-07 15:33 ` Chong Li
2 siblings, 1 reply; 39+ messages in thread
From: Dario Faggioli @ 2015-07-07 14:39 UTC (permalink / raw)
To: Chong Li, Chong Li
Cc: Sisu Xi, george.dunlap, xen-devel, Meng Xu, Jan Beulich, dgolomb
[-- Attachment #1.1: Type: text/plain, Size: 2690 bytes --]
On Tue, 2015-07-07 at 09:59 +0100, Jan Beulich wrote:
> >>> On 29.06.15 at 04:44, <lichong659@gmail.com> wrote:
> > --- a/xen/common/Makefile
> > +++ b/xen/common/Makefile
> > @@ -31,7 +31,6 @@ obj-y += rbtree.o
> > obj-y += rcupdate.o
> > obj-y += sched_credit.o
> > obj-y += sched_credit2.o
> > -obj-y += sched_sedf.o
> > obj-y += sched_arinc653.o
> > obj-y += sched_rt.o
> > obj-y += schedule.o
>
> Stray change. Or perhaps the file doesn't build anymore, in which case
> you should instead have stated that the patch is dependent upon the
> series removing SEDF.
>
This indeed does not belong in here. And of course, things should
build... So, Chong, either deal with SEDF as well, if basing your
patches on a tree where it is still there, or base on top of my patches,
ignore it, but state the dependency, as Jan is asking.
> > @@ -1157,8 +1158,75 @@ rt_dom_cntl(
> > + case XEN_DOMCTL_SCHEDOP_putvcpuinfo:
> > + spin_lock_irqsave(&prv->lock, flags);
> > + for( index = 0; index < op->u.v.nr_vcpus; index++ )
> > + {
> > + if ( copy_from_guest_offset(&local_sched,
> > + op->u.v.vcpus, index, 1) )
> > + {
> > + rc = -EFAULT;
> > + break;
> > + }
> > + if ( local_sched.vcpuid >= d->max_vcpus
> > + || d->vcpu[local_sched.vcpuid] == NULL )
> > + {
> > + rc = -EINVAL;
> > + break;
> > + }
> > + svc = rt_vcpu(d->vcpu[local_sched.vcpuid]);
> > + svc->period = MICROSECS(local_sched.s.rtds.period);
> > + svc->budget = MICROSECS(local_sched.s.rtds.budget);
>
> Are all input values valid here?
>
That's a good point, actually. Right now, SEDF does some range
enforcement, by means of these values:
#define PERIOD_MAX MILLISECS(10000) /* 10s */
#define PERIOD_MIN (MICROSECS(10)) /* 10us */
#define SLICE_MIN (MICROSECS(5)) /* 5us */
Chong, it probably makes sense to (in a separate patch), introduce
something like this in RTDS too (with SLICE_MIN-->BUDGET_MIN), and then
use them, in this patch, for sanity checking the input.
It also makes sense to check and enforce budget<=period, IMO.
About the specific values, I'm open to proposals. I think something like
the SEDF's one is fine. Meng?
Thanks and Regards,
Dario
--
<<This happens because I choose it to happen!>> (Raistlin Majere)
-----------------------------------------------------------------
Dario Faggioli, Ph.D, http://about.me/dario.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: 181 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] 39+ messages in thread
* Re: [PATCH v3 for Xen 4.6 1/4] xen: enable per-VCPU parameter settings for RTDS scheduler
2015-06-29 2:44 ` [PATCH v3 for Xen 4.6 1/4] xen: enable " Chong Li
2015-07-07 8:59 ` Jan Beulich
@ 2015-07-07 14:51 ` Dario Faggioli
1 sibling, 0 replies; 39+ messages in thread
From: Dario Faggioli @ 2015-07-07 14:51 UTC (permalink / raw)
To: Chong Li
Cc: Chong Li, Sisu Xi, george.dunlap, xen-devel, Meng Xu, jbeulich,
dgolomb
[-- Attachment #1.1: Type: text/plain, Size: 1639 bytes --]
So, about the structure of the patch/changelog/etc, taking this patch as
an example.
On Sun, 2015-06-28 at 21:44 -0500, Chong Li wrote:
> Add two hypercalls(XEN_DOMCTL_SCHEDOP_getvcpuinfo/putvcpuinfo) to get/set a domain's
> per-VCPU parameters.
>
This is almost ok. Line should be shorter (try a `git log' with this
patch committed, and check how it looks).
Also, I'd rephrase it as "Add XEN_DOMCTL_SCHEDOP_getvcpuinfo and
_putvcpuinfo hypercalls to independently get and set the scheduling
parameters of each vCPU of a domain"
Or something like this.
> Changes on PATCH v2:
>
So, this kind of summary goes below the '---', as we don't want it in
the final history. And what it should contain what actually changed
between previous versions and this one, while this...
> 1) Change struct xen_domctl_scheduler_op, for transferring per-vcpu parameters
> between libxc and hypervisor.
>
> 2) Handler of XEN_DOMCTL_SCHEDOP_getinfo now just returns the default budget and period values of RTDS scheduler.
>
> 3) Handler of XEN_DOMCTL_SCHEDOP_getvcpuinfo now can return a random subset of the parameters of the VCPUs of a specific domain
>
... looks more like something that could actually be in the actual
changelog (so not under any 'Changes on PATCH v2' preamble), as it
dscribes how the hypercall interface looks like.
Regards,
Dario
--
<<This happens because I choose it to happen!>> (Raistlin Majere)
-----------------------------------------------------------------
Dario Faggioli, Ph.D, http://about.me/dario.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: 181 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] 39+ messages in thread
* Re: [PATCH v3 for Xen 4.6 1/4] xen: enable per-VCPU parameter settings for RTDS scheduler
2015-07-07 8:59 ` Jan Beulich
2015-07-07 14:39 ` Dario Faggioli
@ 2015-07-07 14:55 ` Dario Faggioli
2015-07-07 16:09 ` Jan Beulich
2015-07-07 15:33 ` Chong Li
2 siblings, 1 reply; 39+ messages in thread
From: Dario Faggioli @ 2015-07-07 14:55 UTC (permalink / raw)
To: Jan Beulich
Cc: Chong Li, Sisu Xi, george.dunlap, xen-devel, Meng Xu, Chong Li,
dgolomb
[-- Attachment #1.1.1: Type: text/plain, Size: 9958 bytes --]
On Tue, 2015-07-07 at 09:59 +0100, Jan Beulich wrote:
> >>> On 29.06.15 at 04:44, <lichong659@gmail.com> wrote:
> > --- a/xen/common/Makefile
> > +++ b/xen/common/Makefile
> > @@ -31,7 +31,6 @@ obj-y += rbtree.o
> > obj-y += rcupdate.o
> > obj-y += sched_credit.o
> > obj-y += sched_credit2.o
> > -obj-y += sched_sedf.o
> > obj-y += sched_arinc653.o
> > obj-y += sched_rt.o
> > obj-y += schedule.o
>
> Stray change. Or perhaps the file doesn't build anymore, in which case
> you should instead have stated that the patch is dependent upon the
> series removing SEDF.
>
> > @@ -1157,8 +1158,75 @@ rt_dom_cntl(
> > list_for_each( iter, &sdom->vcpu )
> > {
> > struct rt_vcpu * svc = list_entry(iter, struct rt_vcpu, sdom_elem);
> > - svc->period = MICROSECS(op->u.rtds.period); /* transfer to nanosec */
> > - svc->budget = MICROSECS(op->u.rtds.budget);
> > + svc->period = MICROSECS(op->u.d.rtds.period); /* transfer to nanosec */
> > + svc->budget = MICROSECS(op->u.d.rtds.budget);
> > + }
> > + spin_unlock_irqrestore(&prv->lock, flags);
> > + break;
> > + case XEN_DOMCTL_SCHEDOP_getvcpuinfo:
> > + spin_lock_irqsave(&prv->lock, flags);
> > + for( index = 0; index < op->u.v.nr_vcpus; index++ )
>
> Coding style (more further down).
>
> > + {
> > + if ( copy_from_guest_offset(&local_sched,
> > + op->u.v.vcpus, index, 1) )
>
> Indentation.
>
> > + {
> > + rc = -EFAULT;
> > + break;
> > + }
> > + if ( local_sched.vcpuid >= d->max_vcpus
> > + || d->vcpu[local_sched.vcpuid] == NULL )
>
> || belongs at the end of the first line. Indentation.
>
> > + {
> > + rc = -EINVAL;
> > + break;
> > + }
> > + svc = rt_vcpu(d->vcpu[local_sched.vcpuid]);
> > +
> > + local_sched.vcpuid = svc->vcpu->vcpu_id;
>
> Why? If at all, this should be an ASSERT().
>
> > + local_sched.s.rtds.budget = svc->budget / MICROSECS(1);
> > + local_sched.s.rtds.period = svc->period / MICROSECS(1);
> > + if( index >= op->u.v.nr_vcpus ) /* not enough guest buffer*/
>
> Impossible due to the containing loop's condition.
>
> > + {
> > + rc = -ENOBUFS;
> > + break;
> > + }
> > + if ( copy_to_guest_offset(op->u.v.vcpus, index,
>
> __copy_to_guest_offset()
>
> > + case XEN_DOMCTL_SCHEDOP_putvcpuinfo:
> > + spin_lock_irqsave(&prv->lock, flags);
> > + for( index = 0; index < op->u.v.nr_vcpus; index++ )
> > + {
> > + if ( copy_from_guest_offset(&local_sched,
> > + op->u.v.vcpus, index, 1) )
> > + {
> > + rc = -EFAULT;
> > + break;
> > + }
> > + if ( local_sched.vcpuid >= d->max_vcpus
> > + || d->vcpu[local_sched.vcpuid] == NULL )
> > + {
> > + rc = -EINVAL;
> > + break;
> > + }
> > + svc = rt_vcpu(d->vcpu[local_sched.vcpuid]);
> > + svc->period = MICROSECS(local_sched.s.rtds.period);
> > + svc->budget = MICROSECS(local_sched.s.rtds.budget);
>
> Are all input values valid here?
>
> > --- a/xen/common/schedule.c
> > +++ b/xen/common/schedule.c
> > @@ -65,7 +65,6 @@ DEFINE_PER_CPU(struct schedule_data, schedule_data);
> > DEFINE_PER_CPU(struct scheduler *, scheduler);
> >
> > static const struct scheduler *schedulers[] = {
> > - &sched_sedf_def,
> > &sched_credit_def,
> > &sched_credit2_def,
> > &sched_arinc653_def,
>
> See above.
>
> > @@ -1054,7 +1053,9 @@ long sched_adjust(struct domain *d, struct xen_domctl_scheduler_op *op)
> >
> > if ( (op->sched_id != DOM2OP(d)->sched_id) ||
> > ((op->cmd != XEN_DOMCTL_SCHEDOP_putinfo) &&
> > - (op->cmd != XEN_DOMCTL_SCHEDOP_getinfo)) )
> > + (op->cmd != XEN_DOMCTL_SCHEDOP_getinfo) &&
> > + (op->cmd != XEN_DOMCTL_SCHEDOP_putvcpuinfo) &&
> > + (op->cmd != XEN_DOMCTL_SCHEDOP_getvcpuinfo)) )
> > return -EINVAL;
>
> Convert to switch() please.
>
> > --- a/xen/include/public/domctl.h
> > +++ b/xen/include/public/domctl.h
> > @@ -330,31 +330,59 @@ DEFINE_XEN_GUEST_HANDLE(xen_domctl_max_vcpus_t);
> > #define XEN_SCHEDULER_ARINC653 7
> > #define XEN_SCHEDULER_RTDS 8
> >
> > +typedef struct xen_domctl_sched_sedf {
> > + uint64_aligned_t period;
> > + uint64_aligned_t slice;
> > + uint64_aligned_t latency;
> > + uint32_t extratime;
> > + uint32_t weight;
> > +} xen_domctl_sched_sedf_t;
>
> Indentation.
>
> > +typedef union xen_domctl_schedparam {
> > + xen_domctl_sched_sedf_t sedf;
> > + xen_domctl_sched_credit_t credit;
> > + xen_domctl_sched_credit2_t credit2;
> > + xen_domctl_sched_rtds_t rtds;
> > +} xen_domctl_schedparam_t;
>
> I don't see the need for this extra wrapper type. Nor do I see the
> need for the typedef here and above - they're generally only
> created if you want to also define a matching guest handle type.
>
> > +typedef struct xen_domctl_schedparam_vcpu {
> > + union {
> > + xen_domctl_sched_credit_t credit;
> > + xen_domctl_sched_credit2_t credit2;
> > + xen_domctl_sched_rtds_t rtds;
> > + } s;
> > + uint16_t vcpuid;
>
> Explicit padding please.
>
> > +} xen_domctl_schedparam_vcpu_t;
> > +DEFINE_XEN_GUEST_HANDLE(xen_domctl_schedparam_vcpu_t);
> > +
> > /* Set or get info? */
> > #define XEN_DOMCTL_SCHEDOP_putinfo 0
> > #define XEN_DOMCTL_SCHEDOP_getinfo 1
> > +#define XEN_DOMCTL_SCHEDOP_putvcpuinfo 2
> > +#define XEN_DOMCTL_SCHEDOP_getvcpuinfo 3
> > 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;
> > - struct xen_domctl_sched_rtds {
> > - uint32_t period;
> > - uint32_t budget;
> > - } rtds;
> > + xen_domctl_schedparam_t d;
>
> With this type gone I'm not even sure we need to wrap this in
> another union; not doing so would eliminate some of the other
> changes in this patch.
>
So, Jan, just to be sure, do you mean (apart from the explicit padding)
something like this (attached, also)?
diff --git a/xen/include/public/domctl.h b/xen/include/public/domctl.h
index bc45ea5..8210ecb 100644
--- a/xen/include/public/domctl.h
+++ b/xen/include/public/domctl.h
@@ -330,31 +330,56 @@ DEFINE_XEN_GUEST_HANDLE(xen_domctl_max_vcpus_t);
#define XEN_SCHEDULER_ARINC653 7
#define XEN_SCHEDULER_RTDS 8
+struct xen_domctl_sched_sedf {
+ uint64_aligned_t period;
+ uint64_aligned_t slice;
+ uint64_aligned_t latency;
+ uint32_t extratime;
+ uint32_t weight;
+};
+
+struct xen_domctl_sched_credit {
+ uint16_t weight;
+ uint16_t cap;
+};
+
+struct xen_domctl_sched_credit2 {
+ uint16_t weight;
+};
+
+struct xen_domctl_sched_rtds {
+ uint32_t period;
+ uint32_t budget;
+};
+
+typedef struct xen_domctl_schedparam_vcpu {
+ union {
+ struct xen_domctl_sched_sedf sedf;
+ struct xen_domctl_sched_credit credit;
+ struct xen_domctl_sched_credit2 credit2;
+ struct xen_domctl_sched_rtds rtds;
+ } s;
+ uint16_t vcpuid;
+} xen_domctl_schedparam_vcpu_t;
+DEFINE_XEN_GUEST_HANDLE(xen_domctl_schedparam_vcpu_t);
+
/* Set or get info? */
#define XEN_DOMCTL_SCHEDOP_putinfo 0
#define XEN_DOMCTL_SCHEDOP_getinfo 1
+#define XEN_DOMCTL_SCHEDOP_putvcpuinfo 2
+#define XEN_DOMCTL_SCHEDOP_getvcpuinfo 3
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;
- struct xen_domctl_sched_rtds {
- uint32_t period;
- uint32_t budget;
- } rtds;
+ struct xen_domctl_sched_sedf sedf;
+ struct xen_domctl_sched_credit credit;
+ struct xen_domctl_sched_credit2 credit2;
+ struct xen_domctl_sched_rtds rtds;
+ struct {
+ XEN_GUEST_HANDLE_64(xen_domctl_schedparam_vcpu_t) vcpus;
+ uint16_t nr_vcpus;
+ } v;
} u;
};
typedef struct xen_domctl_scheduler_op xen_domctl_scheduler_op_t;
If yes, I'd be fine with it too (George?)
Thanks and Regards,
Dario
--
<<This happens because I choose it to happen!>> (Raistlin Majere)
-----------------------------------------------------------------
Dario Faggioli, Ph.D, http://about.me/dario.faggioli
Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)
[-- Attachment #1.1.2: rtds-domctl.patch --]
[-- Type: text/x-patch, Size: 2371 bytes --]
diff --git a/xen/include/public/domctl.h b/xen/include/public/domctl.h
index bc45ea5..8210ecb 100644
--- a/xen/include/public/domctl.h
+++ b/xen/include/public/domctl.h
@@ -330,31 +330,56 @@ DEFINE_XEN_GUEST_HANDLE(xen_domctl_max_vcpus_t);
#define XEN_SCHEDULER_ARINC653 7
#define XEN_SCHEDULER_RTDS 8
+struct xen_domctl_sched_sedf {
+ uint64_aligned_t period;
+ uint64_aligned_t slice;
+ uint64_aligned_t latency;
+ uint32_t extratime;
+ uint32_t weight;
+};
+
+struct xen_domctl_sched_credit {
+ uint16_t weight;
+ uint16_t cap;
+};
+
+struct xen_domctl_sched_credit2 {
+ uint16_t weight;
+};
+
+struct xen_domctl_sched_rtds {
+ uint32_t period;
+ uint32_t budget;
+};
+
+typedef struct xen_domctl_schedparam_vcpu {
+ union {
+ struct xen_domctl_sched_sedf sedf;
+ struct xen_domctl_sched_credit credit;
+ struct xen_domctl_sched_credit2 credit2;
+ struct xen_domctl_sched_rtds rtds;
+ } s;
+ uint16_t vcpuid;
+} xen_domctl_schedparam_vcpu_t;
+DEFINE_XEN_GUEST_HANDLE(xen_domctl_schedparam_vcpu_t);
+
/* Set or get info? */
#define XEN_DOMCTL_SCHEDOP_putinfo 0
#define XEN_DOMCTL_SCHEDOP_getinfo 1
+#define XEN_DOMCTL_SCHEDOP_putvcpuinfo 2
+#define XEN_DOMCTL_SCHEDOP_getvcpuinfo 3
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;
- struct xen_domctl_sched_rtds {
- uint32_t period;
- uint32_t budget;
- } rtds;
+ struct xen_domctl_sched_sedf sedf;
+ struct xen_domctl_sched_credit credit;
+ struct xen_domctl_sched_credit2 credit2;
+ struct xen_domctl_sched_rtds rtds;
+ struct {
+ XEN_GUEST_HANDLE_64(xen_domctl_schedparam_vcpu_t) vcpus;
+ uint16_t nr_vcpus;
+ } v;
} u;
};
typedef struct xen_domctl_scheduler_op xen_domctl_scheduler_op_t;
[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 181 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 related [flat|nested] 39+ messages in thread
* Re: [PATCH v3 for Xen 4.6 0/4] Enable per-VCPU parameter settings for RTDS scheduler
2015-06-29 2:44 [PATCH v3 for Xen 4.6 0/4] Enable per-VCPU parameter settings for RTDS scheduler Chong Li
` (3 preceding siblings ...)
2015-06-29 2:44 ` [PATCH v3 for Xen 4.6 4/4] xl: " Chong Li
@ 2015-07-07 15:16 ` Dario Faggioli
2015-07-07 16:11 ` Chong Li
4 siblings, 1 reply; 39+ messages in thread
From: Dario Faggioli @ 2015-07-07 15:16 UTC (permalink / raw)
To: Chong Li
Cc: Chong Li, wei.liu2, george.dunlap, ian.jackson, xen-devel,
ian.campbell, mengxu, jbeulich, dgolomb
[-- Attachment #1.1: Type: text/plain, Size: 4559 bytes --]
On Sun, 2015-06-28 at 21:44 -0500, Chong Li wrote:
> [Usage]
> With this patchset in use, xl sched-rtds tool can:
>
> 1) show the budget and period of each VCPU of each domain, by using "xl sched-rtds -v all" command. An example would be like:
>
Ok. Thanks for this summary, and for including the actual output.
> # xl sched-rtds -v all
> Cpupool Pool-0: sched=RTDS
> Name ID VCPU Period Budget
> Domain-0 0 0 10000 4000
> vm1 1 0 300 150
> vm1 1 1 400 200
> vm1 1 2 10000 4000
> vm1 1 3 1000 500
> vm2 2 0 10000 4000
> vm2 2 1 10000 4000
>
Right. What happens with just `xl sched-rtds'?
>
> 2) show the budget and period of each VCPU of a specific domain, by using,
> e.g., "xl sched-rtds -d vm1 -v all" command. The output would be like:
>
> # xl sched-rtds -d vm1 -v all
> Name ID VCPU Period Budget
> vm1 1 0 300 150
> vm1 1 1 400 200
> vm1 1 2 10000 4000
> vm1 1 3 1000 500
>
Same as above: what happens with just `xl sched-rtds -d vm1'?
> To show a subset of the parameters of the VCPUs of a specific domain, please use, e.g.,"xl sched-rtds -d vm1 -v 0 -v 3" command. The output would be:
>
> # xl sched-rtds -d vm1 -v 0 -v 3
> Name ID VCPU Period Budget
> vm1 1 0 300 150
> vm1 1 3 1000 500
>
>
> 3) Users can set the budget and period of multiple VCPUs of a specific domain
> with only one command, e.g., "xl sched-rtds -d vm1 -v 0 -p 100 -b 50 -v 3 -p 300 -b 150".
>
> Users can set all VCPUs with the same parameters, by one command.
> e.g., "xl sched-rtds -d vm1 -v all -p 500 -b 250".
>
Forgive me for asking, as I really think it's rather obvious you've done
this, but I guess you've stress tested this by feeding the various
switches with arbitrary bad arguments, and checked that things do not
explode?
If you've done it, then fine. It's not necessary that you include (all)
the results of that too in here... I just wanted do double check because
this is certainly not the easiest piece of interface we have in xl.
Anyway, I'll give it a go too.
Regards,
Dario
>
> ---
> CC: <dario.faggioli@citrix.com>
> CC: <george.dunlap@eu.citrix.com>
> CC: <dgolomb@seas.upenn.edu>
> CC: <mengxu@cis.upenn.edu>
> CC: <jbeulich@suse.com>
> CC: <wei.liu2@citrix.com>
> CC: <lichong659@gmail.com>
> CC: <ian.jackson@eu.citrix.com>
> CC: <ian.campbell@eu.citrix.com>
>
>
> Chong Li (4):
> xen: enable per-VCPU parameter settings for RTDS scheduler
> libxc: enable per-VCPU parameter settings for RTDS scheduler
> libxl: enable per-VCPU parameter settings for RTDS scheduler
> xl: enable per-VCPU parameter settings for RTDS scheduler
>
> docs/man/xl.pod.1 | 4 +
> tools/libxc/include/xenctrl.h | 9 ++
> tools/libxc/xc_csched.c | 4 +-
> tools/libxc/xc_csched2.c | 4 +-
> tools/libxc/xc_rt.c | 64 +++++++++-
> tools/libxc/xc_sedf.c | 4 +-
> tools/libxl/libxl.c | 209 +++++++++++++++++++++++++++----
> tools/libxl/libxl.h | 17 +++
> tools/libxl/libxl_types.idl | 16 +++
> tools/libxl/xl_cmdimpl.c | 284 +++++++++++++++++++++++++++++++++++-------
> tools/libxl/xl_cmdtable.c | 10 +-
> xen/common/Makefile | 1 -
> xen/common/domctl.c | 3 +
> xen/common/sched_credit.c | 14 +--
> xen/common/sched_credit2.c | 6 +-
> xen/common/sched_rt.c | 80 +++++++++++-
> xen/common/schedule.c | 5 +-
> xen/include/public/domctl.h | 64 +++++++---
> 18 files changed, 680 insertions(+), 118 deletions(-)
>
--
<<This happens because I choose it to happen!>> (Raistlin Majere)
-----------------------------------------------------------------
Dario Faggioli, Ph.D, http://about.me/dario.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: 181 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] 39+ messages in thread
* Re: [PATCH v3 for Xen 4.6 1/4] xen: enable per-VCPU parameter settings for RTDS scheduler
2015-07-07 8:59 ` Jan Beulich
2015-07-07 14:39 ` Dario Faggioli
2015-07-07 14:55 ` Dario Faggioli
@ 2015-07-07 15:33 ` Chong Li
2015-07-07 15:41 ` Jan Beulich
2015-07-07 15:46 ` Dario Faggioli
2 siblings, 2 replies; 39+ messages in thread
From: Chong Li @ 2015-07-07 15:33 UTC (permalink / raw)
To: Jan Beulich
Cc: Chong Li, Sisu Xi, George Dunlap, dario.faggioli, xen-devel,
Meng Xu, Dagaen Golomb
On Tue, Jul 7, 2015 at 3:59 AM, Jan Beulich <JBeulich@suse.com> wrote:
>>>> On 29.06.15 at 04:44, <lichong659@gmail.com> wrote:
>> --- a/xen/common/Makefile
>> +++ b/xen/common/Makefile
>> @@ -31,7 +31,6 @@ obj-y += rbtree.o
>> obj-y += rcupdate.o
>> obj-y += sched_credit.o
>> obj-y += sched_credit2.o
>> -obj-y += sched_sedf.o
>> obj-y += sched_arinc653.o
>> obj-y += sched_rt.o
>> obj-y += schedule.o
>
> Stray change. Or perhaps the file doesn't build anymore, in which case
> you should instead have stated that the patch is dependent upon the
> series removing SEDF.
>
>> @@ -1157,8 +1158,75 @@ rt_dom_cntl(
>
>> + {
>> + rc = -EINVAL;
>> + break;
>> + }
>> + svc = rt_vcpu(d->vcpu[local_sched.vcpuid]);
>> +
>> + local_sched.vcpuid = svc->vcpu->vcpu_id;
>
> Why? If at all, this should be an ASSERT().
My bad. This should not be there.
>
>> + local_sched.s.rtds.budget = svc->budget / MICROSECS(1);
>> + local_sched.s.rtds.period = svc->period / MICROSECS(1);
>> + if( index >= op->u.v.nr_vcpus ) /* not enough guest buffer*/
>
> Impossible due to the containing loop's condition.
Yes, you're right.
>
>> + {
>> + rc = -ENOBUFS;
>> + break;
>> + }
>> + if ( copy_to_guest_offset(op->u.v.vcpus, index,
>
> __copy_to_guest_offset()
>
>> + case XEN_DOMCTL_SCHEDOP_putvcpuinfo:
>> + spin_lock_irqsave(&prv->lock, flags);
>> + for( index = 0; index < op->u.v.nr_vcpus; index++ )
>> + {
>> + if ( copy_from_guest_offset(&local_sched,
>> + op->u.v.vcpus, index, 1) )
>> + {
>> + rc = -EFAULT;
>> + break;
>> + }
>> + if ( local_sched.vcpuid >= d->max_vcpus
>> + || d->vcpu[local_sched.vcpuid] == NULL )
>> + {
>> + rc = -EINVAL;
>> + break;
>> + }
>> + svc = rt_vcpu(d->vcpu[local_sched.vcpuid]);
>> + svc->period = MICROSECS(local_sched.s.rtds.period);
>> + svc->budget = MICROSECS(local_sched.s.rtds.budget);
>
> Are all input values valid here?
Vcpuid, Period and budget have been validated in libxl. But we can
still repeat that validation here, if it's needed.
>
>> --- a/xen/include/public/domctl.h
>> +++ b/xen/include/public/domctl.h
>> @@ -330,31 +330,59 @@ DEFINE_XEN_GUEST_HANDLE(xen_domctl_max_vcpus_t);
>> #define XEN_SCHEDULER_ARINC653 7
>> #define XEN_SCHEDULER_RTDS 8
>>
>> +typedef struct xen_domctl_sched_sedf {
>> + uint64_aligned_t period;
>> + uint64_aligned_t slice;
>> + uint64_aligned_t latency;
>> + uint32_t extratime;
>> + uint32_t weight;
>> +} xen_domctl_sched_sedf_t;
>
> Indentation.
>
>> +typedef union xen_domctl_schedparam {
>> + xen_domctl_sched_sedf_t sedf;
>> + xen_domctl_sched_credit_t credit;
>> + xen_domctl_sched_credit2_t credit2;
>> + xen_domctl_sched_rtds_t rtds;
>> +} xen_domctl_schedparam_t;
>
> I don't see the need for this extra wrapper type. Nor do I see the
> need for the typedef here and above - they're generally only
> created if you want to also define a matching guest handle type.
>
>> +typedef struct xen_domctl_schedparam_vcpu {
>> + union {
>> + xen_domctl_sched_credit_t credit;
>> + xen_domctl_sched_credit2_t credit2;
>> + xen_domctl_sched_rtds_t rtds;
>> + } s;
>> + uint16_t vcpuid;
>
> Explicit padding please.
>
>> +} xen_domctl_schedparam_vcpu_t;
>> +DEFINE_XEN_GUEST_HANDLE(xen_domctl_schedparam_vcpu_t);
>> +
>> /* Set or get info? */
>> #define XEN_DOMCTL_SCHEDOP_putinfo 0
>> #define XEN_DOMCTL_SCHEDOP_getinfo 1
>> +#define XEN_DOMCTL_SCHEDOP_putvcpuinfo 2
>> +#define XEN_DOMCTL_SCHEDOP_getvcpuinfo 3
>> 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;
>> - struct xen_domctl_sched_rtds {
>> - uint32_t period;
>> - uint32_t budget;
>> - } rtds;
>> + xen_domctl_schedparam_t d;
>
> With this type gone I'm not even sure we need to wrap this in
> another union; not doing so would eliminate some of the other
> changes in this patch.
I see your point. Because of xen_domctl_schedparam_vcpu_t, we still
need to define struct xen_domctl_sched_sedf/credit/credit2/rtds
outside of struct xen_domctl_scheduler_op. Then the struct would be
like:
struct xen_domctl_scheduler_op {
uint32_t sched_id; /* XEN_SCHEDULER_* */
uint32_t cmd; /* XEN_DOMCTL_SCHEDOP_* */
union {
struct xen_domctl_sched_sedf sedf;
struct xen_domctl_sched_credit credit;
struct xen_domctl_sched_credit2 credit2;
struct xen_domctl_sched_rtds rtds;
struct {
XEN_GUEST_HANDLE_64(xen_domctl_schedparam_vcpu_t) vcpus;
uint16_t nr_vcpus;
} v;
} u;
};
typedef struct xen_domctl_scheduler_op xen_domctl_scheduler_op_t;
This design is good for compatibility. Dario, what do you think?
Chong
>
> Jan
--
Chong Li
Department of Computer Science and Engineering
Washington University in St.louis
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v3 for Xen 4.6 4/4] xl: enable per-VCPU parameter settings for RTDS scheduler
2015-06-29 2:44 ` [PATCH v3 for Xen 4.6 4/4] xl: " Chong Li
@ 2015-07-07 15:34 ` Dario Faggioli
2015-07-07 16:01 ` Chong Li
0 siblings, 1 reply; 39+ messages in thread
From: Dario Faggioli @ 2015-07-07 15:34 UTC (permalink / raw)
To: Chong Li
Cc: Chong Li, wei.liu2, Sisu Xi, george.dunlap, xen-devel, Meng Xu,
dgolomb
[-- Attachment #1.1: Type: text/plain, Size: 1434 bytes --]
On Sun, 2015-06-28 at 21:44 -0500, Chong Li wrote:
> Change main_sched_rtds and related output functions to support per-VCPU settings.
>
> Changes on PATCH v2:
>
> 1) Remove per-domain output functions for RTDS scheduler.
>
> 2) Users now use '-v all' to specify all VCPUs.
>
> 3) Support outputting a subset of the parameters of the VCPUs of a specific domain.
>
> 4) When setting all VCPUs with the same parameters (by only one command), no per-domain function is invoked.
>
> Signed-off-by: Chong Li <chong.li@wustl.edu>
> Signed-off-by: Meng Xu <mengxu@cis.upenn.edu>
> Signed-off-by: Sisu Xi <xisisu@gmail.com>
>
I think this patch is fine. I've asked already, when replying to the
cover message, what happens if one uses just:
# xl sched-rtds
or
# xl sched-rtds -d vm1
and it looks to me that, instead of getting rid of
sched_rtds_domain_output(), you can leave it there and make it respond
to the above calls, with the effect of returning the default values, as
agreed and implemented.
What do you think? If not done like this (and if I'm not missing
something), is there a way to retrieve those values from xl?
Dario
--
<<This happens because I choose it to happen!>> (Raistlin Majere)
-----------------------------------------------------------------
Dario Faggioli, Ph.D, http://about.me/dario.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: 181 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] 39+ messages in thread
* Re: [PATCH v3 for Xen 4.6 1/4] xen: enable per-VCPU parameter settings for RTDS scheduler
2015-07-07 15:33 ` Chong Li
@ 2015-07-07 15:41 ` Jan Beulich
2015-07-07 15:46 ` Dario Faggioli
1 sibling, 0 replies; 39+ messages in thread
From: Jan Beulich @ 2015-07-07 15:41 UTC (permalink / raw)
To: Chong Li
Cc: Chong Li, Sisu Xi, George Dunlap, dario.faggioli, xen-devel,
Meng Xu, Dagaen Golomb
>>> On 07.07.15 at 17:33, <lichong659@gmail.com> wrote:
> On Tue, Jul 7, 2015 at 3:59 AM, Jan Beulich <JBeulich@suse.com> wrote:
>>>>> On 29.06.15 at 04:44, <lichong659@gmail.com> wrote:
>>> + case XEN_DOMCTL_SCHEDOP_putvcpuinfo:
>>> + spin_lock_irqsave(&prv->lock, flags);
>>> + for( index = 0; index < op->u.v.nr_vcpus; index++ )
>>> + {
>>> + if ( copy_from_guest_offset(&local_sched,
>>> + op->u.v.vcpus, index, 1) )
>>> + {
>>> + rc = -EFAULT;
>>> + break;
>>> + }
>>> + if ( local_sched.vcpuid >= d->max_vcpus
>>> + || d->vcpu[local_sched.vcpuid] == NULL )
>>> + {
>>> + rc = -EINVAL;
>>> + break;
>>> + }
>>> + svc = rt_vcpu(d->vcpu[local_sched.vcpuid]);
>>> + svc->period = MICROSECS(local_sched.s.rtds.period);
>>> + svc->budget = MICROSECS(local_sched.s.rtds.budget);
>>
>> Are all input values valid here?
>
> Vcpuid, Period and budget have been validated in libxl. But we can
> still repeat that validation here, if it's needed.
Yes, absolutely. Tool stacks may not be fully trusted (namely in
disaggregated setups).
Jan
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v3 for Xen 4.6 1/4] xen: enable per-VCPU parameter settings for RTDS scheduler
2015-07-07 15:33 ` Chong Li
2015-07-07 15:41 ` Jan Beulich
@ 2015-07-07 15:46 ` Dario Faggioli
1 sibling, 0 replies; 39+ messages in thread
From: Dario Faggioli @ 2015-07-07 15:46 UTC (permalink / raw)
To: Chong Li
Cc: Chong Li, Sisu Xi, George Dunlap, xen-devel, Meng Xu, Jan Beulich,
Dagaen Golomb
[-- Attachment #1.1: Type: text/plain, Size: 1702 bytes --]
On Tue, 2015-07-07 at 10:33 -0500, Chong Li wrote:
> On Tue, Jul 7, 2015 at 3:59 AM, Jan Beulich <JBeulich@suse.com> wrote:
> >
> > With this type gone I'm not even sure we need to wrap this in
> > another union; not doing so would eliminate some of the other
> > changes in this patch.
>
> I see your point. Because of xen_domctl_schedparam_vcpu_t, we still
> need to define struct xen_domctl_sched_sedf/credit/credit2/rtds
> outside of struct xen_domctl_scheduler_op. Then the struct would be
> like:
>
> struct xen_domctl_scheduler_op {
> uint32_t sched_id; /* XEN_SCHEDULER_* */
> uint32_t cmd; /* XEN_DOMCTL_SCHEDOP_* */
> union {
> struct xen_domctl_sched_sedf sedf;
> struct xen_domctl_sched_credit credit;
> struct xen_domctl_sched_credit2 credit2;
> struct xen_domctl_sched_rtds rtds;
> struct {
> XEN_GUEST_HANDLE_64(xen_domctl_schedparam_vcpu_t) vcpus;
> uint16_t nr_vcpus;
> } v;
> } u;
> };
> typedef struct xen_domctl_scheduler_op xen_domctl_scheduler_op_t;
>
> This design is good for compatibility. Dario, what do you think?
>
I understood Jan's suggestion in the same exact way as you, as you can
see in my own email, and I like it.
Sorry for being a bit out-of-sync, I've been having internet issues
today (now things are working, but I'm not sure it'd be permanent! :-/).
Dario
--
<<This happens because I choose it to happen!>> (Raistlin Majere)
-----------------------------------------------------------------
Dario Faggioli, Ph.D, http://about.me/dario.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: 181 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] 39+ messages in thread
* Re: [PATCH v3 for Xen 4.6 4/4] xl: enable per-VCPU parameter settings for RTDS scheduler
2015-07-07 15:34 ` Dario Faggioli
@ 2015-07-07 16:01 ` Chong Li
0 siblings, 0 replies; 39+ messages in thread
From: Chong Li @ 2015-07-07 16:01 UTC (permalink / raw)
To: Dario Faggioli
Cc: Chong Li, Wei Liu, Sisu Xi, George Dunlap, xen-devel, Meng Xu,
Dagaen Golomb
On Tue, Jul 7, 2015 at 10:34 AM, Dario Faggioli
<dario.faggioli@citrix.com> wrote:
> On Sun, 2015-06-28 at 21:44 -0500, Chong Li wrote:
>> Change main_sched_rtds and related output functions to support per-VCPU settings.
>>
>> Changes on PATCH v2:
>>
>> 1) Remove per-domain output functions for RTDS scheduler.
>>
>> 2) Users now use '-v all' to specify all VCPUs.
>>
>> 3) Support outputting a subset of the parameters of the VCPUs of a specific domain.
>>
>> 4) When setting all VCPUs with the same parameters (by only one command), no per-domain function is invoked.
>>
>> Signed-off-by: Chong Li <chong.li@wustl.edu>
>> Signed-off-by: Meng Xu <mengxu@cis.upenn.edu>
>> Signed-off-by: Sisu Xi <xisisu@gmail.com>
>>
> I think this patch is fine. I've asked already, when replying to the
> cover message, what happens if one uses just:
>
> # xl sched-rtds
>
> or
>
> # xl sched-rtds -d vm1
>
> and it looks to me that, instead of getting rid of
> sched_rtds_domain_output(), you can leave it there and make it respond
> to the above calls, with the effect of returning the default values, as
> agreed and implemented.
I agree. I'll do that in the next version.
Chong
>
> Dario
> --
> <<This happens because I choose it to happen!>> (Raistlin Majere)
> -----------------------------------------------------------------
> Dario Faggioli, Ph.D, http://about.me/dario.faggioli
> Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)
--
Chong Li
Department of Computer Science and Engineering
Washington University in St.louis
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v3 for Xen 4.6 1/4] xen: enable per-VCPU parameter settings for RTDS scheduler
2015-07-07 14:55 ` Dario Faggioli
@ 2015-07-07 16:09 ` Jan Beulich
0 siblings, 0 replies; 39+ messages in thread
From: Jan Beulich @ 2015-07-07 16:09 UTC (permalink / raw)
To: Dario Faggioli
Cc: Chong Li, Sisu Xi, george.dunlap, xen-devel, Meng Xu, Chong Li,
dgolomb
>>> On 07.07.15 at 16:55, <dario.faggioli@citrix.com> wrote:
> So, Jan, just to be sure, do you mean (apart from the explicit padding)
> something like this (attached, also)?
>
> diff --git a/xen/include/public/domctl.h b/xen/include/public/domctl.h
> index bc45ea5..8210ecb 100644
> --- a/xen/include/public/domctl.h
> +++ b/xen/include/public/domctl.h
> @@ -330,31 +330,56 @@ DEFINE_XEN_GUEST_HANDLE(xen_domctl_max_vcpus_t);
> #define XEN_SCHEDULER_ARINC653 7
> #define XEN_SCHEDULER_RTDS 8
>
> +struct xen_domctl_sched_sedf {
> + uint64_aligned_t period;
> + uint64_aligned_t slice;
> + uint64_aligned_t latency;
> + uint32_t extratime;
> + uint32_t weight;
> +};
> +
> +struct xen_domctl_sched_credit {
> + uint16_t weight;
> + uint16_t cap;
> +};
> +
> +struct xen_domctl_sched_credit2 {
> + uint16_t weight;
> +};
> +
> +struct xen_domctl_sched_rtds {
> + uint32_t period;
> + uint32_t budget;
> +};
> +
> +typedef struct xen_domctl_schedparam_vcpu {
> + union {
> + struct xen_domctl_sched_sedf sedf;
> + struct xen_domctl_sched_credit credit;
> + struct xen_domctl_sched_credit2 credit2;
> + struct xen_domctl_sched_rtds rtds;
> + } s;
> + uint16_t vcpuid;
> +} xen_domctl_schedparam_vcpu_t;
> +DEFINE_XEN_GUEST_HANDLE(xen_domctl_schedparam_vcpu_t);
> +
> /* Set or get info? */
> #define XEN_DOMCTL_SCHEDOP_putinfo 0
> #define XEN_DOMCTL_SCHEDOP_getinfo 1
> +#define XEN_DOMCTL_SCHEDOP_putvcpuinfo 2
> +#define XEN_DOMCTL_SCHEDOP_getvcpuinfo 3
> 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;
> - struct xen_domctl_sched_rtds {
> - uint32_t period;
> - uint32_t budget;
> - } rtds;
> + struct xen_domctl_sched_sedf sedf;
> + struct xen_domctl_sched_credit credit;
> + struct xen_domctl_sched_credit2 credit2;
> + struct xen_domctl_sched_rtds rtds;
> + struct {
> + XEN_GUEST_HANDLE_64(xen_domctl_schedparam_vcpu_t) vcpus;
> + uint16_t nr_vcpus;
> + } v;
> } u;
> };
> typedef struct xen_domctl_scheduler_op xen_domctl_scheduler_op_t;
Yes.
Jan
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v3 for Xen 4.6 0/4] Enable per-VCPU parameter settings for RTDS scheduler
2015-07-07 15:16 ` [PATCH v3 for Xen 4.6 0/4] Enable " Dario Faggioli
@ 2015-07-07 16:11 ` Chong Li
0 siblings, 0 replies; 39+ messages in thread
From: Chong Li @ 2015-07-07 16:11 UTC (permalink / raw)
To: Dario Faggioli
Cc: Chong Li, Wei Liu, George Dunlap, Ian Jackson, xen-devel,
Ian Campbell, Meng Xu, Jan Beulich, Dagaen Golomb
On Tue, Jul 7, 2015 at 10:16 AM, Dario Faggioli
<dario.faggioli@citrix.com> wrote:
> On Sun, 2015-06-28 at 21:44 -0500, Chong Li wrote:
>
>> [Usage]
>> With this patchset in use, xl sched-rtds tool can:
>>
>> 1) show the budget and period of each VCPU of each domain, by using "xl sched-rtds -v all" command. An example would be like:
>>
> Ok. Thanks for this summary, and for including the actual output.
>
>> # xl sched-rtds -v all
>> Cpupool Pool-0: sched=RTDS
>> Name ID VCPU Period Budget
>> Domain-0 0 0 10000 4000
>> vm1 1 0 300 150
>> vm1 1 1 400 200
>> vm1 1 2 10000 4000
>> vm1 1 3 1000 500
>> vm2 2 0 10000 4000
>> vm2 2 1 10000 4000
>>
> Right. What happens with just `xl sched-rtds'?
>>
>> 2) show the budget and period of each VCPU of a specific domain, by using,
>> e.g., "xl sched-rtds -d vm1 -v all" command. The output would be like:
>>
>> # xl sched-rtds -d vm1 -v all
>> Name ID VCPU Period Budget
>> vm1 1 0 300 150
>> vm1 1 1 400 200
>> vm1 1 2 10000 4000
>> vm1 1 3 1000 500
>>
> Same as above: what happens with just `xl sched-rtds -d vm1'?
'xl sched-rtds ( -d vm1)' will return an error message. As you
suggested, I'll make it return per-domain default parameters in the
next version.
>
>> To show a subset of the parameters of the VCPUs of a specific domain, please use, e.g.,"xl sched-rtds -d vm1 -v 0 -v 3" command. The output would be:
>>
>> # xl sched-rtds -d vm1 -v 0 -v 3
>> Name ID VCPU Period Budget
>> vm1 1 0 300 150
>> vm1 1 3 1000 500
>>
>>
>> 3) Users can set the budget and period of multiple VCPUs of a specific domain
>> with only one command, e.g., "xl sched-rtds -d vm1 -v 0 -p 100 -b 50 -v 3 -p 300 -b 150".
>>
>> Users can set all VCPUs with the same parameters, by one command.
>> e.g., "xl sched-rtds -d vm1 -v all -p 500 -b 250".
>>
> Forgive me for asking, as I really think it's rather obvious you've done
> this, but I guess you've stress tested this by feeding the various
> switches with arbitrary bad arguments, and checked that things do not
> explode?
>
> If you've done it, then fine. It's not necessary that you include (all)
> the results of that too in here... I just wanted do double check because
> this is certainly not the easiest piece of interface we have in xl.
Yes, I've done stress test.
>
> Anyway, I'll give it a go too.
>
> Regards,
> Dario
>
>>
>> ---
>> CC: <dario.faggioli@citrix.com>
>> CC: <george.dunlap@eu.citrix.com>
>> CC: <dgolomb@seas.upenn.edu>
>> CC: <mengxu@cis.upenn.edu>
>> CC: <jbeulich@suse.com>
>> CC: <wei.liu2@citrix.com>
>> CC: <lichong659@gmail.com>
>> CC: <ian.jackson@eu.citrix.com>
>> CC: <ian.campbell@eu.citrix.com>
>>
>>
>> Chong Li (4):
>> xen: enable per-VCPU parameter settings for RTDS scheduler
>> libxc: enable per-VCPU parameter settings for RTDS scheduler
>> libxl: enable per-VCPU parameter settings for RTDS scheduler
>> xl: enable per-VCPU parameter settings for RTDS scheduler
>>
>> docs/man/xl.pod.1 | 4 +
>> tools/libxc/include/xenctrl.h | 9 ++
>> tools/libxc/xc_csched.c | 4 +-
>> tools/libxc/xc_csched2.c | 4 +-
>> tools/libxc/xc_rt.c | 64 +++++++++-
>> tools/libxc/xc_sedf.c | 4 +-
>> tools/libxl/libxl.c | 209 +++++++++++++++++++++++++++----
>> tools/libxl/libxl.h | 17 +++
>> tools/libxl/libxl_types.idl | 16 +++
>> tools/libxl/xl_cmdimpl.c | 284 +++++++++++++++++++++++++++++++++++-------
>> tools/libxl/xl_cmdtable.c | 10 +-
>> xen/common/Makefile | 1 -
>> xen/common/domctl.c | 3 +
>> xen/common/sched_credit.c | 14 +--
>> xen/common/sched_credit2.c | 6 +-
>> xen/common/sched_rt.c | 80 +++++++++++-
>> xen/common/schedule.c | 5 +-
>> xen/include/public/domctl.h | 64 +++++++---
>> 18 files changed, 680 insertions(+), 118 deletions(-)
>>
>
> --
> <<This happens because I choose it to happen!>> (Raistlin Majere)
> -----------------------------------------------------------------
> Dario Faggioli, Ph.D, http://about.me/dario.faggioli
> Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)
--
Chong Li
Department of Computer Science and Engineering
Washington University in St.louis
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v3 for Xen 4.6 3/4] libxl: enable per-VCPU parameter settings for RTDS scheduler
2015-06-29 2:44 ` [PATCH v3 for Xen 4.6 3/4] libxl: " Chong Li
2015-06-30 12:26 ` Ian Campbell
@ 2015-07-07 16:23 ` Dario Faggioli
2015-07-08 14:35 ` Chong Li
1 sibling, 1 reply; 39+ messages in thread
From: Dario Faggioli @ 2015-07-07 16:23 UTC (permalink / raw)
To: Chong Li
Cc: Chong Li, wei.liu2, Sisu Xi, george.dunlap, ian.jackson,
xen-devel, ian.campbell, Meng Xu, dgolomb
[-- Attachment #1.1: Type: text/plain, Size: 5358 bytes --]
On Sun, 2015-06-28 at 21:44 -0500, Chong Li wrote:
> Add libxl_vcpu_sched_params_get/set and sched_rtds_vcpu_get/set functions to support
> per-VCPU settings.
>
> Changes on PATCH v2:
>
> 1) New data structure (libxl_vcpu_sched_params and libxl_sched_params) to help per-VCPU settings.
>
> 2) sched_rtds_vcpu_get now can return a random subset of the parameters of the VCPUs of a specific domain.
>
> Signed-off-by: Chong Li <chong.li@wustl.edu>
> Signed-off-by: Meng Xu <mengxu@cis.upenn.edu>
> Signed-off-by: Sisu Xi <xisisu@gmail.com>
>
So, apart from what IanC said already, I think this patch is mostly
fine, apart from:
- the changelog needing to improve, as explained already while
reviewing one other patch in the series (that, in fact, applies to
all of them)
- comments/documentation about the new behavior of per domain
scheduling API calls looks to be missing to me...
- some coding style issues (see below).
> +static int sched_rtds_vcpu_get(libxl__gc *gc, uint32_t domid,
> + libxl_vcpu_sched_params *scinfo)
> +{
> + uint16_t num_vcpus;
> + int rc, i;
> + xc_dominfo_t info;
> + xen_domctl_schedparam_vcpu_t *vcpus;
> +
> + if (scinfo->num_vcpus == 0) {
> + rc = xc_domain_getinfo(CTX->xch, domid, 1, &info);
> + if (rc < 0) {
> + LOGE(ERROR, "getting domain info");
> + return ERROR_FAIL;
> + }
> + num_vcpus = info.max_vcpu_id + 1;
> + } else
> + num_vcpus = scinfo->num_vcpus;
> +
> + GCNEW_ARRAY(vcpus, num_vcpus);
> +
> + if (scinfo->num_vcpus > 0)
> + for(i=0; i < num_vcpus; i++)
spaces ("for (...")
> +static int sched_rtds_vcpu_set(libxl__gc *gc, uint32_t domid,
> + const libxl_vcpu_sched_params *scinfo)
> +{
> + int rc;
> + int i;
> + uint16_t max_vcpuid;
> + xc_dominfo_t info;
> + xen_domctl_schedparam_vcpu_t *vcpus;
> + int num_vcpus;
> +
> + rc = xc_domain_getinfo(CTX->xch, domid, 1, &info);
> + if (rc < 0) {
> + LOGE(ERROR, "getting domain info");
> + return ERROR_FAIL;
> + }
> + max_vcpuid = info.max_vcpu_id;
> +
> + if (scinfo->num_vcpus > 0) {
> + num_vcpus = scinfo->num_vcpus;
> + GCNEW_ARRAY(vcpus, num_vcpus);
> + for (i = 0; i < num_vcpus; i++) {
> + if (scinfo->vcpus[i].vcpuid < 0 ||
> + scinfo->vcpus[i].vcpuid > max_vcpuid) {
> + LOG(ERROR, "VCPU index is out of range, "
> + "valid values are within range from 0 to %d",
> + max_vcpuid);
> + return ERROR_INVAL;
> + }
> + vcpus[i].vcpuid = scinfo->vcpus[i].vcpuid;
> +
> + rc = sched_rtds_validate_params(gc,
> + scinfo->vcpus[i].period, scinfo->vcpus[i].budget,
> + &vcpus[i].s.rtds.period, &vcpus[i].s.rtds.budget);
> + if (rc)
> + return ERROR_INVAL;
> + }
> + } else {
> + num_vcpus = max_vcpuid + 1;
> + GCNEW_ARRAY(vcpus, num_vcpus);
>
Indentation.
> --- a/tools/libxl/libxl.h
> +++ b/tools/libxl/libxl.h
> @@ -200,6 +200,16 @@
> #define LIBXL_HAVE_DEVICETREE_PASSTHROUGH 1
>
> /*
> + * libxl_vcpu_sched_params is used to get/set per-vcpu params
> +*/
> +#define LIBXL_HAVE_VCPU_SCHED_PARAMS 1
> +
> +/*
> + * libxl_sched_params is used to store per-vcpu params
> +*/
> +#define LIBXL_SCHED_PARAMS 1
> +
> +/*
> * libxl ABI compatibility
> *
> * The only guarantee which libxl makes regarding ABI compatibility
> @@ -1554,10 +1564,17 @@ int libxl_sched_credit_params_set(libxl_ctx *ctx, uint32_t poolid,
> #define LIBXL_DOMAIN_SCHED_PARAM_EXTRATIME_DEFAULT -1
> #define LIBXL_DOMAIN_SCHED_PARAM_BUDGET_DEFAULT -1
>
> +/* Per-VCPU parameters*/
> +#define LIBXL_DOMAIN_SCHED_PARAM_VCPU_INDEX_DEFAULT -1
> +
> int libxl_domain_sched_params_get(libxl_ctx *ctx, uint32_t domid,
> libxl_domain_sched_params *params);
> int libxl_domain_sched_params_set(libxl_ctx *ctx, uint32_t domid,
> const libxl_domain_sched_params *params);
> +int libxl_vcpu_sched_params_get(libxl_ctx *ctx, uint32_t domid,
> + libxl_vcpu_sched_params *params);
> +int libxl_vcpu_sched_params_set(libxl_ctx *ctx, uint32_t domid,
> + const libxl_vcpu_sched_params *params);
>
Didn't we say that we wanted the fact that now, at least for RTDS,
libxl_domain_sched_params_*() deals with default parameters to be
documented in a comment somewhere? Have I missed it?
I appreciate that this is happening in lower levels, and that libxl is
just reporting it, but, still, it is something that I would like to find
in the headers of the library, in order to be able to tell the
differences between libxl_vcpu_sched_params_* and
libxl_domain_sched_params_* ... Ian?
Regards,
Dario
--
<<This happens because I choose it to happen!>> (Raistlin Majere)
-----------------------------------------------------------------
Dario Faggioli, Ph.D, http://about.me/dario.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: 181 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] 39+ messages in thread
* Re: [PATCH v3 for Xen 4.6 1/4] xen: enable per-VCPU parameter settings for RTDS scheduler
2015-07-07 14:39 ` Dario Faggioli
@ 2015-07-08 6:06 ` Meng Xu
2015-07-08 8:33 ` Dario Faggioli
0 siblings, 1 reply; 39+ messages in thread
From: Meng Xu @ 2015-07-08 6:06 UTC (permalink / raw)
To: Dario Faggioli
Cc: Chong Li, Sisu Xi, George Dunlap, xen-devel@lists.xen.org,
Meng Xu, Jan Beulich, Chong Li, Dagaen Golomb
2015-07-07 7:39 GMT-07:00 Dario Faggioli <dario.faggioli@citrix.com>:
> On Tue, 2015-07-07 at 09:59 +0100, Jan Beulich wrote:
>> >>> On 29.06.15 at 04:44, <lichong659@gmail.com> wrote:
>> > --- a/xen/common/Makefile
>> > +++ b/xen/common/Makefile
>> > @@ -31,7 +31,6 @@ obj-y += rbtree.o
>> > obj-y += rcupdate.o
>> > obj-y += sched_credit.o
>> > obj-y += sched_credit2.o
>> > -obj-y += sched_sedf.o
>> > obj-y += sched_arinc653.o
>> > obj-y += sched_rt.o
>> > obj-y += schedule.o
>>
>> Stray change. Or perhaps the file doesn't build anymore, in which case
>> you should instead have stated that the patch is dependent upon the
>> series removing SEDF.
>>
> This indeed does not belong in here. And of course, things should
> build... So, Chong, either deal with SEDF as well, if basing your
> patches on a tree where it is still there, or base on top of my patches,
> ignore it, but state the dependency, as Jan is asking.
>
>> > @@ -1157,8 +1158,75 @@ rt_dom_cntl(
>
>> > + case XEN_DOMCTL_SCHEDOP_putvcpuinfo:
>> > + spin_lock_irqsave(&prv->lock, flags);
>> > + for( index = 0; index < op->u.v.nr_vcpus; index++ )
>> > + {
>> > + if ( copy_from_guest_offset(&local_sched,
>> > + op->u.v.vcpus, index, 1) )
>> > + {
>> > + rc = -EFAULT;
>> > + break;
>> > + }
>> > + if ( local_sched.vcpuid >= d->max_vcpus
>> > + || d->vcpu[local_sched.vcpuid] == NULL )
>> > + {
>> > + rc = -EINVAL;
>> > + break;
>> > + }
>> > + svc = rt_vcpu(d->vcpu[local_sched.vcpuid]);
>> > + svc->period = MICROSECS(local_sched.s.rtds.period);
>> > + svc->budget = MICROSECS(local_sched.s.rtds.budget);
>>
>> Are all input values valid here?
>>
> That's a good point, actually. Right now, SEDF does some range
> enforcement, by means of these values:
>
> #define PERIOD_MAX MILLISECS(10000) /* 10s */
> #define PERIOD_MIN (MICROSECS(10)) /* 10us */
> #define SLICE_MIN (MICROSECS(5)) /* 5us */
>
> Chong, it probably makes sense to (in a separate patch), introduce
> something like this in RTDS too (with SLICE_MIN-->BUDGET_MIN), and then
> use them, in this patch, for sanity checking the input.
>
> It also makes sense to check and enforce budget<=period, IMO.
>
> About the specific values, I'm open to proposals. I think something like
> the SEDF's one is fine. Meng?
We are trying to make some range enforcement for RTDS scheduler. Is my
understanding correct? (It should be, but just in case. :-) )
As to the range of period, I think the max value can be as large as
the type of period (ie. s_time_t) can represent. When we want a
dedicated CPU for a guest, we will set budget=period and can set the
period to a very very large value to avoid the unnecessarily
invocation of the scheduler.
As to the min value of period, I think it should be >=100us. The
scheduler overhead of running a large box could be 1us if the runq is
long and competetion of the runq lock is heavy. If the scheduler is
potentially invoked every 10us, the scheduler overhead will be 10% of
total computation time, which seems a lot to me.
As to the range of budget, the min value can be 5us, the same with
SEDF; the max value is the value of period of the same VCPU.
What do you think?
Thanks and best regards,
Meng
-----------
Meng Xu
PhD Student in Computer and Information Science
University of Pennsylvania
http://www.cis.upenn.edu/~mengxu/
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v3 for Xen 4.6 1/4] xen: enable per-VCPU parameter settings for RTDS scheduler
2015-07-08 6:06 ` Meng Xu
@ 2015-07-08 8:33 ` Dario Faggioli
2015-07-09 1:16 ` Meng Xu
0 siblings, 1 reply; 39+ messages in thread
From: Dario Faggioli @ 2015-07-08 8:33 UTC (permalink / raw)
To: Meng Xu
Cc: Chong Li, Sisu Xi, George Dunlap, xen-devel@lists.xen.org,
Meng Xu, Jan Beulich, Chong Li, Dagaen Golomb
[-- Attachment #1.1: Type: text/plain, Size: 4691 bytes --]
On Tue, 2015-07-07 at 23:06 -0700, Meng Xu wrote:
> 2015-07-07 7:39 GMT-07:00 Dario Faggioli <dario.faggioli@citrix.com>:
> > On Tue, 2015-07-07 at 09:59 +0100, Jan Beulich wrote:
> >> >>> On 29.06.15 at 04:44, <lichong659@gmail.com> wrote:
> >> > --- a/xen/common/Makefile
> >> > +++ b/xen/common/Makefile
> >> > @@ -31,7 +31,6 @@ obj-y += rbtree.o
> >> > obj-y += rcupdate.o
> >> > obj-y += sched_credit.o
> >> > obj-y += sched_credit2.o
> >> > -obj-y += sched_sedf.o
> >> > obj-y += sched_arinc653.o
> >> > obj-y += sched_rt.o
> >> > obj-y += schedule.o
> >>
> >> Stray change. Or perhaps the file doesn't build anymore, in which case
> >> you should instead have stated that the patch is dependent upon the
> >> series removing SEDF.
> >>
> > This indeed does not belong in here. And of course, things should
> > build... So, Chong, either deal with SEDF as well, if basing your
> > patches on a tree where it is still there, or base on top of my patches,
> > ignore it, but state the dependency, as Jan is asking.
> >
> >> > @@ -1157,8 +1158,75 @@ rt_dom_cntl(
> >
> >> > + case XEN_DOMCTL_SCHEDOP_putvcpuinfo:
> >> > + spin_lock_irqsave(&prv->lock, flags);
> >> > + for( index = 0; index < op->u.v.nr_vcpus; index++ )
> >> > + {
> >> > + if ( copy_from_guest_offset(&local_sched,
> >> > + op->u.v.vcpus, index, 1) )
> >> > + {
> >> > + rc = -EFAULT;
> >> > + break;
> >> > + }
> >> > + if ( local_sched.vcpuid >= d->max_vcpus
> >> > + || d->vcpu[local_sched.vcpuid] == NULL )
> >> > + {
> >> > + rc = -EINVAL;
> >> > + break;
> >> > + }
> >> > + svc = rt_vcpu(d->vcpu[local_sched.vcpuid]);
> >> > + svc->period = MICROSECS(local_sched.s.rtds.period);
> >> > + svc->budget = MICROSECS(local_sched.s.rtds.budget);
> >>
> >> Are all input values valid here?
> >>
> > That's a good point, actually. Right now, SEDF does some range
> > enforcement, by means of these values:
> >
> > #define PERIOD_MAX MILLISECS(10000) /* 10s */
> > #define PERIOD_MIN (MICROSECS(10)) /* 10us */
> > #define SLICE_MIN (MICROSECS(5)) /* 5us */
> >
> > Chong, it probably makes sense to (in a separate patch), introduce
> > something like this in RTDS too (with SLICE_MIN-->BUDGET_MIN), and then
> > use them, in this patch, for sanity checking the input.
> >
> > It also makes sense to check and enforce budget<=period, IMO.
> >
> > About the specific values, I'm open to proposals. I think something like
> > the SEDF's one is fine. Meng?
>
> We are trying to make some range enforcement for RTDS scheduler. Is my
> understanding correct? (It should be, but just in case. :-) )
>
We are wondering whether that could be necessary/useful, and IMO, it
would.
> As to the range of period, I think the max value can be as large as
> the type of period (ie. s_time_t) can represent. When we want a
> dedicated CPU for a guest, we will set budget=period and can set the
> period to a very very large value to avoid the unnecessarily
> invocation of the scheduler.
>
Makes sense. We do have STIME_MAX and, given that period is something
that is added to current time during scheduling, STIME_DELTA_MAX.
Maybe, put something together basing on those?
> As to the min value of period, I think it should be >=100us. The
> scheduler overhead of running a large box could be 1us if the runq is
> long and competetion of the runq lock is heavy. If the scheduler is
> potentially invoked every 10us, the scheduler overhead will be 10% of
> total computation time, which seems a lot to me.
>
Ok.
> As to the range of budget, the min value can be 5us, the same with
> SEDF;
>
Well, wouldn't the above reasoning about overhead apply here too?
Budgets of 5us mean the scheduler can be invoked every 5us for budget
enforcement. If 10us was unreasonable, 5 is even more so.
Therefore, 100us here too? Or maybe let's allow for lower values (like
50us or 10us), but print a warning?
> the max value is the value of period of the same VCPU.
>
Yep.
And, whatever the values, it would be useful to have comments somewhere
(either when the values are defined or enforced), stating what you said
above.
Regards,
Dario
--
<<This happens because I choose it to happen!>> (Raistlin Majere)
-----------------------------------------------------------------
Dario Faggioli, Ph.D, http://about.me/dario.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: 181 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] 39+ messages in thread
* Re: [PATCH v3 for Xen 4.6 3/4] libxl: enable per-VCPU parameter settings for RTDS scheduler
2015-07-07 16:23 ` Dario Faggioli
@ 2015-07-08 14:35 ` Chong Li
2015-07-08 14:45 ` Dario Faggioli
0 siblings, 1 reply; 39+ messages in thread
From: Chong Li @ 2015-07-08 14:35 UTC (permalink / raw)
To: Dario Faggioli
Cc: Chong Li, Wei Liu, Sisu Xi, George Dunlap, Ian Jackson, xen-devel,
Ian Campbell, Meng Xu, Dagaen Golomb
On Tue, Jul 7, 2015 at 11:23 AM, Dario Faggioli
<dario.faggioli@citrix.com> wrote:
> On Sun, 2015-06-28 at 21:44 -0500, Chong Li wrote:
>> Add libxl_vcpu_sched_params_get/set and sched_rtds_vcpu_get/set functions to support
>> per-VCPU settings.
>>
>>
>> +/* Per-VCPU parameters*/
>> +#define LIBXL_DOMAIN_SCHED_PARAM_VCPU_INDEX_DEFAULT -1
>> +
>> int libxl_domain_sched_params_get(libxl_ctx *ctx, uint32_t domid,
>> libxl_domain_sched_params *params);
>> int libxl_domain_sched_params_set(libxl_ctx *ctx, uint32_t domid,
>> const libxl_domain_sched_params *params);
>> +int libxl_vcpu_sched_params_get(libxl_ctx *ctx, uint32_t domid,
>> + libxl_vcpu_sched_params *params);
>> +int libxl_vcpu_sched_params_set(libxl_ctx *ctx, uint32_t domid,
>> + const libxl_vcpu_sched_params *params);
>>
> Didn't we say that we wanted the fact that now, at least for RTDS,
> libxl_domain_sched_params_*() deals with default parameters to be
> documented in a comment somewhere? Have I missed it?
I'll implement it in the next version.
Do both get/set deal with default parameters? I can easily implement
it in hypervisor, because default parameters
(RTDS_DEFAULT_PERIOD/BUDGET) are just defined there. But for libxl,
how do I deal with it without knowing the default values?
>
> I appreciate that this is happening in lower levels, and that libxl is
> just reporting it, but, still, it is something that I would like to find
> in the headers of the library, in order to be able to tell the
> differences between libxl_vcpu_sched_params_* and
> libxl_domain_sched_params_* ... Ian?
>
> Regards,
> Dario
> --
> <<This happens because I choose it to happen!>> (Raistlin Majere)
> -----------------------------------------------------------------
> Dario Faggioli, Ph.D, http://about.me/dario.faggioli
> Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)
--
Chong Li
Department of Computer Science and Engineering
Washington University in St.louis
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v3 for Xen 4.6 3/4] libxl: enable per-VCPU parameter settings for RTDS scheduler
2015-07-08 14:35 ` Chong Li
@ 2015-07-08 14:45 ` Dario Faggioli
0 siblings, 0 replies; 39+ messages in thread
From: Dario Faggioli @ 2015-07-08 14:45 UTC (permalink / raw)
To: Chong Li
Cc: Chong Li, Wei Liu, Sisu Xi, George Dunlap, Ian Jackson, xen-devel,
Ian Campbell, Meng Xu, Dagaen Golomb
[-- Attachment #1.1: Type: text/plain, Size: 2281 bytes --]
On Wed, 2015-07-08 at 09:35 -0500, Chong Li wrote:
> On Tue, Jul 7, 2015 at 11:23 AM, Dario Faggioli
> >>
> >> +/* Per-VCPU parameters*/
> >> +#define LIBXL_DOMAIN_SCHED_PARAM_VCPU_INDEX_DEFAULT -1
> >> +
> >> int libxl_domain_sched_params_get(libxl_ctx *ctx, uint32_t domid,
> >> libxl_domain_sched_params *params);
> >> int libxl_domain_sched_params_set(libxl_ctx *ctx, uint32_t domid,
> >> const libxl_domain_sched_params *params);
> >> +int libxl_vcpu_sched_params_get(libxl_ctx *ctx, uint32_t domid,
> >> + libxl_vcpu_sched_params *params);
> >> +int libxl_vcpu_sched_params_set(libxl_ctx *ctx, uint32_t domid,
> >> + const libxl_vcpu_sched_params *params);
> >>
> > Didn't we say that we wanted the fact that now, at least for RTDS,
> > libxl_domain_sched_params_*() deals with default parameters to be
> > documented in a comment somewhere? Have I missed it?
>
> I'll implement it in the next version.
>
> Do both get/set deal with default parameters? I can easily implement
> it in hypervisor, because default parameters
> (RTDS_DEFAULT_PERIOD/BUDGET) are just defined there. But for libxl,
> how do I deal with it without knowing the default values?
>
What I meant was that I am ok with this being implemented in the
hypervisor, but I think we want a comment explaining the difference
between the two sets of operations (the _domain_ one and the _vcpu_ one)
here in the library too.
And in such a comment, it would be good to mention what the _domain_
call does for a scheduler that supports per-vcpu parameters (and the
vice-versa, which, yes, is rather trivial, but still).
The aim would be to give an libxl user an idea of what function he/she
wants, depending on what scheduler and on which parameter, to achieve
its purposes, and what (kind of error/behavior) should be expected if
the wrong one is used.
Regards,
Dario
--
<<This happens because I choose it to happen!>> (Raistlin Majere)
-----------------------------------------------------------------
Dario Faggioli, Ph.D, http://about.me/dario.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: 181 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] 39+ messages in thread
* Re: [PATCH v3 for Xen 4.6 1/4] xen: enable per-VCPU parameter settings for RTDS scheduler
2015-07-08 8:33 ` Dario Faggioli
@ 2015-07-09 1:16 ` Meng Xu
2015-07-10 9:51 ` Dario Faggioli
0 siblings, 1 reply; 39+ messages in thread
From: Meng Xu @ 2015-07-09 1:16 UTC (permalink / raw)
To: Dario Faggioli
Cc: Chong Li, Sisu Xi, George Dunlap, xen-devel@lists.xen.org,
Meng Xu, Jan Beulich, Chong Li, Dagaen Golomb
2015-07-08 1:33 GMT-07:00 Dario Faggioli <dario.faggioli@citrix.com>:
> On Tue, 2015-07-07 at 23:06 -0700, Meng Xu wrote:
>> 2015-07-07 7:39 GMT-07:00 Dario Faggioli <dario.faggioli@citrix.com>:
>> > On Tue, 2015-07-07 at 09:59 +0100, Jan Beulich wrote:
>> >> >>> On 29.06.15 at 04:44, <lichong659@gmail.com> wrote:
>> >> > --- a/xen/common/Makefile
>> >> > +++ b/xen/common/Makefile
>> >> > @@ -31,7 +31,6 @@ obj-y += rbtree.o
>> >> > obj-y += rcupdate.o
>> >> > obj-y += sched_credit.o
>> >> > obj-y += sched_credit2.o
>> >> > -obj-y += sched_sedf.o
>> >> > obj-y += sched_arinc653.o
>> >> > obj-y += sched_rt.o
>> >> > obj-y += schedule.o
>> >>
>> >> Stray change. Or perhaps the file doesn't build anymore, in which case
>> >> you should instead have stated that the patch is dependent upon the
>> >> series removing SEDF.
>> >>
>> > This indeed does not belong in here. And of course, things should
>> > build... So, Chong, either deal with SEDF as well, if basing your
>> > patches on a tree where it is still there, or base on top of my patches,
>> > ignore it, but state the dependency, as Jan is asking.
>> >
>> >> > @@ -1157,8 +1158,75 @@ rt_dom_cntl(
>> >
>> >> > + case XEN_DOMCTL_SCHEDOP_putvcpuinfo:
>> >> > + spin_lock_irqsave(&prv->lock, flags);
>> >> > + for( index = 0; index < op->u.v.nr_vcpus; index++ )
>> >> > + {
>> >> > + if ( copy_from_guest_offset(&local_sched,
>> >> > + op->u.v.vcpus, index, 1) )
>> >> > + {
>> >> > + rc = -EFAULT;
>> >> > + break;
>> >> > + }
>> >> > + if ( local_sched.vcpuid >= d->max_vcpus
>> >> > + || d->vcpu[local_sched.vcpuid] == NULL )
>> >> > + {
>> >> > + rc = -EINVAL;
>> >> > + break;
>> >> > + }
>> >> > + svc = rt_vcpu(d->vcpu[local_sched.vcpuid]);
>> >> > + svc->period = MICROSECS(local_sched.s.rtds.period);
>> >> > + svc->budget = MICROSECS(local_sched.s.rtds.budget);
>> >>
>> >> Are all input values valid here?
>> >>
>> > That's a good point, actually. Right now, SEDF does some range
>> > enforcement, by means of these values:
>> >
>> > #define PERIOD_MAX MILLISECS(10000) /* 10s */
>> > #define PERIOD_MIN (MICROSECS(10)) /* 10us */
>> > #define SLICE_MIN (MICROSECS(5)) /* 5us */
>> >
>> > Chong, it probably makes sense to (in a separate patch), introduce
>> > something like this in RTDS too (with SLICE_MIN-->BUDGET_MIN), and then
>> > use them, in this patch, for sanity checking the input.
>> >
>> > It also makes sense to check and enforce budget<=period, IMO.
>> >
>> > About the specific values, I'm open to proposals. I think something like
>> > the SEDF's one is fine. Meng?
>>
>> We are trying to make some range enforcement for RTDS scheduler. Is my
>> understanding correct? (It should be, but just in case. :-) )
>>
> We are wondering whether that could be necessary/useful, and IMO, it
> would.
>
>> As to the range of period, I think the max value can be as large as
>> the type of period (ie. s_time_t) can represent. When we want a
>> dedicated CPU for a guest, we will set budget=period and can set the
>> period to a very very large value to avoid the unnecessarily
>> invocation of the scheduler.
>>
> Makes sense. We do have STIME_MAX and, given that period is something
> that is added to current time during scheduling, STIME_DELTA_MAX.
>
> Maybe, put something together basing on those?
>
>> As to the min value of period, I think it should be >=100us. The
>> scheduler overhead of running a large box could be 1us if the runq is
>> long and competetion of the runq lock is heavy. If the scheduler is
>> potentially invoked every 10us, the scheduler overhead will be 10% of
>> total computation time, which seems a lot to me.
>>
> Ok.
>
>> As to the range of budget, the min value can be 5us, the same with
>> SEDF;
>>
> Well, wouldn't the above reasoning about overhead apply here too?
> Budgets of 5us mean the scheduler can be invoked every 5us for budget
> enforcement. If 10us was unreasonable, 5 is even more so.
>
> Therefore, 100us here too? Or maybe let's allow for lower values (like
> 50us or 10us), but print a warning?
Right. we can use 100us as a threadhold for budget too. Maybe printing
warning is a better idea?
If users know what they are doing and expecting, it is ok. But too
small value (<1us) will potentially cause system freeze.
>
>> the max value is the value of period of the same VCPU.
>>
> Yep.
>
> And, whatever the values, it would be useful to have comments somewhere
> (either when the values are defined or enforced), stating what you said
> above.
right.
Thanks,
Meng
--
-----------
Meng Xu
PhD Student in Computer and Information Science
University of Pennsylvania
http://www.cis.upenn.edu/~mengxu/
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v3 for Xen 4.6 1/4] xen: enable per-VCPU parameter settings for RTDS scheduler
2015-07-09 1:16 ` Meng Xu
@ 2015-07-10 9:51 ` Dario Faggioli
0 siblings, 0 replies; 39+ messages in thread
From: Dario Faggioli @ 2015-07-10 9:51 UTC (permalink / raw)
To: Meng Xu
Cc: Chong Li, Sisu Xi, George Dunlap, xen-devel@lists.xen.org,
Meng Xu, Jan Beulich, Chong Li, Dagaen Golomb
[-- Attachment #1.1: Type: text/plain, Size: 1154 bytes --]
On Wed, 2015-07-08 at 18:16 -0700, Meng Xu wrote:
> 2015-07-08 1:33 GMT-07:00 Dario Faggioli <dario.faggioli@citrix.com>:
> > Well, wouldn't the above reasoning about overhead apply here too?
> > Budgets of 5us mean the scheduler can be invoked every 5us for budget
> > enforcement. If 10us was unreasonable, 5 is even more so.
> >
> > Therefore, 100us here too? Or maybe let's allow for lower values (like
> > 50us or 10us), but print a warning?
>
> Right. we can use 100us as a threadhold for budget too. Maybe printing
> warning is a better idea?
> If users know what they are doing and expecting, it is ok. But too
> small value (<1us) will potentially cause system freeze.
>
I'd be ok with printing a warning for vales below 100us, and rejecting
values below 1us, or even below 10us (I don't think it would be that
practical to go below that anyway).
Regards,
Dario
--
<<This happens because I choose it to happen!>> (Raistlin Majere)
-----------------------------------------------------------------
Dario Faggioli, Ph.D, http://about.me/dario.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: 181 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] 39+ messages in thread
end of thread, other threads:[~2015-07-10 9:51 UTC | newest]
Thread overview: 39+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-06-29 2:44 [PATCH v3 for Xen 4.6 0/4] Enable per-VCPU parameter settings for RTDS scheduler Chong Li
2015-06-29 2:44 ` [PATCH v3 for Xen 4.6 1/4] xen: enable " Chong Li
2015-07-07 8:59 ` Jan Beulich
2015-07-07 14:39 ` Dario Faggioli
2015-07-08 6:06 ` Meng Xu
2015-07-08 8:33 ` Dario Faggioli
2015-07-09 1:16 ` Meng Xu
2015-07-10 9:51 ` Dario Faggioli
2015-07-07 14:55 ` Dario Faggioli
2015-07-07 16:09 ` Jan Beulich
2015-07-07 15:33 ` Chong Li
2015-07-07 15:41 ` Jan Beulich
2015-07-07 15:46 ` Dario Faggioli
2015-07-07 14:51 ` Dario Faggioli
2015-06-29 2:44 ` [PATCH v3 for Xen 4.6 2/4] libxc: " Chong Li
2015-06-30 12:22 ` Ian Campbell
2015-06-30 15:18 ` Chong Li
2015-06-30 15:32 ` Ian Campbell
2015-06-30 15:57 ` Chong Li
2015-06-30 16:04 ` Ian Campbell
2015-06-29 2:44 ` [PATCH v3 for Xen 4.6 3/4] libxl: " Chong Li
2015-06-30 12:26 ` Ian Campbell
2015-06-30 15:42 ` Chong Li
2015-06-30 15:57 ` Ian Campbell
2015-06-30 16:10 ` Chong Li
2015-06-30 16:19 ` Ian Campbell
2015-06-30 16:53 ` Chong Li
2015-07-01 0:54 ` Meng Xu
2015-07-01 8:48 ` Ian Campbell
2015-07-01 12:50 ` Dario Faggioli
2015-07-01 16:59 ` Chong Li
2015-07-07 16:23 ` Dario Faggioli
2015-07-08 14:35 ` Chong Li
2015-07-08 14:45 ` Dario Faggioli
2015-06-29 2:44 ` [PATCH v3 for Xen 4.6 4/4] xl: " Chong Li
2015-07-07 15:34 ` Dario Faggioli
2015-07-07 16:01 ` Chong Li
2015-07-07 15:16 ` [PATCH v3 for Xen 4.6 0/4] Enable " Dario Faggioli
2015-07-07 16:11 ` Chong Li
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).