Re: [Minios-devel] [UNIKRAFT PATCH 3/3] lib/uklock: Make mutex recursive

[Sharan Santhanam <sharan.santhanam@xxxxxxxxx>]

Hello Costin,

Please find the comments inline.

Thanks & Regards

Sharan

On 9/2/19 3:22 PM, Costin Lupu wrote:
> This patch changes the existing mutex implementation to support multiple locking
> by the same thread.
>
> Signed-off-by: Costin Lupu <costin.lupu@xxxxxxxxx>
> ---
>   lib/uklock/include/uk/mutex.h | 29 +++++++++++++++++++++++------
>   lib/uklock/mutex.c            |  2 ++
>   2 files changed, 25 insertions(+), 6 deletions(-)
>
> diff --git a/lib/uklock/include/uk/mutex.h b/lib/uklock/include/uk/mutex.h
> index ff852b45..f9a41a93 100644
> --- a/lib/uklock/include/uk/mutex.h
> +++ b/lib/uklock/include/uk/mutex.h
> @@ -55,41 +55,55 @@ extern "C" {
>    */
>   struct uk_mutex {
>         int locked;
> +       int depth;
Can't we combine depth with locked?

> +       struct uk_thread *owner;
>         struct uk_waitq wait;
>   };
>   
>   #define	UK_MUTEX_INITIALIZER(name)				\
> -       { 0, __WAIT_QUEUE_INITIALIZER((name).wait) }
> +       { 0, 0, NULL, __WAIT_QUEUE_INITIALIZER((name).wait) }
>   
>   void uk_mutex_init(struct uk_mutex *m);
>   
>   static inline void uk_mutex_lock(struct uk_mutex *m)
>   {
> +       struct uk_thread *current;
>         unsigned long irqf;
>   
>   	UK_ASSERT(m);
>   
> +	current = uk_thread_current();
> +
>         for (;;) {
> -               uk_waitq_wait_event(&m->wait, m->locked == 0);
> +               uk_waitq_wait_event(&m->wait,
> +                       m->locked == 0 || m->owner == current);
>                 irqf = ukplat_lcpu_save_irqf();
> -               if (!m->locked)
> +               if (m->locked == 0 || m->owner == current)
>                         break;
>                 ukplat_lcpu_restore_irqf(irqf);
>         }
>         m->locked = 1;
> +       m->depth++;
> +       m->owner = current;
>         ukplat_lcpu_restore_irqf(irqf);
>   }
>   
>   static inline int uk_mutex_trylock(struct uk_mutex *m)
>   {
> +       struct uk_thread *current;
>         unsigned long irqf;
>         int ret = 0;
>   
>   	UK_ASSERT(m);
>   
> +	current = uk_thread_current();
> +
>         irqf = ukplat_lcpu_save_irqf();
> -       if (!m->locked)
> +       if (m->locked == 0 || m->owner == current) {
>                 ret = m->locked = 1;
> +               m->depth++;
> +               m->owner = current;
> +       }
>         ukplat_lcpu_restore_irqf(irqf);
>         return ret;
>   }
> @@ -107,8 +121,11 @@ static inline void uk_mutex_unlock(struct uk_mutex *m)
>   
>   	irqf = ukplat_lcpu_save_irqf();
>         UK_ASSERT(m->locked);
> -       m->locked = 0;
> -       uk_waitq_wake_up(&m->wait);
> +       if (--m->depth == 0) {
> +               m->locked = 0;
> +               m->owner = NULL;
> +               uk_waitq_wake_up(&m->wait);
> +       }
>         ukplat_lcpu_restore_irqf(irqf);
>   }
>   
> diff --git a/lib/uklock/mutex.c b/lib/uklock/mutex.c
> index 183a01db..07faf1e7 100644
> --- a/lib/uklock/mutex.c
> +++ b/lib/uklock/mutex.c
> @@ -3,5 +3,7 @@
>   void uk_mutex_init(struct uk_mutex *m)
>   {
>         m->locked = 0;
> +       m->depth = 0;
> +       m->owner = NULL;
>         uk_waitq_init(&m->wait);
>   }
_______________________________________________
Minios-devel mailing list
Minios-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/minios-devel