From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S933467AbcFPVgU (ORCPT ); Thu, 16 Jun 2016 17:36:20 -0400 Received: from mail-bn1bon0144.outbound.protection.outlook.com ([157.56.111.144]:25762 "EHLO na01-bn1-obe.outbound.protection.outlook.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1754747AbcFPVgO (ORCPT ); Thu, 16 Jun 2016 17:36:14 -0400 Authentication-Results: spf=none (sender IP is ) smtp.mailfrom=waiman.long@hpe.com; Message-ID: <57631BBA.9070505@hpe.com> Date: Thu, 16 Jun 2016 17:35:54 -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: Boqun Feng CC: Peter Zijlstra , Ingo Molnar , , , , , , , Davidlohr Bueso , Jason Low , Dave Chinner , Scott J Norton , Douglas Hatch , Will Deacon Subject: Re: [RFC PATCH-tip v2 1/6] locking/osq: Make lock/unlock proper acquire/release barrier References: <1465944489-43440-1-git-send-email-Waiman.Long@hpe.com> <1465944489-43440-2-git-send-email-Waiman.Long@hpe.com> <20160615080446.GA28443@insomnia> <5761A5FF.5070703@hpe.com> <20160616021951.GA16918@insomnia> In-Reply-To: <20160616021951.GA16918@insomnia> Content-Type: text/plain; charset="ISO-8859-1"; format=flowed Content-Transfer-Encoding: 7bit X-Originating-IP: [71.168.64.218] X-ClientProxiedBy: DM3PR14CA0076.namprd14.prod.outlook.com (10.166.156.172) To CS1PR84MB0311.NAMPRD84.PROD.OUTLOOK.COM (10.162.190.29) X-MS-Office365-Filtering-Correlation-Id: e92a1d14-6c9d-4e9b-30a4-08d3962e3a7a X-Microsoft-Exchange-Diagnostics: 1;CS1PR84MB0311;2:4a3A7dmnWOw/BP/9XgJjPfll//+blJBCLsf5ew37Ihf8uO7/GsDY9aV+sZwpzcag61UEcirgP8aMzLfw0mNutJAGheHepgKLZv7ZJnxTGsMEEHplgdf8tzGQv96meiDykjkkMLlvjTShkuI58vrRgcGK0QdQCsMDkHbAc3tZbrpIw+txrkwix0eoscpvYloj;3:rYaQtJNKFeQTFqXoKYyvj5+WRSYERQSnlEJfvrw1hI81ci8qLuyM23B7KW267h1tbqRCMH7XFw86WVZAiyU6rDLttyS9zCBm9lXNftaZAovgYrkS5cbuJw8FQQk4wx0/ X-Microsoft-Antispam: UriScan:;BCL:0;PCL:0;RULEID:;SRVR:CS1PR84MB0311; X-Microsoft-Exchange-Diagnostics: 1;CS1PR84MB0311;25:8VwV3+rNwo6VzxQgSdm/0rP/k1riBhMpXneuq22rhRu8EfhjZ+bO8NuD/6njaLHrzsra2CuqTVOYvT2yAsjpyPtXvyt9HWPUeJCAzSPZYythGcl3ermqQW+juItq7P5P9zE3X2i88EczNz61Lsyqw9pTJSxNMpoTre8YUQDqlnr+5GOP40gAewGIxEKkH7YWiyHbx8vE7W6PWJYNEO2xq+mHR5w5/Hzc02G/eaGNPNQBUMrDbpUAad3sz7l47mDZ88ByCoWft+A9tu6asDnLCXqLyvaX9BmhtTqsb0TuMya2ReFDGOGaj6yBHq7qzrjKc55OSTuGcYlOvN3BJWMfnkrZourbmMNwpdIxZruzU7Xr2K9vYMJOlK6+xBPtdPw8idsFQr/p3a7N+f6rwQPMk1SOgghSZK1lEU9KKwwXnt/YNouG6iPm6uFthBW/N/nz/6GuzC+E79w8zD0G9Gcb72m7wReTxQnaSrMVJfkrCBZQ9B7eJedWZVhbGpv0FsL79W603hIxplU1RiCFh2TFsqYIdQKAnhCcHYB+2CQICgzy7GQ7R1EIzgVo9IfXpdG1pxNmLXmAUlv4+JbW/QYBfxgh26/dkksPdppXdG04pbi33LjaKAvRrk1MwSCeopxt8tnEE6D7f5DKCGuxFpSaE69s1AvZRZEe2mP5QAHJSfzmXadEuCq9256b99/7iVj4oHXGOZ01o3T+Xv/HUfTHIzOOzfNfFq/xjAUERglfODA= X-Microsoft-Exchange-Diagnostics: 1;CS1PR84MB0311;20:p5ez7Xb7NnIg0r/3afjhgbRWymrRkmrsV5qUMsL/UPcNsh1XoKZikRUp2orNquEXvcg8WFvenzerjVtNWKP74kkrc3V9yKRmQIUbj5CSbYchuPTeajRTOofnuaWtv9tIXSod/lzrQVTQxsvpvHOXOpC+m9zxLq7BGNXBT6OLAJA7o3rjphxQbQTXW0U/JtlHhIM2JpJkdc5ojRJVzzHpASIMt5YQ9ogUyBcIiCFhXk+aUjJewNQBPfI0WM/pIPKnptDbJoU7dRDvhuXTDDQtVGQA6mzJDGQm2xg8UnGomnff8BDWZOOMPcFz+M0upnfZoD9vXuxja1j8Z69mp6Js3q2pvwReFXgLjX52FBIF7E8I+gYhv9nGqHSWYva4BeImjv4zvhfV3a7WeVVmsUksztUr5L0zQER5aOS9V3e7/Oh5B/3gJnsA6YG6UbIXu88c1uAdEsTEWj4oTmbhjNJaz5tfaM+nx4GcOiqEcHL6tOQIAnklVTavxAvH4WOVh6+a;4:3AjoKgaKvd3oY+z0Z8y+QMu2tv1NJL4kZgXP66d2uJ5khdiYVWvcFq2RD1FcuX7fgFwONjp+M43AWTdAFWZcTi8wVJhlnovyzoIZXvbbrXRRq05AIqTaoehsw/rY7RqpIg2q5rwhOuU+EMcfmrFT0Snb+jTUpo4TOEDMaD27Y3/gTDjWcmYax2fmhBUrKVDoX6SCuVVs9bt4M31RF0yEgCIms/laYzgqSRQpXYTrN+I76yyTCWiaM87OP8S8FXJBK274aHXB5fOttZBOOJ+vj3AaJJWaoFxCFe7QzNhsROru8wNKGmBKgX6XmLyVDVDh8C68W7TnwyY2tXzyRtRbjJusQgJeItR1uriB1eOFUepwk9leGwu74SpxcCNUZtvRbHeGHvbHoFpg1eFWOMGuvE4edcpJyj6BruU8pW8/tZjASOOCp90XDe8WvgR8HDWU X-Microsoft-Antispam-PRVS: X-Exchange-Antispam-Report-Test: UriScan:(227479698468861)(788757137089); X-Exchange-Antispam-Report-CFA-Test: BCL:0;PCL:0;RULEID:(601004)(2401047)(5005006)(8121501046)(3002001)(10201501046);SRVR:CS1PR84MB0311;BCL:0;PCL:0;RULEID:;SRVR:CS1PR84MB0311; X-Forefront-PRVS: 09752BC779 X-Forefront-Antispam-Report: SFV:NSPM;SFS:(10019020)(4630300001)(6009001)(6049001)(7916002)(24454002)(199003)(189002)(377454003)(110136002)(83506001)(106356001)(105586002)(80316001)(77096005)(87266999)(76176999)(50986999)(54356999)(189998001)(2906002)(2950100001)(19580395003)(101416001)(19580405001)(93886004)(50466002)(117156001)(230700001)(42186005)(97736004)(4001350100001)(33656002)(65816999)(5008740100001)(59896002)(81156014)(81166006)(8666005)(36756003)(5004730100002)(586003)(23756003)(8676002)(92566002)(99136001)(64126003)(86362001)(65806001)(47776003)(6116002)(4326007)(68736007)(3846002)(66066001)(65956001)(7059030);DIR:OUT;SFP:1102;SCL:1;SRVR:CS1PR84MB0311;H:[192.168.142.155];FPR:;SPF:None;PTR:InfoNoRecords;A:1;MX:1;CAT:NONE;LANG:en;CAT:NONE; X-Microsoft-Exchange-Diagnostics: =?iso-8859-1?Q?1;CS1PR84MB0311;23:9wwNm778dsFJu7hkM3OCG/FeD9uV0D0Itde3fS7?= =?iso-8859-1?Q?tQeOVCBgU8BBUnw7Pj1Ui+0RUhLU47tNMWL7ZyQUQGh7Q8AmTT9ELVUESD?= =?iso-8859-1?Q?2Q3X7t+UpfHG0C+fybBdqLScXFq33oZAiS8qrQJta98NEGATNNvWkXlgoy?= =?iso-8859-1?Q?PyU0WLdUKS6c6OMGa0ZgykcoWq3bG7A7oGd/EUjjXjXf7qBFkM6dyycYIx?= =?iso-8859-1?Q?YOIym5qZFlHRSkkDREwjXQVicpLBhqq2bFplZOl/c23JtGmpWiP2OnVTBM?= =?iso-8859-1?Q?2+y6fN376zLwfCjiCyBHG09mcKcJYcElIDFjbB/XS/ma3PYq/1ytUU383O?= =?iso-8859-1?Q?MhdaG2xJBdmKJ5RlyVx7AawifzPwTlaOVDeNonWeIfyCmuVtrkyLAVwGkB?= =?iso-8859-1?Q?Tjjznc5FK02aAQNL6SweAINwQb+i9Ki/iYoQm+Ad0NIf0nBffdt06zsZDV?= =?iso-8859-1?Q?kdRXd1mPbW+0BxahF4HsIz0YXe43g1sgFvf3e8xxqXUdhH7kHAmiFfb1H3?= =?iso-8859-1?Q?y4s3jRrKtB/OpLY2/xVL261CR/9v3ldxRLbhnFyTsYev/KBwre//bBVV+H?= =?iso-8859-1?Q?Lk47fg9Mk5CHHBo1LKgutR4ERMGgSVrc5KYU8UfmgGM2cKsCxlI2twJeIV?= =?iso-8859-1?Q?W53RDVeZVmiePnnol6bcDYmxFUQKzQp3Xx6d+vyB32I+z4mBTX/pIR0sEX?= =?iso-8859-1?Q?Rq4ZtI8LhKuuFc4IzGcGWgRFN/I6tQzQQITYpSB9VNzLfRjR85JoUJIG1U?= =?iso-8859-1?Q?uGDGseo1C5ws3cmOS8HuxaNNGhcVz2dOk9S2iIC3TSmmHSVTIDY0oQqvNe?= =?iso-8859-1?Q?XLM9ybX8Nr5TUDmtJWdBJydntvkc//i4Bjw6Y74tQ7pT1GkMPxyhs59PlM?= =?iso-8859-1?Q?PBY34/ejlQsybWwH10AKnUA/m1D7gZmdClGgA4HakhAMFd/Y3qTjnPyFTR?= =?iso-8859-1?Q?aiP/kU6UftMADYtAjaeDK+1OzKbNi3QVvKJp6815AX5bmxW12jKUrn3tXW?= =?iso-8859-1?Q?tSzY03O1xScSu6ri7xUCIUSKPnMYFhgcN/6PBvw31axACFcdTi+hcMuH5G?= =?iso-8859-1?Q?biTVoWpJVFTEeZsD36RuDJZhiAapjy4rwaLQM4LUmp9+1g2ZqJS5fwHT1i?= =?iso-8859-1?Q?oEwf149umv3rLGNxud5Yer3TC5VoJw/6c/+tQFf3FZE6LahI6bn95ayIR0?= =?iso-8859-1?Q?igA5HS9afzeGlFAMUoFZ7hdcq/hAiGJIJdYzOmmDbAneMrjRPiWLERKW5Z?= =?iso-8859-1?Q?4ll0Re7CoK5ZcQqazwJQIopUiSpa44trq9j4n6MK/DmpL4YixcpUmmAB6y?= =?iso-8859-1?Q?baXL5dPIsbRotoemn1AFwLUkbylllJaL2vm/jiy5eFx3bvcAc5AO7qIU51?= =?iso-8859-1?Q?8H2lf8wqLvN+dxJWGNCFRb9CPEnwn2vG+OYlvvTPyLuOkPBpJmHwwNRmn2?= =?iso-8859-1?Q?3pAEMwH6GIBRRGKfuog5AC9ibYUx9go5klo?= X-Microsoft-Exchange-Diagnostics: 1;CS1PR84MB0311;6:ok4qg7pNbA9GrynYBoVFNUXDTREUGaqRFbXAoJgWN3BbHNB/SfqU2jQc8pW3MvhzzWzx1nSb/MRx2sEGknV/bZNF2MKZx/zlxqE739upfZ4gOeQyhs6YaYwV9vNYX4e/Zc27uefvp38+9L1vuLKzOOlx7K+gyQMyZAdAiYadQqz9FxqSiYVOb9zEECALis9TjQqZaJHb0pU9T9AeXFP429ymrFfcJs9YKR/K7b8dB6r1pdaGKqcek8CrlPsxIOkpjnLAf8uccP5swZfK8ubDX/TKhmGJ+Y4sH+4RafJU8SA=;5:gJam8lgRZ4DTCkY1sSdaYPk9oDf2WYXwxRhXqWqwG0rx3tIURlrgrSThowcg4Fljod40SWrdZNYBXmu3finJ/DwqAPdfvH0gwO4jlgS3jfninadYxvmrf/QmxQ2fHueJsPBbLIkt1/CytpyYbXDkHg==;24:25BJIhztbR1aL7kGIg5KyZwfYy5ZaMJdpfY1xqVv8G/CTrNd4J/eFcjJvuMGNUhY7fVBuceZ0AgODqMQOIWo/srdI86R1BjmznYG8SjCztg=;7:tetft3IYhdyX8cAf1+BiiLh0GTNUzqw49oJeZDW6JL9AF846MVhI+34ZBDKhET5/0wuP8lK8tEte5FQT+/NOtbj95zhglj6M6wJphFekiZbrbqsU/+/T66C2Ypd37Qg2tnYPsiElsJVEzoPbOamWDBLInsm3R0KSLtMEAzzBvahE7uzAuL3WfoeqKvVzkFZTwDvgAQ5tgTE21ZVd20S4rg== SpamDiagnosticOutput: 1:99 SpamDiagnosticMetadata: NSPM X-OriginatorOrg: hpe.com X-MS-Exchange-CrossTenant-OriginalArrivalTime: 16 Jun 2016 21:36:07.0629 (UTC) X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted X-MS-Exchange-Transport-CrossTenantHeadersStamped: CS1PR84MB0311 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 06/15/2016 10:19 PM, Boqun Feng wrote: > On Wed, Jun 15, 2016 at 03:01:19PM -0400, Waiman Long wrote: >> On 06/15/2016 04:04 AM, Boqun Feng wrote: >>> Hi Waiman, >>> >>> On Tue, Jun 14, 2016 at 06:48:04PM -0400, Waiman Long wrote: >>>> The osq_lock() and osq_unlock() function may not provide the necessary >>>> acquire and release barrier in some cases. This patch makes sure >>>> that the proper barriers are provided when osq_lock() is successful >>>> or when osq_unlock() is called. >>>> >>>> Signed-off-by: Waiman Long >>>> --- >>>> kernel/locking/osq_lock.c | 4 ++-- >>>> 1 files changed, 2 insertions(+), 2 deletions(-) >>>> >>>> diff --git a/kernel/locking/osq_lock.c b/kernel/locking/osq_lock.c >>>> index 05a3785..7dd4ee5 100644 >>>> --- a/kernel/locking/osq_lock.c >>>> +++ b/kernel/locking/osq_lock.c >>>> @@ -115,7 +115,7 @@ bool osq_lock(struct optimistic_spin_queue *lock) >>>> * cmpxchg in an attempt to undo our queueing. >>>> */ >>>> >>>> - while (!READ_ONCE(node->locked)) { >>>> + while (!smp_load_acquire(&node->locked)) { >>>> /* >>>> * If we need to reschedule bail... so we can block. >>>> */ >>>> @@ -198,7 +198,7 @@ void osq_unlock(struct optimistic_spin_queue *lock) >>>> * Second most likely case. >>>> */ >>>> node = this_cpu_ptr(&osq_node); >>>> - next = xchg(&node->next, NULL); >>>> + next = xchg_release(&node->next, NULL); >>>> if (next) { >>>> WRITE_ONCE(next->locked, 1); >>> So we still use WRITE_ONCE() rather than smp_store_release() here? >>> >>> Though, IIUC, This is fine for all the archs but ARM64, because there >>> will always be a xchg_release()/xchg() before the WRITE_ONCE(), which >>> carries a necessary barrier to upgrade WRITE_ONCE() to a RELEASE. >>> >>> Not sure whether it's a problem on ARM64, but I think we certainly need >>> to add some comments here, if we count on this trick. >>> >>> Am I missing something or misunderstanding you here? >>> >>> Regards, >>> Boqun >> The change on the unlock side is more for documentation purpose than is >> actually needed. As you had said, the xchg() call has provided the necessary >> memory barrier. Using the _release variant, however, may have some > But I'm afraid the barrier doesn't remain if we replace xchg() with > xchg_release() on ARM64v8, IIUC, xchg_release() is just a ldxr+stlxr > loop with no barrier on ARM64v8. This means the following code: > > CPU 0 CPU 1 (next) > ======================== ================== > WRITE_ONCE(x, 1); r1 = smp_load_acquire(next->locked, 1); > xchg_release(&node->next, NULL); r2 = READ_ONCE(x); > WRITE_ONCE(next->locked, 1); > > could result in (r1 == 1&& r2 == 0) on ARM64v8, IIUC. If you look into the actual code: next = xchg_release(&node->next, NULL); if (next) { WRITE_ONCE(next->locked, 1); return; } There is a control dependency that WRITE_ONCE() won't happen until xchg_release() returns. For your particular example, I will change it to CPU 0 =================== WRITE_ONCE(x, 1); xchg_relaxed(&node->next, NULL); smp_store_release(next->locked, 1); I don't change WRITE_ONCE to a smp_store_release() because it may not always execute. Cheers, Longman