ia64/xen-unstable

changeset 17583: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>
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;