* [PATCH v2 1/3] xen/atomic: Introduce common atomic header and update includes
2014-07-02 13:47 [PATCH v2 0/3] Improvements to allow refcoutning of DOMCTL_{, un}pausedomain Andrew Cooper
@ 2014-07-02 13:47 ` Andrew Cooper
2014-07-02 14:01 ` Jan Beulich
2014-07-02 13:47 ` [PATCH v2 2/3] xen/atomic: Implement atomic_{inc, dec}_bounded() Andrew Cooper
2014-07-02 13:47 ` [PATCH v2 3/3] xen/common: Properly reference count DOMCTL_{, un}pausedomain hypercalls Andrew Cooper
2 siblings, 1 reply; 8+ messages in thread
From: Andrew Cooper @ 2014-07-02 13:47 UTC (permalink / raw)
To: Xen-devel
Cc: Keir Fraser, Ian Campbell, Andrew Cooper, Tim Deegan,
Stefano Stabellini, Jan Beulich
This patch does reorder the #includes in asm-arm/atomic.h to pull in
definitions in the correct order, which would otherwise cause a build failure
in patch #2 because of struct vcpu_guest_core_regs being referenced before
definition.
No functional change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
CC: Keir Fraser <keir@xen.org>
CC: Jan Beulich <JBeulich@suse.com>
CC: Ian Campbell <ian.campbell@citrix.com>
CC: Stefano Stabellini <stefano.stabellini@citrix.com>
CC: Tim Deegan <tim@xen.org>
---
xen/arch/x86/apic.c | 2 +-
xen/arch/x86/cpu/mcheck/barrier.h | 2 +-
xen/arch/x86/cpu/mcheck/mce.h | 2 +-
xen/arch/x86/crash.c | 2 +-
xen/arch/x86/i8259.c | 2 +-
xen/arch/x86/mm/mem_sharing.c | 2 +-
xen/arch/x86/mm/shadow/private.h | 2 +-
xen/arch/x86/traps.c | 2 +-
xen/common/kexec.c | 2 +-
xen/common/rcupdate.c | 2 +-
xen/common/sched_credit.c | 2 +-
xen/common/spinlock.c | 2 +-
xen/common/timer.c | 2 +-
xen/common/trace.c | 2 +-
xen/drivers/passthrough/arm/smmu.c | 2 +-
xen/include/acpi/platform/aclinux.h | 2 +-
xen/include/asm-arm/atomic.h | 2 +-
xen/include/asm-x86/irq.h | 2 +-
xen/include/asm-x86/spinlock.h | 2 +-
xen/include/xen/atomic.h | 6 ++++++
xen/include/xen/gdbstub.h | 2 +-
xen/include/xen/sched.h | 2 +-
xen/xsm/flask/avc.c | 2 +-
23 files changed, 28 insertions(+), 22 deletions(-)
create mode 100644 xen/include/xen/atomic.h
diff --git a/xen/arch/x86/apic.c b/xen/arch/x86/apic.c
index 0e5e302..47f8ef9 100644
--- a/xen/arch/x86/apic.c
+++ b/xen/arch/x86/apic.c
@@ -28,7 +28,7 @@
#include <xen/softirq.h>
#include <asm/mc146818rtc.h>
#include <asm/msr.h>
-#include <asm/atomic.h>
+#include <xen/atomic.h>
#include <asm/mpspec.h>
#include <asm/flushtlb.h>
#include <asm/hardirq.h>
diff --git a/xen/arch/x86/cpu/mcheck/barrier.h b/xen/arch/x86/cpu/mcheck/barrier.h
index 87f7550..ca33f5d 100644
--- a/xen/arch/x86/cpu/mcheck/barrier.h
+++ b/xen/arch/x86/cpu/mcheck/barrier.h
@@ -1,7 +1,7 @@
#ifndef _MCHECK_BARRIER_H
#define _MCHECK_BARRIER_H
-#include <asm/atomic.h>
+#include <xen/atomic.h>
/* MCE handling */
struct mce_softirq_barrier {
diff --git a/xen/arch/x86/cpu/mcheck/mce.h b/xen/arch/x86/cpu/mcheck/mce.h
index e83d431..2891469 100644
--- a/xen/arch/x86/cpu/mcheck/mce.h
+++ b/xen/arch/x86/cpu/mcheck/mce.h
@@ -7,7 +7,7 @@
#include <xen/smp.h>
#include <asm/types.h>
#include <asm/traps.h>
-#include <asm/atomic.h>
+#include <xen/atomic.h>
#include <asm/percpu.h>
#include "x86_mca.h"
diff --git a/xen/arch/x86/crash.c b/xen/arch/x86/crash.c
index c0b83df..974fd0f 100644
--- a/xen/arch/x86/crash.c
+++ b/xen/arch/x86/crash.c
@@ -8,7 +8,7 @@
* - Magnus Damm <magnus@valinux.co.jp>
*/
-#include <asm/atomic.h>
+#include <xen/atomic.h>
#include <asm/elf.h>
#include <asm/percpu.h>
#include <xen/types.h>
diff --git a/xen/arch/x86/i8259.c b/xen/arch/x86/i8259.c
index c2c9005..2a8cf76 100644
--- a/xen/arch/x86/i8259.c
+++ b/xen/arch/x86/i8259.c
@@ -12,7 +12,7 @@
#include <xen/errno.h>
#include <xen/sched.h>
#include <xen/irq.h>
-#include <asm/atomic.h>
+#include <xen/atomic.h>
#include <asm/system.h>
#include <asm/io.h>
#include <asm/desc.h>
diff --git a/xen/arch/x86/mm/mem_sharing.c b/xen/arch/x86/mm/mem_sharing.c
index 7293f31..92a68eb 100644
--- a/xen/arch/x86/mm/mem_sharing.c
+++ b/xen/arch/x86/mm/mem_sharing.c
@@ -31,7 +31,7 @@
#include <asm/string.h>
#include <asm/p2m.h>
#include <asm/mem_event.h>
-#include <asm/atomic.h>
+#include <xen/atomic.h>
#include <xen/rcupdate.h>
#include <asm/event.h>
#include <xsm/xsm.h>
diff --git a/xen/arch/x86/mm/shadow/private.h b/xen/arch/x86/mm/shadow/private.h
index b778fcf..4958500 100644
--- a/xen/arch/x86/mm/shadow/private.h
+++ b/xen/arch/x86/mm/shadow/private.h
@@ -30,7 +30,7 @@
#include <xen/domain_page.h>
#include <asm/x86_emulate.h>
#include <asm/hvm/support.h>
-#include <asm/atomic.h>
+#include <xen/atomic.h>
#include "../mm-locks.h"
diff --git a/xen/arch/x86/traps.c b/xen/arch/x86/traps.c
index 677074b..45f9ed6 100644
--- a/xen/arch/x86/traps.c
+++ b/xen/arch/x86/traps.c
@@ -52,7 +52,7 @@
#include <xen/watchdog.h>
#include <asm/system.h>
#include <asm/io.h>
-#include <asm/atomic.h>
+#include <xen/atomic.h>
#include <xen/bitops.h>
#include <asm/desc.h>
#include <asm/debugreg.h>
diff --git a/xen/common/kexec.c b/xen/common/kexec.c
index 2239ee8..5055461 100644
--- a/xen/common/kexec.c
+++ b/xen/common/kexec.c
@@ -20,7 +20,7 @@
#include <xen/keyhandler.h>
#include <public/kexec.h>
#include <xen/cpumask.h>
-#include <asm/atomic.h>
+#include <xen/atomic.h>
#include <xen/spinlock.h>
#include <xen/version.h>
#include <xen/console.h>
diff --git a/xen/common/rcupdate.c b/xen/common/rcupdate.c
index e9979cd..5d7a31b 100644
--- a/xen/common/rcupdate.c
+++ b/xen/common/rcupdate.c
@@ -39,7 +39,7 @@
#include <xen/smp.h>
#include <xen/rcupdate.h>
#include <xen/sched.h>
-#include <asm/atomic.h>
+#include <xen/atomic.h>
#include <xen/bitops.h>
#include <xen/percpu.h>
#include <xen/softirq.h>
diff --git a/xen/common/sched_credit.c b/xen/common/sched_credit.c
index 8b02b7b..68caa8f 100644
--- a/xen/common/sched_credit.c
+++ b/xen/common/sched_credit.c
@@ -18,7 +18,7 @@
#include <xen/time.h>
#include <xen/sched-if.h>
#include <xen/softirq.h>
-#include <asm/atomic.h>
+#include <xen/atomic.h>
#include <asm/div64.h>
#include <xen/errno.h>
#include <xen/keyhandler.h>
diff --git a/xen/common/spinlock.c b/xen/common/spinlock.c
index 575cc6d..11a63c7 100644
--- a/xen/common/spinlock.c
+++ b/xen/common/spinlock.c
@@ -8,7 +8,7 @@
#include <xen/preempt.h>
#include <public/sysctl.h>
#include <asm/processor.h>
-#include <asm/atomic.h>
+#include <xen/atomic.h>
#ifndef NDEBUG
diff --git a/xen/common/timer.c b/xen/common/timer.c
index f36aebc..96573ac 100644
--- a/xen/common/timer.c
+++ b/xen/common/timer.c
@@ -23,7 +23,7 @@
#include <xen/symbols.h>
#include <asm/system.h>
#include <asm/desc.h>
-#include <asm/atomic.h>
+#include <xen/atomic.h>
/* We program the time hardware this far behind the closest deadline. */
static unsigned int timer_slop __read_mostly = 50000; /* 50 us */
diff --git a/xen/common/trace.c b/xen/common/trace.c
index f651cf3..0a7b2c3 100644
--- a/xen/common/trace.c
+++ b/xen/common/trace.c
@@ -31,7 +31,7 @@
#include <xen/percpu.h>
#include <xen/pfn.h>
#include <xen/cpu.h>
-#include <asm/atomic.h>
+#include <xen/atomic.h>
#include <public/sysctl.h>
#ifdef CONFIG_COMPAT
diff --git a/xen/drivers/passthrough/arm/smmu.c b/xen/drivers/passthrough/arm/smmu.c
index f4eb2a2..5189d46 100644
--- a/xen/drivers/passthrough/arm/smmu.c
+++ b/xen/drivers/passthrough/arm/smmu.c
@@ -43,7 +43,7 @@
#include <xen/vmap.h>
#include <xen/rbtree.h>
#include <xen/sched.h>
-#include <asm/atomic.h>
+#include <xen/atomic.h>
#include <asm/device.h>
#include <asm/io.h>
#include <asm/platform.h>
diff --git a/xen/include/acpi/platform/aclinux.h b/xen/include/acpi/platform/aclinux.h
index 239ced2..4f304c3 100644
--- a/xen/include/acpi/platform/aclinux.h
+++ b/xen/include/acpi/platform/aclinux.h
@@ -54,7 +54,7 @@
#include <xen/ctype.h>
#include <xen/spinlock.h>
#include <asm/system.h>
-#include <asm/atomic.h>
+#include <xen/atomic.h>
#include <asm/div64.h>
#include <asm/acpi.h>
#include <asm/current.h>
diff --git a/xen/include/asm-arm/atomic.h b/xen/include/asm-arm/atomic.h
index 7d15fb0..ee29d4e 100644
--- a/xen/include/asm-arm/atomic.h
+++ b/xen/include/asm-arm/atomic.h
@@ -2,8 +2,8 @@
#define __ARCH_ARM_ATOMIC__
#include <xen/config.h>
-#include <xen/prefetch.h>
#include <asm/system.h>
+#include <xen/prefetch.h>
#define build_atomic_read(name, size, width, type, reg)\
static inline type name(const volatile type *addr) \
diff --git a/xen/include/asm-x86/irq.h b/xen/include/asm-x86/irq.h
index 9066d38..9906035 100644
--- a/xen/include/asm-x86/irq.h
+++ b/xen/include/asm-x86/irq.h
@@ -4,7 +4,7 @@
/* (C) 1992, 1993 Linus Torvalds, (C) 1997 Ingo Molnar */
#include <xen/config.h>
-#include <asm/atomic.h>
+#include <xen/atomic.h>
#include <xen/cpumask.h>
#include <xen/smp.h>
#include <xen/hvm/irq.h>
diff --git a/xen/include/asm-x86/spinlock.h b/xen/include/asm-x86/spinlock.h
index 6bc044c..a315e88 100644
--- a/xen/include/asm-x86/spinlock.h
+++ b/xen/include/asm-x86/spinlock.h
@@ -3,7 +3,7 @@
#include <xen/config.h>
#include <xen/lib.h>
-#include <asm/atomic.h>
+#include <xen/atomic.h>
typedef struct {
volatile s16 lock;
diff --git a/xen/include/xen/atomic.h b/xen/include/xen/atomic.h
new file mode 100644
index 0000000..141f9e9
--- /dev/null
+++ b/xen/include/xen/atomic.h
@@ -0,0 +1,6 @@
+#ifndef __XEN_ATOMIC_H__
+#define __XEN_ATOMIC_H__
+
+#include <asm/atomic.h>
+
+#endif /* __XEN_ATOMIC_H__ */
diff --git a/xen/include/xen/gdbstub.h b/xen/include/xen/gdbstub.h
index 67d7410..6b11425 100644
--- a/xen/include/xen/gdbstub.h
+++ b/xen/include/xen/gdbstub.h
@@ -21,7 +21,7 @@
#ifndef __XEN_GDBSTUB_H__
#define __XEN_GDBSTUB_H__
-#include <asm/atomic.h>
+#include <xen/atomic.h>
#include <asm/page.h>
#ifdef CRASH_DEBUG
diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h
index f920e1a..a953469 100644
--- a/xen/include/xen/sched.h
+++ b/xen/include/xen/sched.h
@@ -17,7 +17,7 @@
#include <xen/mm.h>
#include <xen/smp.h>
#include <xen/perfc.h>
-#include <asm/atomic.h>
+#include <xen/atomic.h>
#include <xen/wait.h>
#include <public/xen.h>
#include <public/domctl.h>
diff --git a/xen/xsm/flask/avc.c b/xen/xsm/flask/avc.c
index fc6580e..234aaba 100644
--- a/xen/xsm/flask/avc.c
+++ b/xen/xsm/flask/avc.c
@@ -26,7 +26,7 @@
#include <xen/sched.h>
#include <xen/init.h>
#include <xen/rcupdate.h>
-#include <asm/atomic.h>
+#include <xen/atomic.h>
#include <asm/current.h>
#include <public/xsm/flask_op.h>
--
1.7.10.4
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [PATCH v2 1/3] xen/atomic: Introduce common atomic header and update includes
2014-07-02 13:47 ` [PATCH v2 1/3] xen/atomic: Introduce common atomic header and update includes Andrew Cooper
@ 2014-07-02 14:01 ` Jan Beulich
2014-07-02 14:07 ` Andrew Cooper
0 siblings, 1 reply; 8+ messages in thread
From: Jan Beulich @ 2014-07-02 14:01 UTC (permalink / raw)
To: Andrew Cooper
Cc: Keir Fraser, Tim Deegan, Stefano Stabellini, Ian Campbell,
Xen-devel
>>> On 02.07.14 at 15:47, <andrew.cooper3@citrix.com> wrote:
> --- a/xen/arch/x86/apic.c
> +++ b/xen/arch/x86/apic.c
> @@ -28,7 +28,7 @@
> #include <xen/softirq.h>
> #include <asm/mc146818rtc.h>
> #include <asm/msr.h>
> -#include <asm/atomic.h>
> +#include <xen/atomic.h>
I appreciate you having taken the time to replace all these, but - do
we really need to do this replacement at this point? Especially arch
code should be fine using the asm/ header instad of the xen/ one.
Jan
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2 1/3] xen/atomic: Introduce common atomic header and update includes
2014-07-02 14:01 ` Jan Beulich
@ 2014-07-02 14:07 ` Andrew Cooper
0 siblings, 0 replies; 8+ messages in thread
From: Andrew Cooper @ 2014-07-02 14:07 UTC (permalink / raw)
To: Jan Beulich
Cc: Keir Fraser, Tim Deegan, Stefano Stabellini, Ian Campbell,
Xen-devel
On 02/07/14 15:01, Jan Beulich wrote:
>>>> On 02.07.14 at 15:47, <andrew.cooper3@citrix.com> wrote:
>> --- a/xen/arch/x86/apic.c
>> +++ b/xen/arch/x86/apic.c
>> @@ -28,7 +28,7 @@
>> #include <xen/softirq.h>
>> #include <asm/mc146818rtc.h>
>> #include <asm/msr.h>
>> -#include <asm/atomic.h>
>> +#include <xen/atomic.h>
> I appreciate you having taken the time to replace all these, but - do
> we really need to do this replacement at this point? Especially arch
> code should be fine using the asm/ header instad of the xen/ one.
>
> Jan
>
Not strictly, but I figured it was cleaner to update everything at once
than to have a mix of common and arch specific includes.
(Also it didn't take long at all. `git grep ... | xargs sed` is a
remakably quick refactoring tool, and gcc will quickly tell you if it
went wrong)
~Andrew
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v2 2/3] xen/atomic: Implement atomic_{inc, dec}_bounded()
2014-07-02 13:47 [PATCH v2 0/3] Improvements to allow refcoutning of DOMCTL_{, un}pausedomain Andrew Cooper
2014-07-02 13:47 ` [PATCH v2 1/3] xen/atomic: Introduce common atomic header and update includes Andrew Cooper
@ 2014-07-02 13:47 ` Andrew Cooper
2014-07-02 14:05 ` Jan Beulich
2014-07-02 13:47 ` [PATCH v2 3/3] xen/common: Properly reference count DOMCTL_{, un}pausedomain hypercalls Andrew Cooper
2 siblings, 1 reply; 8+ messages in thread
From: Andrew Cooper @ 2014-07-02 13:47 UTC (permalink / raw)
To: Xen-devel; +Cc: Andrew Cooper, Keir Fraser, Jan Beulich
These will increment/decremented an atomic_t, while ensuring that the atomic_t
doesn't hit a specific bound.
This involves introducing atomic_cmpxchg() for x86, which previously only
existed on ARM.
Suggested-by: Don Slutz <dslutz@verizon.com>
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
CC: Keir Fraser <keir@xen.org>
CC: Jan Beulich <JBeulich@suse.com>
---
xen/common/Makefile | 1 +
xen/common/atomic.c | 35 +++++++++++++++++++++++++++++++++++
xen/include/asm-x86/atomic.h | 5 +++++
xen/include/xen/atomic.h | 20 ++++++++++++++++++++
4 files changed, 61 insertions(+)
create mode 100644 xen/common/atomic.c
diff --git a/xen/common/Makefile b/xen/common/Makefile
index 3683ae3..7c1ee1d 100644
--- a/xen/common/Makefile
+++ b/xen/common/Makefile
@@ -1,3 +1,4 @@
+obj-y += atomic.o
obj-y += bitmap.o
obj-y += core_parking.o
obj-y += cpu.o
diff --git a/xen/common/atomic.c b/xen/common/atomic.c
new file mode 100644
index 0000000..64408e3
--- /dev/null
+++ b/xen/common/atomic.c
@@ -0,0 +1,35 @@
+#include <xen/atomic.h>
+
+bool_t atomic_inc_bounded(atomic_t *v, int bound)
+{
+ int old, new, prev = atomic_read(v);
+
+ do
+ {
+ old = prev;
+ new = old + 1;
+ if ( new >= bound )
+ return 0;
+
+ prev = atomic_cmpxchg(v, old, new);
+ } while ( prev != old );
+
+ return 1;
+}
+
+bool_t atomic_dec_bounded(atomic_t *v, int bound)
+{
+ int old, new, prev = atomic_read(v);
+
+ do
+ {
+ old = prev;
+ new = old - 1;
+ if ( new <= bound )
+ return 0;
+
+ prev = atomic_cmpxchg(v, old, new);
+ } while ( prev != old );
+
+ return 1;
+}
diff --git a/xen/include/asm-x86/atomic.h b/xen/include/asm-x86/atomic.h
index 8972463..43a250b 100644
--- a/xen/include/asm-x86/atomic.h
+++ b/xen/include/asm-x86/atomic.h
@@ -255,4 +255,9 @@ static inline atomic_t atomic_compareandswap(
return rc;
}
+static inline int atomic_cmpxchg(atomic_t *ptr, int old, int new)
+{
+ return __cmpxchg(&ptr->counter, old, new, sizeof(ptr->counter));
+}
+
#endif /* __ARCH_X86_ATOMIC__ */
diff --git a/xen/include/xen/atomic.h b/xen/include/xen/atomic.h
index 141f9e9..d1a6b57 100644
--- a/xen/include/xen/atomic.h
+++ b/xen/include/xen/atomic.h
@@ -3,4 +3,24 @@
#include <asm/atomic.h>
+/**
+ * atomic_inc_bounded - increment @v if it wouldn't exceed @bound
+ * @v: pointer of type atomic_t
+ * @bound: boundary to compare against
+ *
+ * returns true if @v was incremented, or false if incrementing @v would hit
+ * @bound.
+ */
+bool_t atomic_inc_bounded(atomic_t *v, int bound);
+
+/**
+ * atomic_dec_bounded - decrement @v if it wouldn't exceed @bound
+ * @v: pointer of type atomic_t
+ * @bound: boundary to compare against
+ *
+ * returns true if @v was decremented, or false if decrementing @v would hit
+ * @bound.
+ */
+bool_t atomic_dec_bounded(atomic_t *v, int bound);
+
#endif /* __XEN_ATOMIC_H__ */
--
1.7.10.4
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [PATCH v2 2/3] xen/atomic: Implement atomic_{inc, dec}_bounded()
2014-07-02 13:47 ` [PATCH v2 2/3] xen/atomic: Implement atomic_{inc, dec}_bounded() Andrew Cooper
@ 2014-07-02 14:05 ` Jan Beulich
2014-07-02 14:17 ` Andrew Cooper
0 siblings, 1 reply; 8+ messages in thread
From: Jan Beulich @ 2014-07-02 14:05 UTC (permalink / raw)
To: Andrew Cooper; +Cc: Keir Fraser, Xen-devel
>>> On 02.07.14 at 15:47, <andrew.cooper3@citrix.com> wrote:
> These will increment/decremented an atomic_t, while ensuring that the
> atomic_t
> doesn't hit a specific bound.
>
> This involves introducing atomic_cmpxchg() for x86, which previously only
> existed on ARM.
>
> Suggested-by: Don Slutz <dslutz@verizon.com>
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> CC: Keir Fraser <keir@xen.org>
> CC: Jan Beulich <JBeulich@suse.com>
You should be getting used to Cc all "REST" maintainers on respective
changes.
> --- /dev/null
> +++ b/xen/common/atomic.c
> @@ -0,0 +1,35 @@
> +#include <xen/atomic.h>
> +
> +bool_t atomic_inc_bounded(atomic_t *v, int bound)
> +{
> + int old, new, prev = atomic_read(v);
> +
> + do
> + {
> + old = prev;
> + new = old + 1;
> + if ( new >= bound )
> + return 0;
> +
> + prev = atomic_cmpxchg(v, old, new);
> + } while ( prev != old );
> +
> + return 1;
> +}
> +
> +bool_t atomic_dec_bounded(atomic_t *v, int bound)
> +{
> + int old, new, prev = atomic_read(v);
> +
> + do
> + {
> + old = prev;
> + new = old - 1;
> + if ( new <= bound )
> + return 0;
> +
> + prev = atomic_cmpxchg(v, old, new);
> + } while ( prev != old );
> +
> + return 1;
> +}
Same question here: Do we really need these? There are various uses
of cmpxchg() in common code already, and in patch 3 you don't really
need the cmpxchg to happen on an atomic_t, so plain cmpxchg() with
the little bit of extra logic open coded would seem fine to me.
Jan
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH v2 2/3] xen/atomic: Implement atomic_{inc, dec}_bounded()
2014-07-02 14:05 ` Jan Beulich
@ 2014-07-02 14:17 ` Andrew Cooper
0 siblings, 0 replies; 8+ messages in thread
From: Andrew Cooper @ 2014-07-02 14:17 UTC (permalink / raw)
To: Jan Beulich; +Cc: Keir Fraser, Xen-devel
On 02/07/14 15:05, Jan Beulich wrote:
>>>> On 02.07.14 at 15:47, <andrew.cooper3@citrix.com> wrote:
>> These will increment/decremented an atomic_t, while ensuring that the
>> atomic_t
>> doesn't hit a specific bound.
>>
>> This involves introducing atomic_cmpxchg() for x86, which previously only
>> existed on ARM.
>>
>> Suggested-by: Don Slutz <dslutz@verizon.com>
>> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
>> CC: Keir Fraser <keir@xen.org>
>> CC: Jan Beulich <JBeulich@suse.com>
> You should be getting used to Cc all "REST" maintainers on respective
> changes.
Oops yes - I will see about updating my auto-cc list hooks.
>
>> --- /dev/null
>> +++ b/xen/common/atomic.c
>> @@ -0,0 +1,35 @@
>> +#include <xen/atomic.h>
>> +
>> +bool_t atomic_inc_bounded(atomic_t *v, int bound)
>> +{
>> + int old, new, prev = atomic_read(v);
>> +
>> + do
>> + {
>> + old = prev;
>> + new = old + 1;
>> + if ( new >= bound )
>> + return 0;
>> +
>> + prev = atomic_cmpxchg(v, old, new);
>> + } while ( prev != old );
>> +
>> + return 1;
>> +}
>> +
>> +bool_t atomic_dec_bounded(atomic_t *v, int bound)
>> +{
>> + int old, new, prev = atomic_read(v);
>> +
>> + do
>> + {
>> + old = prev;
>> + new = old - 1;
>> + if ( new <= bound )
>> + return 0;
>> +
>> + prev = atomic_cmpxchg(v, old, new);
>> + } while ( prev != old );
>> +
>> + return 1;
>> +}
> Same question here: Do we really need these? There are various uses
> of cmpxchg() in common code already, and in patch 3 you don't really
> need the cmpxchg to happen on an atomic_t, so plain cmpxchg() with
> the little bit of extra logic open coded would seem fine to me.
>
> Jan
>
These as in atomic_{inc,dec}_bounded() ?
They logically fit alongside atomic_{inc,dec}_and_test(), and I came to
the conclusion that they would likely be useful elsewhere.
I realise that these are not fantastic reasons alone...
~Andrew
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v2 3/3] xen/common: Properly reference count DOMCTL_{, un}pausedomain hypercalls
2014-07-02 13:47 [PATCH v2 0/3] Improvements to allow refcoutning of DOMCTL_{, un}pausedomain Andrew Cooper
2014-07-02 13:47 ` [PATCH v2 1/3] xen/atomic: Introduce common atomic header and update includes Andrew Cooper
2014-07-02 13:47 ` [PATCH v2 2/3] xen/atomic: Implement atomic_{inc, dec}_bounded() Andrew Cooper
@ 2014-07-02 13:47 ` Andrew Cooper
2 siblings, 0 replies; 8+ messages in thread
From: Andrew Cooper @ 2014-07-02 13:47 UTC (permalink / raw)
To: Xen-devel
Cc: Keir Fraser, Ian Campbell, Andrew Cooper, Tim Deegan,
Stefano Stabellini, Jan Beulich
For safety reasons, c/s 6ae2df93c27 "mem_access: Add helper API to setup
ring and enable mem_access" has to pause the domain while it performs a set of
operations.
However without properly reference counted hypercalls, xc_mem_event_enable()
now unconditionally unpauses a previously paused domain.
To prevent toolstack software running wild, there is an arbitrary limit of 255
on the toolstack pause count. This is high enough for several components of
the toolstack to safely use, but prevents over/underflow of d->pause_count.
The previous domain_{,un}pause_by_systemcontroller() functions are updated to
return an error code. domain_pause_by_systemcontroller() is modified to have
a common stub and take a pause_fn pointer, allowing for both sync and nosync
domain pauses. domain_pause_for_debugger() has a hand-rolled nosync pause
replaced with the new domain_pause_by_systemcontroller_nosync(), and has its
variables shuffled slightly to avoid rereading current multiple times.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
CC: Keir Fraser <keir@xen.org>
CC: Jan Beulich <JBeulich@suse.com>
CC: Ian Campbell <ian.campbell@citrix.com>
CC: Stefano Stabellini <stefano.stabellini@citrix.com>
CC: Tim Deegan <tim@xen.org>
---
This has been functionally tested on x86, and compile tested on both flavours
of ARM.
v2:
* Use atomic_{inc,dec}_bounded() instead of having a spinlock.
---
xen/arch/x86/domctl.c | 7 ++++---
xen/common/domain.c | 41 ++++++++++++++++++++++++-----------------
xen/common/domctl.c | 9 ++++-----
xen/include/xen/sched.h | 15 ++++++++++++---
4 files changed, 44 insertions(+), 28 deletions(-)
diff --git a/xen/arch/x86/domctl.c b/xen/arch/x86/domctl.c
index a4effc3..efa8746 100644
--- a/xen/arch/x86/domctl.c
+++ b/xen/arch/x86/domctl.c
@@ -1027,7 +1027,7 @@ long arch_do_domctl(
struct vcpu *v;
ret = -EBUSY;
- if ( !d->is_paused_by_controller )
+ if ( atomic_read(&d->controller_pause_count) > 0 )
break;
ret = -EINVAL;
if ( domctl->u.gdbsx_pauseunp_vcpu.vcpu >= MAX_VIRT_CPUS ||
@@ -1043,7 +1043,7 @@ long arch_do_domctl(
struct vcpu *v;
ret = -EBUSY;
- if ( !d->is_paused_by_controller )
+ if ( atomic_read(&d->controller_pause_count) > 0 )
break;
ret = -EINVAL;
if ( domctl->u.gdbsx_pauseunp_vcpu.vcpu >= MAX_VIRT_CPUS ||
@@ -1061,7 +1061,8 @@ long arch_do_domctl(
struct vcpu *v;
domctl->u.gdbsx_domstatus.vcpu_id = -1;
- domctl->u.gdbsx_domstatus.paused = d->is_paused_by_controller;
+ domctl->u.gdbsx_domstatus.paused =
+ atomic_read(&d->controller_pause_count) > 0;
if ( domctl->u.gdbsx_domstatus.paused )
{
for_each_vcpu ( d, v )
diff --git a/xen/common/domain.c b/xen/common/domain.c
index c3a576e..f81e989 100644
--- a/xen/common/domain.c
+++ b/xen/common/domain.c
@@ -317,7 +317,7 @@ struct domain *domain_create(
if ( (err = xsm_domain_create(XSM_HOOK, d, ssidref)) != 0 )
goto fail;
- d->is_paused_by_controller = 1;
+ _atomic_set(&d->controller_pause_count, 1);
atomic_inc(&d->pause_count);
if ( !is_hardware_domain(d) )
@@ -755,18 +755,13 @@ void vcpu_end_shutdown_deferral(struct vcpu *v)
#ifdef HAS_GDBSX
void domain_pause_for_debugger(void)
{
- struct domain *d = current->domain;
- struct vcpu *v;
+ struct vcpu *v = current;
+ struct domain *d = v->domain;
- atomic_inc(&d->pause_count);
- if ( test_and_set_bool(d->is_paused_by_controller) )
- domain_unpause(d); /* race-free atomic_dec(&d->pause_count) */
-
- for_each_vcpu ( d, v )
- vcpu_sleep_nosync(v);
+ domain_pause_by_systemcontroller_nosync(d);
/* if gdbsx active, we just need to pause the domain */
- if (current->arch.gdbsx_vcpu_event == 0)
+ if (v->arch.gdbsx_vcpu_event == 0)
send_global_virq(VIRQ_DEBUGGER);
}
#endif
@@ -914,17 +909,29 @@ void domain_unpause(struct domain *d)
vcpu_wake(v);
}
-void domain_pause_by_systemcontroller(struct domain *d)
+int __domain_pause_by_systemcontroller(struct domain *d,
+ void (*pause_fn)(struct domain *d))
{
- domain_pause(d);
- if ( test_and_set_bool(d->is_paused_by_controller) )
- domain_unpause(d);
+ /*
+ * Limit the toolstack pause count to an arbitrary 255 to prevent the
+ * toolstack overflowing d->pause_count with many repeated hypercalls.
+ */
+ if ( !atomic_inc_bounded(&d->controller_pause_count, 256) )
+ return -EUSERS;
+
+ pause_fn(d);
+
+ return 0;
}
-void domain_unpause_by_systemcontroller(struct domain *d)
+int domain_unpause_by_systemcontroller(struct domain *d)
{
- if ( test_and_clear_bool(d->is_paused_by_controller) )
- domain_unpause(d);
+ if ( !atomic_dec_bounded(&d->controller_pause_count, -1) )
+ return -EINVAL;
+
+ domain_unpause(d);
+
+ return 0;
}
int vcpu_reset(struct vcpu *v)
diff --git a/xen/common/domctl.c b/xen/common/domctl.c
index 000993f..7af1605 100644
--- a/xen/common/domctl.c
+++ b/xen/common/domctl.c
@@ -181,7 +181,8 @@ void getdomaininfo(struct domain *d, struct xen_domctl_getdomaininfo *info)
info->flags = (info->nr_online_vcpus ? flags : 0) |
((d->is_dying == DOMDYING_dead) ? XEN_DOMINF_dying : 0) |
(d->is_shut_down ? XEN_DOMINF_shutdown : 0) |
- (d->is_paused_by_controller ? XEN_DOMINF_paused : 0) |
+ (atomic_read(&d->controller_pause_count) > 0
+ ? XEN_DOMINF_paused : 0) |
(d->debugger_attached ? XEN_DOMINF_debugged : 0) |
d->shutdown_code << XEN_DOMINF_shutdownshift;
@@ -398,16 +399,14 @@ long do_domctl(XEN_GUEST_HANDLE_PARAM(xen_domctl_t) u_domctl)
ret = -EINVAL;
if ( d != current->domain )
{
- domain_pause_by_systemcontroller(d);
- ret = 0;
+ ret = domain_pause_by_systemcontroller(d);
}
}
break;
case XEN_DOMCTL_unpausedomain:
{
- domain_unpause_by_systemcontroller(d);
- ret = 0;
+ ret = domain_unpause_by_systemcontroller(d);
}
break;
diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h
index a953469..41179a0 100644
--- a/xen/include/xen/sched.h
+++ b/xen/include/xen/sched.h
@@ -366,7 +366,7 @@ struct domain
/* Is this guest dying (i.e., a zombie)? */
enum { DOMDYING_alive, DOMDYING_dying, DOMDYING_dead } is_dying;
/* Domain is paused by controller software? */
- bool_t is_paused_by_controller;
+ atomic_t controller_pause_count;
/* Domain's VCPUs are pinned 1:1 to physical CPUs? */
bool_t is_pinned;
@@ -769,8 +769,17 @@ void domain_pause(struct domain *d);
void domain_pause_nosync(struct domain *d);
void vcpu_unpause(struct vcpu *v);
void domain_unpause(struct domain *d);
-void domain_pause_by_systemcontroller(struct domain *d);
-void domain_unpause_by_systemcontroller(struct domain *d);
+int domain_unpause_by_systemcontroller(struct domain *d);
+int __domain_pause_by_systemcontroller(struct domain *d,
+ void (*pause_fn)(struct domain *d));
+static inline int domain_pause_by_systemcontroller(struct domain *d)
+{
+ return __domain_pause_by_systemcontroller(d, domain_pause);
+}
+static inline int domain_pause_by_systemcontroller_nosync(struct domain *d)
+{
+ return __domain_pause_by_systemcontroller(d, domain_pause_nosync);
+}
void cpu_init(void);
struct scheduler;
--
1.7.10.4
^ permalink raw reply related [flat|nested] 8+ messages in thread