[Xen-devel] [PATCH 4/8] xen: add locks with timeouts for keyhandlers

[Juergen Gross <jgross@xxxxxxxx>]

Most keyhandlers are used to dump hypervisor data to the console and
they are used mostly for debugging purposes. In those cases it might
happen that some data structures are locked and thus are blocking the
handler to access the data.

In order to be able to still get some information don't use plain
locking functions in the keyhandlers, but a variant of trylocks with
a timeout value. This allows to wait for some time and to give up in
case the lock was not obtained.

Add the main infrastructure for this feature including a new runtime
parameter allowing to specify the timeout value in milliseconds.

Use the new locking scheme in the handlers defined in keyhandler.c.

Signed-off-by: Juergen Gross <jgross@xxxxxxxx>
---
 docs/misc/xen-command-line.pandoc |  9 +++++++++
 xen/arch/x86/domain.c             |  9 +++++++--
 xen/common/keyhandler.c           | 29 ++++++++++++++++++++++++++++-
 xen/common/rangeset.c             |  7 +++++--
 xen/include/xen/keyhandler.h      | 26 ++++++++++++++++++++++++++
 5 files changed, 75 insertions(+), 5 deletions(-)

diff --git a/docs/misc/xen-command-line.pandoc 
b/docs/misc/xen-command-line.pandoc
index 5051583a5d..ee3d031771 100644
--- a/docs/misc/xen-command-line.pandoc
+++ b/docs/misc/xen-command-line.pandoc
@@ -1384,6 +1384,15 @@ Force the use of `[<seg>:]<bus>:<device>.<func>` as 
device ID of IO-APIC
 `<ioapic>` instead of the one specified by the IVHD sub-tables of the IVRS
 ACPI table.
 
+### keyhandler-lock-timeout
+> `= <integer>`
+
+> Default: `1`
+
+> Can be modified at runtime
+
+Specify the lock timeout of keyhandlers in milliseconds.
+
 ### lapic (x86)
 > `= <boolean>`
 
diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c
index f53ae5ff86..1d09911dc0 100644
--- a/xen/arch/x86/domain.c
+++ b/xen/arch/x86/domain.c
@@ -22,6 +22,7 @@
 #include <xen/grant_table.h>
 #include <xen/iocap.h>
 #include <xen/kernel.h>
+#include <xen/keyhandler.h>
 #include <xen/hypercall.h>
 #include <xen/multicall.h>
 #include <xen/irq.h>
@@ -222,7 +223,8 @@ void dump_pageframe_info(struct domain *d)
     {
         printk("    DomPage list too long to display\n");
     }
-    else
+    else if ( keyhandler_spin_lock(&d->page_alloc_lock,
+                                   "could not read page_list") )
     {
         unsigned long total[MASK_EXTR(PGT_type_mask, PGT_type_mask) + 1] = {};
 
@@ -251,7 +253,10 @@ void dump_pageframe_info(struct domain *d)
     if ( is_hvm_domain(d) )
         p2m_pod_dump_data(d);
 
-    spin_lock(&d->page_alloc_lock);
+    if ( !keyhandler_spin_lock(&d->page_alloc_lock,
+                               "could not read page_list") )
+        return;
+
     page_list_for_each ( page, &d->xenpage_list )
     {
         printk("    XenPage %p: caf=%08lx, taf=%" PRtype_info "\n",
diff --git a/xen/common/keyhandler.c b/xen/common/keyhandler.c
index f50490d0f3..c393d83b70 100644
--- a/xen/common/keyhandler.c
+++ b/xen/common/keyhandler.c
@@ -14,8 +14,10 @@
 #include <xen/rangeset.h>
 #include <xen/compat.h>
 #include <xen/ctype.h>
+#include <xen/param.h>
 #include <xen/perfc.h>
 #include <xen/mm.h>
+#include <xen/time.h>
 #include <xen/watchdog.h>
 #include <xen/init.h>
 #include <asm/debugger.h>
@@ -71,6 +73,30 @@ static struct keyhandler {
 #undef KEYHANDLER
 };
 
+static unsigned int lock_timeout = 1;
+integer_runtime_param("keyhandler-lock-timeout", lock_timeout);
+
+s_time_t keyhandler_lock_timeout(void)
+{
+    return NOW() + MILLISECS(lock_timeout);
+}
+
+bool keyhandler_spin_lock(spinlock_t *lock, const char *msg)
+{
+    keyhandler_lock_body(bool, spin_trylock(lock), "%s\n", msg);
+}
+
+bool keyhandler_spin_lock_irqsave(spinlock_t *lock, unsigned long *flags,
+                                  const char *msg)
+{
+    keyhandler_lock_body(bool, spin_trylock_irqsave(lock, *flags), "%s\n", 
msg);
+}
+
+bool keyhandler_read_lock(rwlock_t *lock, const char *msg)
+{
+    keyhandler_lock_body(bool, read_trylock(lock), "%s\n", msg);
+}
+
 static void keypress_action(void *unused)
 {
     handle_keypress(keypress_key, NULL);
@@ -378,7 +404,8 @@ static void read_clocks(unsigned char key)
     static u32 count = 0;
     static DEFINE_SPINLOCK(lock);
 
-    spin_lock(&lock);
+    if ( !keyhandler_spin_lock(&lock, "could not read clock stats") )
+        return;
 
     smp_call_function(read_clocks_slave, NULL, 0);
 
diff --git a/xen/common/rangeset.c b/xen/common/rangeset.c
index 4ebba30ba3..97104abb45 100644
--- a/xen/common/rangeset.c
+++ b/xen/common/rangeset.c
@@ -9,6 +9,7 @@
 
 #include <xen/sched.h>
 #include <xen/errno.h>
+#include <xen/keyhandler.h>
 #include <xen/rangeset.h>
 #include <xsm/xsm.h>
 
@@ -546,7 +547,8 @@ static void rangeset_printk(struct rangeset *r)
     int nr_printed = 0;
     struct range *x;
 
-    read_lock(&r->lock);
+    if ( !keyhandler_read_lock(&r->lock, "could not read rangeset") )
+        return;
 
     printk("%-10s {", r->name);
 
@@ -575,7 +577,8 @@ void rangeset_domain_printk(
 
     printk("Rangesets belonging to domain %u:\n", d->domain_id);
 
-    spin_lock(&d->rangesets_lock);
+    if ( !keyhandler_spin_lock(&d->rangesets_lock, "could not get rangesets") )
+        return;
 
     if ( list_empty(&d->rangesets) )
         printk("    None\n");
diff --git a/xen/include/xen/keyhandler.h b/xen/include/xen/keyhandler.h
index 5131e86cbc..cc8e0b18f5 100644
--- a/xen/include/xen/keyhandler.h
+++ b/xen/include/xen/keyhandler.h
@@ -10,6 +10,9 @@
 #ifndef __XEN_KEYHANDLER_H__
 #define __XEN_KEYHANDLER_H__
 
+#include <xen/rwlock.h>
+#include <xen/spinlock.h>
+#include <xen/time.h>
 #include <xen/types.h>
 
 /*
@@ -48,4 +51,27 @@ void register_irq_keyhandler(unsigned char key,
 /* Inject a keypress into the key-handling subsystem. */
 extern void handle_keypress(unsigned char key, struct cpu_user_regs *regs);
 
+/* Locking primitives for inside keyhandlers (like trylock). */
+bool keyhandler_spin_lock(spinlock_t *lock, const char *msg);
+bool keyhandler_spin_lock_irqsave(spinlock_t *lock, unsigned long *flags,
+                                  const char *msg);
+bool keyhandler_read_lock(rwlock_t *lock, const char *msg);
+
+/* Primitives for custom keyhandler lock functions. */
+s_time_t keyhandler_lock_timeout(void);
+#define keyhandler_lock_body(type, lockfunc, arg...) \
+    s_time_t end = keyhandler_lock_timeout();        \
+    type ret;                                        \
+                                                     \
+    do {                                             \
+        ret = lockfunc;                              \
+        if ( ret )                                   \
+            return ret;                              \
+        cpu_relax();                                 \
+    } while ( NOW() < end );                         \
+                                                     \
+    printk("-->lock conflict: " arg);                \
+                                                     \
+    return ret
+
 #endif /* __XEN_KEYHANDLER_H__ */
-- 
2.16.4


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel