From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751454AbcETXB7 (ORCPT ); Fri, 20 May 2016 19:01:59 -0400 Received: from mail-bn1bon0114.outbound.protection.outlook.com ([157.56.111.114]:8199 "EHLO na01-bn1-obe.outbound.protection.outlook.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1751179AbcETXB6 (ORCPT ); Fri, 20 May 2016 19:01:58 -0400 Authentication-Results: hpe.com; dkim=none (message not signed) header.d=none;hpe.com; dmarc=none action=none header.from=hpe.com; Message-ID: <573F9757.9020805@hpe.com> Date: Fri, 20 May 2016 19:01:43 -0400 From: Waiman Long User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:10.0.12) Gecko/20130109 Thunderbird/10.0.12 MIME-Version: 1.0 To: Jason Low CC: Peter Zijlstra , Ingo Molnar , , Davidlohr Bueso , "Paul E. McKenney" , Terry Rudd , Scott J Norton , Jason Low Subject: Re: [PATCH v2] locking/mutex: Set and clear owner using WRITE_ONCE() References: <1463782776.2479.9.camel@j-VirtualBox> In-Reply-To: <1463782776.2479.9.camel@j-VirtualBox> Content-Type: text/plain; charset="UTF-8"; format=flowed Content-Transfer-Encoding: 7bit X-Originating-IP: [72.71.243.228] X-ClientProxiedBy: BY1PR20CA0034.namprd20.prod.outlook.com (10.162.140.44) To AT5PR84MB0305.NAMPRD84.PROD.OUTLOOK.COM (10.162.138.27) X-MS-Office365-Filtering-Correlation-Id: 64a17395-0e3e-466c-09fb-08d38102bd37 X-Microsoft-Exchange-Diagnostics: 1;AT5PR84MB0305;2:rN2XqYdBcz8fqwhR0hi7hg0nr0w507dWDJeVO31exLCQguHHjJYHqzrAPRYVWCGG3PewWfI500bzEursQv9O18K0hBTDMPkwgNwtjf8yGTMmAnW0V2uoifKc7569sQiTeO6TXT/+JfltiRIBaeaBE+hPyAX3zhHINzXssDz4G5+1lPc/clIt6Sr8Xz63/WFX;3:53icYkBDQ/LzM7p4vjiZu/DzGFcznZ8fgOVotQIwSZctEPOtZvpCQgOGV1doCM6ExTJdNVHm4CZbjrgLw4TmV2uSVPOkjkxHArkxiuB+MpH5W7hwK0fqE91lna5URj+V;25:Nue/A//QS8khSHViopI/9COkYIA/dqN+hFL4fB7Jh85DoWgMWASXADH0NohVcd2wh1RobnsCLumbnpC56bjzFNSpOK/wSCmu5fq1SXIlcJ2BMXYAgkDvAUS5uDHeL/yN6TxCj3tcmI399mUFLARqXAz08gEl8u/L0hH8rgTpe8xOGitAWXItqAXd3vUSr8pFEO8gN7dKLfBN38n3Nts4bCSuOKFqhkuisYnvsGhmB7MLW7vLxPjNQoIs9/XjaCDKp3Qk6dZy4YRE8783jJaTy+RZRL3HQ2DGBpxBiBwjqSdwMRDylBaTfr848eH5awyXWXrWE4SfI7il/lfv4oWFz/f92vDMwcYqsyXPNAtvFDhLKxklSDFTcyUsuuqyXWekBs0SLv0PZuSqa5cinCg3hJmH+5KTBvryBNnae5+YXSw= X-Microsoft-Antispam: UriScan:;BCL:0;PCL:0;RULEID:;SRVR:AT5PR84MB0305; X-LD-Processed: 105b2061-b669-4b31-92ac-24d304d195dc,ExtAddr X-Microsoft-Exchange-Diagnostics: 1;AT5PR84MB0305;20:xh3HxsCiE9tRbzP/KzRqFvj9SBQwyFYL77pFkA6PrhvXDihj6vyo1NQ9H+lNKIBHbHt38XROEX/Gh8eb/LGSP4cq9XlkQ8eOnYRaeKBag+e9TXU2JlRHtMsik9WXCXf4H8ZhybX8OveuvBkiOQBscFdji6jOUNkA0E6DOE0EqL8/C4rUFmcKkZAkvjuU2YQnabJKuxxqMoUQaIyuhzK/Ll/g958/7OY3gEKgUoQfdXdITCat+WlQ8YK5Z2+TExrbIAIR4s97ydaYcugopTxKsMY99igz4nin8sYn2j6D7j3W01HFlRwctOK4hoDbnVE4DBIK1zpzyfZ0/aUwi3kxTQ==;4:UjnpDgntDL8uRqQree8zvdQU+Ph6ktcRbmCIQxefjLlHEy/2LvZm5jfHK66nCuuyB+8xHF2zW55+4UWueR3g6Z99/+OQjpwtnWSFd1UVIDaSRkrJv/dvuEUqsb4zrHFVcrRc+6B+Y9Lh07aJXl2oixijqCq0imaavWDXEktbt22/PVZlp5BXxfeWjltGlJPYqfPdf1G6mMIvKzxSfT5uxCkCyJTv0tp2V5q7SXXLnMa4uqvLarpLq2znF2BV4dppVNrX5RZRTYqnIPqzKUn5yNkRfG2AaZOTs0p49qT+FV3fQ48c2RyaBsqukCJx1xL2oMRpUMSF2GLJxfj7IyrD5ORLb/t7k8n1t8MvaV5PAv//XfDjeiCRhBMry1cjZD/4 X-Microsoft-Antispam-PRVS: X-Exchange-Antispam-Report-Test: UriScan:; X-Exchange-Antispam-Report-CFA-Test: BCL:0;PCL:0;RULEID:(601004)(2401047)(8121501046)(5005006)(10201501046)(3002001);SRVR:AT5PR84MB0305;BCL:0;PCL:0;RULEID:;SRVR:AT5PR84MB0305; X-Forefront-PRVS: 09480768F8 X-Forefront-Antispam-Report: SFV:NSPM;SFS:(10019020)(4630300001)(6049001)(6009001)(24454002)(377454003)(64126003)(4001350100001)(36756003)(6116002)(189998001)(65956001)(65806001)(66066001)(3846002)(59896002)(81166006)(8676002)(47776003)(92566002)(117156001)(2906002)(23676002)(586003)(4326007)(8666003)(110136002)(2950100001)(50986999)(83506001)(5008740100001)(86362001)(50466002)(19580395003)(80316001)(19580405001)(230700001)(42186005)(87266999)(76176999)(54356999)(33656002)(77096005)(5004730100002)(7059030);DIR:OUT;SFP:1102;SCL:1;SRVR:AT5PR84MB0305;H:[192.168.142.135];FPR:;SPF:None;MLV:sfv;LANG:en; X-Microsoft-Exchange-Diagnostics: =?utf-8?B?MTtBVDVQUjg0TUIwMzA1OzIzOnlPRGZESGlhUy9PTXNsMDRaeGJIZ2wwakxI?= =?utf-8?B?dDlPbVJUcDlzT2c5dWJjTDZtbDZmcmpLajRUNkpYL0Nic0lDelJpZzI5NUt4?= =?utf-8?B?WDhTR0VKZVZsSWxlSUFGUHcwN0oyMEI0enVGanV4UmpHOWRwVjZnY01lNy9K?= =?utf-8?B?L3R3d2JqU251YSs1aFp5L3BSNFNoUGU1c0lpVWp2UHo1VlRUUEQ2eXdJZ2xD?= =?utf-8?B?OENkVThrNHBaUzE5Ui9TWGtJTlJTTkswN2dYSHppK2t3cWI2U3pvbzkxKzhY?= =?utf-8?B?Z29WN1RVaUNkaElwcjlIZGlVazZlMk9WODNaSFBwWWpDbXEwa1NxdCt2SXN5?= =?utf-8?B?bEVieFc4UTRMaFZ4aElpV1F3UDExTkUzOXhCZ1NiekdleWZ0T0NCL1N2OGhV?= =?utf-8?B?NUV0R0VYVUx5OTVkOTF4TTdwY0QwY3ZJQ2lTditsNTgzVEl4V0E2UU9mR2pH?= =?utf-8?B?SCtCWTd5emE2cDQ1U0dxakVEUzdHSk9vdjNzWW1uQmcvWk1VbW0wYXM5U2ZM?= =?utf-8?B?WlVwekFUSHZHQmNobXlhTGp3TmE2TUdDaS83QS9LTW5tdzhwOHJPV3BqYWlS?= =?utf-8?B?RlcvYm1CaEkyTnlrWHpXNzZ5RUZlTVV0Mi9MRHlQaDBUajRUN01aMmIrbWpE?= =?utf-8?B?bkJiUU9PVm5NMmx0V3lHRUdpa2kzc3owUjZWTVUrcG9HbHRSbGFxZ2k0Vjdz?= =?utf-8?B?N1QycGxtRWxZTnJwL2Q4Vlp0ZVpzOUFSZXVYM21LNDgyRXFRMU1ETm1GVWxC?= =?utf-8?B?TnQ3RUJISVAzMkVXcXZtZTAyb3ovdEdIcGQ0Z1l6TnljTHRaNzFEUGZwTEY1?= =?utf-8?B?ZEpRZjBQQVNhQklEdUNPengwN0YyQVZxa3U0NHY0OEhSTlRqVHI5RGJ2dmtB?= =?utf-8?B?dGhTQ205UC9kRW5JTTZnVGhtT3R5R3lZSWp1enZ0ajNYNzJaZnlRaXVsL3ZF?= =?utf-8?B?MUJZeWhEVTA4dk1SelpiVVp1TVZNd0N5dFRqcHp3R2NBOTRZeTY4NkVYdW43?= =?utf-8?B?enNiUlZoTHkyMmlBVGs5Z200NWYzV29tam5VZThQSzFpb0o0WGluTy84ODgr?= =?utf-8?B?UjhRaG42WldEZTg1dlBpYmJKY0lHaUFBWlQ5V2ZGSkZmYVZWSXovZUMzM3l0?= =?utf-8?B?dDVPQ0trSlNRaW01TUVOeS81bFlqcGVldlhNTkxYVE1jdUVoN1FJZXpsNm0x?= =?utf-8?B?QVF1STV2SG5kY090Q25sZEp6TEUyci9CeDdGRDB2NnpyQ2Z6S1RZVGhwNytx?= =?utf-8?B?M1pSdUxhNEZsUzlWb2Q3eDNzSTlGVVZaUlFJQlRVZmxlSXhuR1pFYUNyb1NH?= =?utf-8?B?ZmhWU2o3dWZKa2l5VTRpc3FxdnpjK3F3bkJKK3h6WjAvMjIzbWhYS2l4YlZC?= =?utf-8?B?SXpGR3VCVVluelUwUHVlYVVoSXRFTzVjcjV6cXpaeG5JYUIvRkdTR0JzTzc5?= =?utf-8?B?cVBnaHJYOSs1NFkxZ2hmcngwcTdKbi9lbWtlQ0oxblpkb3hYd1BORG5EKy9C?= =?utf-8?Q?5zo3mTs8JrEeJlyL5+/QhPRKcuo6iXN/zY5mLjQbNfnq8J?= X-Microsoft-Exchange-Diagnostics: 1;AT5PR84MB0305;5:M51c4GFiza9gWSDBbRVJ2YX0RA2Ha0dAgAx68AvUFv0rcR1S7pj10MTckjUD2RADMlsscqEVSFeQQBQLDHY1xu8GGXWW/cceewshk1C5M1mAgIs+ABOsGa82vgMqQttsoPCDDoaJrw4PyuN1NMV+9w==;24:CBZNm0SfA89qjBZFm+ZDOf4U8Igc/Eq4kZKBCKRt8oF8hfpaL/BKnV4zBvlCmAkDqlRvQhr5TsmBeCY/Ui6DOM0yRypHNXRxH7lhItZqaAg=;7:bSmy0NhkPG2TvWtKseklr0PSD3N+8P1xR5xOE9vppKGAZKDYLSejmqWvFt7Zd5zd8KxDUNEHufauWs02IaiP1H7UqsFNayEsP+v7gr0D5FqyfmB8GTS/SadecgW0YUq9n434TyFdfUe4yqTXScdQP4BH6tJxXnkFDFktr6EOBYws/vxluWxkiR4GtWZ0s1kO SpamDiagnosticOutput: 1:23 SpamDiagnosticMetadata: NSPM X-OriginatorOrg: hpe.com X-MS-Exchange-CrossTenant-OriginalArrivalTime: 20 May 2016 23:01:53.7070 (UTC) X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted X-MS-Exchange-Transport-CrossTenantHeadersStamped: AT5PR84MB0305 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 05/20/2016 06:19 PM, Jason Low wrote: > The mutex owner can get read and written to locklessly. > 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 > Acked-by: Davidlohr Bueso > --- > kernel/locking/mutex-debug.h | 4 ++-- > kernel/locking/mutex.h | 10 ++++++++-- > 2 files changed, 10 insertions(+), 4 deletions(-) > > diff --git a/kernel/locking/mutex-debug.h b/kernel/locking/mutex-debug.h > index 0799fd3..372e653 100644 > --- a/kernel/locking/mutex-debug.h > +++ b/kernel/locking/mutex-debug.h > @@ -29,12 +29,12 @@ extern void debug_mutex_init(struct mutex *lock, const char *name, > > 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); > } > > #define spin_lock_mutex(lock, flags) \ > diff --git a/kernel/locking/mutex.h b/kernel/locking/mutex.h > index 5cda397..12f9619 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) Acked-by: Waiman Long