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;
