[PATCH v3 0/6] add hypercall option to temporarily pin a vcpu

[PATCH v3 0/6] add hypercall option to temporarily pin a vcpu

[Juergen Gross]

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

[PATCH v3 1/6] xen, cpupool: correct error handling when removing cpu from cpupool

[Juergen Gross]

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

[PATCH v3 2/6] xen: add hypercall option to override and restore vcpu affinity

[Juergen Gross]

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

[PATCH v3 3/6] xen: add force flag to xen_domctl_vcpuaffinity for undoing pin override

[Juergen Gross]

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

[PATCH v3 4/6] libxc: do some retries in xc_cpupool_removecpu() for EBUSY case

[Juergen Gross]

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

[PATCH v3 5/6] libxl: print message how to recover from xl cpupool-cpu-remove errors

[Juergen Gross]

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

[PATCH v3 6/6] libxl: add force option for xl vcpu-pin

[Juergen Gross]

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

Re: [PATCH v3 1/6] xen, cpupool: correct error handling when removing cpu from cpupool

[Jan Beulich]

>>> 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

Re: [PATCH v3 2/6] xen: add hypercall option to override and restore vcpu affinity

[Jan Beulich]

>>> 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

Re: [PATCH v3 3/6] xen: add force flag to xen_domctl_vcpuaffinity for undoing pin override

[Jan Beulich]

>>> 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

Re: [PATCH v3 1/6] xen, cpupool: correct error handling when removing cpu from cpupool

[Juergen Gross]

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

Re: [PATCH v3 1/6] xen, cpupool: correct error handling when removing cpu from cpupool

[Jan Beulich]

>>> 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

Re: [PATCH v3 1/6] xen, cpupool: correct error handling when removing cpu from cpupool

[Dario Faggioli]

[-- 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

Re: [PATCH v3 2/6] xen: add hypercall option to override and restore vcpu affinity

[Dario Faggioli]

[-- 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

Re: [PATCH v3 4/6] libxc: do some retries in xc_cpupool_removecpu() for EBUSY case

[Dario Faggioli]

[-- 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

Re: [PATCH v3 5/6] libxl: print message how to recover from xl cpupool-cpu-remove errors

[Dario Faggioli]

[-- 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

Re: [PATCH v3 6/6] libxl: add force option for xl vcpu-pin

[Dario Faggioli]

[-- 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

Re: [PATCH v3 6/6] libxl: add force option for xl vcpu-pin

[Wei Liu]

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

Re: [PATCH v3 5/6] libxl: print message how to recover from xl cpupool-cpu-remove errors

[Wei Liu]

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

Re: [PATCH v3 6/6] libxl: add force option for xl vcpu-pin

[Juergen Gross]

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

Re: [PATCH v3 4/6] libxc: do some retries in xc_cpupool_removecpu() for EBUSY case

[Juergen Gross]

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

Re: [PATCH v3 6/6] libxl: add force option for xl vcpu-pin

[Dario Faggioli]

[-- 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

Re: [PATCH v3 6/6] libxl: add force option for xl vcpu-pin

[Juergen Gross]

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