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
-}
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, 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
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
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
Apparently Analagous Threads
- mini-os: Notify shutdown through weak function call instead of wake queue
- [RFC] PVFB: Add refresh period to XenStore parameters?
- [PATCH v4 00/23] Xenstore stub domain
- xen-unstable stubdom build error version(3432abcf9380)
- [PATCH 4 of 13] DisplayState interface change