Re: [PATCH v2 02/11] locking/ww_mutex: Re-check ww->ctx in the inner optimistic spin loop

From: Peter Zijlstra
Date: Tue Dec 06 2016 - 10:06:46 EST


On Thu, Dec 01, 2016 at 03:06:45PM +0100, Nicolai Hähnle wrote:
> +++ b/kernel/locking/mutex.c
> @@ -350,7 +350,8 @@ ww_mutex_set_context_slowpath(struct ww_mutex *lock,
> * access and not reliable.
> */
> static noinline
> -bool mutex_spin_on_owner(struct mutex *lock, struct task_struct *owner)
> +bool mutex_spin_on_owner(struct mutex *lock, struct task_struct *owner,
> + bool use_ww_ctx, struct ww_acquire_ctx *ww_ctx)
> {
> bool ret = true;
>
> @@ -373,6 +374,28 @@ bool mutex_spin_on_owner(struct mutex *lock, struct task_struct *owner)
> break;
> }
>
> + if (use_ww_ctx && ww_ctx->acquired > 0) {
> + struct ww_mutex *ww;
> +
> + ww = container_of(lock, struct ww_mutex, base);
> +
> + /*
> + * If ww->ctx is set the contents are undefined, only
> + * by acquiring wait_lock there is a guarantee that
> + * they are not invalid when reading.
> + *
> + * As such, when deadlock detection needs to be
> + * performed the optimistic spinning cannot be done.
> + *
> + * Check this in every inner iteration because we may
> + * be racing against another thread's ww_mutex_lock.
> + */
> + if (READ_ONCE(ww->ctx)) {
> + ret = false;
> + break;
> + }
> + }
> +
> cpu_relax();
> }
> rcu_read_unlock();

Aside from the valid question about mutex_can_spin_on_owner() there's
another 'problem' here, mutex_spin_on_owner() is marked noinline, so all
the use_ww_ctx stuff doesn't 'work' here.

As is, I think we're missing an __always_inline on
mutex_optimistic_spin, I'll have to go see what that does for code
generation, but given both @use_ww_ctx and @waiter there that makes
sense.