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;