From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751206AbcEIFg0 (ORCPT ); Mon, 9 May 2016 01:36:26 -0400 Received: from mail-pa0-f49.google.com ([209.85.220.49]:35910 "EHLO mail-pa0-f49.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751167AbcEIFgZ (ORCPT ); Mon, 9 May 2016 01:36:25 -0400 Subject: Re: [PATCH 1/4] locking/rwsem: Avoid stale ->count for rwsem_down_write_failed() To: Davidlohr Bueso References: <1462769770-29363-1-git-send-email-dave@stgolabs.net> <1462769770-29363-2-git-send-email-dave@stgolabs.net> Cc: mingo@kernel.org, peterz@infradead.org, tglx@linutronix.de, Waiman.Long@hpe.com, jason.low2@hp.com, linux-kernel@vger.kernel.org, Davidlohr Bueso From: Peter Hurley Message-ID: <573021D5.4070406@hurleysoftware.com> Date: Sun, 8 May 2016 22:36:21 -0700 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.7.2 MIME-Version: 1.0 In-Reply-To: <1462769770-29363-2-git-send-email-dave@stgolabs.net> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 05/08/2016 09:56 PM, Davidlohr Bueso wrote: > The field is obviously updated w.o the lock and needs a READ_ONCE > while waiting for lock holder(s) to go away, just like we do with > all other ->count accesses. This isn't actually fixing a bug because it's passed through several full barriers which will force reloading from sem->count. I think the patch is ok if you want it just for consistency anyway, but please change $subject and changelog. Regards, Peter Hurley > Signed-off-by: Davidlohr Bueso > --- > kernel/locking/rwsem-xadd.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/kernel/locking/rwsem-xadd.c b/kernel/locking/rwsem-xadd.c > index df4dcb883b50..7d62772600cf 100644 > --- a/kernel/locking/rwsem-xadd.c > +++ b/kernel/locking/rwsem-xadd.c > @@ -494,7 +494,7 @@ __rwsem_down_write_failed_common(struct rw_semaphore *sem, int state) > } > schedule(); > set_current_state(state); > - } while ((count = sem->count) & RWSEM_ACTIVE_MASK); > + } while ((count = READ_ONCE(sem->count)) & RWSEM_ACTIVE_MASK); > > raw_spin_lock_irq(&sem->wait_lock); > } >