From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752000AbcETWKE (ORCPT ); Fri, 20 May 2016 18:10:04 -0400 Received: from g1t6214.austin.hp.com ([15.73.96.122]:55388 "EHLO g1t6214.austin.hp.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751032AbcETWKB (ORCPT ); Fri, 20 May 2016 18:10:01 -0400 Message-ID: <1463782185.2479.6.camel@j-VirtualBox> Subject: Re: [PATCH] locking/mutex: Set and clear owner using WRITE_ONCE() From: Jason Low To: Waiman Long Cc: jason.low2@hp.com, Peter Zijlstra , Ingo Molnar , linux-kernel@vger.kernel.org, Davidlohr Bueso , "Paul E. McKenney" , Terry Rudd , Scott J Norton Date: Fri, 20 May 2016 15:09:45 -0700 In-Reply-To: <573F734C.7050708@hpe.com> References: <1463696630.2587.95.camel@j-VirtualBox> <573F734C.7050708@hpe.com> Content-Type: text/plain; charset="UTF-8" X-Mailer: Evolution 3.10.4-0ubuntu2 Mime-Version: 1.0 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Fri, 2016-05-20 at 16:27 -0400, Waiman Long wrote: > On 05/19/2016 06:23 PM, Jason Low wrote: > > The mutex owner can get read and written to without the wait_lock. > > Use WRITE_ONCE when setting and clearing the owner field in order > > to avoid optimizations such as store tearing. This avoids > > situations where the owner field gets written to with multiple > > stores and another thread could concurrently read and use a > > partially written owner value. > > > > Signed-off-by: Jason Low > > --- > > kernel/locking/mutex.h | 10 ++++++++-- > > 1 file changed, 8 insertions(+), 2 deletions(-) > > > > diff --git a/kernel/locking/mutex.h b/kernel/locking/mutex.h > > index 5cda397..469b61e 100644 > > --- a/kernel/locking/mutex.h > > +++ b/kernel/locking/mutex.h > > @@ -17,14 +17,20 @@ > > __list_del((waiter)->list.prev, (waiter)->list.next) > > > > #ifdef CONFIG_MUTEX_SPIN_ON_OWNER > > +/* > > + * The mutex owner can get read and written to locklessly. > > + * We should use WRITE_ONCE() when writing the owner value to > > + * avoid store tearing, otherwise, a thread could potentially > > + * read a partially written and incomplete owner value. > > + */ > > static inline void mutex_set_owner(struct mutex *lock) > > { > > - lock->owner = current; > > + WRITE_ONCE(lock->owner, current); > > } > > > > static inline void mutex_clear_owner(struct mutex *lock) > > { > > - lock->owner = NULL; > > + WRITE_ONCE(lock->owner, NULL); > > } > > #else > > static inline void mutex_set_owner(struct mutex *lock) > > I think mutex-debug.h also needs similar changes for completeness. Good point, I will add the changes to those functions in the debug case to this patch. Thanks, Jason