Re: [PATCH] mini-os: Fix forp xenbus and cleanup

[Grzegorz Milos <gm281@cam.ac.uk>]

That looks good. Could you please apply Keir?

Thanks.
Gregor

Dietmar Hahn wrote:
> Hi,
> 
> attached is a small patch which fixes a problem in xenbus.c.
> In xenbus_wait_for_value() the function wait_for_watch() is called which adds 
> an waiter to watch_queue but never removes this.
> Besides this I made wait_for_watch() a global function and changed the types 
> of 2 variables in hypervisor.c to the type the reference variable has.
> Thanks.
> 
> Dietmar.
> 
> 
> ------------------------------------------------------------------------
> 
> # HG changeset patch
> # User dietmar.hahn@fujitsu-siemens.com
> # Date 1174053768 -3600
> # Node ID 60f93bf1835096d6807aedbdffaab9d15054d564
> # Parent  cd8d2a4d46e44dd91fbd7eee3a4553dc9633a9cc
> - Fixed type of variables in hypervisor.c.
> - Fixed not removing a waiter from watch_queue.
> - Make wait_for_watch() global accessible.
> 
> Signed-off-by: Dietmar Hahn <dietmar.hahn@fujitsu-siemens.com>
> 
> diff -r cd8d2a4d46e4 -r 60f93bf18350 extras/mini-os/hypervisor.c
> --- a/extras/mini-os/hypervisor.c	Thu Mar 15 23:03:22 2007 +0000
> +++ b/extras/mini-os/hypervisor.c	Fri Mar 16 15:02:48 2007 +0100
> @@ -35,8 +35,8 @@
>  
>  void do_hypervisor_callback(struct pt_regs *regs)
>  {
> -    u32 	       l1, l2;
> -    unsigned int   l1i, l2i, port;
> +    unsigned long  l1, l2, l1i, l2i;
> +    unsigned int   port;
>      int            cpu = 0;
>      shared_info_t *s = HYPERVISOR_shared_info;
>      vcpu_info_t   *vcpu_info = &s->vcpu_info[cpu];
> diff -r cd8d2a4d46e4 -r 60f93bf18350 extras/mini-os/include/wait.h
> --- a/extras/mini-os/include/wait.h	Thu Mar 15 23:03:22 2007 +0000
> +++ b/extras/mini-os/include/wait.h	Fri Mar 16 15:02:48 2007 +0100
> @@ -74,6 +74,13 @@ static inline void wake_up(struct wait_q
>      local_irq_restore(flags);   \
>  } while (0)
>  
> +#define remove_waiter(w) do {   \
> +    unsigned long flags;        \
> +    local_irq_save(flags);      \
> +    remove_wait_queue(&w);      \
> +    local_irq_restore(flags);   \
> +} while (0)
> +
>  #define wait_event(wq, condition) do{             \
>      unsigned long flags;                          \
>      if(condition)                                 \
> diff -r cd8d2a4d46e4 -r 60f93bf18350 extras/mini-os/include/xenbus.h
> --- a/extras/mini-os/include/xenbus.h	Thu Mar 15 23:03:22 2007 +0000
> +++ b/extras/mini-os/include/xenbus.h	Fri Mar 16 15:02:48 2007 +0100
> @@ -13,6 +13,7 @@ char *xenbus_read(xenbus_transaction_t x
>  char *xenbus_read(xenbus_transaction_t xbt, const char *path, char **value);
>  
>  char *xenbus_watch_path(xenbus_transaction_t xbt, const char *path);
> +void wait_for_watch(void);
>  char* xenbus_wait_for_value(const char*,const char*);
>  
>  /* Associates a value with a path.  Returns a malloc'd error string on
> diff -r cd8d2a4d46e4 -r 60f93bf18350 extras/mini-os/xenbus/xenbus.c
> --- a/extras/mini-os/xenbus/xenbus.c	Thu Mar 15 23:03:22 2007 +0000
> +++ b/extras/mini-os/xenbus/xenbus.c	Fri Mar 16 15:02:48 2007 +0100
> @@ -72,11 +72,12 @@ static void memcpy_from_ring(const void 
>      memcpy(dest + c1, ring, c2);
>  }
>  
> -static inline void wait_for_watch(void)
> +void wait_for_watch(void)
>  {
>      DEFINE_WAIT(w);
>      add_waiter(w,watch_queue);
>      schedule();
> +    remove_waiter(w);
>      wake(current);
>  }
>