From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932466AbcEaUBc (ORCPT ); Tue, 31 May 2016 16:01:32 -0400 Received: from mail-bn1on0115.outbound.protection.outlook.com ([157.56.110.115]:14752 "EHLO na01-bn1-obe.outbound.protection.outlook.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S932335AbcEaUBZ (ORCPT ); Tue, 31 May 2016 16:01:25 -0400 Authentication-Results: infradead.org; dkim=none (message not signed) header.d=none;infradead.org; dmarc=none action=none header.from=hpe.com; Message-ID: <574DED82.9080200@hpe.com> Date: Tue, 31 May 2016 16:01:06 -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: Peter Zijlstra CC: , , , , , , , , , , , , , , Subject: Re: [PATCH -v3 7/8] locking: Move smp_cond_load_acquire() and friends into asm-generic/barrier.h References: <20160531094134.606249808@infradead.org> <20160531094844.282806055@infradead.org> In-Reply-To: <20160531094844.282806055@infradead.org> Content-Type: text/plain; charset="UTF-8"; format=flowed Content-Transfer-Encoding: 7bit X-Originating-IP: [72.71.243.245] X-ClientProxiedBy: BY1PR15CA0034.namprd15.prod.outlook.com (10.162.17.172) To DF4PR84MB0316.NAMPRD84.PROD.OUTLOOK.COM (10.162.193.30) X-MS-Office365-Filtering-Correlation-Id: 56af49b9-07de-4f0a-8b6b-08d3898e567c X-Microsoft-Exchange-Diagnostics: 1;DF4PR84MB0316;2:atw5gjAdMMVJocng7+xAZ4BFBY6re0AzYWlhdWJWLIyPOYwETcpYWdTB8sfZMN+9Chwf7AtS47ZVKwCyfIgRc5IphBkU9rTgf1CtvwwNCc1U5LGymBU0YuFstTxJMO0eVNqPbSELTwlgf7Zx1q0yL71bBk4XDD2rRgQaE1eU3YKrAlEy2xQdwPsWqsbPcHsP;3:JOdlr8UI+R7ExBSXmtMzNFmMOpro0LD1hYGHu6nnifI8nsGbffDMslX0t44t1isAmSxu3VD3CPyFvgWWvLxA/XDwh+6gsfNH4MmeELcyIMQvQ7/kyVXY4CsZ+3SahXFX;25:2wjFFdg7BZc1wHoIwUSAmA00ZJJiJdyOEiiuDf8liF2T4a72Yc7141HYqd083h7ue4pOGSyzreUZIgvIGA/XChJ6pQa+T3dZtUUkMya5tF4KuGJRSoa+queQGixrYrZkVb3IJOySe4o1OI5ccmplUh1TQewSU5YoFSEMWSUAKltarDyl0yVCBDlluBFYUdEOshmWQEZEJFTbcOe6q4LjnRWKQq58HaEVFV9vHkQ9RgWlTY+Qq/CCqobnahxgKJDrwnzmDI6xgnAhJH+EMuSWaDCgh19MUkTfVu3ob9AX5qCRQBr2CR5qvMDHGrr3IlniKNNEdE2+KZsvtObdkzmJEpiMoxzU4W/8JD7cUMyKlEKNV1zruPadtDG5A873lF5bWH0dkuCBXPIahC5eshxNaP3AnLiG9oSuVf6vbPaS+p4= X-Microsoft-Antispam: UriScan:;BCL:0;PCL:0;RULEID:;SRVR:DF4PR84MB0316; X-Microsoft-Exchange-Diagnostics: 1;DF4PR84MB0316;20:isxAoaHuLKszdlcMTi7QaeTcQhD6xMe+HNOmtkqkDX6uD4uxirZAEnBsNzfMp4IruwJGPNU8UaJmNV7wKozjpvIVnMw9HMOX8OihoHzZ3Us6ZmdGKqT7ThlmoY3c0wN1qg4bN8vcLzUC/wsQb5RwsEhKdQk1XOIBkGVAofrUDe2Jz1yhYceQ/zVn9RYMaqdFKeTkELoqLK6bWOJpVXM5u3m1Q/0tP+lWYK7l/KhqlVDULDfFLDXFopP89HlKcWXw6Uq/kVSLswoOef5IAc+YIojRWqnaXyF1DQofZQHzYRhaSbxeDoc2OZ1QfsTr+2uC6tksaPNPTIbL+3Io48nTMA==;4:JXE0T7zlJ+9r1uIM9R6fnHExYOR6ECmLFkXNg+sAQ9TtIRygf4HgbtWMylJ18JgHDRmLR183n4q7KZ7T2NIPapvAJO8KbZvdCj1Nv7Rmq3ZT9JZ9sNKnsof1Kba/oKscqDuC30pe4C+8edeOhXT6LbSLvM1M5XUcRvyINlF0ZM5rX2IcBgtMwnkrETtmzpDU77VKFgOkxA95E7kCNQGi8Ikdp5774Z6LkKxV1VHW7Gt0evgfRVZTq5A6hTt1qsMv5FSrb5zD3AgMzxy8VGG4lL8zgbkDeR3/vWN2dprltimQYCNmM+mE1XCV2SjG5GJKvlhyCtPW2zxZG4tXOREe5lWLROaJWiYx7gP5yCxoiQDqfWhO2BqWdbD2eLS0Ezrt 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:DF4PR84MB0316;BCL:0;PCL:0;RULEID:;SRVR:DF4PR84MB0316; X-Forefront-PRVS: 095972DF2F X-Forefront-Antispam-Report: SFV:NSPM;SFS:(10019020)(4630300001)(6009001)(6049001)(377454003)(24454002)(2906002)(586003)(65816999)(50986999)(4326007)(54356999)(189998001)(50466002)(19580405001)(19580395003)(230700001)(92566002)(59896002)(117156001)(80316001)(81166006)(110136002)(8676002)(3846002)(6116002)(76176999)(23676002)(42186005)(65956001)(65806001)(36756003)(86362001)(77096005)(66066001)(64126003)(47776003)(33656002)(5008740100001)(5004730100002)(2950100001)(41533002);DIR:OUT;SFP:1102;SCL:1;SRVR:DF4PR84MB0316;H:[192.168.142.143];FPR:;SPF:None;MLV:sfv;LANG:en; X-Microsoft-Exchange-Diagnostics: =?utf-8?B?MTtERjRQUjg0TUIwMzE2OzIzOk5vTS95SEtDRHViZGdVT0FFTWQ1a2FGNUhZ?= =?utf-8?B?a1MyazhWck5sUXRWTExhM1JOWFVZMURaSEZESXNFQ2hsamJuN2ZDQStrSWZU?= =?utf-8?B?RHUrTXNScFJyay9VYlNCQXNuNmEvNzZxRVhNUThnQjB6Slc0aWRPbGs0dnFR?= =?utf-8?B?THg0bVozQUpkTFBSV3pCU3Y3VEZhTm9sMlFBQTFQZHN2UVU0N05GNXpudGt2?= =?utf-8?B?VXcyZ0xib1MyYkVYK05aaEUrbWRTMnVPMU5WOStrT3lLaW9QSVZEZ3NpRmY2?= =?utf-8?B?V3l2ZUpaM2tZMjE2SCtIbmcxd0tyUXY3T3l5QVlHanMycTJLZXExTXRJOGUy?= =?utf-8?B?c1EzbWZUdmIySFJiVHNRSGZ3bmcwL2ZCbG1xYmJGT2ZXdmdnWUZMMDBjb0dq?= =?utf-8?B?TU9sNHJuK3Y0WUpDdWNId1JLalc0Sm1KR2pRNjRYbUdXL1YyaC9RWVFIbk5Z?= =?utf-8?B?bW96aVdYWlpqaUFKT3pVcmh1VVhqb1IycVVyL05PK0IzcytKcGdFUm5OQ2Fs?= =?utf-8?B?bXZQaldpNmFWY3orOTU1OU43ZllKTS9RRy9qWlcrd0tRME5FbG5JdWFidTRL?= =?utf-8?B?cEhmU0szY3dNVG9ReFJBK0pZTERFbmJ2UzBaY0VqQy9yckhSSG1uQ1ZqeUZP?= =?utf-8?B?V0xHd0lxVFkvQzZtT055c2JlWkgrMXZseVkraHZpRTBab08xUThPMWpwcEhE?= =?utf-8?B?WXZlcFFOVWJqWXUxc2xVRHhjbVFzUUpmL2RhYWp4REJsd0J0b3JhNDZFVWg5?= =?utf-8?B?bWdFbmh4TldJc1B6LzcvV1c2SzVSVnlneEJKRStLcU1QNW9MbDZuMmtMT1NY?= =?utf-8?B?WFpMVFhGeGhZdXBvaFNMaCsySnpaMmtGT3NjQ3JROXlicjBjWXJ3YnBZQWJv?= =?utf-8?B?L1I2clE4UlRSU0tzeGtGcTgya3dyUG1PYXVaMkxudktRK1hXMSs5cHNYbE5V?= =?utf-8?B?ek1ZRjFwUGxLaWdaT2s0am5wVit2VlFGRmRaT0hYOWFhR1hTbFdEczlHMjlh?= =?utf-8?B?MDV5RmFPaFpOQ0dTWWRCek9XdlFFMENUbDdaNDREOEhrSGt1RTBTd3BSYXAr?= =?utf-8?B?TGlpRXhXSjNVeWxqMEpxZ3lkMFBNY1IvblJFKzhpOVJ6QnkzdU5aWnVxVHB4?= =?utf-8?B?N3lWYThQaXlmT2lxLzhnWVkxQ0RMYklTWmJFOWpFSXhZUyszUVBxc1JaZkNX?= =?utf-8?B?ZTBiN0pJcHIyd2VtUWxhaFFKR3cyTWw1aVF6UEZvSmUzQU1hQlludG9lYVZ1?= =?utf-8?B?WWNELytlRzdOU1dyZjNWd1RqbDBvQm1iTTZLRUV4aEJkWjBvOGJleEhtVFA5?= =?utf-8?B?cVRJZzdVSjRUOFp6cXNQVkZic21zUlF6TnlZcHJPVmZuaElqTno4TjJYbkMz?= =?utf-8?B?TUUxS2NscFhDRDR0RXYzUUtFVlZhN3ErSlpoMytHYm5NSlB2eHAwYjhpeGR5?= =?utf-8?Q?7K02CS2qHFOIfh6qLjSe9CrPviR?= X-Microsoft-Exchange-Diagnostics: 1;DF4PR84MB0316;5:NIxC3wiXxlmBPuGMILBWzwsWO+Huo2i5nNsaD8MYLIalzwa9EDxFyiE2evJa2UAOwb+nd5cnGVRZxwapVBfiNgui+c/roVJLgIyQ8QcygCTPdb45jM8eHyaLI9zoyYInp72KHNYCYnh9WM3XDc2Aug==;24:a9Rj6/94Oy4/tLFhxbk8y1CJ85fa9XI7X+8SI03Y6LAAOUeX0ILSR2442206jYCsDVibWgeVUz92EzNLNFQjBXUg4cWPOi9qFP4eqXa+PiM=;7:pKk9Hc7hPPTTgbgoxiU34HtDE3lGZxIbfeW1SNGiA4tslEatCeyjY7bh9a2ftvp99cwxCUnTnTm+m/WfYooiFx//IlrUqYeNdAoC1cYD1gcoIfPc4M0Kc8XTy2C1xN+ibJGWvjDmMKo0exBmG5BBKPjMMil/dPyc4axxY33MfZyBbZ2QHfrzyRD36cizGTAz SpamDiagnosticOutput: 1:23 SpamDiagnosticMetadata: NSPM X-OriginatorOrg: hpe.com X-MS-Exchange-CrossTenant-OriginalArrivalTime: 31 May 2016 20:01:18.8109 (UTC) X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted X-MS-Exchange-Transport-CrossTenantHeadersStamped: DF4PR84MB0316 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 05/31/2016 05:41 AM, Peter Zijlstra wrote: > Since all asm/barrier.h should/must include asm-generic/barrier.h the > latter is a good place for generic infrastructure like this. > > This also allows archs to override the new > smp_acquire__after_ctrl_dep(). > > Signed-off-by: Peter Zijlstra (Intel) > > --- a/include/asm-generic/barrier.h > +++ b/include/asm-generic/barrier.h > @@ -194,7 +194,7 @@ do { \ > }) > #endif > > -#endif > +#endif /* CONFIG_SMP */ > > /* Barriers for virtual machine guests when talking to an SMP host */ > #define virt_mb() __smp_mb() > @@ -207,5 +207,61 @@ do { \ > #define virt_store_release(p, v) __smp_store_release(p, v) > #define virt_load_acquire(p) __smp_load_acquire(p) > > +/** > + * smp_acquire__after_ctrl_dep() - Provide ACQUIRE ordering after a control dependency > + * > + * A control dependency provides a LOAD->STORE order, the additional RMB > + * provides LOAD->LOAD order, together they provide LOAD->{LOAD,STORE} order, > + * aka. (load)-ACQUIRE. > + * > + * Architectures that do not do load speculation can have this be barrier(). > + */ > +#ifndef smp_acquire__after_ctrl_dep > +#define smp_acquire__after_ctrl_dep() smp_rmb() > +#endif > + > +/** > + * cmpwait - compare and wait for a variable to change > + * @ptr: pointer to the variable to wait on > + * @val: the value it should change from > + * > + * A simple constuct that waits for a variable to change from a known > + * value; some architectures can do this in hardware. > + */ > +#ifndef cmpwait > +#define cmpwait(ptr, val) do { \ > + typeof (ptr) __ptr = (ptr); \ > + typeof (val) __val = (val); \ > + while (READ_ONCE(*__ptr) == __val) \ > + cpu_relax(); \ > +} while (0) > +#endif > + > +/** > + * smp_cond_load_acquire() - (Spin) wait for cond with ACQUIRE ordering > + * @ptr: pointer to the variable to wait on > + * @cond: boolean expression to wait for > + * > + * Equivalent to using smp_load_acquire() on the condition variable but employs > + * the control dependency of the wait to reduce the barrier on many platforms. > + * > + * Due to C lacking lambda expressions we load the value of *ptr into a > + * pre-named variable @VAL to be used in @cond. > + */ > +#ifndef smp_cond_load_acquire > +#define smp_cond_load_acquire(ptr, cond_expr) ({ \ > + typeof(ptr) __PTR = (ptr); \ > + typeof(*ptr) VAL; \ > + for (;;) { \ > + VAL = READ_ONCE(*__PTR); \ > + if (cond_expr) \ > + break; \ > + cmpwait(__PTR, VAL); \ > + } \ > + smp_acquire__after_ctrl_dep(); \ > + VAL; \ > +}) > +#endif > You are doing two READ_ONCE's in the smp_cond_load_acquire loop. Can we change it to do just one READ_ONCE, like --- a/include/asm-generic/barrier.h +++ b/include/asm-generic/barrier.h @@ -229,12 +229,18 @@ do { * value; some architectures can do this in hardware. */ #ifndef cmpwait -#define cmpwait(ptr, val) do { \ +#define cmpwait(ptr, val) ({ \ typeof (ptr) __ptr = (ptr); \ - typeof (val) __val = (val); \ - while (READ_ONCE(*__ptr) == __val) \ + typeof (val) __old = (val); \ + typeof (val) __new; \ + for (;;) { \ + __new = READ_ONCE(*__ptr); \ + if (__new != __old) \ + break; \ cpu_relax(); \ -} while (0) + } \ + __new; \ +}) #endif /** @@ -251,12 +257,11 @@ do { #ifndef smp_cond_load_acquire #define smp_cond_load_acquire(ptr, cond_expr) ({ \ typeof(ptr) __PTR = (ptr); \ - typeof(*ptr) VAL; \ + typeof(*ptr) VAL = READ_ONCE(*__PTR); \ for (;;) { \ - VAL = READ_ONCE(*__PTR); \ if (cond_expr) \ break; \ - cmpwait(__PTR, VAL); \ + VAL = cmpwait(__PTR, VAL); \ } \ smp_acquire__after_ctrl_dep(); \ VAL; \ Cheers, Longman