Re: [PATCH 3/4] locking/rwsem: Enable lockless waiter wakeup(s)

[Peter Zijlstra]

On Sun, May 08, 2016 at 09:56:09PM -0700, Davidlohr Bueso wrote:
> @@ -129,12 +133,14 @@ __rwsem_do_wake(struct rw_semaphore *sem, enum rwsem_wake_type wake_type)
>  	waiter = list_entry(sem->wait_list.next, struct rwsem_waiter, list);
>  	if (waiter->type == RWSEM_WAITING_FOR_WRITE) {
>  		if (wake_type == RWSEM_WAKE_ANY)
> -			/* Wake writer at the front of the queue, but do not
> -			 * grant it the lock yet as we want other writers
> -			 * to be able to steal it.  Readers, on the other hand,
> -			 * will block as they will notice the queued writer.
> +			/*
> +			 * Mark writer at the front of the queue for wakeup.
> +			 * Until the task is actually later awoken later by
> +			 * the caller, other writers are able to steal it the
> +			 * lock to be able to steal it.  Readers, on the other,
> +			 * hand, will block as they will notice the queued writer.
>  			 */
> -			wake_up_process(waiter->task);
> +			wake_q_add(wake_q, waiter->task);

Thanks for fixing that comment; that bugged the hell out of me ;-)

>  		goto out;
>  	}
>  
> @@ -196,12 +202,11 @@ __rwsem_do_wake(struct rw_semaphore *sem, enum rwsem_wake_type wake_type)
>  		 */
>  		smp_mb();
>  		waiter->task = NULL;
> -		wake_up_process(tsk);
> +		wake_q_add(wake_q, tsk);


However, note that per the race in the previous email; this is too late
to acquire the tsk refcount. I think it'll work if you do wake_q_add()
_before_ the waiter->task = NULL.

On that same note; I think that you can replace:

	smp_mb();
	waiter->task = NULL;

with:

	smp_store_release(&waiter->task, NULL);

>  	} while (--loop);
>  
>  	sem->wait_list.next = next;
>  	next->prev = &sem->wait_list;