debuggers.hg
changeset 17614:a3bddc22d2f5
minios: fix thread safety of xenbus watches by requiring callers to
provide their own queue of events, because else we can not dispatch to
watchers running in parallel.
Signed-off-by: Samuel Thibault <samuel.thibault@eu.citrix.com>
provide their own queue of events, because else we can not dispatch to
watchers running in parallel.
Signed-off-by: Samuel Thibault <samuel.thibault@eu.citrix.com>
author | Keir Fraser <keir.fraser@citrix.com> |
---|---|
date | Tue May 06 13:34:52 2008 +0100 (2008-05-06) |
parents | 01aa7c088e98 |
children | ab2d9e75098d |
files | extras/mini-os/blkfront.c extras/mini-os/fbfront.c extras/mini-os/fs-front.c extras/mini-os/include/lib.h extras/mini-os/include/xenbus.h extras/mini-os/netfront.c extras/mini-os/xenbus/xenbus.c |
line diff
1.1 --- a/extras/mini-os/blkfront.c Tue May 06 13:32:18 2008 +0100 1.2 +++ b/extras/mini-os/blkfront.c Tue May 06 13:34:52 2008 +0100 1.3 @@ -50,6 +50,8 @@ struct blkfront_dev { 1.4 char *backend; 1.5 struct blkfront_info info; 1.6 1.7 + xenbus_event_queue events; 1.8 + 1.9 #ifdef HAVE_LIBC 1.10 int fd; 1.11 #endif 1.12 @@ -101,6 +103,8 @@ struct blkfront_dev *init_blkfront(char 1.13 1.14 dev->ring_ref = gnttab_grant_access(dev->dom,virt_to_mfn(s),0); 1.15 1.16 + dev->events = NULL; 1.17 + 1.18 // FIXME: proper frees on failures 1.19 again: 1.20 err = xenbus_transaction_start(&xbt); 1.21 @@ -166,11 +170,9 @@ done: 1.22 1.23 snprintf(path, sizeof(path), "%s/state", dev->backend); 1.24 1.25 - xenbus_watch_path(XBT_NIL, path); 1.26 + xenbus_watch_path_token(XBT_NIL, path, path, &dev->events); 1.27 1.28 - xenbus_wait_for_value(path,"4"); 1.29 - 1.30 - xenbus_unwatch_path(XBT_NIL, path); 1.31 + xenbus_wait_for_value(path, "4", &dev->events); 1.32 1.33 snprintf(path, sizeof(path), "%s/info", dev->backend); 1.34 dev->info.info = xenbus_read_integer(path); 1.35 @@ -211,10 +213,12 @@ void shutdown_blkfront(struct blkfront_d 1.36 1.37 snprintf(path, sizeof(path), "%s/state", dev->backend); 1.38 err = xenbus_printf(XBT_NIL, nodename, "state", "%u", 5); /* closing */ 1.39 - xenbus_wait_for_value(path,"5"); 1.40 + xenbus_wait_for_value(path, "5", &dev->events); 1.41 1.42 err = xenbus_printf(XBT_NIL, nodename, "state", "%u", 6); 1.43 - xenbus_wait_for_value(path,"6"); 1.44 + xenbus_wait_for_value(path, "6", &dev->events); 1.45 + 1.46 + xenbus_unwatch_path(XBT_NIL, path); 1.47 1.48 unbind_evtchn(dev->evtchn); 1.49
2.1 --- a/extras/mini-os/fbfront.c Tue May 06 13:32:18 2008 +0100 2.2 +++ b/extras/mini-os/fbfront.c Tue May 06 13:34:52 2008 +0100 2.3 @@ -31,6 +31,8 @@ struct kbdfront_dev { 2.4 char *nodename; 2.5 char *backend; 2.6 2.7 + xenbus_event_queue events; 2.8 + 2.9 #ifdef HAVE_LIBC 2.10 int fd; 2.11 #endif 2.12 @@ -75,6 +77,8 @@ struct kbdfront_dev *init_kbdfront(char 2.13 dev->page = s = (struct xenkbd_page*) alloc_page(); 2.14 memset(s,0,PAGE_SIZE); 2.15 2.16 + dev->events = NULL; 2.17 + 2.18 s->in_cons = s->in_prod = 0; 2.19 s->out_cons = s->out_prod = 0; 2.20 2.21 @@ -136,11 +140,9 @@ done: 2.22 2.23 snprintf(path, sizeof(path), "%s/state", dev->backend); 2.24 2.25 - xenbus_watch_path(XBT_NIL, path); 2.26 + xenbus_watch_path_token(XBT_NIL, path, path, &dev->events); 2.27 2.28 - xenbus_wait_for_value(path,"4"); 2.29 - 2.30 - xenbus_unwatch_path(XBT_NIL, path); 2.31 + xenbus_wait_for_value(path, "4", &dev->events); 2.32 2.33 printk("%s connected\n", dev->backend); 2.34 2.35 @@ -199,10 +201,12 @@ void shutdown_kbdfront(struct kbdfront_d 2.36 2.37 snprintf(path, sizeof(path), "%s/state", dev->backend); 2.38 err = xenbus_printf(XBT_NIL, nodename, "state", "%u", 5); /* closing */ 2.39 - xenbus_wait_for_value(path,"5"); 2.40 + xenbus_wait_for_value(path, "5", &dev->events); 2.41 2.42 err = xenbus_printf(XBT_NIL, nodename, "state", "%u", 6); 2.43 - xenbus_wait_for_value(path,"6"); 2.44 + xenbus_wait_for_value(path, "6", &dev->events); 2.45 + 2.46 + xenbus_unwatch_path(XBT_NIL, path); 2.47 2.48 unbind_evtchn(dev->evtchn); 2.49 2.50 @@ -249,6 +253,8 @@ struct fbfront_dev { 2.51 int stride; 2.52 int mem_length; 2.53 int offset; 2.54 + 2.55 + xenbus_event_queue events; 2.56 }; 2.57 2.58 void fbfront_handler(evtchn_port_t port, struct pt_regs *regs, void *data) 2.59 @@ -292,6 +298,7 @@ struct fbfront_dev *init_fbfront(char *n 2.60 dev->stride = s->line_length = stride; 2.61 dev->mem_length = s->mem_length = n * PAGE_SIZE; 2.62 dev->offset = 0; 2.63 + dev->events = NULL; 2.64 2.65 const int max_pd = sizeof(s->pd) / sizeof(s->pd[0]); 2.66 unsigned long mapped = 0; 2.67 @@ -368,14 +375,12 @@ done: 2.68 2.69 snprintf(path, sizeof(path), "%s/state", dev->backend); 2.70 2.71 - xenbus_watch_path(XBT_NIL, path); 2.72 + xenbus_watch_path_token(XBT_NIL, path, path, &dev->events); 2.73 2.74 - xenbus_wait_for_value(path,"4"); 2.75 + xenbus_wait_for_value(path, "4", &dev->events); 2.76 2.77 printk("%s connected\n", dev->backend); 2.78 2.79 - xenbus_unwatch_path(XBT_NIL, path); 2.80 - 2.81 snprintf(path, sizeof(path), "%s/request-update", dev->backend); 2.82 dev->request_update = xenbus_read_integer(path); 2.83 2.84 @@ -463,10 +468,12 @@ void shutdown_fbfront(struct fbfront_dev 2.85 2.86 snprintf(path, sizeof(path), "%s/state", dev->backend); 2.87 err = xenbus_printf(XBT_NIL, nodename, "state", "%u", 5); /* closing */ 2.88 - xenbus_wait_for_value(path,"5"); 2.89 + xenbus_wait_for_value(path, "5", &dev->events); 2.90 2.91 err = xenbus_printf(XBT_NIL, nodename, "state", "%u", 6); 2.92 - xenbus_wait_for_value(path,"6"); 2.93 + xenbus_wait_for_value(path, "6", &dev->events); 2.94 + 2.95 + xenbus_unwatch_path(XBT_NIL, path); 2.96 2.97 unbind_evtchn(dev->evtchn); 2.98
3.1 --- a/extras/mini-os/fs-front.c Tue May 06 13:32:18 2008 +0100 3.2 +++ b/extras/mini-os/fs-front.c Tue May 06 13:34:52 2008 +0100 3.3 @@ -917,6 +917,7 @@ static int init_fs_import(struct fs_impo 3.4 struct fsif_sring *sring; 3.5 int retry = 0; 3.6 domid_t self_id; 3.7 + xenbus_event_queue events = NULL; 3.8 3.9 printk("Initialising FS fortend to backend dom %d\n", import->dom_id); 3.10 /* Allocate page for the shared ring */ 3.11 @@ -1026,8 +1027,8 @@ done: 3.12 sprintf(r_nodename, "%s/state", import->backend); 3.13 sprintf(token, "fs-front-%d", import->import_id); 3.14 /* The token will not be unique if multiple imports are inited */ 3.15 - xenbus_watch_path(XBT_NIL, r_nodename/*, token*/); 3.16 - xenbus_wait_for_value(/*token,*/ r_nodename, STATE_READY); 3.17 + xenbus_watch_path_token(XBT_NIL, r_nodename, r_nodename, &events); 3.18 + xenbus_wait_for_value(r_nodename, STATE_READY, &events); 3.19 xenbus_unwatch_path(XBT_NIL, r_nodename); 3.20 printk("Backend ready.\n"); 3.21
4.1 --- a/extras/mini-os/include/lib.h Tue May 06 13:32:18 2008 +0100 4.2 +++ b/extras/mini-os/include/lib.h Tue May 06 13:34:52 2008 +0100 4.3 @@ -178,7 +178,7 @@ extern struct file { 4.4 struct { 4.5 /* To each xenbus FD is associated a queue of watch events for this 4.6 * FD. */ 4.7 - struct xenbus_event *volatile events; 4.8 + xenbus_event_queue events; 4.9 } xenbus; 4.10 }; 4.11 volatile int read; /* maybe available for read */
5.1 --- a/extras/mini-os/include/xenbus.h Tue May 06 13:32:18 2008 +0100 5.2 +++ b/extras/mini-os/include/xenbus.h Tue May 06 13:34:52 2008 +0100 5.3 @@ -19,17 +19,18 @@ struct xenbus_event { 5.4 char *token; 5.5 struct xenbus_event *next; 5.6 }; 5.7 +typedef struct xenbus_event *xenbus_event_queue; 5.8 5.9 -char *xenbus_watch_path_token(xenbus_transaction_t xbt, const char *path, const char *token, struct xenbus_event *volatile *events); 5.10 +char *xenbus_watch_path_token(xenbus_transaction_t xbt, const char *path, const char *token, xenbus_event_queue *events); 5.11 char *xenbus_unwatch_path_token(xenbus_transaction_t xbt, const char *path, const char *token); 5.12 extern struct wait_queue_head xenbus_watch_queue; 5.13 -void xenbus_wait_for_watch(void); 5.14 -char **xenbus_wait_for_watch_return(void); 5.15 -char* xenbus_wait_for_value(const char *path, const char *value); 5.16 +void xenbus_wait_for_watch(xenbus_event_queue *queue); 5.17 +char **xenbus_wait_for_watch_return(xenbus_event_queue *queue); 5.18 +char* xenbus_wait_for_value(const char *path, const char *value, xenbus_event_queue *queue); 5.19 5.20 /* When no token is provided, use a global queue. */ 5.21 #define XENBUS_WATCH_PATH_TOKEN "xenbus_watch_path" 5.22 -extern struct xenbus_event * volatile xenbus_events; 5.23 +extern xenbus_event_queue xenbus_events; 5.24 #define xenbus_watch_path(xbt, path) xenbus_watch_path_token(xbt, path, XENBUS_WATCH_PATH_TOKEN, NULL) 5.25 #define xenbus_unwatch_path(xbt, path) xenbus_unwatch_path_token(xbt, path, XENBUS_WATCH_PATH_TOKEN) 5.26
6.1 --- a/extras/mini-os/netfront.c Tue May 06 13:32:18 2008 +0100 6.2 +++ b/extras/mini-os/netfront.c Tue May 06 13:34:52 2008 +0100 6.3 @@ -53,6 +53,8 @@ struct netfront_dev { 6.4 char *nodename; 6.5 char *backend; 6.6 6.7 + xenbus_event_queue events; 6.8 + 6.9 #ifdef HAVE_LIBC 6.10 int fd; 6.11 unsigned char *data; 6.12 @@ -328,6 +330,8 @@ struct netfront_dev *init_netfront(char 6.13 6.14 dev->netif_rx = thenetif_rx; 6.15 6.16 + dev->events = NULL; 6.17 + 6.18 // FIXME: proper frees on failures 6.19 again: 6.20 err = xenbus_transaction_start(&xbt); 6.21 @@ -399,11 +403,9 @@ done: 6.22 char path[strlen(dev->backend) + 1 + 5 + 1]; 6.23 snprintf(path, sizeof(path), "%s/state", dev->backend); 6.24 6.25 - xenbus_watch_path(XBT_NIL, path); 6.26 + xenbus_watch_path_token(XBT_NIL, path, path, &dev->events); 6.27 6.28 - xenbus_wait_for_value(path,"4"); 6.29 - 6.30 - xenbus_unwatch_path(XBT_NIL, path); 6.31 + xenbus_wait_for_value(path, "4", &dev->events); 6.32 6.33 if (ip) { 6.34 snprintf(path, sizeof(path), "%s/ip", dev->backend); 6.35 @@ -458,10 +460,12 @@ void shutdown_netfront(struct netfront_d 6.36 6.37 snprintf(path, sizeof(path), "%s/state", dev->backend); 6.38 err = xenbus_printf(XBT_NIL, nodename, "state", "%u", 5); /* closing */ 6.39 - xenbus_wait_for_value(path,"5"); 6.40 + xenbus_wait_for_value(path, "5", &dev->events); 6.41 6.42 err = xenbus_printf(XBT_NIL, nodename, "state", "%u", 6); 6.43 - xenbus_wait_for_value(path,"6"); 6.44 + xenbus_wait_for_value(path, "6", &dev->events); 6.45 + 6.46 + xenbus_unwatch_path(XBT_NIL, path); 6.47 6.48 unbind_evtchn(dev->evtchn); 6.49
7.1 --- a/extras/mini-os/xenbus/xenbus.c Tue May 06 13:32:18 2008 +0100 7.2 +++ b/extras/mini-os/xenbus/xenbus.c Tue May 06 13:34:52 2008 +0100 7.3 @@ -45,10 +45,10 @@ static struct xenstore_domain_interface 7.4 static DECLARE_WAIT_QUEUE_HEAD(xb_waitq); 7.5 DECLARE_WAIT_QUEUE_HEAD(xenbus_watch_queue); 7.6 7.7 -struct xenbus_event *volatile xenbus_events; 7.8 +xenbus_event_queue xenbus_events; 7.9 static struct watch { 7.10 char *token; 7.11 - struct xenbus_event *volatile *events; 7.12 + xenbus_event_queue *events; 7.13 struct watch *next; 7.14 } *watches; 7.15 struct xenbus_req_info 7.16 @@ -75,28 +75,34 @@ static void memcpy_from_ring(const void 7.17 memcpy(dest + c1, ring, c2); 7.18 } 7.19 7.20 -char **xenbus_wait_for_watch_return() 7.21 +char **xenbus_wait_for_watch_return(xenbus_event_queue *queue) 7.22 { 7.23 struct xenbus_event *event; 7.24 + if (!queue) 7.25 + queue = &xenbus_events; 7.26 DEFINE_WAIT(w); 7.27 - while (!(event = xenbus_events)) { 7.28 + while (!(event = *queue)) { 7.29 add_waiter(w, xenbus_watch_queue); 7.30 schedule(); 7.31 } 7.32 remove_waiter(w); 7.33 - xenbus_events = event->next; 7.34 + *queue = event->next; 7.35 return &event->path; 7.36 } 7.37 7.38 -void xenbus_wait_for_watch(void) 7.39 +void xenbus_wait_for_watch(xenbus_event_queue *queue) 7.40 { 7.41 char **ret; 7.42 - ret = xenbus_wait_for_watch_return(); 7.43 + if (!queue) 7.44 + queue = &xenbus_events; 7.45 + ret = xenbus_wait_for_watch_return(queue); 7.46 free(ret); 7.47 } 7.48 7.49 -char* xenbus_wait_for_value(const char* path, const char* value) 7.50 +char* xenbus_wait_for_value(const char* path, const char* value, xenbus_event_queue *queue) 7.51 { 7.52 + if (!queue) 7.53 + queue = &xenbus_events; 7.54 for(;;) 7.55 { 7.56 char *res, *msg; 7.57 @@ -109,7 +115,7 @@ char* xenbus_wait_for_value(const char* 7.58 free(res); 7.59 7.60 if(r==0) break; 7.61 - else xenbus_wait_for_watch(); 7.62 + else xenbus_wait_for_watch(queue); 7.63 } 7.64 return NULL; 7.65 } 7.66 @@ -147,8 +153,8 @@ static void xenbus_thread_func(void *ign 7.67 7.68 if(msg.type == XS_WATCH_EVENT) 7.69 { 7.70 - struct xenbus_event *event = malloc(sizeof(*event) + msg.len), 7.71 - *volatile *events = NULL; 7.72 + struct xenbus_event *event = malloc(sizeof(*event) + msg.len); 7.73 + xenbus_event_queue *events = NULL; 7.74 char *data = (char*)event + sizeof(*event); 7.75 struct watch *watch; 7.76 7.77 @@ -167,8 +173,6 @@ static void xenbus_thread_func(void *ign 7.78 events = watch->events; 7.79 break; 7.80 } 7.81 - if (!events) 7.82 - events = &xenbus_events; 7.83 7.84 event->next = *events; 7.85 *events = event; 7.86 @@ -463,7 +467,7 @@ char *xenbus_write(xenbus_transaction_t 7.87 return NULL; 7.88 } 7.89 7.90 -char* xenbus_watch_path_token( xenbus_transaction_t xbt, const char *path, const char *token, struct xenbus_event *volatile *events) 7.91 +char* xenbus_watch_path_token( xenbus_transaction_t xbt, const char *path, const char *token, xenbus_event_queue *events) 7.92 { 7.93 struct xsd_sockmsg *rep; 7.94 7.95 @@ -474,6 +478,9 @@ char* xenbus_watch_path_token( xenbus_tr 7.96 7.97 struct watch *watch = malloc(sizeof(*watch)); 7.98 7.99 + if (!events) 7.100 + events = &xenbus_events; 7.101 + 7.102 watch->token = strdup(token); 7.103 watch->events = events; 7.104 watch->next = watches;