From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753496AbcEIHso (ORCPT ); Mon, 9 May 2016 03:48:44 -0400 Received: from merlin.infradead.org ([205.233.59.134]:56996 "EHLO merlin.infradead.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753420AbcEIHsm (ORCPT ); Mon, 9 May 2016 03:48:42 -0400 Date: Mon, 9 May 2016 09:48:30 +0200 From: Peter Zijlstra To: Davidlohr Bueso Cc: mingo@kernel.org, tglx@linutronix.de, Waiman.Long@hpe.com, jason.low2@hp.com, linux-kernel@vger.kernel.org, Davidlohr Bueso Subject: Re: [PATCH 3/4] locking/rwsem: Enable lockless waiter wakeup(s) Message-ID: <20160509074830.GD3430@twins.programming.kicks-ass.net> References: <1462769770-29363-1-git-send-email-dave@stgolabs.net> <1462769770-29363-4-git-send-email-dave@stgolabs.net> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1462769770-29363-4-git-send-email-dave@stgolabs.net> User-Agent: Mutt/1.5.21 (2012-12-30) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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;