[minios] Add xenbus shutdown control support

[minios] Add xenbus shutdown control support

[Samuel Thibault]

Add a thread watching the xenbus shutdown control path and notifies a
wait queue.

Add HYPERVISOR_shutdown convenient inline for minios shutdown.

Add proper shutdown to the minios test application.

Signed-off-by: Samuel Thibault <samuel.thibault@ens-lyon.org>

diff -r fdf241ea6ff4 extras/mini-os/include/kernel.h
--- a/extras/mini-os/include/kernel.h	Wed Nov 28 21:29:18 2012 +0100
+++ b/extras/mini-os/include/kernel.h	Wed Nov 28 22:53:42 2012 +0100
@@ -1,6 +1,9 @@
 #ifndef _KERNEL_H_
 #define _KERNEL_H_
 
+extern unsigned int do_shutdown;
+extern unsigned int shutdown_reason;
+extern struct wait_queue_head shutdown_queue;
 extern void do_exit(void) __attribute__((noreturn));
 extern void stop_kernel(void);
 
diff -r fdf241ea6ff4 extras/mini-os/include/x86/x86_32/hypercall-x86_32.h
--- a/extras/mini-os/include/x86/x86_32/hypercall-x86_32.h	Wed Nov 28 21:29:18 2012 +0100
+++ b/extras/mini-os/include/x86/x86_32/hypercall-x86_32.h	Wed Nov 28 22:53:42 2012 +0100
@@ -172,6 +172,14 @@
 	return _hypercall2(int, sched_op, cmd, arg);
 }
 
+static inline int
+HYPERVISOR_shutdown(
+	unsigned int reason)
+{
+	struct sched_shutdown shutdown = { .reason = reason };
+	return _hypercall2(int, sched_op, SCHEDOP_shutdown, &shutdown);
+}
+
 static inline long
 HYPERVISOR_set_timer_op(
 	uint64_t timeout)
diff -r fdf241ea6ff4 extras/mini-os/include/x86/x86_64/hypercall-x86_64.h
--- a/extras/mini-os/include/x86/x86_64/hypercall-x86_64.h	Wed Nov 28 21:29:18 2012 +0100
+++ b/extras/mini-os/include/x86/x86_64/hypercall-x86_64.h	Wed Nov 28 22:53:42 2012 +0100
@@ -176,6 +176,14 @@
 	return _hypercall2(int, sched_op, cmd, arg);
 }
 
+static inline int
+HYPERVISOR_shutdown(
+	unsigned int reason)
+{
+	struct sched_shutdown shutdown = { .reason = reason };
+	return _hypercall2(int, sched_op, SCHEDOP_shutdown, &shutdown);
+}
+
 static inline long
 HYPERVISOR_set_timer_op(
 	uint64_t timeout)
diff -r fdf241ea6ff4 extras/mini-os/kernel.c
--- a/extras/mini-os/kernel.c	Wed Nov 28 21:29:18 2012 +0100
+++ b/extras/mini-os/kernel.c	Wed Nov 28 22:53:42 2012 +0100
@@ -48,6 +48,10 @@
 
 uint8_t xen_features[XENFEAT_NR_SUBMAPS * 32];
 
+unsigned int do_shutdown = 0;
+unsigned int shutdown_reason;
+DECLARE_WAIT_QUEUE_HEAD(shutdown_queue);
+
 void setup_xen_features(void)
 {
     xen_feature_info_t fi;
@@ -64,6 +68,36 @@
     }
 }
 
+static void shutdown_thread(void *p)
+{
+    const char *path = "control/shutdown";
+    const char *token = path;
+    xenbus_event_queue events = NULL;
+    char *shutdown, *err;
+    xenbus_watch_path_token(XBT_NIL, path, token, &events);
+    while ((err = xenbus_read(XBT_NIL, path, &shutdown)) != NULL)
+    {
+        free(err);
+        xenbus_wait_for_watch(&events);
+    }
+    xenbus_unwatch_path_token(XBT_NIL, path, token);
+    xenbus_write(XBT_NIL, path, "");
+    printk("Shutting down (%s)\n", shutdown);
+
+    if (!strcmp(shutdown, "poweroff"))
+        shutdown_reason = SHUTDOWN_poweroff;
+    else if (!strcmp(shutdown, "reboot"))
+        shutdown_reason = SHUTDOWN_reboot;
+    else
+        /* Unknown */
+        shutdown_reason = SHUTDOWN_crash;
+    wmb();
+    do_shutdown = 1;
+    wmb();
+    wake_up(&shutdown_queue);
+}
+
+
 /* This should be overridden by the application we are linked against. */
 __attribute__((weak)) int app_main(start_info_t *si)
 {
@@ -126,6 +160,8 @@
     /* Init XenBus */
     init_xenbus();
 
+    create_thread("shutdown", shutdown_thread, NULL);
+
     /* Call (possibly overridden) app_main() */
     app_main(&start_info);
 
diff -r fdf241ea6ff4 extras/mini-os/test.c
--- a/extras/mini-os/test.c	Wed Nov 28 21:29:18 2012 +0100
+++ b/extras/mini-os/test.c	Wed Nov 28 22:53:42 2012 +0100
@@ -46,6 +46,7 @@
 #include <xen/version.h>
 
 static struct netfront_dev *net_dev;
+static struct semaphore net_sem = __SEMAPHORE_INITIALIZER(net_sem, 0);
 
 void test_xenbus(void);
 
@@ -70,12 +71,14 @@
 static void netfront_thread(void *p)
 {
     net_dev = init_netfront(NULL, NULL, NULL, NULL);
+    up(&net_sem);
 }
 
 static struct blkfront_dev *blk_dev;
 static struct blkfront_info blk_info;
 static uint64_t blk_size_read;
 static uint64_t blk_size_write;
+static struct semaphore blk_sem = __SEMAPHORE_INITIALIZER(blk_sem, 0);;
 
 struct blk_req {
     struct blkfront_aiocb aiocb;
@@ -189,8 +192,10 @@
     time_t lasttime = 0;
 
     blk_dev = init_blkfront(NULL, &blk_info);
-    if (!blk_dev)
+    if (!blk_dev) {
+        up(&blk_sem);
         return;
+    }
 
     if (blk_info.info & VDISK_CDROM)
         printk("Block device is a CDROM\n");
@@ -210,7 +215,7 @@
         blk_read_sector(blk_info.sectors-1);
     }
 
-    while (1) {
+    while (!do_shutdown) {
         uint64_t sector = rand() % blk_info.sectors;
         struct timeval tv;
 #ifdef BLKTEST_WRITE
@@ -235,6 +240,7 @@
         }
 #endif
     }
+    up(&blk_sem);
 }
 
 #define WIDTH 800
@@ -293,7 +299,6 @@
     xfree(mfns);
     if (!fb_dev) {
         xfree(fb);
-        return;
     }
     up(&fbfront_sem);
 }
@@ -330,17 +335,21 @@
 }
 
 static struct kbdfront_dev *kbd_dev;
+static struct semaphore kbd_sem = __SEMAPHORE_INITIALIZER(kbd_sem, 0);
 static void kbdfront_thread(void *p)
 {
     DEFINE_WAIT(w);
     DEFINE_WAIT(w2);
+    DEFINE_WAIT(w3);
     int x = WIDTH / 2, y = HEIGHT / 2, z = 0;
 
     kbd_dev = init_kbdfront(NULL, 1);
-    if (!kbd_dev)
+    down(&fbfront_sem);
+    if (!kbd_dev) {
+        up(&kbd_sem);
         return;
+    }
 
-    down(&fbfront_sem);
     refresh_cursor(x, y);
     while (1) {
         union xenkbd_in_event kbdevent;
@@ -349,6 +358,11 @@
 
         add_waiter(w, kbdfront_queue);
         add_waiter(w2, fbfront_queue);
+        add_waiter(w3, shutdown_queue);
+
+        rmb();
+        if (do_shutdown)
+            break;
 
         while (kbdfront_receive(kbd_dev, &kbdevent, 1) != 0) {
             sleep = 0;
@@ -391,9 +405,11 @@
                         fbfront_update(fb_dev, x - 16, y - 16, 33, 33);
                     }
                 } else if (kbdevent.key.keycode == KEY_Q) {
-                    struct sched_shutdown sched_shutdown = { .reason = SHUTDOWN_poweroff };
-                    HYPERVISOR_sched_op(SCHEDOP_shutdown, &sched_shutdown);
-                    do_exit();
+                    shutdown_reason = SHUTDOWN_poweroff;
+                    wmb();
+                    do_shutdown = 1;
+                    wmb();
+                    wake_up(&shutdown_queue);
                 }
                 break;
             }
@@ -410,11 +426,16 @@
         }
         if (sleep)
             schedule();
+        remove_waiter(w3, shutdown_queue);
+        remove_waiter(w2, fbfront_queue);
+        remove_waiter(w, kbdfront_queue);
     }
+    up(&kbd_sem);
 }
 
 #ifdef CONFIG_PCIFRONT
 static struct pcifront_dev *pci_dev;
+static struct semaphore pci_sem = __SEMAPHORE_INITIALIZER(pci_sem, 0);
 
 static void print_pcidev(unsigned int domain, unsigned int bus, unsigned int slot, unsigned int fun)
 {
@@ -432,13 +453,60 @@
 {
     pcifront_watches(NULL);
     pci_dev = init_pcifront(NULL);
-    if (!pci_dev)
+    if (!pci_dev) {
+        up(&pci_sem);
         return;
+    }
     printk("PCI devices:\n");
     pcifront_scan(pci_dev, print_pcidev);
+    up(&pci_sem);
 }
 #endif
 
+void shutdown_frontends(void)
+{
+    down(&net_sem);
+    if (net_dev)
+        shutdown_netfront(net_dev);
+
+    down(&blk_sem);
+    if (blk_dev)
+        shutdown_blkfront(blk_dev);
+
+    if (fb_dev)
+        shutdown_fbfront(fb_dev);
+
+    down(&kbd_sem);
+    if (kbd_dev)
+        shutdown_kbdfront(kbd_dev);
+
+#ifdef CONFIG_PCIFRONT
+    down(&pci_sem);
+    if (pci_dev)
+        shutdown_pcifront(pci_dev);
+#endif
+}
+
+static void shutdown_thread(void *p)
+{
+    DEFINE_WAIT(w);
+
+    while (1) {
+        add_waiter(w, shutdown_queue);
+        rmb();
+        if (do_shutdown) {
+            rmb();
+            break;
+        }
+        schedule();
+        remove_waiter(w, shutdown_queue);
+    }
+
+    shutdown_frontends();
+
+    HYPERVISOR_shutdown(shutdown_reason);
+}
+
 int app_main(start_info_t *si)
 {
     printk("Test main: start_info=%p\n", si);
@@ -451,25 +519,6 @@
 #ifdef CONFIG_PCIFRONT
     create_thread("pcifront", pcifront_thread, si);
 #endif
+    create_thread("shutdown", shutdown_thread, si);
     return 0;
 }
-
-void shutdown_frontends(void)
-{
-    if (net_dev)
-        shutdown_netfront(net_dev);
-
-    if (blk_dev)
-        shutdown_blkfront(blk_dev);
-
-    if (fb_dev)
-        shutdown_fbfront(fb_dev);
-
-    if (kbd_dev)
-        shutdown_kbdfront(kbd_dev);
-
-#ifdef CONFIG_PCIFRONT
-    if (pci_dev)
-        shutdown_pcifront(pci_dev);
-#endif
-}

Re: [minios] Add xenbus shutdown control support

[Ian Campbell]

On Wed, 2012-11-28 at 21:57 +0000, Samuel Thibault wrote:
> Add a thread watching the xenbus shutdown control path and notifies a
> wait queue.

Why a wait queue rather than a weak function call?

> Add HYPERVISOR_shutdown convenient inline for minios shutdown.
> 
> Add proper shutdown to the minios test application.

The use of locks in here is pretty exciting, with the up/downs being
spread over various places, but I think I've worked it out and it's only
a test app ;-)

> Signed-off-by: Samuel Thibault <samuel.thibault@ens-lyon.org>
> 
> diff -r fdf241ea6ff4 extras/mini-os/include/kernel.h
> --- a/extras/mini-os/include/kernel.h	Wed Nov 28 21:29:18 2012 +0100
> +++ b/extras/mini-os/include/kernel.h	Wed Nov 28 22:53:42 2012 +0100
> @@ -1,6 +1,9 @@
>  #ifndef _KERNEL_H_
>  #define _KERNEL_H_
>  
> +extern unsigned int do_shutdown;
> +extern unsigned int shutdown_reason;
> +extern struct wait_queue_head shutdown_queue;
>  extern void do_exit(void) __attribute__((noreturn));
>  extern void stop_kernel(void);
>  
> diff -r fdf241ea6ff4 extras/mini-os/include/x86/x86_32/hypercall-x86_32.h
> --- a/extras/mini-os/include/x86/x86_32/hypercall-x86_32.h	Wed Nov 28 21:29:18 2012 +0100
> +++ b/extras/mini-os/include/x86/x86_32/hypercall-x86_32.h	Wed Nov 28 22:53:42 2012 +0100
> @@ -172,6 +172,14 @@
>  	return _hypercall2(int, sched_op, cmd, arg);
>  }
>  
> +static inline int
> +HYPERVISOR_shutdown(
> +	unsigned int reason)
> +{
> +	struct sched_shutdown shutdown = { .reason = reason };
> +	return _hypercall2(int, sched_op, SCHEDOP_shutdown, &shutdown);
> +}
> +
>  static inline long
>  HYPERVISOR_set_timer_op(
>  	uint64_t timeout)
> diff -r fdf241ea6ff4 extras/mini-os/include/x86/x86_64/hypercall-x86_64.h
> --- a/extras/mini-os/include/x86/x86_64/hypercall-x86_64.h	Wed Nov 28 21:29:18 2012 +0100
> +++ b/extras/mini-os/include/x86/x86_64/hypercall-x86_64.h	Wed Nov 28 22:53:42 2012 +0100
> @@ -176,6 +176,14 @@
>  	return _hypercall2(int, sched_op, cmd, arg);
>  }
>  
> +static inline int
> +HYPERVISOR_shutdown(
> +	unsigned int reason)
> +{
> +	struct sched_shutdown shutdown = { .reason = reason };
> +	return _hypercall2(int, sched_op, SCHEDOP_shutdown, &shutdown);
> +}
> +
>  static inline long
>  HYPERVISOR_set_timer_op(
>  	uint64_t timeout)
> diff -r fdf241ea6ff4 extras/mini-os/kernel.c
> --- a/extras/mini-os/kernel.c	Wed Nov 28 21:29:18 2012 +0100
> +++ b/extras/mini-os/kernel.c	Wed Nov 28 22:53:42 2012 +0100
> @@ -48,6 +48,10 @@
>  
>  uint8_t xen_features[XENFEAT_NR_SUBMAPS * 32];
>  
> +unsigned int do_shutdown = 0;
> +unsigned int shutdown_reason;
> +DECLARE_WAIT_QUEUE_HEAD(shutdown_queue);
> +
>  void setup_xen_features(void)
>  {
>      xen_feature_info_t fi;
> @@ -64,6 +68,36 @@
>      }
>  }
>  
> +static void shutdown_thread(void *p)
> +{
> +    const char *path = "control/shutdown";
> +    const char *token = path;
> +    xenbus_event_queue events = NULL;
> +    char *shutdown, *err;
> +    xenbus_watch_path_token(XBT_NIL, path, token, &events);
> +    while ((err = xenbus_read(XBT_NIL, path, &shutdown)) != NULL)
> +    {
> +        free(err);
> +        xenbus_wait_for_watch(&events);
> +    }
> +    xenbus_unwatch_path_token(XBT_NIL, path, token);
> +    xenbus_write(XBT_NIL, path, "");
> +    printk("Shutting down (%s)\n", shutdown);
> +
> +    if (!strcmp(shutdown, "poweroff"))
> +        shutdown_reason = SHUTDOWN_poweroff;
> +    else if (!strcmp(shutdown, "reboot"))
> +        shutdown_reason = SHUTDOWN_reboot;
> +    else
> +        /* Unknown */
> +        shutdown_reason = SHUTDOWN_crash;
> +    wmb();
> +    do_shutdown = 1;
> +    wmb();
> +    wake_up(&shutdown_queue);
> +}
> +
> +
>  /* This should be overridden by the application we are linked against. */
>  __attribute__((weak)) int app_main(start_info_t *si)
>  {
> @@ -126,6 +160,8 @@
>      /* Init XenBus */
>      init_xenbus();
>  
> +    create_thread("shutdown", shutdown_thread, NULL);
> +
>      /* Call (possibly overridden) app_main() */
>      app_main(&start_info);
>  
> diff -r fdf241ea6ff4 extras/mini-os/test.c
> --- a/extras/mini-os/test.c	Wed Nov 28 21:29:18 2012 +0100
> +++ b/extras/mini-os/test.c	Wed Nov 28 22:53:42 2012 +0100
> @@ -46,6 +46,7 @@
>  #include <xen/version.h>
>  
>  static struct netfront_dev *net_dev;
> +static struct semaphore net_sem = __SEMAPHORE_INITIALIZER(net_sem, 0);
>  
>  void test_xenbus(void);
>  
> @@ -70,12 +71,14 @@
>  static void netfront_thread(void *p)
>  {
>      net_dev = init_netfront(NULL, NULL, NULL, NULL);
> +    up(&net_sem);
>  }
>  
>  static struct blkfront_dev *blk_dev;
>  static struct blkfront_info blk_info;
>  static uint64_t blk_size_read;
>  static uint64_t blk_size_write;
> +static struct semaphore blk_sem = __SEMAPHORE_INITIALIZER(blk_sem, 0);;
>  
>  struct blk_req {
>      struct blkfront_aiocb aiocb;
> @@ -189,8 +192,10 @@
>      time_t lasttime = 0;
>  
>      blk_dev = init_blkfront(NULL, &blk_info);
> -    if (!blk_dev)
> +    if (!blk_dev) {
> +        up(&blk_sem);
>          return;
> +    }
>  
>      if (blk_info.info & VDISK_CDROM)
>          printk("Block device is a CDROM\n");
> @@ -210,7 +215,7 @@
>          blk_read_sector(blk_info.sectors-1);
>      }
>  
> -    while (1) {
> +    while (!do_shutdown) {
>          uint64_t sector = rand() % blk_info.sectors;
>          struct timeval tv;
>  #ifdef BLKTEST_WRITE
> @@ -235,6 +240,7 @@
>          }
>  #endif
>      }
> +    up(&blk_sem);
>  }
>  
>  #define WIDTH 800
> @@ -293,7 +299,6 @@
>      xfree(mfns);
>      if (!fb_dev) {
>          xfree(fb);
> -        return;
>      }
>      up(&fbfront_sem);
>  }
> @@ -330,17 +335,21 @@
>  }
>  
>  static struct kbdfront_dev *kbd_dev;
> +static struct semaphore kbd_sem = __SEMAPHORE_INITIALIZER(kbd_sem, 0);
>  static void kbdfront_thread(void *p)
>  {
>      DEFINE_WAIT(w);
>      DEFINE_WAIT(w2);
> +    DEFINE_WAIT(w3);
>      int x = WIDTH / 2, y = HEIGHT / 2, z = 0;
>  
>      kbd_dev = init_kbdfront(NULL, 1);
> -    if (!kbd_dev)
> +    down(&fbfront_sem);
> +    if (!kbd_dev) {
> +        up(&kbd_sem);
>          return;
> +    }
>  
> -    down(&fbfront_sem);
>      refresh_cursor(x, y);
>      while (1) {
>          union xenkbd_in_event kbdevent;
> @@ -349,6 +358,11 @@
>  
>          add_waiter(w, kbdfront_queue);
>          add_waiter(w2, fbfront_queue);
> +        add_waiter(w3, shutdown_queue);
> +
> +        rmb();
> +        if (do_shutdown)
> +            break;
>  
>          while (kbdfront_receive(kbd_dev, &kbdevent, 1) != 0) {
>              sleep = 0;
> @@ -391,9 +405,11 @@
>                          fbfront_update(fb_dev, x - 16, y - 16, 33, 33);
>                      }
>                  } else if (kbdevent.key.keycode == KEY_Q) {
> -                    struct sched_shutdown sched_shutdown = { .reason = SHUTDOWN_poweroff };
> -                    HYPERVISOR_sched_op(SCHEDOP_shutdown, &sched_shutdown);
> -                    do_exit();
> +                    shutdown_reason = SHUTDOWN_poweroff;
> +                    wmb();
> +                    do_shutdown = 1;
> +                    wmb();
> +                    wake_up(&shutdown_queue);
>                  }
>                  break;
>              }
> @@ -410,11 +426,16 @@
>          }
>          if (sleep)
>              schedule();
> +        remove_waiter(w3, shutdown_queue);
> +        remove_waiter(w2, fbfront_queue);
> +        remove_waiter(w, kbdfront_queue);
>      }
> +    up(&kbd_sem);
>  }
>  
>  #ifdef CONFIG_PCIFRONT
>  static struct pcifront_dev *pci_dev;
> +static struct semaphore pci_sem = __SEMAPHORE_INITIALIZER(pci_sem, 0);
>  
>  static void print_pcidev(unsigned int domain, unsigned int bus, unsigned int slot, unsigned int fun)
>  {
> @@ -432,13 +453,60 @@
>  {
>      pcifront_watches(NULL);
>      pci_dev = init_pcifront(NULL);
> -    if (!pci_dev)
> +    if (!pci_dev) {
> +        up(&pci_sem);
>          return;
> +    }
>      printk("PCI devices:\n");
>      pcifront_scan(pci_dev, print_pcidev);
> +    up(&pci_sem);
>  }
>  #endif
>  
> +void shutdown_frontends(void)
> +{
> +    down(&net_sem);
> +    if (net_dev)
> +        shutdown_netfront(net_dev);
> +
> +    down(&blk_sem);
> +    if (blk_dev)
> +        shutdown_blkfront(blk_dev);
> +
> +    if (fb_dev)
> +        shutdown_fbfront(fb_dev);
> +
> +    down(&kbd_sem);
> +    if (kbd_dev)
> +        shutdown_kbdfront(kbd_dev);
> +
> +#ifdef CONFIG_PCIFRONT
> +    down(&pci_sem);
> +    if (pci_dev)
> +        shutdown_pcifront(pci_dev);
> +#endif
> +}
> +
> +static void shutdown_thread(void *p)
> +{
> +    DEFINE_WAIT(w);
> +
> +    while (1) {
> +        add_waiter(w, shutdown_queue);
> +        rmb();
> +        if (do_shutdown) {
> +            rmb();
> +            break;
> +        }
> +        schedule();
> +        remove_waiter(w, shutdown_queue);
> +    }
> +
> +    shutdown_frontends();
> +
> +    HYPERVISOR_shutdown(shutdown_reason);
> +}
> +
>  int app_main(start_info_t *si)
>  {
>      printk("Test main: start_info=%p\n", si);
> @@ -451,25 +519,6 @@
>  #ifdef CONFIG_PCIFRONT
>      create_thread("pcifront", pcifront_thread, si);
>  #endif
> +    create_thread("shutdown", shutdown_thread, si);
>      return 0;
>  }
> -
> -void shutdown_frontends(void)
> -{
> -    if (net_dev)
> -        shutdown_netfront(net_dev);
> -
> -    if (blk_dev)
> -        shutdown_blkfront(blk_dev);
> -
> -    if (fb_dev)
> -        shutdown_fbfront(fb_dev);
> -
> -    if (kbd_dev)
> -        shutdown_kbdfront(kbd_dev);
> -
> -#ifdef CONFIG_PCIFRONT
> -    if (pci_dev)
> -        shutdown_pcifront(pci_dev);
> -#endif
> -}
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel

Re: [minios] Add xenbus shutdown control support

[Samuel Thibault]

Re,

Ian Campbell, le Thu 29 Nov 2012 11:14:19 +0000, a écrit :
> On Wed, 2012-11-28 at 21:57 +0000, Samuel Thibault wrote:
> > Add a thread watching the xenbus shutdown control path and notifies a
> > wait queue.
> 
> Why a wait queue rather than a weak function call?

Because it integrates well with existing wait loops.

Samuel

Re: [minios] Add xenbus shutdown control support

[Ian Campbell]

On Tue, 2012-12-04 at 00:30 +0000, Samuel Thibault wrote:
> Re,
> 
> Ian Campbell, le Thu 29 Nov 2012 11:14:19 +0000, a écrit :
> > On Wed, 2012-11-28 at 21:57 +0000, Samuel Thibault wrote:
> > > Add a thread watching the xenbus shutdown control path and notifies a
> > > wait queue.
> > 
> > Why a wait queue rather than a weak function call?
> 
> Because it integrates well with existing wait loops.

I was imagining that someone using such a wait loop would simply provide
the weak function to kick the queue themselves, rather than imposing
this design on them from the core. It's not a big deal though.

Ian.


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [minios] Add xenbus shutdown control support

[Samuel Thibault]

Ian Campbell, le Tue 04 Dec 2012 09:32:45 +0000, a écrit :
> On Tue, 2012-12-04 at 00:30 +0000, Samuel Thibault wrote:
> > Ian Campbell, le Thu 29 Nov 2012 11:14:19 +0000, a écrit :
> > > On Wed, 2012-11-28 at 21:57 +0000, Samuel Thibault wrote:
> > > > Add a thread watching the xenbus shutdown control path and notifies a
> > > > wait queue.
> > > 
> > > Why a wait queue rather than a weak function call?
> > 
> > Because it integrates well with existing wait loops.
> 
> I was imagining that someone using such a wait loop would simply provide
> the weak function to kick the queue themselves,

Ah, right, in that case it doesn't need much synchronization with the
shutdown event, so it doesn't pose problem.

> rather than imposing this design on them from the core. It's not a big
> deal though.

But it's nicer to get it the way people would prefer. Daniel, Matthew,
what do you think?

Samuel