* [PATCH v3 0/6] add hypercall option to temporarily pin a vcpu @ 2016-03-03 16:48 Juergen Gross 2016-03-03 16:48 ` [PATCH v3 1/6] xen, cpupool: correct error handling when removing cpu from cpupool Juergen Gross ` (5 more replies) 0 siblings, 6 replies; 23+ messages in thread From: Juergen Gross @ 2016-03-03 16:48 UTC (permalink / raw) To: xen-devel Cc: Juergen Gross, Wei Liu, Stefano Stabellini, George Dunlap, Andrew Cooper, Dario Faggioli, Ian Jackson, David Vrabel, Jan Beulich Some hardware (e.g. Dell studio 1555 laptops) require SMIs to be called on physical cpu 0 only. Linux drivers like dcdbas or i8k try to achieve this by pinning the running thread to cpu 0, but in Dom0 this is not enough: the vcpu must be pinned to physical cpu 0 via Xen, too. This patch series adds a stable hypercall option to achieve this. And adds several related corrections and add-ons. Changes in V3: - removed old patch 1 as already accepted - new patch 1 to correct error in cpupool_unassign_cpu_helper() - patch 2: adjust coding style as requested by Jan Beulich - patch 2: rename pin_temp to pin_override as requested by Jan Beulich and suggested by David Vrabel - patch 2: change return type of do_pin_temp() to int as requested by Jan Beulich - patch 2: swap error checks in do_sched_op() as requested by Jan Beulich - patch 2: adjust comment in xen/include/public/sched.h as requested by Jan Beulich - new patch 3 to add possibility in hypervisor to undo pin override via tools - patch 4 (was 3): adjust coding style as requested by Wei Liu - new patch 5 to print message how to recover from xl cpupool-cpu-remove errors - new patch 6 to undo pin override via xl vcpu-pin (force option) Changes in V2: - add patch 1 to silence messages on suspend/resume - add patch 3 to handle EBUSY case when removing cpu from cpupool - limit operation to hardware domain as suggested by Jan Beulich - some style issues corrected as requested by Jan Beulich - use fixed width types in interface as requested by Jan Beulich - add compat layer checking as requested by Jan Beulich Juergen Gross (6): xen, cpupool: correct error handling when removing cpu from cpupool xen: add hypercall option to override and restore vcpu affinity xen: add force flag to xen_domctl_vcpuaffinity for undoing pin override libxc: do some retries in xc_cpupool_removecpu() for EBUSY case libxl: print message how to recover from xl cpupool-cpu-remove errors libxl: add force option for xl vcpu-pin tools/libxc/xc_cpupool.c | 13 ++++++- tools/libxl/libxl.c | 31 ++++++++++++--- tools/libxl/libxl.h | 4 ++ tools/libxl/xl_cmdimpl.c | 33 +++++++++++++--- tools/libxl/xl_cmdtable.c | 3 +- xen/common/compat/schedule.c | 4 ++ xen/common/cpupool.c | 18 ++++++--- xen/common/domctl.c | 6 +++ xen/common/schedule.c | 93 +++++++++++++++++++++++++++++++++++++++++--- xen/include/public/domctl.h | 3 ++ xen/include/public/sched.h | 17 ++++++++ xen/include/xen/sched.h | 1 + xen/include/xlat.lst | 1 + 13 files changed, 203 insertions(+), 24 deletions(-) Cc: Andrew Cooper <andrew.cooper3@citrix.com> Cc: Dario Faggioli <dario.faggioli@citrix.com> Cc: David Vrabel <david.vrabel@citrix.com> Cc: George Dunlap <george.dunlap@eu.citrix.com> Cc: Ian Jackson <ian.jackson@eu.citrix.com> Cc: Jan Beulich <jbeulich@suse.com> Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com> Cc: Wei Liu <wei.liu2@citrix.com> -- 2.6.2 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel ^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH v3 1/6] xen, cpupool: correct error handling when removing cpu from cpupool 2016-03-03 16:48 [PATCH v3 0/6] add hypercall option to temporarily pin a vcpu Juergen Gross @ 2016-03-03 16:48 ` Juergen Gross 2016-03-04 9:42 ` Jan Beulich ` (2 more replies) 2016-03-03 16:48 ` [PATCH v3 2/6] xen: add hypercall option to override and restore vcpu affinity Juergen Gross ` (4 subsequent siblings) 5 siblings, 3 replies; 23+ messages in thread From: Juergen Gross @ 2016-03-03 16:48 UTC (permalink / raw) To: xen-devel; +Cc: Juergen Gross, Dario Faggioli, Jan Beulich When schedule_cpu_switch() called from cpupool_unassign_cpu_helper() returns an error, the domlist_read_lock isn't released again. As cpu_disable_scheduler() might have changed affinity of some domains domain_update_node_affinity() must be called for all domains in the cpupool even in error case. Even if looking weird it is okay to let the to be removed cpu set in cpupool_free_cpus in case of an error returned by cpu_disable_scheduler(). Add a comment explaining the reason for this. Cc: Dario Faggioli <dario.faggioli@citrix.com> Cc: Jan Beulich <jbeulich@suse.com> Signed-off-by: Juergen Gross <jgross@suse.com> --- xen/common/cpupool.c | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/xen/common/cpupool.c b/xen/common/cpupool.c index 8e7b723..d0189f8 100644 --- a/xen/common/cpupool.c +++ b/xen/common/cpupool.c @@ -312,17 +312,25 @@ static long cpupool_unassign_cpu_helper(void *info) rcu_read_lock(&domlist_read_lock); ret = cpu_disable_scheduler(cpu); cpumask_set_cpu(cpu, &cpupool_free_cpus); + + /* + * cpu_disable_scheduler() returning an error doesn't require resetting + * cpupool_free_cpus' cpu bit. All error cases should be of temporary + * nature and tools will retry the operation. Even if the number of + * retries may be limited, the in-between state can easily be repaired + * by adding the cpu to the cpupool again. + */ if ( !ret ) { ret = schedule_cpu_switch(cpu, NULL); if ( ret ) - { cpumask_clear_cpu(cpu, &cpupool_free_cpus); - goto out; + else + { + cpupool_moving_cpu = -1; + cpupool_put(cpupool_cpu_moving); + cpupool_cpu_moving = NULL; } - cpupool_moving_cpu = -1; - cpupool_put(cpupool_cpu_moving); - cpupool_cpu_moving = NULL; } for_each_domain_in_cpupool(d, c) -- 2.6.2 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel ^ permalink raw reply related [flat|nested] 23+ messages in thread
* Re: [PATCH v3 1/6] xen, cpupool: correct error handling when removing cpu from cpupool 2016-03-03 16:48 ` [PATCH v3 1/6] xen, cpupool: correct error handling when removing cpu from cpupool Juergen Gross @ 2016-03-04 9:42 ` Jan Beulich [not found] ` <56D9668602000078000D9400@suse.com> 2016-03-08 10:46 ` Dario Faggioli 2 siblings, 0 replies; 23+ messages in thread From: Jan Beulich @ 2016-03-04 9:42 UTC (permalink / raw) To: Juergen Gross; +Cc: Dario Faggioli, xen-devel >>> On 03.03.16 at 17:48, <JGross@suse.com> wrote: > if ( !ret ) > { > ret = schedule_cpu_switch(cpu, NULL); > if ( ret ) > - { > cpumask_clear_cpu(cpu, &cpupool_free_cpus); > - goto out; > + else > + { > + cpupool_moving_cpu = -1; > + cpupool_put(cpupool_cpu_moving); > + cpupool_cpu_moving = NULL; > } > - cpupool_moving_cpu = -1; > - cpupool_put(cpupool_cpu_moving); > - cpupool_cpu_moving = NULL; > } > > for_each_domain_in_cpupool(d, c) So this now also eliminates the bypass of the loop the beginning of which we can still see here - is that really intended, or wouldn't this better be made conditional upon !ret? Jan _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel ^ permalink raw reply [flat|nested] 23+ messages in thread
[parent not found: <56D9668602000078000D9400@suse.com>]
* Re: [PATCH v3 1/6] xen, cpupool: correct error handling when removing cpu from cpupool [not found] ` <56D9668602000078000D9400@suse.com> @ 2016-03-04 9:54 ` Juergen Gross 2016-03-04 10:03 ` Jan Beulich 0 siblings, 1 reply; 23+ messages in thread From: Juergen Gross @ 2016-03-04 9:54 UTC (permalink / raw) To: Jan Beulich; +Cc: Dario Faggioli, xen-devel On 04/03/16 10:42, Jan Beulich wrote: >>>> On 03.03.16 at 17:48, <JGross@suse.com> wrote: >> if ( !ret ) >> { >> ret = schedule_cpu_switch(cpu, NULL); >> if ( ret ) >> - { >> cpumask_clear_cpu(cpu, &cpupool_free_cpus); >> - goto out; >> + else >> + { >> + cpupool_moving_cpu = -1; >> + cpupool_put(cpupool_cpu_moving); >> + cpupool_cpu_moving = NULL; >> } >> - cpupool_moving_cpu = -1; >> - cpupool_put(cpupool_cpu_moving); >> - cpupool_cpu_moving = NULL; >> } >> >> for_each_domain_in_cpupool(d, c) > > So this now also eliminates the bypass of the loop the beginning of > which we can still see here - is that really intended, or wouldn't > this better be made conditional upon !ret? Did you look at the commit message? It is explaining exactly this behaviour. Juergen _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v3 1/6] xen, cpupool: correct error handling when removing cpu from cpupool 2016-03-04 9:54 ` Juergen Gross @ 2016-03-04 10:03 ` Jan Beulich 0 siblings, 0 replies; 23+ messages in thread From: Jan Beulich @ 2016-03-04 10:03 UTC (permalink / raw) To: Juergen Gross; +Cc: Dario Faggioli, xen-devel >>> On 04.03.16 at 10:54, <JGross@suse.com> wrote: > On 04/03/16 10:42, Jan Beulich wrote: >>>>> On 03.03.16 at 17:48, <JGross@suse.com> wrote: >>> if ( !ret ) >>> { >>> ret = schedule_cpu_switch(cpu, NULL); >>> if ( ret ) >>> - { >>> cpumask_clear_cpu(cpu, &cpupool_free_cpus); >>> - goto out; >>> + else >>> + { >>> + cpupool_moving_cpu = -1; >>> + cpupool_put(cpupool_cpu_moving); >>> + cpupool_cpu_moving = NULL; >>> } >>> - cpupool_moving_cpu = -1; >>> - cpupool_put(cpupool_cpu_moving); >>> - cpupool_cpu_moving = NULL; >>> } >>> >>> for_each_domain_in_cpupool(d, c) >> >> So this now also eliminates the bypass of the loop the beginning of >> which we can still see here - is that really intended, or wouldn't >> this better be made conditional upon !ret? > > Did you look at the commit message? It is explaining exactly this > behaviour. Argh - I had looked, even twice (once before writing the comment, and once before hitting the send button), but nevertheless managed to not connect things together. I'm sorry. And I think this aspect (and only it) makes this a backport candidate. Jan _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v3 1/6] xen, cpupool: correct error handling when removing cpu from cpupool 2016-03-03 16:48 ` [PATCH v3 1/6] xen, cpupool: correct error handling when removing cpu from cpupool Juergen Gross 2016-03-04 9:42 ` Jan Beulich [not found] ` <56D9668602000078000D9400@suse.com> @ 2016-03-08 10:46 ` Dario Faggioli 2 siblings, 0 replies; 23+ messages in thread From: Dario Faggioli @ 2016-03-08 10:46 UTC (permalink / raw) To: Juergen Gross, xen-devel; +Cc: Jan Beulich [-- Attachment #1.1: Type: text/plain, Size: 1072 bytes --] On Thu, 2016-03-03 at 17:48 +0100, Juergen Gross wrote: > When schedule_cpu_switch() called from cpupool_unassign_cpu_helper() > returns an error, the domlist_read_lock isn't released again. > > As cpu_disable_scheduler() might have changed affinity of some > domains domain_update_node_affinity() must be called for all domains > in the cpupool even in error case. > > Even if looking weird it is okay to let the to be removed cpu set in > cpupool_free_cpus in case of an error returned by > cpu_disable_scheduler(). Add a comment explaining the reason for > this. > > Cc: Dario Faggioli <dario.faggioli@citrix.com> > Cc: Jan Beulich <jbeulich@suse.com> > Signed-off-by: Juergen Gross <jgross@suse.com> > Acked-by: Dario Faggioli <dario.faggioli@citrix.com> 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] 23+ messages in thread
* [PATCH v3 2/6] xen: add hypercall option to override and restore vcpu affinity 2016-03-03 16:48 [PATCH v3 0/6] add hypercall option to temporarily pin a vcpu Juergen Gross 2016-03-03 16:48 ` [PATCH v3 1/6] xen, cpupool: correct error handling when removing cpu from cpupool Juergen Gross @ 2016-03-03 16:48 ` Juergen Gross 2016-03-04 9:44 ` Jan Beulich 2016-03-08 11:45 ` Dario Faggioli 2016-03-03 16:48 ` [PATCH v3 3/6] xen: add force flag to xen_domctl_vcpuaffinity for undoing pin override Juergen Gross ` (3 subsequent siblings) 5 siblings, 2 replies; 23+ messages in thread From: Juergen Gross @ 2016-03-03 16:48 UTC (permalink / raw) To: xen-devel Cc: Juergen Gross, Wei Liu, Stefano Stabellini, George Dunlap, Andrew Cooper, Dario Faggioli, Ian Jackson, David Vrabel, Jan Beulich Some hardware (e.g. Dell studio 1555 laptops) require SMIs to be called on physical cpu 0 only. Linux drivers like dcdbas or i8k try to achieve this by pinning the running thread to cpu 0, but in Dom0 this is not enough: the vcpu must be pinned to physical cpu 0 via Xen, too. Add a stable hypercall option SCHEDOP_pin_override to the sched_op hypercall to achieve this. It is taking a physical cpu number as parameter. If pinning is possible (the calling domain has the privilege to make the call and the cpu is available in the domain's cpupool) the calling vcpu is pinned to the specified cpu. The old cpu affinity is saved. To undo the override pinning a negative cpu value is specified. This will restore the original cpu affinity of the vcpu. Cc: George Dunlap <george.dunlap@eu.citrix.com> Cc: Dario Faggioli <dario.faggioli@citrix.com> Cc: Jan Beulich <jbeulich@suse.com> Cc: Andrew Cooper <andrew.cooper3@citrix.com> Cc: Ian Jackson <ian.jackson@eu.citrix.com> Cc: David Vrabel <david.vrabel@citrix.com> Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com> Cc: Wei Liu <wei.liu2@citrix.com> Signed-off-by: Juergen Gross <jgross@suse.com> --- V2: - limit operation to hardware domain as suggested by Jan Beulich - some style issues corrected as requested by Jan Beulich - use fixed width types in interface as requested by Jan Beulich - add compat layer checking as requested by Jan Beulich V3: - adjust coding style as requested by Jan Beulich - rename pin_temp to pin_override as requested by Jan Beulich and suggested by David Vrabel - change return type of do_pin_temp() to int as requested by Jan Beulich - swap error checks in do_sched_op() as requested by Jan Beulich - adjust comment in xen/include/public/sched.h as requested by Jan Beulich --- xen/common/compat/schedule.c | 4 ++ xen/common/schedule.c | 93 +++++++++++++++++++++++++++++++++++++++++--- xen/include/public/sched.h | 17 ++++++++ xen/include/xen/sched.h | 1 + xen/include/xlat.lst | 1 + 5 files changed, 111 insertions(+), 5 deletions(-) diff --git a/xen/common/compat/schedule.c b/xen/common/compat/schedule.c index 812c550..8b6e6f1 100644 --- a/xen/common/compat/schedule.c +++ b/xen/common/compat/schedule.c @@ -10,6 +10,10 @@ #define do_sched_op compat_sched_op +#define xen_sched_pin_override sched_pin_override +CHECK_sched_pin_override; +#undef xen_sched_pin_override + #define xen_sched_shutdown sched_shutdown CHECK_sched_shutdown; #undef xen_sched_shutdown diff --git a/xen/common/schedule.c b/xen/common/schedule.c index 13803ec..e57b659 100644 --- a/xen/common/schedule.c +++ b/xen/common/schedule.c @@ -271,6 +271,12 @@ int sched_move_domain(struct domain *d, struct cpupool *c) struct scheduler *old_ops; void *old_domdata; + for_each_vcpu ( d, v ) + { + if ( v->affinity_broken ) + return -EBUSY; + } + domdata = SCHED_OP(c->sched, alloc_domdata, d); if ( domdata == NULL ) return -ENOMEM; @@ -669,6 +675,14 @@ int cpu_disable_scheduler(unsigned int cpu) if ( cpumask_empty(&online_affinity) && cpumask_test_cpu(cpu, v->cpu_hard_affinity) ) { + if ( v->affinity_broken ) + { + /* The vcpu is temporarily pinned, can't move it. */ + vcpu_schedule_unlock_irqrestore(lock, flags, v); + ret = -EBUSY; + break; + } + if (system_state == SYS_STATE_suspend) { cpumask_copy(v->cpu_hard_affinity_saved, @@ -752,14 +766,22 @@ static int vcpu_set_affinity( struct vcpu *v, const cpumask_t *affinity, cpumask_t *which) { spinlock_t *lock; + int ret = 0; lock = vcpu_schedule_lock_irq(v); - cpumask_copy(which, affinity); + if ( v->affinity_broken ) + ret = -EBUSY; + else + { + cpumask_copy(which, affinity); - /* Always ask the scheduler to re-evaluate placement - * when changing the affinity */ - set_bit(_VPF_migrating, &v->pause_flags); + /* + * Always ask the scheduler to re-evaluate placement + * when changing the affinity. + */ + set_bit(_VPF_migrating, &v->pause_flags); + } vcpu_schedule_unlock_irq(lock, v); @@ -771,7 +793,7 @@ static int vcpu_set_affinity( vcpu_migrate(v); } - return 0; + return ret; } int vcpu_set_hard_affinity(struct vcpu *v, const cpumask_t *affinity) @@ -982,6 +1004,50 @@ void watchdog_domain_destroy(struct domain *d) kill_timer(&d->watchdog_timer[i]); } +int vcpu_pin_override(struct vcpu *v, int cpu) +{ + spinlock_t *lock; + int ret = -EINVAL; + + lock = vcpu_schedule_lock_irq(v); + + if ( cpu < 0 ) + { + if ( v->affinity_broken ) + { + cpumask_copy(v->cpu_hard_affinity, v->cpu_hard_affinity_saved); + v->affinity_broken = 0; + set_bit(_VPF_migrating, &v->pause_flags); + ret = 0; + } + } + else if ( cpu < nr_cpu_ids ) + { + if ( v->affinity_broken ) + ret = -EBUSY; + else if ( cpumask_test_cpu(cpu, VCPU2ONLINE(v)) ) + { + cpumask_copy(v->cpu_hard_affinity_saved, v->cpu_hard_affinity); + v->affinity_broken = 1; + cpumask_copy(v->cpu_hard_affinity, cpumask_of(cpu)); + set_bit(_VPF_migrating, &v->pause_flags); + ret = 0; + } + } + + vcpu_schedule_unlock_irq(lock, v); + + domain_update_node_affinity(v->domain); + + if ( v->pause_flags & VPF_migrating ) + { + vcpu_sleep_nosync(v); + vcpu_migrate(v); + } + + return ret; +} + typedef long ret_t; #endif /* !COMPAT */ @@ -1091,6 +1157,23 @@ ret_t do_sched_op(int cmd, XEN_GUEST_HANDLE_PARAM(void) arg) break; } + case SCHEDOP_pin_override: + { + struct sched_pin_override sched_pin_override; + + ret = -EPERM; + if ( !is_hardware_domain(current->domain) ) + break; + + ret = -EFAULT; + if ( copy_from_guest(&sched_pin_override, arg, 1) ) + break; + + ret = vcpu_pin_override(current, sched_pin_override.pcpu); + + break; + } + default: ret = -ENOSYS; } diff --git a/xen/include/public/sched.h b/xen/include/public/sched.h index 2219696..82c338f 100644 --- a/xen/include/public/sched.h +++ b/xen/include/public/sched.h @@ -118,6 +118,17 @@ * With id != 0 and timeout != 0, poke watchdog timer and set new timeout. */ #define SCHEDOP_watchdog 6 + +/* + * Override the current vcpu affinity by pinning it to one physical cpu or undo + * this override restoring the previous affinity. + * @arg == pointer to sched_pin_override_t structure. + * + * A negative pcpu value will undo a previous pin override and restore the + * previous cpu affinity. + * This call is allowed for the hardware domain only. + */ +#define SCHEDOP_pin_override 7 /* ` } */ struct sched_shutdown { @@ -148,6 +159,12 @@ struct sched_watchdog { typedef struct sched_watchdog sched_watchdog_t; DEFINE_XEN_GUEST_HANDLE(sched_watchdog_t); +struct sched_pin_override { + int32_t pcpu; +}; +typedef struct sched_pin_override sched_pin_override_t; +DEFINE_XEN_GUEST_HANDLE(sched_pin_override_t); + /* * Reason codes for SCHEDOP_shutdown. These may be interpreted by control * software to determine the appropriate action. For the most part, Xen does diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h index cc77d70..09fff5f 100644 --- a/xen/include/xen/sched.h +++ b/xen/include/xen/sched.h @@ -811,6 +811,7 @@ int cpu_disable_scheduler(unsigned int cpu); int vcpu_set_hard_affinity(struct vcpu *v, const cpumask_t *affinity); int vcpu_set_soft_affinity(struct vcpu *v, const cpumask_t *affinity); void restore_vcpu_affinity(struct domain *d); +int vcpu_pin_override(struct vcpu *v, int cpu); void vcpu_runstate_get(struct vcpu *v, struct vcpu_runstate_info *runstate); uint64_t get_cpu_idle_time(unsigned int cpu); diff --git a/xen/include/xlat.lst b/xen/include/xlat.lst index fda1137..23befb3 100644 --- a/xen/include/xlat.lst +++ b/xen/include/xlat.lst @@ -104,6 +104,7 @@ ? pmu_data pmu.h ? pmu_params pmu.h ! sched_poll sched.h +? sched_pin_override sched.h ? sched_remote_shutdown sched.h ? sched_shutdown sched.h ? tmem_oid tmem.h -- 2.6.2 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel ^ permalink raw reply related [flat|nested] 23+ messages in thread
* Re: [PATCH v3 2/6] xen: add hypercall option to override and restore vcpu affinity 2016-03-03 16:48 ` [PATCH v3 2/6] xen: add hypercall option to override and restore vcpu affinity Juergen Gross @ 2016-03-04 9:44 ` Jan Beulich 2016-03-08 11:45 ` Dario Faggioli 1 sibling, 0 replies; 23+ messages in thread From: Jan Beulich @ 2016-03-04 9:44 UTC (permalink / raw) To: xen-devel, Juergen Gross Cc: Wei Liu, Stefano Stabellini, George Dunlap, Andrew Cooper, Dario Faggioli, Ian Jackson, David Vrabel >>> On 03.03.16 at 17:48, <JGross@suse.com> wrote: > Some hardware (e.g. Dell studio 1555 laptops) require SMIs to be > called on physical cpu 0 only. Linux drivers like dcdbas or i8k try > to achieve this by pinning the running thread to cpu 0, but in Dom0 > this is not enough: the vcpu must be pinned to physical cpu 0 via > Xen, too. > > Add a stable hypercall option SCHEDOP_pin_override to the sched_op > hypercall to achieve this. It is taking a physical cpu number as > parameter. If pinning is possible (the calling domain has the > privilege to make the call and the cpu is available in the domain's > cpupool) the calling vcpu is pinned to the specified cpu. The old > cpu affinity is saved. To undo the override pinning a negative cpu > value is specified. This will restore the original cpu affinity of > the vcpu. > > Cc: George Dunlap <george.dunlap@eu.citrix.com> > Cc: Dario Faggioli <dario.faggioli@citrix.com> > Cc: Jan Beulich <jbeulich@suse.com> > Cc: Andrew Cooper <andrew.cooper3@citrix.com> > Cc: Ian Jackson <ian.jackson@eu.citrix.com> > Cc: David Vrabel <david.vrabel@citrix.com> > Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com> > Cc: Wei Liu <wei.liu2@citrix.com> > > Signed-off-by: Juergen Gross <jgross@suse.com> Acked-by: Jan Beulich <jbeulich@suse.com> _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v3 2/6] xen: add hypercall option to override and restore vcpu affinity 2016-03-03 16:48 ` [PATCH v3 2/6] xen: add hypercall option to override and restore vcpu affinity Juergen Gross 2016-03-04 9:44 ` Jan Beulich @ 2016-03-08 11:45 ` Dario Faggioli 1 sibling, 0 replies; 23+ messages in thread From: Dario Faggioli @ 2016-03-08 11:45 UTC (permalink / raw) To: Juergen Gross, xen-devel Cc: Wei Liu, Stefano Stabellini, George Dunlap, Andrew Cooper, Ian Jackson, David Vrabel, Jan Beulich [-- Attachment #1.1: Type: text/plain, Size: 1994 bytes --] On Thu, 2016-03-03 at 17:48 +0100, Juergen Gross wrote: > Some hardware (e.g. Dell studio 1555 laptops) require SMIs to be > called on physical cpu 0 only. Linux drivers like dcdbas or i8k try > to achieve this by pinning the running thread to cpu 0, but in Dom0 > this is not enough: the vcpu must be pinned to physical cpu 0 via > Xen, too. > > Add a stable hypercall option SCHEDOP_pin_override to the sched_op > hypercall to achieve this. It is taking a physical cpu number as > parameter. If pinning is possible (the calling domain has the > privilege to make the call and the cpu is available in the domain's > cpupool) the calling vcpu is pinned to the specified cpu. > I would have added the "and the cpu is available in the domain's cpupool" part in the comment in public headers too, such as: > --- a/xen/include/public/sched.h > +++ b/xen/include/public/sched.h > @@ -118,6 +118,17 @@ > * With id != 0 and timeout != 0, poke watchdog timer and set new > timeout. > */ > #define SCHEDOP_watchdog 6 > + > +/* > + * Override the current vcpu affinity by pinning it to one physical > cpu or undo > + * this override restoring the previous affinity. > + * @arg == pointer to sched_pin_override_t structure. > + * > + * A negative pcpu value will undo a previous pin override and > restore the > + * previous cpu affinity. > + * This call is allowed for the hardware domain only. ", and succeeds only if the specified cpu is available in the domain's cpupool." > + */ > +#define SCHEDOP_pin_override 7 > /* ` } */ > > struct sched_shutdown { > In any case, the scheduling part is: Acked-by: Dario Faggioli <dario.faggioli@citrix.com> 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] 23+ messages in thread
* [PATCH v3 3/6] xen: add force flag to xen_domctl_vcpuaffinity for undoing pin override 2016-03-03 16:48 [PATCH v3 0/6] add hypercall option to temporarily pin a vcpu Juergen Gross 2016-03-03 16:48 ` [PATCH v3 1/6] xen, cpupool: correct error handling when removing cpu from cpupool Juergen Gross 2016-03-03 16:48 ` [PATCH v3 2/6] xen: add hypercall option to override and restore vcpu affinity Juergen Gross @ 2016-03-03 16:48 ` Juergen Gross 2016-03-04 9:45 ` Jan Beulich 2016-03-03 16:48 ` [PATCH v3 4/6] libxc: do some retries in xc_cpupool_removecpu() for EBUSY case Juergen Gross ` (2 subsequent siblings) 5 siblings, 1 reply; 23+ messages in thread From: Juergen Gross @ 2016-03-03 16:48 UTC (permalink / raw) To: xen-devel Cc: Juergen Gross, Wei Liu, Stefano Stabellini, Andrew Cooper, Ian Jackson, David Vrabel, Jan Beulich Add a XEN_VCPUAFFINITY_FORCE flag to xen_domctl_vcpuaffinity structure which will allow to undo a SCHEDOP_pin_override in case of a driver error of the hardware domain which didn't do the expected SCHEDOP_pin_override with cpu < 0 which would have done the undo operation. Cc: Jan Beulich <jbeulich@suse.com> Cc: Andrew Cooper <andrew.cooper3@citrix.com> Cc: Ian Jackson <ian.jackson@eu.citrix.com> Cc: David Vrabel <david.vrabel@citrix.com> Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com> Cc: Wei Liu <wei.liu2@citrix.com> Signed-off-by: Juergen Gross <jgross@suse.com> --- xen/common/domctl.c | 6 ++++++ xen/include/public/domctl.h | 3 +++ 2 files changed, 9 insertions(+) diff --git a/xen/common/domctl.c b/xen/common/domctl.c index b34c0a1..e43904e 100644 --- a/xen/common/domctl.c +++ b/xen/common/domctl.c @@ -782,6 +782,12 @@ long do_domctl(XEN_GUEST_HANDLE_PARAM(xen_domctl_t) u_domctl) break; } + /* Undo a stuck SCHED_pin_override? */ + if ( vcpuaff->flags & XEN_VCPUAFFINITY_FORCE ) + vcpu_pin_override(v, -1); + + ret = 0; + /* * We both set a new affinity and report back to the caller what * the scheduler will be effectively using. diff --git a/xen/include/public/domctl.h b/xen/include/public/domctl.h index a934318..076b1ae 100644 --- a/xen/include/public/domctl.h +++ b/xen/include/public/domctl.h @@ -307,6 +307,9 @@ struct xen_domctl_vcpuaffinity { /* Set/get the soft affinity for vcpu */ #define _XEN_VCPUAFFINITY_SOFT 1 #define XEN_VCPUAFFINITY_SOFT (1U<<_XEN_VCPUAFFINITY_SOFT) + /* Undo SCHEDOP_pin_override */ +#define _XEN_VCPUAFFINITY_FORCE 2 +#define XEN_VCPUAFFINITY_FORCE (1U<<_XEN_VCPUAFFINITY_FORCE) uint32_t flags; /* * IN/OUT variables. -- 2.6.2 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel ^ permalink raw reply related [flat|nested] 23+ messages in thread
* Re: [PATCH v3 3/6] xen: add force flag to xen_domctl_vcpuaffinity for undoing pin override 2016-03-03 16:48 ` [PATCH v3 3/6] xen: add force flag to xen_domctl_vcpuaffinity for undoing pin override Juergen Gross @ 2016-03-04 9:45 ` Jan Beulich 0 siblings, 0 replies; 23+ messages in thread From: Jan Beulich @ 2016-03-04 9:45 UTC (permalink / raw) To: Juergen Gross Cc: Wei Liu, Stefano Stabellini, Andrew Cooper, Ian Jackson, xen-devel, David Vrabel >>> On 03.03.16 at 17:48, <JGross@suse.com> wrote: > Add a XEN_VCPUAFFINITY_FORCE flag to xen_domctl_vcpuaffinity structure > which will allow to undo a SCHEDOP_pin_override in case of a driver > error of the hardware domain which didn't do the expected > SCHEDOP_pin_override with cpu < 0 which would have done the undo > operation. > > Cc: Jan Beulich <jbeulich@suse.com> > Cc: Andrew Cooper <andrew.cooper3@citrix.com> > Cc: Ian Jackson <ian.jackson@eu.citrix.com> > Cc: David Vrabel <david.vrabel@citrix.com> > Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com> > Cc: Wei Liu <wei.liu2@citrix.com> > Signed-off-by: Juergen Gross <jgross@suse.com> Acked-by: Jan Beulich <jbeulich@suse.com> _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel ^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH v3 4/6] libxc: do some retries in xc_cpupool_removecpu() for EBUSY case 2016-03-03 16:48 [PATCH v3 0/6] add hypercall option to temporarily pin a vcpu Juergen Gross ` (2 preceding siblings ...) 2016-03-03 16:48 ` [PATCH v3 3/6] xen: add force flag to xen_domctl_vcpuaffinity for undoing pin override Juergen Gross @ 2016-03-03 16:48 ` Juergen Gross 2016-03-08 13:16 ` Dario Faggioli 2016-03-03 16:48 ` [PATCH v3 5/6] libxl: print message how to recover from xl cpupool-cpu-remove errors Juergen Gross 2016-03-03 16:48 ` [PATCH v3 6/6] libxl: add force option for xl vcpu-pin Juergen Gross 5 siblings, 1 reply; 23+ messages in thread From: Juergen Gross @ 2016-03-03 16:48 UTC (permalink / raw) To: xen-devel; +Cc: Juergen Gross, Wei Liu, Ian Jackson, Stefano Stabellini The hypervisor might return EBUSY when trying to remove a cpu from a cpupool when a domain running in this cpupool has pinned a vcpu temporarily. Do some retries in this case, perhaps the situation cleans up. Cc: Ian Jackson <ian.jackson@eu.citrix.com> Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com> Cc: Wei Liu <wei.liu2@citrix.com> Signed-off-by: Juergen Gross <jgross@suse.com> --- V3: adjust coding style as requested by Wei Liu --- tools/libxc/xc_cpupool.c | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/tools/libxc/xc_cpupool.c b/tools/libxc/xc_cpupool.c index c42273e..9626699 100644 --- a/tools/libxc/xc_cpupool.c +++ b/tools/libxc/xc_cpupool.c @@ -20,8 +20,11 @@ */ #include <stdarg.h> +#include <unistd.h> #include "xc_private.h" +#define LIBXC_BUSY_RETRIES 5 + static int do_sysctl_save(xc_interface *xch, struct xen_sysctl *sysctl) { int ret; @@ -141,13 +144,21 @@ int xc_cpupool_removecpu(xc_interface *xch, uint32_t poolid, int cpu) { + unsigned retries; + int err; DECLARE_SYSCTL; sysctl.cmd = XEN_SYSCTL_cpupool_op; sysctl.u.cpupool_op.op = XEN_SYSCTL_CPUPOOL_OP_RMCPU; sysctl.u.cpupool_op.cpupool_id = poolid; sysctl.u.cpupool_op.cpu = (cpu < 0) ? XEN_SYSCTL_CPUPOOL_PAR_ANY : cpu; - return do_sysctl_save(xch, &sysctl); + for ( retries = 0; retries < LIBXC_BUSY_RETRIES; retries++ ) { + err = do_sysctl_save(xch, &sysctl); + if ( err >= 0 || errno != EBUSY ) + break; + sleep(1); + } + return err; } int xc_cpupool_movedomain(xc_interface *xch, -- 2.6.2 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel ^ permalink raw reply related [flat|nested] 23+ messages in thread
* Re: [PATCH v3 4/6] libxc: do some retries in xc_cpupool_removecpu() for EBUSY case 2016-03-03 16:48 ` [PATCH v3 4/6] libxc: do some retries in xc_cpupool_removecpu() for EBUSY case Juergen Gross @ 2016-03-08 13:16 ` Dario Faggioli 2016-03-08 16:12 ` Juergen Gross 0 siblings, 1 reply; 23+ messages in thread From: Dario Faggioli @ 2016-03-08 13:16 UTC (permalink / raw) To: Juergen Gross, xen-devel; +Cc: Ian Jackson, Wei Liu, Stefano Stabellini [-- Attachment #1.1: Type: text/plain, Size: 2740 bytes --] On Thu, 2016-03-03 at 17:48 +0100, Juergen Gross wrote: > The hypervisor might return EBUSY when trying to remove a cpu from a > cpupool when a domain running in this cpupool has pinned a vcpu > temporarily. Do some retries in this case, perhaps the situation > cleans up. > I now I'm at high risk of being called nitpicker (or, more likely, much worse names), but I think that: > --- a/tools/libxc/xc_cpupool.c > +++ b/tools/libxc/xc_cpupool.c > @@ -20,8 +20,11 @@ > */ > > #include <stdarg.h> > +#include <unistd.h> > #include "xc_private.h" > > +#define LIBXC_BUSY_RETRIES 5 > + This name makes me think about something which wants to be more generic than it is actually the case... Like some number of retries that libxc does in general, while it's only applicable to a very specific cpupool operation. Just something like CPUPOOL_NUM_REMOVECPU_RETRIES (or, maybe, even without the CPUPOOL_ prefix, as we're already inside cpupool.c) would be more appropriate. I'd also define it closer to xc_cpupool_removecpu() (but that is a lot about personal taste, I guess) and would add a brief comment (basically, a summary of what's in the changelog already), if only to save people having to go through `git blame'. > @@ -141,13 +144,21 @@ int xc_cpupool_removecpu(xc_interface *xch, > uint32_t poolid, > int cpu) > { > + unsigned retries; > + int err; > DECLARE_SYSCTL; > > sysctl.cmd = XEN_SYSCTL_cpupool_op; > sysctl.u.cpupool_op.op = XEN_SYSCTL_CPUPOOL_OP_RMCPU; > sysctl.u.cpupool_op.cpupool_id = poolid; > sysctl.u.cpupool_op.cpu = (cpu < 0) ? XEN_SYSCTL_CPUPOOL_PAR_ANY > : cpu; > - return do_sysctl_save(xch, &sysctl); > + for ( retries = 0; retries < LIBXC_BUSY_RETRIES; retries++ ) { > + err = do_sysctl_save(xch, &sysctl); > + if ( err >= 0 || errno != EBUSY ) > + break; > + sleep(1); > + } > Doing this the other way round (basically, exactly as the same thing is done in do_sysctl_save() already), reads, IMHO, more natural: for (...) { err = do_sysctl_save(..); if ( err < 0 && errno == EBUSY ) sleep(1); else break; } But yeah, this really is nitpicking. :-) 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] 23+ messages in thread
* Re: [PATCH v3 4/6] libxc: do some retries in xc_cpupool_removecpu() for EBUSY case 2016-03-08 13:16 ` Dario Faggioli @ 2016-03-08 16:12 ` Juergen Gross 0 siblings, 0 replies; 23+ messages in thread From: Juergen Gross @ 2016-03-08 16:12 UTC (permalink / raw) To: Dario Faggioli, xen-devel; +Cc: Ian Jackson, Wei Liu, Stefano Stabellini On 08/03/16 14:16, Dario Faggioli wrote: > On Thu, 2016-03-03 at 17:48 +0100, Juergen Gross wrote: >> The hypervisor might return EBUSY when trying to remove a cpu from a >> cpupool when a domain running in this cpupool has pinned a vcpu >> temporarily. Do some retries in this case, perhaps the situation >> cleans up. >> > I now I'm at high risk of being called nitpicker (or, more likely, much > worse names), but I think that: > >> --- a/tools/libxc/xc_cpupool.c >> +++ b/tools/libxc/xc_cpupool.c >> @@ -20,8 +20,11 @@ >> */ >> >> #include <stdarg.h> >> +#include <unistd.h> >> #include "xc_private.h" >> >> +#define LIBXC_BUSY_RETRIES 5 >> + > This name makes me think about something which wants to be more generic > than it is actually the case... Like some number of retries that libxc > does in general, while it's only applicable to a very specific cpupool > operation. > > Just something like CPUPOOL_NUM_REMOVECPU_RETRIES (or, maybe, even > without the CPUPOOL_ prefix, as we're already inside cpupool.c) would > be more appropriate. > > I'd also define it closer to xc_cpupool_removecpu() (but that is a lot > about personal taste, I guess) and would add a brief comment > (basically, a summary of what's in the changelog already), if only to > save people having to go through `git blame'. > >> @@ -141,13 +144,21 @@ int xc_cpupool_removecpu(xc_interface *xch, >> uint32_t poolid, >> int cpu) >> { >> + unsigned retries; >> + int err; >> DECLARE_SYSCTL; >> >> sysctl.cmd = XEN_SYSCTL_cpupool_op; >> sysctl.u.cpupool_op.op = XEN_SYSCTL_CPUPOOL_OP_RMCPU; >> sysctl.u.cpupool_op.cpupool_id = poolid; >> sysctl.u.cpupool_op.cpu = (cpu < 0) ? XEN_SYSCTL_CPUPOOL_PAR_ANY >> : cpu; >> - return do_sysctl_save(xch, &sysctl); >> + for ( retries = 0; retries < LIBXC_BUSY_RETRIES; retries++ ) { >> + err = do_sysctl_save(xch, &sysctl); >> + if ( err >= 0 || errno != EBUSY ) >> + break; >> + sleep(1); >> + } >> > Doing this the other way round (basically, exactly as the same thing is > done in do_sysctl_save() already), reads, IMHO, more natural: > > for (...) { > err = do_sysctl_save(..); > if ( err < 0 && errno == EBUSY ) > sleep(1); > else > break; > } > > But yeah, this really is nitpicking. :-) Nevertheless I can do it. Need to respin anyway. Juergen _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel ^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH v3 5/6] libxl: print message how to recover from xl cpupool-cpu-remove errors 2016-03-03 16:48 [PATCH v3 0/6] add hypercall option to temporarily pin a vcpu Juergen Gross ` (3 preceding siblings ...) 2016-03-03 16:48 ` [PATCH v3 4/6] libxc: do some retries in xc_cpupool_removecpu() for EBUSY case Juergen Gross @ 2016-03-03 16:48 ` Juergen Gross 2016-03-08 13:18 ` Dario Faggioli 2016-03-08 15:58 ` Wei Liu 2016-03-03 16:48 ` [PATCH v3 6/6] libxl: add force option for xl vcpu-pin Juergen Gross 5 siblings, 2 replies; 23+ messages in thread From: Juergen Gross @ 2016-03-03 16:48 UTC (permalink / raw) To: xen-devel; +Cc: Juergen Gross, Wei Liu, Ian Jackson, Stefano Stabellini An error occurring when calling "xl cpupool-cpu-remove" might leave the system in a state where a cpu is neither completely free nor in a cpupool. This can easily be repaired by adding the cpu via "xl cpupool-cpu-add" to the cpupool where it was removed from before. Print a message telling this the user in case of an error. Cc: Ian Jackson <ian.jackson@eu.citrix.com> Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com> Cc: Wei Liu <wei.liu2@citrix.com> Signed-off-by: Juergen Gross <jgross@suse.com> --- tools/libxl/xl_cmdimpl.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c index 990d3c9..411473d 100644 --- a/tools/libxl/xl_cmdimpl.c +++ b/tools/libxl/xl_cmdimpl.c @@ -7716,8 +7716,10 @@ int main_cpupoolcpuremove(int argc, char **argv) goto out; } - if (libxl_cpupool_cpuremove_cpumap(ctx, poolid, &cpumap)) - fprintf(stderr, "some cpus may not have been removed from %s\n", pool); + if (libxl_cpupool_cpuremove_cpumap(ctx, poolid, &cpumap)) { + fprintf(stderr, "Some cpus may have not or only partially been removed from '%s'.\n", pool); + fprintf(stderr, "If a cpu can't be added to another cpupool, add it to '%s' again and retry.\n", pool); + } rc = EXIT_SUCCESS; -- 2.6.2 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel ^ permalink raw reply related [flat|nested] 23+ messages in thread
* Re: [PATCH v3 5/6] libxl: print message how to recover from xl cpupool-cpu-remove errors 2016-03-03 16:48 ` [PATCH v3 5/6] libxl: print message how to recover from xl cpupool-cpu-remove errors Juergen Gross @ 2016-03-08 13:18 ` Dario Faggioli 2016-03-08 15:58 ` Wei Liu 1 sibling, 0 replies; 23+ messages in thread From: Dario Faggioli @ 2016-03-08 13:18 UTC (permalink / raw) To: Juergen Gross, xen-devel; +Cc: Ian Jackson, Wei Liu, Stefano Stabellini [-- Attachment #1.1: Type: text/plain, Size: 938 bytes --] On Thu, 2016-03-03 at 17:48 +0100, Juergen Gross wrote: > An error occurring when calling "xl cpupool-cpu-remove" might leave > the system in a state where a cpu is neither completely free nor in > a cpupool. This can easily be repaired by adding the cpu via > "xl cpupool-cpu-add" to the cpupool where it was removed from before. > Print a message telling this the user in case of an error. > > Cc: Ian Jackson <ian.jackson@eu.citrix.com> > Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com> > Cc: Wei Liu <wei.liu2@citrix.com> > Signed-off-by: Juergen Gross <jgross@suse.com> > Reviewed-by: Dario Faggioli <dario.faggioli@citrix.com> 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] 23+ messages in thread
* Re: [PATCH v3 5/6] libxl: print message how to recover from xl cpupool-cpu-remove errors 2016-03-03 16:48 ` [PATCH v3 5/6] libxl: print message how to recover from xl cpupool-cpu-remove errors Juergen Gross 2016-03-08 13:18 ` Dario Faggioli @ 2016-03-08 15:58 ` Wei Liu 1 sibling, 0 replies; 23+ messages in thread From: Wei Liu @ 2016-03-08 15:58 UTC (permalink / raw) To: Juergen Gross; +Cc: Wei Liu, Stefano Stabellini, Ian Jackson, xen-devel On Thu, Mar 03, 2016 at 05:48:49PM +0100, Juergen Gross wrote: > An error occurring when calling "xl cpupool-cpu-remove" might leave > the system in a state where a cpu is neither completely free nor in > a cpupool. This can easily be repaired by adding the cpu via > "xl cpupool-cpu-add" to the cpupool where it was removed from before. > Print a message telling this the user in case of an error. > > Cc: Ian Jackson <ian.jackson@eu.citrix.com> > Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com> > Cc: Wei Liu <wei.liu2@citrix.com> > Signed-off-by: Juergen Gross <jgross@suse.com> Acked-by: Wei Liu <wei.liu2@citrix.com> > --- > tools/libxl/xl_cmdimpl.c | 6 ++++-- > 1 file changed, 4 insertions(+), 2 deletions(-) > > diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c > index 990d3c9..411473d 100644 > --- a/tools/libxl/xl_cmdimpl.c > +++ b/tools/libxl/xl_cmdimpl.c > @@ -7716,8 +7716,10 @@ int main_cpupoolcpuremove(int argc, char **argv) > goto out; > } > > - if (libxl_cpupool_cpuremove_cpumap(ctx, poolid, &cpumap)) > - fprintf(stderr, "some cpus may not have been removed from %s\n", pool); > + if (libxl_cpupool_cpuremove_cpumap(ctx, poolid, &cpumap)) { > + fprintf(stderr, "Some cpus may have not or only partially been removed from '%s'.\n", pool); > + fprintf(stderr, "If a cpu can't be added to another cpupool, add it to '%s' again and retry.\n", pool); > + } > > rc = EXIT_SUCCESS; > > -- > 2.6.2 > _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel ^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH v3 6/6] libxl: add force option for xl vcpu-pin 2016-03-03 16:48 [PATCH v3 0/6] add hypercall option to temporarily pin a vcpu Juergen Gross ` (4 preceding siblings ...) 2016-03-03 16:48 ` [PATCH v3 5/6] libxl: print message how to recover from xl cpupool-cpu-remove errors Juergen Gross @ 2016-03-03 16:48 ` Juergen Gross 2016-03-08 13:29 ` Dario Faggioli ` (2 more replies) 5 siblings, 3 replies; 23+ messages in thread From: Juergen Gross @ 2016-03-03 16:48 UTC (permalink / raw) To: xen-devel; +Cc: Juergen Gross, Wei Liu, Ian Jackson, Stefano Stabellini In order to be able to undo a vcpu pin override in case of a kernel driver error add a flag "-f" to the "xl vcpu-pin" command forcing the hypervisor to undo the override. Cc: Ian Jackson <ian.jackson@eu.citrix.com> Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com> Cc: Wei Liu <wei.liu2@citrix.com> Signed-off-by: Juergen Gross <jgross@suse.com> --- tools/libxl/libxl.c | 31 +++++++++++++++++++++++++------ tools/libxl/libxl.h | 4 ++++ tools/libxl/xl_cmdimpl.c | 27 +++++++++++++++++++++++---- tools/libxl/xl_cmdtable.c | 3 ++- 4 files changed, 54 insertions(+), 11 deletions(-) diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c index 4cdc169..53f0100 100644 --- a/tools/libxl/libxl.c +++ b/tools/libxl/libxl.c @@ -5301,18 +5301,20 @@ err: return NULL; } -int libxl_set_vcpuaffinity(libxl_ctx *ctx, uint32_t domid, uint32_t vcpuid, - const libxl_bitmap *cpumap_hard, - const libxl_bitmap *cpumap_soft) +static int libxl__set_vcpuaffinity(libxl_ctx *ctx, uint32_t domid, + uint32_t vcpuid, + const libxl_bitmap *cpumap_hard, + const libxl_bitmap *cpumap_soft, + unsigned flags) { GC_INIT(ctx); libxl_bitmap hard, soft; - int rc, flags = 0; + int rc; libxl_bitmap_init(&hard); libxl_bitmap_init(&soft); - if (!cpumap_hard && !cpumap_soft) { + if (!cpumap_hard && !cpumap_soft && !flags) { rc = ERROR_INVAL; goto out; } @@ -5327,7 +5329,7 @@ int libxl_set_vcpuaffinity(libxl_ctx *ctx, uint32_t domid, uint32_t vcpuid, goto out; libxl__bitmap_copy_best_effort(gc, &hard, cpumap_hard); - flags = XEN_VCPUAFFINITY_HARD; + flags |= XEN_VCPUAFFINITY_HARD; } if (cpumap_soft) { rc = libxl_cpu_bitmap_alloc(ctx, &soft, 0); @@ -5378,6 +5380,23 @@ int libxl_set_vcpuaffinity(libxl_ctx *ctx, uint32_t domid, uint32_t vcpuid, return rc; } +int libxl_set_vcpuaffinity(libxl_ctx *ctx, uint32_t domid, uint32_t vcpuid, + const libxl_bitmap *cpumap_hard, + const libxl_bitmap *cpumap_soft) +{ + return libxl__set_vcpuaffinity(ctx, domid, vcpuid, cpumap_hard, + cpumap_soft, 0); +} + +int libxl_set_vcpuaffinity_force(libxl_ctx *ctx, uint32_t domid, + uint32_t vcpuid, + const libxl_bitmap *cpumap_hard, + const libxl_bitmap *cpumap_soft) +{ + return libxl__set_vcpuaffinity(ctx, domid, vcpuid, cpumap_hard, + cpumap_soft, XEN_VCPUAFFINITY_FORCE); +} + int libxl_set_vcpuaffinity_all(libxl_ctx *ctx, uint32_t domid, unsigned int max_vcpus, const libxl_bitmap *cpumap_hard, diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h index f9e3ef5..19ec076 100644 --- a/tools/libxl/libxl.h +++ b/tools/libxl/libxl.h @@ -1715,6 +1715,10 @@ int libxl_get_physinfo(libxl_ctx *ctx, libxl_physinfo *physinfo); int libxl_set_vcpuaffinity(libxl_ctx *ctx, uint32_t domid, uint32_t vcpuid, const libxl_bitmap *cpumap_hard, const libxl_bitmap *cpumap_soft); +int libxl_set_vcpuaffinity_force(libxl_ctx *ctx, uint32_t domid, + uint32_t vcpuid, + const libxl_bitmap *cpumap_hard, + const libxl_bitmap *cpumap_soft); int libxl_set_vcpuaffinity_all(libxl_ctx *ctx, uint32_t domid, unsigned int max_vcpus, const libxl_bitmap *cpumap_hard, diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c index 411473d..120f30b 100644 --- a/tools/libxl/xl_cmdimpl.c +++ b/tools/libxl/xl_cmdimpl.c @@ -5344,6 +5344,10 @@ int main_vcpulist(int argc, char **argv) int main_vcpupin(int argc, char **argv) { + static struct option opts[] = { + {"force", 0, 0, 'f'}, + COMMON_LONG_OPTS + }; libxl_vcpuinfo *vcpuinfo; libxl_bitmap cpumap_hard, cpumap_soft;; libxl_bitmap *soft = &cpumap_soft, *hard = &cpumap_hard; @@ -5355,13 +5359,17 @@ int main_vcpupin(int argc, char **argv) long vcpuid; const char *vcpu, *hard_str, *soft_str; char *endptr; - int opt, nb_cpu, nb_vcpu, rc = EXIT_FAILURE; + int opt, nb_cpu, nb_vcpu, force = 0, rc = EXIT_FAILURE; libxl_bitmap_init(&cpumap_hard); libxl_bitmap_init(&cpumap_soft); - SWITCH_FOREACH_OPT(opt, "", NULL, "vcpu-pin", 3) { - /* No options */ + SWITCH_FOREACH_OPT(opt, "f", opts, "vcpu-pin", 3) { + case 'f': + force = 1; + break; + default: + break; } domid = find_domain(argv[optind]); @@ -5376,6 +5384,10 @@ int main_vcpupin(int argc, char **argv) fprintf(stderr, "Error: Invalid argument %s as VCPU.\n", vcpu); goto out; } + if (force) { + fprintf(stderr, "Error: --force and 'all' as VCPU not allowed.\n"); + goto out; + } vcpuid = -1; } @@ -5437,7 +5449,14 @@ int main_vcpupin(int argc, char **argv) goto out; } - if (vcpuid != -1) { + if (force) { + if (libxl_set_vcpuaffinity_force(ctx, domid, vcpuid, hard, soft)) { + fprintf(stderr, "Could not set affinity for vcpu `%ld'.\n", + vcpuid); + goto out; + } + } + else if (vcpuid != -1) { if (libxl_set_vcpuaffinity(ctx, domid, vcpuid, hard, soft)) { fprintf(stderr, "Could not set affinity for vcpu `%ld'.\n", vcpuid); diff --git a/tools/libxl/xl_cmdtable.c b/tools/libxl/xl_cmdtable.c index fdc1ac6..7cc0401 100644 --- a/tools/libxl/xl_cmdtable.c +++ b/tools/libxl/xl_cmdtable.c @@ -222,7 +222,8 @@ struct cmd_spec cmd_table[] = { { "vcpu-pin", &main_vcpupin, 1, 1, "Set which CPUs a VCPU can use", - "<Domain> <VCPU|all> <Hard affinity|-|all> <Soft affinity|-|all>", + "[option] <Domain> <VCPU|all> <Hard affinity|-|all> <Soft affinity|-|all>", + "-f, --force undo an override pinning done by the kernel", }, { "vcpu-set", &main_vcpuset, 0, 1, -- 2.6.2 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel ^ permalink raw reply related [flat|nested] 23+ messages in thread
* Re: [PATCH v3 6/6] libxl: add force option for xl vcpu-pin 2016-03-03 16:48 ` [PATCH v3 6/6] libxl: add force option for xl vcpu-pin Juergen Gross @ 2016-03-08 13:29 ` Dario Faggioli 2016-03-08 15:58 ` Wei Liu 2016-03-08 17:16 ` Dario Faggioli 2 siblings, 0 replies; 23+ messages in thread From: Dario Faggioli @ 2016-03-08 13:29 UTC (permalink / raw) To: Juergen Gross, xen-devel; +Cc: Ian Jackson, Wei Liu, Stefano Stabellini [-- Attachment #1.1: Type: text/plain, Size: 1712 bytes --] On Thu, 2016-03-03 at 17:48 +0100, Juergen Gross wrote: > In order to be able to undo a vcpu pin override in case of a kernel > driver error add a flag "-f" to the "xl vcpu-pin" command forcing the > hypervisor to undo the override. > > Cc: Ian Jackson <ian.jackson@eu.citrix.com> > Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com> > Cc: Wei Liu <wei.liu2@citrix.com> > Signed-off-by: Juergen Gross <jgross@suse.com> > Reviewed-by: Dario Faggioli <dario.faggioli@citrix.com> With the only comment that, here: > --- a/tools/libxl/xl_cmdimpl.c > +++ b/tools/libxl/xl_cmdimpl.c > @@ -5344,6 +5344,10 @@ int main_vcpulist(int argc, char **argv) > > int main_vcpupin(int argc, char **argv) > { > + static struct option opts[] = { > + {"force", 0, 0, 'f'}, > + COMMON_LONG_OPTS > + }; > libxl_vcpuinfo *vcpuinfo; > libxl_bitmap cpumap_hard, cpumap_soft;; > libxl_bitmap *soft = &cpumap_soft, *hard = &cpumap_hard; > @@ -5355,13 +5359,17 @@ int main_vcpupin(int argc, char **argv) > long vcpuid; > const char *vcpu, *hard_str, *soft_str; > char *endptr; > - int opt, nb_cpu, nb_vcpu, rc = EXIT_FAILURE; > + int opt, nb_cpu, nb_vcpu, force = 0, rc = EXIT_FAILURE; > force can be bool. The Reviewed-by stands both with that changed, and as the patch looks now. 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] 23+ messages in thread
* Re: [PATCH v3 6/6] libxl: add force option for xl vcpu-pin 2016-03-03 16:48 ` [PATCH v3 6/6] libxl: add force option for xl vcpu-pin Juergen Gross 2016-03-08 13:29 ` Dario Faggioli @ 2016-03-08 15:58 ` Wei Liu 2016-03-08 16:11 ` Juergen Gross 2016-03-08 17:16 ` Dario Faggioli 2 siblings, 1 reply; 23+ messages in thread From: Wei Liu @ 2016-03-08 15:58 UTC (permalink / raw) To: Juergen Gross; +Cc: Ian Jackson, Stefano Stabellini, Wei Liu, xen-devel On Thu, Mar 03, 2016 at 05:48:50PM +0100, Juergen Gross wrote: [...] > int libxl_set_vcpuaffinity_all(libxl_ctx *ctx, uint32_t domid, > unsigned int max_vcpus, > const libxl_bitmap *cpumap_hard, > diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h > index f9e3ef5..19ec076 100644 > --- a/tools/libxl/libxl.h > +++ b/tools/libxl/libxl.h > @@ -1715,6 +1715,10 @@ int libxl_get_physinfo(libxl_ctx *ctx, libxl_physinfo *physinfo); > int libxl_set_vcpuaffinity(libxl_ctx *ctx, uint32_t domid, uint32_t vcpuid, > const libxl_bitmap *cpumap_hard, > const libxl_bitmap *cpumap_soft); > +int libxl_set_vcpuaffinity_force(libxl_ctx *ctx, uint32_t domid, > + uint32_t vcpuid, > + const libxl_bitmap *cpumap_hard, > + const libxl_bitmap *cpumap_soft); With the introduction of this new API you need to have a #define in libxl.h Wei. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v3 6/6] libxl: add force option for xl vcpu-pin 2016-03-08 15:58 ` Wei Liu @ 2016-03-08 16:11 ` Juergen Gross 0 siblings, 0 replies; 23+ messages in thread From: Juergen Gross @ 2016-03-08 16:11 UTC (permalink / raw) To: Wei Liu; +Cc: Stefano Stabellini, Ian Jackson, xen-devel On 08/03/16 16:58, Wei Liu wrote: > On Thu, Mar 03, 2016 at 05:48:50PM +0100, Juergen Gross wrote: > [...] >> int libxl_set_vcpuaffinity_all(libxl_ctx *ctx, uint32_t domid, >> unsigned int max_vcpus, >> const libxl_bitmap *cpumap_hard, >> diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h >> index f9e3ef5..19ec076 100644 >> --- a/tools/libxl/libxl.h >> +++ b/tools/libxl/libxl.h >> @@ -1715,6 +1715,10 @@ int libxl_get_physinfo(libxl_ctx *ctx, libxl_physinfo *physinfo); >> int libxl_set_vcpuaffinity(libxl_ctx *ctx, uint32_t domid, uint32_t vcpuid, >> const libxl_bitmap *cpumap_hard, >> const libxl_bitmap *cpumap_soft); >> +int libxl_set_vcpuaffinity_force(libxl_ctx *ctx, uint32_t domid, >> + uint32_t vcpuid, >> + const libxl_bitmap *cpumap_hard, >> + const libxl_bitmap *cpumap_soft); > > With the introduction of this new API you need to have a #define in > libxl.h Okay. Juergen _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v3 6/6] libxl: add force option for xl vcpu-pin 2016-03-03 16:48 ` [PATCH v3 6/6] libxl: add force option for xl vcpu-pin Juergen Gross 2016-03-08 13:29 ` Dario Faggioli 2016-03-08 15:58 ` Wei Liu @ 2016-03-08 17:16 ` Dario Faggioli 2016-03-08 17:19 ` Juergen Gross 2 siblings, 1 reply; 23+ messages in thread From: Dario Faggioli @ 2016-03-08 17:16 UTC (permalink / raw) To: Juergen Gross, xen-devel; +Cc: Ian Jackson, Wei Liu, Stefano Stabellini [-- Attachment #1.1: Type: text/plain, Size: 1128 bytes --] On Thu, 2016-03-03 at 17:48 +0100, Juergen Gross wrote: > In order to be able to undo a vcpu pin override in case of a kernel > driver error add a flag "-f" to the "xl vcpu-pin" command forcing the > hypervisor to undo the override. > > Cc: Ian Jackson <ian.jackson@eu.citrix.com> > Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com> > Cc: Wei Liu <wei.liu2@citrix.com> > Signed-off-by: Juergen Gross <jgross@suse.com> > --- > tools/libxl/libxl.c | 31 +++++++++++++++++++++++++------ > tools/libxl/libxl.h | 4 ++++ > tools/libxl/xl_cmdimpl.c | 27 +++++++++++++++++++++++---- > tools/libxl/xl_cmdtable.c | 3 ++- > Actually, there's something I always forget when reviewing xl stuff, which is that the xl manpage should be modified as well. Sorry for (nearly) missing this! :-/ 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] 23+ messages in thread
* Re: [PATCH v3 6/6] libxl: add force option for xl vcpu-pin 2016-03-08 17:16 ` Dario Faggioli @ 2016-03-08 17:19 ` Juergen Gross 0 siblings, 0 replies; 23+ messages in thread From: Juergen Gross @ 2016-03-08 17:19 UTC (permalink / raw) To: Dario Faggioli, xen-devel; +Cc: Ian Jackson, Wei Liu, Stefano Stabellini On 08/03/16 18:16, Dario Faggioli wrote: > On Thu, 2016-03-03 at 17:48 +0100, Juergen Gross wrote: >> In order to be able to undo a vcpu pin override in case of a kernel >> driver error add a flag "-f" to the "xl vcpu-pin" command forcing the >> hypervisor to undo the override. >> >> Cc: Ian Jackson <ian.jackson@eu.citrix.com> >> Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com> >> Cc: Wei Liu <wei.liu2@citrix.com> >> Signed-off-by: Juergen Gross <jgross@suse.com> >> --- >> tools/libxl/libxl.c | 31 +++++++++++++++++++++++++------ >> tools/libxl/libxl.h | 4 ++++ >> tools/libxl/xl_cmdimpl.c | 27 +++++++++++++++++++++++---- >> tools/libxl/xl_cmdtable.c | 3 ++- >> > Actually, there's something I always forget when reviewing xl stuff, > which is that the xl manpage should be modified as well. Yeah, I already thought of this, too. Will add it. Juergen > > Sorry for (nearly) missing this! :-/ > > Regards, > Dario > _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel ^ permalink raw reply [flat|nested] 23+ messages in thread
end of thread, other threads:[~2016-03-08 17:19 UTC | newest] Thread overview: 23+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2016-03-03 16:48 [PATCH v3 0/6] add hypercall option to temporarily pin a vcpu Juergen Gross 2016-03-03 16:48 ` [PATCH v3 1/6] xen, cpupool: correct error handling when removing cpu from cpupool Juergen Gross 2016-03-04 9:42 ` Jan Beulich [not found] ` <56D9668602000078000D9400@suse.com> 2016-03-04 9:54 ` Juergen Gross 2016-03-04 10:03 ` Jan Beulich 2016-03-08 10:46 ` Dario Faggioli 2016-03-03 16:48 ` [PATCH v3 2/6] xen: add hypercall option to override and restore vcpu affinity Juergen Gross 2016-03-04 9:44 ` Jan Beulich 2016-03-08 11:45 ` Dario Faggioli 2016-03-03 16:48 ` [PATCH v3 3/6] xen: add force flag to xen_domctl_vcpuaffinity for undoing pin override Juergen Gross 2016-03-04 9:45 ` Jan Beulich 2016-03-03 16:48 ` [PATCH v3 4/6] libxc: do some retries in xc_cpupool_removecpu() for EBUSY case Juergen Gross 2016-03-08 13:16 ` Dario Faggioli 2016-03-08 16:12 ` Juergen Gross 2016-03-03 16:48 ` [PATCH v3 5/6] libxl: print message how to recover from xl cpupool-cpu-remove errors Juergen Gross 2016-03-08 13:18 ` Dario Faggioli 2016-03-08 15:58 ` Wei Liu 2016-03-03 16:48 ` [PATCH v3 6/6] libxl: add force option for xl vcpu-pin Juergen Gross 2016-03-08 13:29 ` Dario Faggioli 2016-03-08 15:58 ` Wei Liu 2016-03-08 16:11 ` Juergen Gross 2016-03-08 17:16 ` Dario Faggioli 2016-03-08 17:19 ` Juergen Gross
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).