From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S933650AbcFQPl5 (ORCPT ); Fri, 17 Jun 2016 11:41:57 -0400 Received: from mail-bn1bbn0108.outbound.protection.outlook.com ([157.56.111.108]:61120 "EHLO na01-bn1-obe.outbound.protection.outlook.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1751757AbcFQPlw (ORCPT ); Fri, 17 Jun 2016 11:41:52 -0400 Authentication-Results: spf=none (sender IP is ) smtp.mailfrom=waiman.long@hpe.com; Message-ID: <576416B1.6020006@hpe.com> Date: Fri, 17 Jun 2016 11:26:41 -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> <57631BBA.9070505@hpe.com> <20160617004837.GB16918@insomnia> In-Reply-To: <20160617004837.GB16918@insomnia> Content-Type: text/plain; charset="ISO-8859-1"; format=flowed Content-Transfer-Encoding: 7bit X-Originating-IP: [71.168.64.123] X-ClientProxiedBy: BY2PR11CA0020.namprd11.prod.outlook.com (10.163.150.30) To AT5PR84MB0306.NAMPRD84.PROD.OUTLOOK.COM (10.162.138.28) X-MS-Office365-Filtering-Correlation-Id: 87c8837e-159d-4e35-c3dc-08d396c3d275 X-Microsoft-Exchange-Diagnostics: 1;AT5PR84MB0306;2:lA+uq8wHxfcHcbYLEtR+v30PgukXfboco0R6K6LqHh1ytHOvoThNHC4qpz+0M45ocqyanuPPRom4d8+GJP78++fV4E+0iV+HBEpiANN7h7jgPbWFnBDjgzQLCKizZnUPWNvR78mdmonykXjrVkFy3TNmGryJmEWsOYkEsu6UuSpH9/vVhbuf9H4Y4F3A0zvw;3:WcvgWrKC6pEExkrGqSi5j0M0izwItzTrDgdRBkqBHgkntxVeuHiAyFUNuV6x05O5ER9qZKC57ASnJP5XPrWoRhVD7cDWeNnBl7OhFob6/qghPLLP86hO4bMaUQMfnfZf X-Microsoft-Antispam: UriScan:;BCL:0;PCL:0;RULEID:;SRVR:AT5PR84MB0306; X-Microsoft-Exchange-Diagnostics: 1;AT5PR84MB0306;25:iv1XwhuZa0a0PhyEHMaFry8JrNr6CcoDcbPDBBEtlFXVpyyDUZSTfp5B9TY0xXfOvzFF2cz2p33QC/g7ZdM86OhwXE3BDPGx50rt0gR2DzfqmPwwzSKpYukmNwnXkQWTSqC09lSHYCYvygWhYtExU0olG53VRx1R9eaxQguVyLKEBwe4siMwG7Hmrxb3H9PDeDrwNvD6i9FnLrCXbZK/YnEwPTg2d1eg4tu5Z5OuxR6YC8JtuDpLxTdaPvTSbG7DGModVi441fN7wUUGDY+q2BSqjVc6dbA5qM4va7jPZ4VzWhj1plNCVrMyoi6i/8aoBWi3tFaJ6ATClyKpyGGKRoCPRO8jVTisU/ra6QH3XdicjVbwszTvR6nUb1iZ3A/yUqe15hTVgJYMY8yyEbBVo9/qSPtqnlt4DVmojxxk5vzApkvoYH+O26MdJ/DTApNF8sIkPqEDqG86kmQ8NGd839qYpb/zrdT3XAoz/kbD+la+SVL2LKQFCZi3VeO12UJb3dHmGHmCjsLF8MHwKmvgjcH7R/TSApxsieXwgzsc9uELkD/UizAC5Fnw4ONBx1IzgYEjKsK/prYm7At2PCgtIGTp7AnlMrZlmobwn1P6aqX2zCSaggAHICva4O60XI9CJpzEOd/u1rb6ckpCW84LnbHpDaz6lpf3sIGQBCyD/B9c/LjpTj6oD7ExvmhOxTGVgxYXFyxO9Y2xc9QPvFiOwu6gf/Zyr4HX7Xms0CUqjYo= X-Microsoft-Exchange-Diagnostics: 1;AT5PR84MB0306;20:HLUeUpu5O2ZcZj4fz7+W8eW/Qa4vBbH3uB2QYUUMJMC7jODf8JUFN2YbNr2xTY45f4y3WMztmX2SzZKbW8T8fcNg3X3CidN4MjFIIQa5IG1p4Paa7dbJcpfc0MUx7anuzF8X7MNf6xKG6A4ZwDU9gAZP/Yf+YB6nflLZ9GxPNWNcC9uJ7ebhxB2yZ6zlgmbEG93na73EX3FpA7OvUUoC/HkmNEpTOsz1UHHWRLzKX6JTlLa0fGs4PpcQdK4a98oKhdJhX/ZO3aOFIqsJ/A7dqIc30GJgbILWX6iLlpbJhQ3zK43yxczcYVSvsyhfHfjQmknF1xeWtUj1zEQIEhfdwhlKL3ZaDuuxmqlGgXUGGLGs1Azi+kLzJUIkffOKQxUF/OZ2mBHCuco7p2belnQ55BBVNhZo9S5FLd6U4/hB45mSSvJKHJsCueHrS3R/jPCjQxv5JbNMjYm5WEAAxE3v05GrUhm+gUaovSQJ5roHaEbRSbqiPdDwEXb4+NtF49yA;4:j98vLbasV1EXnwb1MsAMYLN7ffEi0RGczgjn9vk4SdmFDW5EzPqjkx/4AjIbjMrhj0DiikJ0xRVV0Vyv3iMuFKPcGmSZMpLcyL/hRj9V8oa8tPbS871xmLYYjT3XlbaEdKqL8ICYGvW2gD8FsAnP0S6rU/VXY7MOWKMz0bPMgkJQPPPrm5tkUreaK1yU8oDHRSkitGBbkcfR8lSxlZ1gHTA5huMbtaxc95AmGkC+fnR1XZFh9ZwKBDLsCcFabEKTATdi1feucaFGBSavCYfsZt7t/RnitoJyfRjKpXsafSo3TAlg4VIOR3rBI7dxtJCIPp5mP5AY5z2jaw017Qcnr3Xm2qCDNCxJNyRfUw5dlDbARaQT40Evc9RrIAIhb0f/gB/qSPOzKluAYbIeJEAmBJMK7jEwdEPy10RT/Y6QwN5QhHY11tzetvQT84QAD92Q 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)(8121501046)(5005006)(10201501046)(3002001);SRVR:AT5PR84MB0306;BCL:0;PCL:0;RULEID:;SRVR:AT5PR84MB0306; X-Forefront-PRVS: 09760A0505 X-Forefront-Antispam-Report: SFV:NSPM;SFS:(10019020)(4630300001)(6009001)(6049001)(7916002)(199003)(24454002)(377454003)(189002)(33656002)(86362001)(64126003)(5004730100002)(65956001)(36756003)(59896002)(66066001)(23756003)(6116002)(5008740100001)(3846002)(42186005)(586003)(93886004)(83506001)(68736007)(65806001)(92566002)(189998001)(230700001)(19580395003)(4001350100001)(110136002)(19580405001)(97736004)(105586002)(87266999)(54356999)(65816999)(76176999)(99136001)(8666005)(50986999)(117156001)(106356001)(47776003)(2950100001)(2906002)(50466002)(101416001)(4326007)(8676002)(77096005)(81166006)(80316001)(81156014)(7059030);DIR:OUT;SFP:1102;SCL:1;SRVR:AT5PR84MB0306;H:[192.168.142.156];FPR:;SPF:None;PTR:InfoNoRecords;A:1;MX:1;LANG:en; X-Microsoft-Exchange-Diagnostics: =?iso-8859-1?Q?1;AT5PR84MB0306;23:MWdKbHnDbsw4fastRykyXZBn5hNIqwPWVMpDhp0?= =?iso-8859-1?Q?ReiA3nuu1jKbsF9s3R7FP6yIlvVLqFx3p4RJIROrI41nn87mgSFx9GmsUH?= =?iso-8859-1?Q?Sno08dY+NTB1goG+wqRF4VYP7bzaGuQmsARJvTuQhAcqvv+SkrpxXWxFep?= =?iso-8859-1?Q?rpRAC1igEA816CL+KIep9PgMCeOpsOwvM2hfZnAiSZNoJULFHm9waIuwFd?= =?iso-8859-1?Q?dx50ifIApubJEbQJVi3k6vhmpkNjQIZJOVoFWa6t6uj6795lEIUXJLoaS9?= =?iso-8859-1?Q?UcgMwjnjKAIgP8h+8XdWDyf+xULoUC+hO+L8QQzCfSbbppz7PlZbhO4jnY?= =?iso-8859-1?Q?E7BSR6PbvqFVKXiO7pkUAjiuZJ4lCW3zIDVVdq0JeFcvZj2ifJ51/FHwpt?= =?iso-8859-1?Q?hrA6RfeSFBosEFltIFy6ZsSp2VCNdho2c61knbJCrwty2T8zkRc7MpuVLm?= =?iso-8859-1?Q?9wZxkacJRyllANy84dcnidaKX6kxWS1boXelxrnjnIwniOqisUG2Q94ajI?= =?iso-8859-1?Q?NVE7hixoTzlHIvkrmArDTJy+QPgIUK8Tznr1JXP85QTUew1x+9HI3ULZqQ?= =?iso-8859-1?Q?sr17UkTrQZAjMvZUrwNu+/WfRQWIQpw59oLljiNe/0tzkDFAQve45WPeLF?= =?iso-8859-1?Q?exmrUquLHuq6G8sEmqDaAl+v0zr7tCWCM07t0Bxsms/c26P8P3/YpR79wR?= =?iso-8859-1?Q?5t3iqhRqjsLDimQnE8TdWKu1S4v0TuYd2bXb8p3w0Ks0gFCGc/J3tstIqB?= =?iso-8859-1?Q?klAi+erxZprTFcFMAEqP9zHURAgAp9di1bSb0nFWPWYOlxhYXM+p74xucl?= =?iso-8859-1?Q?v8iBeiM3g2ePd56nXEtJrK/01MM0G/qc5GiMk/WhzdcUjxbkcqX5iux00o?= =?iso-8859-1?Q?65wKvAQpKC+lCPdwY/Dj/RTIWL71gpR+Ad9dVanhYUHR1q8bSQVtWL5lrL?= =?iso-8859-1?Q?mTa+TttNAQNL9Lf8zYmX3ol5kJes5hz7XJS37kyKssPTmyyDo1lioRy7jJ?= =?iso-8859-1?Q?t//n6T4qq1mGizQWpItwxoV8qwp01LtWl+GApTS1n9vPOZxGOzG40sTcy/?= =?iso-8859-1?Q?bz8gzdhBCfms/j5l65xd48QzjzI9GEzj5QPErACqDgQ1Z9Tx1m7awiVZxC?= =?iso-8859-1?Q?K1EJTWT/8XGeKucWUzk+4mXCjalRX6DuzkObbnjQzkWYQtRQC69sWO57IJ?= =?iso-8859-1?Q?VTZ/KBuVAswfB0+/fR5C8DtLam3Xm0MBYOpSkJ5nkVnpXsfXCqk3TpKciX?= =?iso-8859-1?Q?aZUaQ8yeLvuygcaUGWL0bvCbBjgvBc9/i50eWaTuJVbcIoIRtQUQgs3V5t?= =?iso-8859-1?Q?yTF1lfKnRxfh74FJFcMnCTNe4kf3/4wpYPoPE5nGMkarUuA2pqzK4ehaq4?= =?iso-8859-1?Q?jPgsz1Uw/HxRYRnn7vfaX2NJpnutzr9IxaJDoPIn9IWZkfTlQ6mxZdHf9s?= =?iso-8859-1?Q?cUa6i0m1TzCXJKzZlj1TtioexHri2rPUzCx?= X-Microsoft-Exchange-Diagnostics: 1;AT5PR84MB0306;6:pVNU0cFDch5h+PjUrqjzkBknvdtzzreG6mtVxyqmSCNTr2FFqxIKqKY/fCtaPBUXksHBPUSfBnpljRX64dikSDV7JIyuLkglf8YOBHpmTRz29jXmnekcBA0jd2vclqBeGqmgUtbKXFODvFmPtzRvhzeYMssPKmx1tQL8mX18cASRYj0l9L6Uvte11Wj5sH2f+naNfaM6Eu0UM9P5ZpROHs1FVJQyIeW0LILcq/VGQ9HxZQmyNdjxYCLxob0+Ag6JES8jXb1in0cuMQjcjvU3+5WlcTMC+wBPCtglML+++Bk=;5:M8CjLt6w4kXSbOMYkPo/cCX98ye57sR1GJVVJIFLZ67oK6YroLDX8od/nseN8rbOqFg3schER/ZdEzFf5Tq+FEqvOzRwHMFoy2L/v3popieuBNTQ82cO9fGwSbP17C0E3BpI4POd18Djc47bR4P2rA==;24:2PuWccZ/AacoSnYG0HiruZ1/5Pdrw2pC1ueR76VIxttctqoCYMLbCNwZNyYP5U2leIOHIq61OFF+4OCw3YPoKBgs8/nVuLtryteEpf1ueSw=;7:SGYzGIFO8VI5tjC8IfCr87ifsSzRKdVGH4xfQdI/B53zbIRnmtLmNWZyzSRqQ37uO6vg3hCCxrtU95LT9YpVuq/KFtVw+cDQpD1KI/km4ifnAOJ8PinrsjPe4r4tsjKjBdRMfVvc3QgK8xUvInpBUHdZ16g/eTcVilvVcUyd1kgJMeIdzlkMh+o9L3g83gQcIZ1v0UvjcvT2R2V08O6/CQEijKqR81n9mKBhlAtGe0N95Jny+EHq8YgsgDiboUzL SpamDiagnosticOutput: 1:99 SpamDiagnosticMetadata: NSPM X-OriginatorOrg: hpe.com X-MS-Exchange-CrossTenant-OriginalArrivalTime: 17 Jun 2016 15:26:55.9144 (UTC) X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted X-MS-Exchange-Transport-CrossTenantHeadersStamped: AT5PR84MB0306 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 06/16/2016 08:48 PM, Boqun Feng wrote: > On Thu, Jun 16, 2016 at 05:35:54PM -0400, Waiman Long wrote: >> 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 > But a control dependency only orders LOAD->STORE pairs, right? And here > the control dependency orders the LOAD part of xchg_release() and the > WRITE_ONCE(). > > Along with the fact that RELEASE only orders the STORE part of xchg with > the memory operations preceding the STORE part, so for the following > code: > > WRTIE_ONCE(x,1); > next = xchg_release(&node->next, NULL); > if (next) > WRITE_ONCE(next->locked, 1); > > such a reordering is allowed to happen on ARM64v8 > > next = ldxr [&node->next] // LOAD part of xchg_release() > > if (next) > WRITE_ONCE(next->locked, 1); > > WRITE_ONCE(x,1); > stlxr NULL [&node->next] // STORE part of xchg_releae() > > Am I missing your point here? > > Regards, > Boqun My understanding of the release barrier is that both prior LOADs and STOREs can't move after the barrier. If WRITE_ONCE(x, 1) can move to below as shown above, it is not a real release barrier and we may need to change the barrier code. Cheers, Longman