On Fri, Jun 14, 2019 at 02:41:20PM +0200, Christian König wrote:
> The ww_mutex implementation allows for detection deadlocks when multiple
> threads try to acquire the same set of locks in different order.
> 
> The problem is that handling those deadlocks was the burden of the user of
> the ww_mutex implementation and at least some users didn't got that right
> on the first try.
> 
> I'm not a big fan of macros, but it still better then implementing the same
> logic at least halve a dozen times. So this patch adds two macros to lock
> and unlock multiple ww_mutex instances with the necessary deadlock handling.
> 
> Signed-off-by: Christian König <christian.koe...@amd.com>
> ---
>  include/linux/ww_mutex.h | 75 ++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 75 insertions(+)
> 
> diff --git a/include/linux/ww_mutex.h b/include/linux/ww_mutex.h
> index 3af7c0e03be5..a0d893b5b114 100644
> --- a/include/linux/ww_mutex.h
> +++ b/include/linux/ww_mutex.h
> @@ -369,4 +369,79 @@ static inline bool ww_mutex_is_locked(struct ww_mutex 
> *lock)
>       return mutex_is_locked(&lock->base);
>  }
>  
> +/**
> + * ww_mutex_unlock_for_each - cleanup after error or contention
> + * @loop: for loop code fragment iterating over all the locks
> + * @pos: code fragment returning the currently handled lock
> + * @contended: the last contended ww_mutex or NULL or ERR_PTR
> + *
> + * Helper to make cleanup after error or lock contention easier.
> + * First unlock the last contended lock and then all other locked ones.
> + */
> +#define ww_mutex_unlock_for_each(loop, pos, contended)       \
> +     if (!IS_ERR(contended)) {                       \
> +             if (contended)                          \
> +                     ww_mutex_unlock(contended);     \
> +             contended = (pos);                      \
> +             loop {                                  \
> +                     if (contended == (pos))         \
> +                             break;                  \
> +                     ww_mutex_unlock(pos);           \
> +             }                                       \
> +     }
> +
> +/**
> + * ww_mutex_lock_for_each - implement ww_mutex deadlock handling
> + * @loop: for loop code fragment iterating over all the locks
> + * @pos: code fragment returning the currently handled lock
> + * @contended: ww_mutex pointer variable for state handling
> + * @ret: int variable to store the return value of ww_mutex_lock()
> + * @intr: If true ww_mutex_lock_interruptible() is used
> + * @ctx: ww_acquire_ctx pointer for the locking
> + *
> + * This macro implements the necessary logic to lock multiple ww_mutex
> + * instances. Lock contention with backoff and re-locking is handled by the
> + * macro so that the loop body only need to handle other errors and
> + * successfully acquired locks.
> + *
> + * With the @loop and @pos code fragment it is possible to apply this logic
> + * to all kind of containers (array, list, tree, etc...) holding multiple
> + * ww_mutex instances.
> + *
> + * @contended is used to hold the current state we are in. -ENOENT is used to
> + * signal that we are just starting the handling. -EDEADLK means that the
> + * current position is contended and we need to restart the loop after 
> locking
> + * it. NULL means that there is no contention to be handled. Any other value 
> is
> + * the last contended ww_mutex.
> + */
> +#define ww_mutex_lock_for_each(loop, pos, contended, ret, intr, ctx) \
> +     for (contended = ERR_PTR(-ENOENT); ({                           \
> +             __label__ relock, next;                                 \
> +             ret = -ENOENT;                                          \
> +             if (contended == ERR_PTR(-ENOENT))                      \
> +                     contended = NULL;                               \
> +             else if (contended == ERR_PTR(-EDEADLK))                \
> +                     contended = (pos);                              \
> +             else                                                    \
> +                     goto next;                                      \
> +             loop {                                                  \
> +                     if (contended == (pos)) {                       \
> +                             contended = NULL;                       \
> +                             continue;                               \
> +                     }                                               \
> +relock:                                                                      
> \
> +                     ret = !(intr) ? ww_mutex_lock(pos, ctx) :       \
> +                             ww_mutex_lock_interruptible(pos, ctx);  \
> +                     if (ret == -EDEADLK) {                          \
> +                             ww_mutex_unlock_for_each(loop, pos,     \
> +                                                      contended);    \
> +                             contended = ERR_PTR(-EDEADLK);          \
> +                             goto relock;                            \
> +                     }                                               \
> +                     break;                                          \
> +next:                                                                        
> \
> +                     continue;                                       \
> +             }                                                       \
> +     }), ret != -ENOENT;)
> +

Yea gawds, that's eyebleeding bad, even for macros :/

It also breaks with pretty much all other *for_each*() macros in
existence by not actually including the loop itself but farming that out
to an argument statement (@loop), suggesting these really should not be
called for_each.


_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Reply via email to