From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755862AbcHCEFz (ORCPT ); Wed, 3 Aug 2016 00:05:55 -0400 Received: from mail-sn1nam02on0102.outbound.protection.outlook.com ([104.47.36.102]:36421 "EHLO NAM02-SN1-obe.outbound.protection.outlook.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1751351AbcHCEFI (ORCPT ); Wed, 3 Aug 2016 00:05:08 -0400 Authentication-Results: spf=none (sender IP is ) smtp.mailfrom=waiman.long@hpe.com; Message-ID: <57A164E3.105@hpe.com> Date: Tue, 2 Aug 2016 23:28:35 -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: Wanpeng Li CC: Ingo Molnar , Peter Zijlstra , , , Wanpeng Li , Davidlohr Bueso Subject: Re: [PATCH RESEND v4] locking/pvqspinlock: Fix double hash race References: <1469619037-13826-1-git-send-email-wanpeng.li@hotmail.com> In-Reply-To: <1469619037-13826-1-git-send-email-wanpeng.li@hotmail.com> Content-Type: text/plain; charset="ISO-8859-1"; format=flowed Content-Transfer-Encoding: 7bit X-Originating-IP: [72.71.243.12] X-ClientProxiedBy: BY2PR1001CA0032.namprd10.prod.outlook.com (10.164.163.170) To CS1PR84MB0310.NAMPRD84.PROD.OUTLOOK.COM (10.162.190.28) X-MS-Office365-Filtering-Correlation-Id: 2809ea24-74d7-4cc0-7583-08d3bb4e454c X-Microsoft-Exchange-Diagnostics: 1;CS1PR84MB0310;2:bBjgFboyqE7jlXPrRofKXdFko0yQrtVSDn5AT1xgZfxvQpZ8rmFQoe5bXvCJoVtCk6wy7heH2W+glHVs9LgvcjUlqGu8wCP8sCp6aWOsmChXqSgtKLeRn/zO4Se25gU9ApJ7ICk2UNJXW0QTjPPAkAd2suQeD1xJuixavCit3M6LuUM5JY+X26UKR7EF/ORI;3:fjFOeAWx5BJ6RIgUBpEggLGAGnhJNpkmm6+/+NUH1r2wX8pGKbjb3ODw65JGA+atWZN0WU1UNeGyTwLLC1NZTz1ecOMn+QuKiKAX9INzjL4dj8dUVEXQrYfXxl9EcZcT X-Microsoft-Antispam: UriScan:;BCL:0;PCL:0;RULEID:;SRVR:CS1PR84MB0310; X-Microsoft-Exchange-Diagnostics: 1;CS1PR84MB0310;25:zv5zv2AqekFv9dZj0sWnylCz9RsrtluNbH16ongRgzEww6mnnDlPP3+V2f38mBXfao+n6im7cp7ivA38GN8OVgO3Lz/4vLfVvFVs0oox0q8UiAwl7/nhINLrClKPOsOrYAEuEmF9YLf8g+Q9hL/RMZyuAeRLs4HmhdsdNa4sM6OAEJrsfNCsN+EdH7JKhkmWk3QLlJ6Q+jPBoJimknKwaunWxXfUm+KQiPuLtKB2cl5JKcYUrIYxvirX3FlKot8m2o9uNo9IWWncG4sQ4gg2vh6Hnw7l/V7qSKTnIqDuFwCaIG3DMn9aAPF/4fkNRnNZ1LGGoZY5QKW2aksibTR3NFXsYJ08T6TElYFsY6UrfYsDJTnqfLFB3kBd5Uif/xRCyAmmXTITUKvGpYbmjbvqshA4aj63f+9z/e5x/q/C5ENxEqgNVMOzmQXex6Jrp+HiexN4XFzROtcQ6nkmr7F/EjX2uiEN7tvLwe7CenonLahowsKBXbukS5xr1YjlXgydWPY/4DAueQe8+EjCAJpoKq7g4AlIAqSZxi19qE8z40fWwigbQ3ArdkgvwntORhCSeNUwSnpxBLlfyIzekhrmFbGR9jmsM7x9A4nUWyiNlHbi4ArpOx6VprS/v9Iw+uxtmd7bdPiM3WUeGT4HJ+Z9jP5iNbJP3Nsb7tXxF3Ese3IzFR+l6z3Gnd2Ps2cg1eST;31:aahS3a3MwUHBNOvtrSATeFFc0mAJsTr0E5fvHWvsdkrQN2il35W1BV8voWQzFDRZbJ2vFzbMb7WkVobLT4dCsE6Vzi8Z2i6qlRJu7ljNj3VScj+yDsDK9wdW97i4pKJ6IUba9dx1ZI0y/F6iQECJ7jkQG4vArscB2dfb1zPjnKnXvYbzUItAZctkFNqWRH97I1QlEQJHIKuw2l86E+HudMpBv9jkjR26eJV5EukxMUg= X-Microsoft-Exchange-Diagnostics: 1;CS1PR84MB0310;20:Hl2noGSBpecBbsHoHKxOkorSMaQ2+/njr/ZFmJ21QxVwoUNPVdBulUbznagAV0Lapdtjq+d/WXix3EgyKzr0xccEB0v76ILdsOjEnJHCyzimT2Aq1MyykbqOjp505uBhFwvok0yOsAZc+S3D3AaT+5jWB5iQ5hqO5GIGB7Ru23oYKkY96rBe5PwJugnr+eiBxhqnzUeJcU9uW4KrqzwuMfErnA+E+Jm56jeB7fzJ3M0R1+WlI+0pcZmu5xNvi5pI0IpKLClD82pxRTLMtaip2ssOGdg0rSBgsOyayTOQr4TF4kBh+An+YIxOupx0bgJG4YBvqi213GjzTHyQGGrhQq1VRVLnhPg8g0SVjKjje+VDge3A48ibOGu9kZjXg9WCFnMEHlA9dlx4UB/dO3tHcBElV0A+q4gjtq5T4pNxaAMc2krQA6EFYLFF2cytaqeSebjWxzC69DSj14VggKyc9ET7SJlE/pl0lr7t06pBJmrXthf0ovWwwwT6y+Aky+z5 X-Microsoft-Antispam-PRVS: X-Exchange-Antispam-Report-Test: UriScan:(227479698468861)(194151415913766)(104084551191319); X-Exchange-Antispam-Report-CFA-Test: BCL:0;PCL:0;RULEID:(601004)(2401047)(8121501046)(5005006)(3002001)(10201501046)(6055026);SRVR:CS1PR84MB0310;BCL:0;PCL:0;RULEID:;SRVR:CS1PR84MB0310; X-Microsoft-Exchange-Diagnostics: 1;CS1PR84MB0310;4:Cb4M5wfbJuZ+io2XTL11Ylz8s9M+GjwpyyuN049LcGhAQW7D8zMQGi3PGQpI6OviYkC0N0hfRnPto/SDw9+jKDPlEKKBiElVu7Aj2jjvFmo/xFQkjufM9ayRVlyIMWgq9sOqJtB1VjxS8AT8uvvJVF627/sHDzp4hSrSdS+B7/mpCJJLTJdzaAphHU1QZyoWWgh9F2OcA8SfDxbUDCgz7oU+m+13omtygsoma3Mqc16AnZhs8hL768xv0gX57ocrxtMsxWkIkpb61ECUttpcT3WdQR0vUoD52zxpS3kBOkUL2hl928yPNHFSrizOrYEesmzmNjJXJBFBtmiU3qXoa52PmF0Ze/9uDGlwfvUQoZawdU/RwNmhpXGcnv+EzZtrSXMTb4qyZaUcL9VKNAa+e78oiC6FsfhfrchZJ/j4QOr82iwen0aem1v4yRVK/shsi6gYPy4mVq40iGEBwKFtXlVXled3Gdd/jPOYCJ7nQMJKV1QH24s1tH1t6QIX/LI5 X-Forefront-PRVS: 00235A1EEF X-Forefront-Antispam-Report: SFV:NSPM;SFS:(10019020)(4630300001)(6049001)(6009001)(7916002)(199003)(189002)(377454003)(24454002)(33656002)(7846002)(7736002)(64126003)(305945005)(66066001)(54356999)(42186005)(50986999)(76176999)(1411001)(110136002)(77096005)(81156014)(81166006)(8676002)(36756003)(86362001)(189998001)(101416001)(65956001)(117156001)(65806001)(4326007)(4001350100001)(230700001)(47776003)(105586002)(92566002)(6116002)(586003)(3846002)(68736007)(65816999)(23756003)(19580405001)(106356001)(2950100001)(97736004)(19580395003)(2906002)(50466002)(83506001)(217873001);DIR:OUT;SFP:1102;SCL:1;SRVR:CS1PR84MB0310;H:[192.168.142.177];FPR:;SPF:None;PTR:InfoNoRecords;A:1;MX:1;LANG:en; X-Microsoft-Exchange-Diagnostics: =?iso-8859-1?Q?1;CS1PR84MB0310;23:TSQdVxjTOpVDYXK/Fjzn9CEh0XDzMzM9gZl93W/?= =?iso-8859-1?Q?Idu3MqsyhFpgr1a68Otuc+hY0FuUiDW38ESCtL76Lc95xLD3ykAhJlxlRM?= =?iso-8859-1?Q?lR3v5p98qmVI0MLNjjOyklHfymF3tUijdb6hTqH/hGxmYBfExrJPdjkT50?= =?iso-8859-1?Q?qW1LD6qZjGY7Jybvj0ffu/qFHvjBdEtkEFrO/if2LHMw9GIyNQCS7oP9/6?= =?iso-8859-1?Q?hEI/4PpofxoR3Zcmmoa5Hk9Iumx1dZyLguQBBDbr4YyigcJEchcNfk7ArI?= =?iso-8859-1?Q?Ia0j9V6mznQuoMhPy8Z6ky6pYRy4n5UMGeT+7kixj1UmHZ3QnMAkPyoIz6?= =?iso-8859-1?Q?Cm10SJLBYHASgnzCDpMJ0P9z3a8/dsEc5fojOhWAevAM43tvj7oz+Qh+oJ?= =?iso-8859-1?Q?Dzj2Q8L8nANujLOmSxm26MbOxFgJ9jHQ3uEKMZe24qQHkuR7qMte1Xj4Wt?= =?iso-8859-1?Q?yiYX7m6wsVPHkWduFOEn5/DRtcjp6gmRZt0L4JhoLJlazCZBA23QS3VlWe?= =?iso-8859-1?Q?Y3LZ4RigSIiK2jiOVbolkPa2voZYRQop9UF5JFPjsvzToPdNi6jDUYrr/k?= =?iso-8859-1?Q?X0LjPzQSnMmXRWrEUAsfw0n+LNNiN0HuP5grxFX1JkWkUz0Bhjy8v8TPqr?= =?iso-8859-1?Q?VrR87nyKhRRguetmDTyvbdpBtGCf6EgJEFsm5c2dmwYZmMvfJ963nbKNop?= =?iso-8859-1?Q?GC7jUoEVuBQsiU8Zly+IRKi5wEk/IwnQgOKxKQVSO0eH59AlxVE80KrNm7?= =?iso-8859-1?Q?IuCmuE3B3J9IOrH40pcDoPlEjj98DY3nt8p49O0lDyY5dJlxCPQ/DCfAzT?= =?iso-8859-1?Q?cwbb87h7UO5aOt2gaQ4wPf4SnYsFS4f/laN6WMfhU6OQPI6OrarO1+JD/n?= =?iso-8859-1?Q?7HZ/OrV2B2n1EqvaOK89zLRvQdN3QR4H4gmBF5UrY9R3GwCn+IulQoYIzd?= =?iso-8859-1?Q?/39DetBt76LWOsgNhfxWYP+JZo9uHRSBTPGJZlkpQjMkUPU9C2HdYJp0Ey?= =?iso-8859-1?Q?uU4xaMHo6W0Tuv0M/x+TxtRupXaLps/LrETgfzOvEl69jTWxWRbJXf9TZN?= =?iso-8859-1?Q?kF4BoFMXpHUzqNmIW1DUnjiZmsqAqyHFN8BAgNLfLhq7m0UY3WRqupt3V8?= =?iso-8859-1?Q?weK3nlg5yejlHQAHsKe9EUjJnqQ/2oC4sP+I8ZBNCH3DZVH2Gzx2YfwRPC?= =?iso-8859-1?Q?doSZjM9AasXH1oT9Gfq8jZkVZy2pyKwH/5bz78KlCBiuSrl/sSxk6wR5lo?= =?iso-8859-1?Q?TnKX6TWO2NNh32gK27a/hPMtYL/oar+qkrxg9Y22TszqhjhWQ+2wvGZy2d?= =?iso-8859-1?Q?zrMuMwIIaXITnR4w2aBvwfeZQOUmrn7P34B5vMdca7clA=3D=3D?= X-Microsoft-Exchange-Diagnostics: 1;CS1PR84MB0310;6:OJ0wGCpEgxch23vTQI8RPzzprwZ/xd3ima9Vr0MhiuRBGgQp8x0n+UKOKz6hLi5VEBNEbRyovvspEZMiGnqnyTSenDSJQWc8jL+swNdehalIn8u4ATL4q58KlNHV25bdO4TT/D6WzRyJdBVMMxWituZtzpMNiRn+IhV442rA4RnV3fBo8xygQ7O5QRbClTMsFrHu3EDous4DRlFN4tqnJ9FwTKLQ7sx/nx22dtx/Uijn+yqqM5wuhJcLPUU9H2i59tSfu8QfyxPuOYoCubSwmctsIGKhPK2VWw51Z8yYYi/6OIuIIC89lh+bqv1z9R3vY+zACVa2Hxqp43XtgLSOmw==;5:cUfC4HnEDbfOwmQQQwtW+bvqlLGkd2rZkFZL55vifoW4oHIH83Vfjklpo/LK+UodAs5MAoVDRJjlOx56P2o6qXbgywhxZG673xUP2eacEP9mMVr/RJ4sGnQG6Yn88iSVa+CwKqu/jfyH5PsjRntUcg==;24:YUOp+4OVrUm9273qnVHxbjDM/EIc+gJ5EnSkCFvAQ0AMzY4xy73JsZ1FDJ0Uzx70OhTGtsS0VHCPk4UDqqrBvHkR00Mbn/We6NuS+2KzmNQ=;7:5BOj6JQArUTD746c0KihHL1xjcCzUaDvNx/Mz4NHM1xcUuJv+l0wJ8vjhp/g83z7AzGwnOXQD0wzzaCIlyTgKu1WHTskV3mozg3TXWfV5E3G+jIY6ecZv8Q0+ZDER2+zWHHJ52+vB+HdjK6yQxFiufS5I0XkTHaghc4tWHG6DNj3WduNSM4R0k7A+bSnOShFSPUApBOaXpiQvwxq69wh3XDIH7mFREVlD4kbPFlTaUfjAI/kGPhoRvdlVLgNcIjf SpamDiagnosticOutput: 1:99 SpamDiagnosticMetadata: NSPM X-OriginatorOrg: hpe.com X-MS-Exchange-CrossTenant-OriginalArrivalTime: 03 Aug 2016 03:28:42.0301 (UTC) X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted X-MS-Exchange-Transport-CrossTenantHeadersStamped: CS1PR84MB0310 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 07/27/2016 07:30 AM, Wanpeng Li wrote: > From: Wanpeng Li > > When the lock holder vCPU is racing with the queue head vCPU: > > lock holder vCPU queue head vCPU > ===================== ================== > > node->locked = 1; > READ_ONCE(node->locked) > ... pv_wait_head_or_lock(): > SPIN_THRESHOLD loop; > pv_hash(); > lock->locked = _Q_SLOW_VAL; > node->state = vcpu_hashed; > pv_kick_node(): > cmpxchg(node->state, > vcpu_halted, vcpu_hashed); > lock->locked = _Q_SLOW_VAL; > pv_hash(); > > With preemption at the right moment, it is possible that both the > lock holder and queue head vCPUs can be racing to set node->state > which can result in hash entry race. Making sure the state is never > set to vcpu_halted will prevent this racing from happening. > > This patch fix it by setting vcpu_hashed after we did all hash thing. > > Reviewed-by: Davidlohr Bueso > Reviewed-by: Pan Xinhui > Cc: Peter Zijlstra (Intel) > Cc: Ingo Molnar > Cc: Waiman Long > Cc: Davidlohr Bueso > Signed-off-by: Wanpeng Li > --- > v3 -> v4: > * update patch subject > * add code comments > v2 -> v3: > * fix typo in patch description > v1 -> v2: > * adjust patch description > > kernel/locking/qspinlock_paravirt.h | 23 ++++++++++++++++++++++- > 1 file changed, 22 insertions(+), 1 deletion(-) > > diff --git a/kernel/locking/qspinlock_paravirt.h b/kernel/locking/qspinlock_paravirt.h > index 21ede57..ca96db4 100644 > --- a/kernel/locking/qspinlock_paravirt.h > +++ b/kernel/locking/qspinlock_paravirt.h > @@ -450,7 +450,28 @@ pv_wait_head_or_lock(struct qspinlock *lock, struct mcs_spinlock *node) > goto gotlock; > } > } > - WRITE_ONCE(pn->state, vcpu_halted); > + /* > + * lock holder vCPU queue head vCPU > + * ---------------- --------------- > + * node->locked = 1; > + * READ_ONCE(node->locked) > + * ... pv_wait_head_or_lock(): > + * SPIN_THRESHOLD loop; > + * pv_hash(); > + * lock->locked = _Q_SLOW_VAL; > + * node->state = vcpu_hashed; > + * pv_kick_node(): > + * cmpxchg(node->state, > + * vcpu_halted, vcpu_hashed); > + * lock->locked = _Q_SLOW_VAL; > + * pv_hash(); > + * > + * With preemption at the right moment, it is possible that both the > + * lock holder and queue head vCPUs can be racing to set node->state. > + * Making sure the state is never set to vcpu_halted will prevent this > + * racing from happening. > + */ > + WRITE_ONCE(pn->state, vcpu_hashed); > qstat_inc(qstat_pv_wait_head, true); > qstat_inc(qstat_pv_wait_again, waitcnt); > pv_wait(&l->locked, _Q_SLOW_VAL); Acked-by: Waiman Long Cheers, Longman