From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751915AbcGOTrs (ORCPT ); Fri, 15 Jul 2016 15:47:48 -0400 Received: from mail-co1nam03on0137.outbound.protection.outlook.com ([104.47.40.137]:52403 "EHLO NAM03-CO1-obe.outbound.protection.outlook.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1751669AbcGOTrq (ORCPT ); Fri, 15 Jul 2016 15:47:46 -0400 Authentication-Results: spf=none (sender IP is ) smtp.mailfrom=waiman.long@hpe.com; Message-ID: <57893DD8.8000707@hpe.com> Date: Fri, 15 Jul 2016 15:47:36 -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: Ingo Molnar , , Pan Xinhui , Boqun Feng , Scott J Norton , Douglas Hatch Subject: Re: [PATCH v2 2/5] locking/pvqspinlock: Fix missed PV wakeup problem References: <1464713631-1066-1-git-send-email-Waiman.Long@hpe.com> <1464713631-1066-3-git-send-email-Waiman.Long@hpe.com> <20160715084732.GF30921@twins.programming.kicks-ass.net> In-Reply-To: <20160715084732.GF30921@twins.programming.kicks-ass.net> Content-Type: text/plain; charset="ISO-8859-1"; format=flowed Content-Transfer-Encoding: 7bit X-Originating-IP: [72.71.243.182] X-ClientProxiedBy: SN1PR0501CA0027.namprd05.prod.outlook.com (10.163.126.165) To TU4PR84MB0320.NAMPRD84.PROD.OUTLOOK.COM (10.162.186.30) X-MS-Office365-Filtering-Correlation-Id: 663fdc4e-ad62-48d5-3e44-08d3ace8e399 X-Microsoft-Exchange-Diagnostics: 1;TU4PR84MB0320;2:7Buuw+goycoFtNa8CB0GhXRpnCDVe4W5gR1J+Pe2avNPYjBfiAwUnXC3tN645uFPtV3ByFz59bbzy2eYtagvvM6Ss8rwuh+T5i/bDYV4pY2G3RwpYPLy+5kHZ30B+2UdGB9sCwPT9Hr1Cg1DvPingelxQUbb473Rn/VYWwHkW7U7pWBxegiKr1G9xOuUwwUo;3:VoHbNz735DVpk1AClB3di2bhOtEmX0QLBDAtoPShAc66vQO+HtJ/5UJ6kKXjFXariArS3dPKxkSY0EekIaDC544LPTytD1s6ogEsBvIRNDK7ri2yr79UGa2KJfqhXez5;25:+L7hwXnWOYA+E9GI6hrrkxJGz29yF2ijVve1v79bsqOIqqMnKAvXK1y61FWLne3ROXrPFWeu4quE7bKwdkrgo23/gfhUYr12Ra6yU3sqcZN6Bt9LPVBMRM7gix01bghp+sbaUP7hiLwHXtdUp0S/irBZKhoPVEzHeuvkxqluPZnk1JwXQ/mR3sLf+QzbIBDDAlWDDMVbh1Te9pJJ/MGxAJiK5mn3meg/T81KZfFqyEz3Z2InH3vFipPewGdw7VzVNhiP4InBsdAwTYeK7RhnbAWRQaaZVpWxz4zMQak3SJ34l8I4b0boSV3a6bcmuRpgOLFw3wz6pob+KcgWk6uWliFaXDBlOO4kzMXljMbBltSyMYlxqDYkL9fHKi+KZHqVbVEbS/B8Ed9Xmm27KzuZ8V3t025/Gf40KEsgEbCyE0M=;31:72H12K1Nptd9K/FsiuoRbBSL05nR6MnKh1thi1UA/ZXC189mMF3BdMROQfVcvxFi5NbU+Ox6ndGOLPSwlU237RI3hyj1O8ITRoeC3hj3R8n5ciEXT1olZRUME04dPqL9TjjH8Jp99pK/pRI5UW0FxVxWHw+LTzFlRhb+GMN3SYEZ8i3WyM8C1DSgyq/Sue0nFhP6g0rsDKkfjC2/EIePxQ== X-Microsoft-Antispam: UriScan:;BCL:0;PCL:0;RULEID:;SRVR:TU4PR84MB0320; X-Microsoft-Exchange-Diagnostics: 1;TU4PR84MB0320;20:ubEJ3WJetD3xof2aJJerCEGDcg9jLKYVivDFwtspE0jieVWBP5EYam4cOl+hTTu98UvYqbIC2zAxUORV26jr5xRU2ngF9SndWkU4LK2NjG0Y5DGSRBEwYpPWerYRlBX5hYqz7LOb2S7XcSgF2BivaDMAhp+LmtTWmhtW4HCMm+HsxGWeQfTClhtESCWFr8JbC5tvne/vCXaUVFdRBLJ5UI0M0usORvhnNBO5SXNz2fU/a+xdOBwbtJGnD/QpMLj0S3aKU6m04CJvlCvD9IZ49N9fJt7Hk5rzntdko6zn4+Wb2L7w9fXIcX1ptsHB0nKVB6hb8M2t/GCWCWdD0uRecapO+jfDUT0zrwRMblAm9fo9qE0LZTYq/MV1p7Jui7cOp21nTayRO9ro6ImOMf7Xev0t/ps463wPa68YMJtG1WSlVoX2/Thkk+4KMM6tMS7BvqAIOKYsoaPute5pCC8OYFcjs5UHeCuQPgBj5BSri7J4o9xLU0sjSY8tQ7JwuzJ3;4:g3bIBWHs2nueyVwpitEeZUxzYC/W9M6JFfP5M9VORCM8DeDSIR17dvkM/91r8XSg30QiXMj+2vsjaeads8bZ/xMI5U7Nn5lhu1BLP8zlqCwsL5bTgf84n1kPHtFizrqJC3lK/4kcCyVZVIOjuw9zgQ3XfrgX/k/ScZFRVs00kEkpVPgYqpfe89Tub97dcQhNOdvoPFJL7AfcsWbG/lqpS+epIwI2qVX3pjQtupR2gudsXvvcRVSKqDGepUUySxLkSVzlmrZMtdcTJ/TIr6prfIQG97jydCe8aApkUpHE9EqUGZiWfrcEfFv6sop07FcM98MN4TeYCnfS8UnUHzKCVtXjF0piUGzbeja0nduJj5YcAjnu84D0qjlVYDU9pofie7ED/XKIC7UHsmfz0r9pcA== 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)(6055026);SRVR:TU4PR84MB0320;BCL:0;PCL:0;RULEID:;SRVR:TU4PR84MB0320; X-Forefront-PRVS: 00046D390F X-Forefront-Antispam-Report: SFV:NSPM;SFS:(10019020)(4630300001)(6049001)(6009001)(7916002)(24454002)(189002)(199003)(377454003)(97736004)(23756003)(4001350100001)(65806001)(59896002)(42186005)(86362001)(110136002)(189998001)(99136001)(586003)(68736007)(80316001)(81166006)(83506001)(81156014)(4326007)(3846002)(6116002)(2906002)(92566002)(105586002)(76176999)(54356999)(87266999)(50986999)(101416001)(77096005)(65816999)(106356001)(305945005)(7736002)(36756003)(117156001)(2950100001)(7846002)(50466002)(65956001)(66066001)(33656002)(64126003)(8676002)(47776003)(230700001);DIR:OUT;SFP:1102;SCL:1;SRVR:TU4PR84MB0320;H:[192.168.142.168];FPR:;SPF:None;PTR:InfoNoRecords;MX:1;A:1;LANG:en; X-Microsoft-Exchange-Diagnostics: =?iso-8859-1?Q?1;TU4PR84MB0320;23:+F11PwxdwWkLbnmjFyiEGEC+Du3xv+z9sFYesVD?= =?iso-8859-1?Q?3EQa5OhhmRUVmq4PzSFDnNU1zXpZ1jsIbvsmtphc4Yh+0bbIF9PSKbbl6M?= =?iso-8859-1?Q?5imQcPoCKVa63xOhycZQ7a8jf+vDTtw6Cy0z2oYL8tW52tglX8sF4JoWiL?= =?iso-8859-1?Q?PFztGI6e0HLt8Q68TQ1O3VWgVjLkzoOLjJHuDj6FX67bJ9uJwAFuKsCPxw?= =?iso-8859-1?Q?KNknlIjohc+bUFxGM2t70HPtenfjKvkWHOJYaaVfFpkLMqH1thamg8C8YV?= =?iso-8859-1?Q?ZrfLdhmAYVdSKQsPoXCKllvcZisIL0Ld73GoIkBVDNhFnUrTjTSVjT9pic?= =?iso-8859-1?Q?YTgYptqA1Kg2CJqCwuGUvCn3RPAa4HK/4cX5DLRDFpAJbA19Jfu22dpuXu?= =?iso-8859-1?Q?VFM8EfLDU17Xz9poD0rYqipKr2LKHrb56uo9On0ePLy3ycIusggO8257iU?= =?iso-8859-1?Q?p1oIqOFwtzho0WKvokJVgoIBGUPh6+Jyz4Z4dUjN4e2nMyCnf4rxSZLDBQ?= =?iso-8859-1?Q?3hrumQIbMlqTxIG9Lqs+chxCaC96BW8nkFDSGYL93oTWB040p/9W8CU73L?= =?iso-8859-1?Q?SoCC/cqT2vwaQWs9wPWuB2UpXdCpsR5W39dXnA4f07RwCaTGvQ5VP58p1T?= =?iso-8859-1?Q?Nw2OwsV4FYe8/V/MkHK4Emaip7Xrj7BzPSBCeel8LdBGv2+GkoWP3/IP1w?= =?iso-8859-1?Q?CGUCtFkh2E61oIMdIup85yfY6xOw3w0gGMM39Hs5/ACZ/LQkOBcQ1rz+Rt?= =?iso-8859-1?Q?KFEd0kmQ4tkK1Qj0GcZAnkHEFSxZLMKi1KGcAPRoKbaKxoJVrgz4AbYnVc?= =?iso-8859-1?Q?W0JYbDlI2+B/x5oZZWdpgyU9ZpM/u27ILVMVuEe2svW5ONSorbQSSRvI5y?= =?iso-8859-1?Q?KVqgH9S9vZ1cIqMnJvi5ke8UeUZQ5FzOV+Zmh7lemyuavzrT1qy3L+WFO5?= =?iso-8859-1?Q?AXwdn8cCSXpJ52KZrCrvRLGtCG7UnQUlkD8oTQJr4JOQpzN2dQaGof9jxR?= =?iso-8859-1?Q?2Xji4vzOrhMAHWMkmmTDJA4KkXnq6MrmkcDIOMQ/F2CY/SXkblkvArbgzY?= =?iso-8859-1?Q?iXEhWw3Rf4VjPYfWi7D0wMTZpqouoGM9NksbNFCydxU+QWHeFwjJKkFUcg?= =?iso-8859-1?Q?43pSTCeFyqgDquOInftvkplxub9665V8LbExQsXFdVLgcK5QeCzDlg8iNJ?= =?iso-8859-1?Q?9OFnAlTfH0xA6zqPlDxqSZ1lhykoeuQxfgCkTX2WGDT6CJe5JEMN8D+9Iq?= =?iso-8859-1?Q?7FPoa9wmWp0bxd/mjI507856oG5/KFDdsHMaxfSygxDkUqeM0OB/+h3gf8?= =?iso-8859-1?Q?WE2UbOITz8K3PC2mY7ye3kq?= X-Microsoft-Exchange-Diagnostics: 1;TU4PR84MB0320;6:4qOOsvgVA4zB0WK8tVYbL7H618j7U1Mx5gCbAT8RNXxar1nSgkcen/zwiiuwOtjAXFwFJFqoL0AhW5MbepzDmKrCgwJ+4BTgS/AGj6zf4yqKzSNwtAe0NDqqSY0NOBRCdlI781xyQAKMkrqwVC0/ik8mAO2OOizjBAiHoR5w2wkv2fC+mr6EHpSlOXAxc4ysvxQx4g635yP2mFhpCjfrEPpUarJOG+GRqrnyr0Ag976sVQmZapk/Sd/3lLTbdfbeTHQyP5on/g01BdtOYLFQjTLcCIZl+Uv/1COUGzqxzzrnHvCoYjWoq75UnphDqOdvvpoh6qXbktZojC2C7m3VeA==;5:JGfY876InzhPUeWS7gbk4F3Sij7XnrSk5W+fp+AZGkVT3OXlay96WNJgF5ZTzbpeY1r5rotoDv+uQN0PMMpLtYjsDLpyrjqajDeptastti/SrWL7fdnblFiVGhDUDG5zW5b3NZW49OGYGnrs/OQ+nQ==;24:50vkfzfvPygJ4H7SvEDZRgugcxI4gW4bqlV+v46d/NBC1ijkg0JotNxqVepEOLYulw4jx5iS+VmuBggCgOjT3XLjjDYKbeiGe+NpA8F43JY=;7:GiE9bRsOkON3nMblmzUQLHRNdunLsqcs8u0hQh4zfM+kPvtQpdbHdK4o0hS/A5TcGXRDGwu97FFuGcNxSbA6d0VLcKDFbPEVt73szfCyobFQLFsFlrTTNcEwvsFUjCWSaMJPzKhvegZQC6fg1yibxSfjDuF4Z7RS3bVc1SvYblzqakpO4VWZDMIYHmaLV6YU7dVVwd92qhn38HfQP1rD9cxiTdDjtjvwJghdpHA4q81Dv6OYmyy/GX9/8Jkt8oie SpamDiagnosticOutput: 1:99 SpamDiagnosticMetadata: NSPM X-OriginatorOrg: hpe.com X-MS-Exchange-CrossTenant-OriginalArrivalTime: 15 Jul 2016 19:47:43.2295 (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 07/15/2016 04:47 AM, Peter Zijlstra wrote: > So the reason I never get around to this is because the patch stinks. > > It simply doesn't make sense... Remember, the harder you make a reviewer > work the less likely the review will be done. > > Present things in clear concise language and draw a picture. > > On Tue, May 31, 2016 at 12:53:48PM -0400, Waiman Long wrote: >> Currently, calling pv_hash() and setting _Q_SLOW_VAL is only >> done once for any pv_node. It is either in pv_kick_node() or in >> pv_wait_head_or_lock(). > So far so good.... > >> Because of lock stealing, a pv_kick'ed node is >> not guaranteed to get the lock before the spinning threshold expires >> and has to call pv_wait() again. As a result, the new lock holder >> won't see _Q_SLOW_VAL and so won't wake up the sleeping vCPU. > *brain melts* what!? pv_kick'ed node reads like pv_kick_node() and that > doesn't make any kind of sense. Sorry for the confusing. I will clean up the submit log to discuss what I actually mean. > I'm thinking you're trying to say this: > > > CPU0 CPU1 CPU2 > > __pv_queued_spin_unlock_slowpath() > ... > smp_store_release(&l->locked, 0); > __pv_queued_spin_lock_slowpath() > ... > pv_queued_spin_steal_lock() > cmpxchg(&l->locked, 0, _Q_LOCKED_VAL) == 0 > > > pv_wait_head_or_lock() > > pv_kick(node->cpu); ----------------------> pv_wait(&l->locked, _Q_SLOW_VAL); > > __pv_queued_spin_unlock() > cmpxchg(&l->locked, _Q_LOCKED_VAL, 0) == _Q_LOCKED_VAL > > for () { > trylock_clear_pending(); > cpu_relax(); > } > > pv_wait(&l->locked, _Q_SLOW_VAL); > Yes, that is the scenario that I have in mind. > Which is indeed 'bad', but not fatal, note that the later pv_wait() will > not in fact go wait, since l->locked will _not_ be _Q_SLOW_VAL. > > Is this indeed the 3 CPU scenario you tried to describe in a scant 4 > lines of text, or is there more to it? You are right. The vCPU won't actually going to wait. It will get out and spin again. I will correct the patch title. However, it is still not good as it is not doing what it is suppose to do. Cheers, Longman