From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751168AbcETU06 (ORCPT ); Fri, 20 May 2016 16:26:58 -0400 Received: from mail-by2on0146.outbound.protection.outlook.com ([207.46.100.146]:37570 "EHLO na01-by2-obe.outbound.protection.outlook.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1750795AbcETU05 (ORCPT ); Fri, 20 May 2016 16:26:57 -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: <573F7302.6040609@hpe.com> Date: Fri, 20 May 2016 16:26:42 -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: , Davidlohr Bueso , Peter Zijlstra , Ingo Molnar , , "Paul E. McKenney" , Scott J Norton , Douglas Hatch Subject: Re: [PATCH v4 2/5] locking/rwsem: Protect all writes to owner by WRITE_ONCE References: <1463534783-38814-1-git-send-email-Waiman.Long@hpe.com> <1463534783-38814-3-git-send-email-Waiman.Long@hpe.com> <20160518140436.GA6273@linux-uzut.site> <1463592095.3369.10.camel@j-VirtualBox> <573CB496.4010707@hpe.com> <1463601515.2587.24.camel@j-VirtualBox> <1463696487.2587.93.camel@j-VirtualBox> In-Reply-To: <1463696487.2587.93.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: BY1PR20CA0021.namprd20.prod.outlook.com (10.162.140.31) To TU4PR84MB0320.NAMPRD84.PROD.OUTLOOK.COM (10.162.186.30) X-MS-Office365-Filtering-Correlation-Id: 629e911e-2ffc-47ad-c71c-08d380ed1557 X-Microsoft-Exchange-Diagnostics: 1;TU4PR84MB0320;2:2RsbaXYQAxHUpryrR5Dy0aStsymZLEgrQQ5vRFTaP6hfUaZNU8PUA7d22yNXRRoS0WlwsdcJDwAFh0AktQTz1SurTKCGD3Syh2YoCvP3v+W1MN3hL1q3Wy2Pl+hJifXryba8hRej50KAkfnOC4Xo4yHTWQD7K5x48/U0F51Aa9POQZxH4QgWwFRWoC92lXiB;3:UzDvRouqXsr9i3+Po9T7EeYaZSUMGVNsHly4+6mzNl6GXeU2axWScPTG5Xq6BCQAPBJynN2TxwrEtGLOBBmS65O/MaumqhZkkAPo1jsVssgRJSCJk/Cts2O8JfVFhflq X-Microsoft-Antispam: UriScan:;BCL:0;PCL:0;RULEID:;SRVR:TU4PR84MB0320; X-Microsoft-Exchange-Diagnostics: 1;TU4PR84MB0320;25:ODyNPM9LnqWsajk9oXiLMlOwZ3kSzQVrAVFbvPPPEiFRpebPcjbneW2YHW420BxaQHQyCMe8a00SFt2emE5a99XMGcrAFgs1/foCc5XT50Ylyijf77hHafY8zF/1PMvl5sAZAbFnbhX/DOiyTFSBOXhdCMhSUp0JqkocxEnyO0OAxy1UBQagpo+ZvvXCWGpsvXTp6v7sbpDbsJSsAeb6QQ4cCwgQ8SVd3LoHJ6WVbF42RfXAt9Gg3nJ8TWrYFFf/fygKrVMllcwv7vNry/Ly5Ecm54XzJ7frngKnBkvAe7hn5+Itxy3/TMStVbi9/cY7V9wcF+EE/yOXGb7qnfRFDDOES9dOJBBsQFXHf2OBiaXOJg+C/T2YcWaYXTYzqO9kbfglTAXudYWOMM3KtVzlr5EhU9Vk3W7Jt42KnxwbzeTS43OT+66u1WN2/zNwiKrBPBxLaMVVXBhVKxpVLtHZbmugbtFDSFPnx6rcIp6P2W+5mHVini2d5eMoJpc8J0oURbqk5wJiFtifK5nub0AxOCD731HxpUjSmjcdP5uEPgS71n0F+cwTz0ybE92sMYfOCqzAUqvcdpyIobrjR+qN4n1XFdAS34jTrBmArQxmDL2VP86GccteNZ3E18xufJEukRa8sN8D9vPslW5qVp/wK60WcF6XCP3vRzg2oQqEkftoSy0HPvNa215OrY+o9c7zY3UA8ooJNF0dXXgtM6wFTA== X-LD-Processed: 105b2061-b669-4b31-92ac-24d304d195dc,ExtAddr X-Microsoft-Exchange-Diagnostics: 1;TU4PR84MB0320;20:tPxdWHymP65dIgNsA8VHvunCS+9cYWR9hHVDawiDlNm75nQPtyVMfeq5g0+hFf3jkdNS+olle3TD+ZAaLxepuOf6Dm1byX2EefOtBpn3vF/AbTd3Me9VLC35KetUU6Sl3GS2OknMVRPwnDdt/3tZ8bKkgJhHaqtpPLapbsAWxPfQ2AxsQyUaIlqzI0h9Gwd5EJRMB/DtrYRtUvs9JNV1CxxgboL52KwmUaIzpVtZAX90w9FtWfaWwWRJWTR875j+aBMf94rgVUYako8tA/guG6UlKlTDJcptOZkzd+/M/G5g4YnSo+yVtutYY8np+L1Pa0uCWVUloITjhcKWMtBXVA==;4:O1D+Gq/w3hqxB+WU56f6suPUeqWKnw6K7w17Cswh4k+h9idwIuv6J1xWnNo78XkAWEkEnKbUudGgDrWF3P6wUhBeywl9H5SBM5Xi1b32ao112dW1ABfpoIB1c42Hot9+Mwp0Z12db1L3bC7j9RoQb/Dzr6QGNOtFYz5Yft8tjwJ51mr3qys4MXPDUMZXO4BLU95UyBnIIdWncdV2ru1hN7UViK3gUpcGMGS9TTEbrrkn4QrztFV/r8wS6gqFVScjtAFik5y1gQZCceJCB/LYVqjqw8a0fDZOUjvPNqRnEGQcsxeMNhHRjdxM5eK6HRzIWJLZlkERSayRrwVxelamf9zxQ5a5eSTARXYXe/xwvgJHdY4rrBxZWNRg1YfiEBAZ 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:TU4PR84MB0320;BCL:0;PCL:0;RULEID:;SRVR:TU4PR84MB0320; X-Forefront-PRVS: 09480768F8 X-Forefront-Antispam-Report: SFV:NSPM;SFS:(10019020)(4630300001)(6009001)(6049001)(24454002)(377454003)(377424004)(42186005)(4001350100001)(92566002)(189998001)(7520500002)(65956001)(65806001)(50466002)(64126003)(86362001)(59896002)(66066001)(117156001)(77096005)(47776003)(8666003)(230700001)(110136002)(36756003)(6116002)(3846002)(87266999)(54356999)(76176999)(5004730100002)(586003)(2906002)(83506001)(93886004)(2950100001)(33656002)(65816999)(50986999)(4326007)(23676002)(8676002)(5008740100001)(81166006)(7059030);DIR:OUT;SFP:1102;SCL:1;SRVR:TU4PR84MB0320;H:[192.168.142.135];FPR:;SPF:None;MLV:sfv;LANG:en; X-Microsoft-Exchange-Diagnostics: =?utf-8?B?MTtUVTRQUjg0TUIwMzIwOzIzOmZGdDQ5eW9BRTRxY2psNFpIVDl4YlVqOUVJ?= =?utf-8?B?N1dRMWtxTVJIUHV1SE1TdXEra3RuN3Zzak1CakROVlNhT1pQU1pxeUlxcUdM?= =?utf-8?B?VHdNODdGZGhobVNScmV5R1ViaVpvRUZ1K0phTDh1V3hyUGNOU0xTVEpINWNZ?= =?utf-8?B?R3RMTjNqQWpwOFF3T09DWTREM2lJVWh3ekRXLzJmZm0yR2ppSXRaMjE0SlQz?= =?utf-8?B?U1MrUXVMdTl3V3BvYnYvQ2loUExkL3dEcG5jUE5xUzd2ZFN4dkJ3N1dLWkNn?= =?utf-8?B?WmdRTzJUT2pFVTB5Z0VPMnV5eTBQVTFEaldpNTRwdEoyR2NTemlqWitKQ0NO?= =?utf-8?B?N0FmZ3lUbDFMVmY0NjRRdFB6RWNmYTdrWG1HU2Y5VE5aUmhqZFVpclBYSEMy?= =?utf-8?B?N2tDMzJlOGduU2dUaGdJaVk5d0JXdGZXRWZpMHRTWmVPREZjMnpXQWN2eTdn?= =?utf-8?B?eW5lS1hyTDViN0ZPMWZwQlordEpyd2tzYUN6MFZsaHFEUWNWU2tKYUpjdElh?= =?utf-8?B?UzVkaGc3UVc3aEhvbm1RZlZoU2hXSy9ubjNzc0l3SnFDNVBWL25NVTljbW93?= =?utf-8?B?a3Z0ZFFYRzV4YmkzNFRFSjFjcGpvMVY1ZkZlU3d5SFlHR0lISW9VbjlJZXNq?= =?utf-8?B?V1FZVWcyd1NDd2lia0JrZXRUbjZ1RmpXYWlHb1Fsd0ZVVUk4cTNvclJuczBy?= =?utf-8?B?a1VVN2tvalVzazVXK1FkUHhDbjYxN04wTGkzbklJUldtdVZON2RWZDBwOVc5?= =?utf-8?B?bldldHlqOFdLR3VVcWJBU0sxbEx4WDhIaC9Za0c4ajNNRTNtV1dXZ0lYUDdz?= =?utf-8?B?L1h2SGRrSnFsWUhWSi9jZEdkTG9YNjV0UG9tTjJUdm05NklsK2tsL1RUdDll?= =?utf-8?B?elIrTWI4eTZWSEY1Yzdzb3NudEoxK2QvdFAvSWt6RHgvdGF2WjhiVEpDY3lB?= =?utf-8?B?bkQ2ZHdNdE5ZdE9MN09XRERRSXQreCtuNzVPZHlIbzlvSzE0UXhNUy9rdDZF?= =?utf-8?B?OHhMSmtzWUcyY2lydVhrU3NHdHozWHIrVEhGQzBlYk4rem5qZlJiWk9aMlhi?= =?utf-8?B?WFpCL0RTYk1VWXFsaFdndkVleDlYRFRqQWtnRzhDNmwwNmJwa1NyL1pKaUU5?= =?utf-8?B?VFUvUjZhd29sYVR1V2dEay9hV2RQL1JYOVB0TnZRRjJQWGdjT0lMVWFrVWg5?= =?utf-8?B?aHdKTmNvazY5Sm5BZlRrSGJOMjMrYmhpRUk1ZVN2TlF3OGQ1MDh6WGpXaXpv?= =?utf-8?B?bTVRMWdtRWZleXlqTHZ0TDh3SUhLdThOZC9qSDdmTU1mdEU3RmRGU2tIK2cx?= =?utf-8?B?Z2s5bDRtTFZhUERpcFY2MXp6WnNtYlRDQTlTUWFDMS9ZNGN6c2pvRTFPSXJW?= =?utf-8?B?WWtpRmJzUG5VbnVqTmtKdVBTeVpjZWh2ZmhrcUxSWjVpWUJKWFpXM3htTUMy?= =?utf-8?B?enkxMmlxcmZ3ekd2Y3FhWGFPRjNoL0lVMnZaVkxIdFd6ZGlnRHlYcElOdmFz?= =?utf-8?B?d2hZTXNWR2s4ZjI2ZWxBSUROcGloNFRCRWJCcWxQSjJJYWxXVWN2WFlVMWdm?= =?utf-8?B?R1RTaGc0Q25RN1N6RUF0dlNvUnNpY3c9PQ==?= X-Microsoft-Exchange-Diagnostics: 1;TU4PR84MB0320;5:3IayBormfj5DfpL9UDE0OSkCzUh6jdG41xvGEmA4O63JtHCFnCzhI7WPC/u1KWZ4/Zk8y3ebavzYSgWaZxsARjV0dfy7ub/m/0/aVvcHkiPM1zCbLPmtCXQ7VmJrWjXwA+AYJmqksqj5vbIcFI9PGQ==;24:QdU9V3lwoLMJNQA+8jCicHRCGzCy2uaTi/3LnnxJNJoMpEQS8kS2UrgLUOMu5w1MegJ291eaw8hr0MNrTzePW5/O7AfUJ0fem7+s+N6rsnw=;7:ebf4xlMy0YhcIYtPH8io7zJG5tZWc2IgbknyhmwtkdVHOLUuwgFZpb/0m3j1/cVNRMX0TNM6waQ+wHf4Sh90g0p5IYls1YVpJNqAxBwtVhRMVYHXkPtstRMsCbCG1Z/gSZ5FJ338HWaBU5kFV17CGYqgc24mXEwCuVnAU03Mtl8OOVx00u8/hXmAU+qwD9W3 SpamDiagnosticOutput: 1:23 SpamDiagnosticMetadata: NSPM X-OriginatorOrg: hpe.com X-MS-Exchange-CrossTenant-OriginalArrivalTime: 20 May 2016 20:26:52.7112 (UTC) X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted X-MS-Exchange-Transport-CrossTenantHeadersStamped: TU4PR84MB0320 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 05/19/2016 06:21 PM, Jason Low wrote: > On Wed, 2016-05-18 at 12:58 -0700, Jason Low wrote: >> On Wed, 2016-05-18 at 14:29 -0400, Waiman Long wrote: >>> On 05/18/2016 01:21 PM, Jason Low wrote: >>>> On Wed, 2016-05-18 at 07:04 -0700, Davidlohr Bueso wrote: >>>>> On Tue, 17 May 2016, Waiman Long wrote: >>>>> >>>>>> Without using WRITE_ONCE(), the compiler can potentially break a >>>>>> write into multiple smaller ones (store tearing). So a read from the >>>>>> same data by another task concurrently may return a partial result. >>>>>> This can result in a kernel crash if the data is a memory address >>>>>> that is being dereferenced. >>>>>> >>>>>> This patch changes all write to rwsem->owner to use WRITE_ONCE() >>>>>> to make sure that store tearing will not happen. READ_ONCE() may >>>>>> not be needed for rwsem->owner as long as the value is only used for >>>>>> comparison and not dereferencing. >>>> It might be okay to leave out READ_ONCE() for reading rwsem->owner, but >>>> couldn't we include it to at least document that we're performing a >>>> "special" lockless read? >>>> >>> Using READ_ONCE() does have a bit of cost as it limits compiler >>> optimization. If we changes all access to rwsem->owner to READ_ONCE() >>> and WRITE_ONCE(), we may as well change its type to volatile and be done >>> with. >> Right, although there are still places like the init function where >> WRITE_ONCE isn't necessary. >> >>> I am not against doing that, but it feels a bit over-reach for me. >>> On the other hand, we may define a do-nothing macro that designates the >>> owner as a special variable for documentation purpose, but don't need >>> protection at that particular call site. >> It should be fine to use the standard READ_ONCE here, even if it's just >> for documentation, as it's probably not going to cost anything in >> practice. It would be better to avoid adding any special macros for this >> which may just add more complexity. > By the way, this potential "partial write" issue may also apply to > mutexes as well, so we should also make a similar change to > mutex_set_owner() and mutex_clear_owner(). > > Jason > Yes, I am aware of that. I just don't have the time to to do a mutex patch yet. As you have sent out a patch on that, this is now covered. Cheers, Longman