Re: [Xen-devel] [PATCH 04/11] mini-os/xenbus: Change type of xenbus_event_queue

[Samuel Thibault <samuel.thibault@xxxxxxxxxxxx>]

Ian Jackson, le Fri 20 Jun 2014 20:04:43 +0100, a écrit :
> We change xenbus_event_queue from a pointer typedef to a struct, for
> two reasons:
> 
> 1. In a moment we are going to want to extend this struct to include
>    a minios scheduler wait queue.
> 
> 2. We can replace the open-coded list with a MINIOS_STAILQ.
> 
> All the xenbus users need to call the new initialisation function
> instead of simply initialising the struct to NULL, and have their
> parameter type changed.
> 
> There is a functional side-effect: now we are using a tail queue,
> rather than a tailless queue, we insert events at the end rather than
> the beginning.  So watch events now come out in chronological order,
> rather than their order possibly being scrambled in the queue.
> 
> Signed-off-by: Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx>

Acked-by: Samuel Thibault <samuel.thibault@xxxxxxxxxxxx>

> ---
>  include/mini-os/console.h |    2 +-
>  include/mini-os/xenbus.h  |   22 +++++++++++++---------
>  xen/blkfront.c            |    4 ++--
>  xen/console/xenbus.c      |    2 +-
>  xen/netfront.c            |    4 ++--
>  xen/pcifront.c            |    7 ++++---
>  xen/xenbus/xenbus.c       |   31 +++++++++++++++++++------------
>  7 files changed, 42 insertions(+), 30 deletions(-)
> 
> diff --git a/include/mini-os/console.h b/include/mini-os/console.h
> index 1b04b13..49990b9 100644
> --- a/include/mini-os/console.h
> +++ b/include/mini-os/console.h
> @@ -56,7 +56,7 @@ struct consfront_dev {
>      char *nodename;
>      char *backend;
>  
> -    xenbus_event_queue events;
> +    struct xenbus_event_queue events;
>  
>  };
>  
> diff --git a/include/mini-os/xenbus.h b/include/mini-os/xenbus.h
> index f3594cb..4dad4c8 100644
> --- a/include/mini-os/xenbus.h
> +++ b/include/mini-os/xenbus.h
> @@ -2,6 +2,7 @@
>  #define MINIOS_XENBUS_H__
>  
>  #include <xen/io/xenbus.h>
> +#include <mini-os/queue.h>
>  
>  typedef unsigned long xenbus_transaction_t;
>  #define XBT_NIL ((xenbus_transaction_t)0)
> @@ -25,22 +26,25 @@ struct xenbus_event {
>      /* Keep these two as this for xs.c */
>      char *path;
>      char *token;
> -    struct xenbus_event *next;
> +    MINIOS_STAILQ_ENTRY(xenbus_event) entry;
>  };
> -typedef struct xenbus_event *xenbus_event_queue;
> +struct xenbus_event_queue {
> +    MINIOS_STAILQ_HEAD(, xenbus_event) events;
> +};
> +
> +void xenbus_event_queue_init(struct xenbus_event_queue *queue);
>  
> -char *xenbus_watch_path_token(xenbus_transaction_t xbt, const char *path, 
> const char *token, xenbus_event_queue *events);
> +char *xenbus_watch_path_token(xenbus_transaction_t xbt, const char *path, 
> const char *token, struct xenbus_event_queue *events);
>  char *xenbus_unwatch_path_token(xenbus_transaction_t xbt, const char *path, 
> const char *token);
> -extern struct wait_queue_head xenbus_watch_queue;
> -void xenbus_wait_for_watch(xenbus_event_queue *queue);
> -char **xenbus_wait_for_watch_return(xenbus_event_queue *queue);
> -char* xenbus_wait_for_value(const char *path, const char *value, 
> xenbus_event_queue *queue);
> -char *xenbus_wait_for_state_change(const char* path, XenbusState *state, 
> xenbus_event_queue *queue);
> +void xenbus_wait_for_watch(struct xenbus_event_queue *queue);
> +char **xenbus_wait_for_watch_return(struct xenbus_event_queue *queue);
> +char* xenbus_wait_for_value(const char *path, const char *value, struct 
> xenbus_event_queue *queue);
> +char *xenbus_wait_for_state_change(const char* path, XenbusState *state, 
> struct xenbus_event_queue *queue);
>  char *xenbus_switch_state(xenbus_transaction_t xbt, const char* path, 
> XenbusState state);
>  
>  /* When no token is provided, use a global queue. */
>  #define XENBUS_WATCH_PATH_TOKEN "xenbus_watch_path"
> -extern xenbus_event_queue xenbus_events;
> +extern struct xenbus_event_queue xenbus_events;
>  #define xenbus_watch_path(xbt, path) xenbus_watch_path_token(xbt, path, 
> XENBUS_WATCH_PATH_TOKEN, NULL)
>  #define xenbus_unwatch_path(xbt, path) xenbus_unwatch_path_token(xbt, path, 
> XENBUS_WATCH_PATH_TOKEN)
>  
> diff --git a/xen/blkfront.c b/xen/blkfront.c
> index 347c68c..a5da53d 100644
> --- a/xen/blkfront.c
> +++ b/xen/blkfront.c
> @@ -47,7 +47,7 @@ struct blkfront_dev {
>      char *backend;
>      struct blkfront_info info;
>  
> -    xenbus_event_queue events;
> +    struct xenbus_event_queue events;
>  
>  };
>  
> @@ -103,7 +103,7 @@ struct blkfront_dev *init_blkfront(char *_nodename, 
> struct blkfront_info *info)
>  
>      dev->ring_ref = gnttab_grant_access(dev->dom,virt_to_mfn(s),0);
>  
> -    dev->events = NULL;
> +    xenbus_event_queue_init(&dev->events);
>  
>  again:
>      err = xenbus_transaction_start(&xbt);
> diff --git a/xen/console/xenbus.c b/xen/console/xenbus.c
> index d848af8..a164314 100644
> --- a/xen/console/xenbus.c
> +++ b/xen/console/xenbus.c
> @@ -96,7 +96,7 @@ struct consfront_dev *init_consfront(char *_nodename)
>      memset(dev->ring, 0, PAGE_SIZE);
>      dev->ring_ref = gnttab_grant_access(dev->dom, virt_to_mfn(dev->ring), 0);
>  
> -    dev->events = NULL;
> +    xenbus_event_queue_init(&dev->events);
>  
>  again:
>      err = xenbus_transaction_start(&xbt);
> diff --git a/xen/netfront.c b/xen/netfront.c
> index 9e17a86..a89a0dd 100644
> --- a/xen/netfront.c
> +++ b/xen/netfront.c
> @@ -51,7 +51,7 @@ struct netfront_dev {
>      char *backend;
>      char *mac;
>  
> -    xenbus_event_queue events;
> +    struct xenbus_event_queue events;
>  
>  
>      void (*netif_rx)(struct netfront_dev *, unsigned char* data, int len);
> @@ -313,7 +313,7 @@ struct netfront_dev *init_netfront(char *_nodename, void 
> (*thenetif_rx)(struct n
>  
>      dev->netif_rx = thenetif_rx;
>  
> -    dev->events = NULL;
> +    xenbus_event_queue_init(&dev->events);
>  
>  again:
>      err = xenbus_transaction_start(&xbt);
> diff --git a/xen/pcifront.c b/xen/pcifront.c
> index 6f52b77..7bd3788 100644
> --- a/xen/pcifront.c
> +++ b/xen/pcifront.c
> @@ -31,7 +31,7 @@ struct pcifront_dev {
>      char *nodename;
>      char *backend;
>  
> -    xenbus_event_queue events;
> +    struct xenbus_event_queue events;
>  };
>  
>  void pcifront_handler(evtchn_port_t port, struct pt_regs *regs, void *data)
> @@ -64,7 +64,8 @@ void pcifront_watches(void *opaque)
>      char* nodename = opaque ? opaque : "device/pci/0";
>      char path[strlen(nodename) + 9];
>      char fe_state[strlen(nodename) + 7];
> -    xenbus_event_queue events = NULL;
> +    struct xenbus_event_queue events;
> +    xenbus_event_queue_init(&events);
>  
>      snprintf(path, sizeof(path), "%s/backend", nodename);
>      snprintf(fe_state, sizeof(fe_state), "%s/state", nodename);
> @@ -174,7 +175,7 @@ struct pcifront_dev *init_pcifront(char *_nodename)
>  
>      dev->info_ref = gnttab_grant_access(dev->dom,virt_to_mfn(dev->info),0);
>  
> -    dev->events = NULL;
> +    xenbus_event_queue_init(&dev->events);
>  
>  again:
>      err = xenbus_transaction_start(&xbt);
> diff --git a/xen/xenbus/xenbus.c b/xen/xenbus/xenbus.c
> index a06dc30..8a14c3b 100644
> --- a/xen/xenbus/xenbus.c
> +++ b/xen/xenbus/xenbus.c
> @@ -48,10 +48,10 @@ static DECLARE_WAIT_QUEUE_HEAD(xb_waitq);
>  static spinlock_t xb_lock = SPIN_LOCK_UNLOCKED; /* protects xenbus req ring 
> */
>  DECLARE_WAIT_QUEUE_HEAD(xenbus_watch_queue);
>  
> -xenbus_event_queue xenbus_events;
> +struct xenbus_event_queue xenbus_events;
>  static struct watch {
>      char *token;
> -    xenbus_event_queue *events;
> +    struct xenbus_event_queue *events;
>      struct watch *next;
>  } *watches;
>  struct xenbus_req_info 
> @@ -61,6 +61,13 @@ struct xenbus_req_info
>      void *reply;
>  };
>  
> +
> +void xenbus_event_queue_init(struct xenbus_event_queue *queue)
> +{
> +    MINIOS_STAILQ_INIT(&queue->events);
> +}
> +
> +
>  #define NR_REQS 32
>  static struct xenbus_req_info req_info[NR_REQS];
>  
> @@ -78,22 +85,22 @@ static void memcpy_from_ring(const void *Ring,
>      memcpy(dest + c1, ring, c2);
>  }
>  
> -char **xenbus_wait_for_watch_return(xenbus_event_queue *queue)
> +char **xenbus_wait_for_watch_return(struct xenbus_event_queue *queue)
>  {
>      struct xenbus_event *event;
>      DEFINE_WAIT(w);
>      if (!queue)
>          queue = &xenbus_events;
> -    while (!(event = *queue)) {
> +    while (!(event = MINIOS_STAILQ_FIRST(&queue->events))) {
>          add_waiter(w, xenbus_watch_queue);
>          schedule();
>      }
>      remove_waiter(w, xenbus_watch_queue);
> -    *queue = event->next;
> +    MINIOS_STAILQ_REMOVE_HEAD(&queue->events, entry);
>      return &event->path;
>  }
>  
> -void xenbus_wait_for_watch(xenbus_event_queue *queue)
> +void xenbus_wait_for_watch(struct xenbus_event_queue *queue)
>  {
>      char **ret;
>      if (!queue)
> @@ -105,7 +112,7 @@ void xenbus_wait_for_watch(xenbus_event_queue *queue)
>          printk("unexpected path returned by watch\n");
>  }
>  
> -char* xenbus_wait_for_value(const char* path, const char* value, 
> xenbus_event_queue *queue)
> +char* xenbus_wait_for_value(const char* path, const char* value, struct 
> xenbus_event_queue *queue)
>  {
>      if (!queue)
>          queue = &xenbus_events;
> @@ -168,7 +175,7 @@ exit:
>      return msg;
>  }
>  
> -char *xenbus_wait_for_state_change(const char* path, XenbusState *state, 
> xenbus_event_queue *queue)
> +char *xenbus_wait_for_state_change(const char* path, XenbusState *state, 
> struct xenbus_event_queue *queue)
>  {
>      if (!queue)
>          queue = &xenbus_events;
> @@ -227,7 +234,7 @@ static void xenbus_thread_func(void *ign)
>              if(msg.type == XS_WATCH_EVENT)
>              {
>               struct xenbus_event *event = malloc(sizeof(*event) + msg.len);
> -                xenbus_event_queue *events = NULL;
> +                struct xenbus_event_queue *events = NULL;
>               char *data = (char*)event + sizeof(*event);
>                  struct watch *watch;
>  
> @@ -248,8 +255,7 @@ static void xenbus_thread_func(void *ign)
>                      }
>  
>                  if (events) {
> -                    event->next = *events;
> -                    *events = event;
> +                    MINIOS_STAILQ_INSERT_TAIL(&events->events, event, entry);
>                      wake_up(&xenbus_watch_queue);
>                  } else {
>                      printk("unexpected watch token %s\n", event->token);
> @@ -332,6 +338,7 @@ void init_xenbus(void)
>  {
>      int err;
>      DEBUG("init_xenbus called.\n");
> +    xenbus_event_queue_init(&xenbus_events);
>      xenstore_buf = mfn_to_virt(start_info.store_mfn);
>      create_thread("xenstore", xenbus_thread_func, NULL);
>      DEBUG("buf at %p.\n", xenstore_buf);
> @@ -561,7 +568,7 @@ char *xenbus_write(xenbus_transaction_t xbt, const char 
> *path, const char *value
>      return NULL;
>  }
>  
> -char* xenbus_watch_path_token( xenbus_transaction_t xbt, const char *path, 
> const char *token, xenbus_event_queue *events)
> +char* xenbus_watch_path_token( xenbus_transaction_t xbt, const char *path, 
> const char *token, struct xenbus_event_queue *events)
>  {
>      struct xsd_sockmsg *rep;
>  
> -- 
> 1.7.10.4
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@xxxxxxxxxxxxx
> http://lists.xen.org/xen-devel
> 

-- 
Samuel
<P> moo
<N> moo ?
<D> P: keski t'arrive? :))
<P> moooo
<N> moooooo ?
<P> rien le net marche je suis content :)
 -+- #ens-mim - accro du net -+-

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