From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751676AbcGQXLJ (ORCPT ); Sun, 17 Jul 2016 19:11:09 -0400 Received: from mail-bn3nam01on0098.outbound.protection.outlook.com ([104.47.33.98]:33712 "EHLO NAM01-BN3-obe.outbound.protection.outlook.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1751577AbcGQXLE (ORCPT ); Sun, 17 Jul 2016 19:11:04 -0400 Authentication-Results: spf=none (sender IP is ) smtp.mailfrom=waiman.long@hpe.com; Message-ID: <578C107F.70000@hpe.com> Date: Sun, 17 Jul 2016 19:10:55 -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 , Pan Xinhui , Ingo Molnar , , 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> <3c5d5c29-7956-572f-2638-b85299c72432@linux.vnet.ibm.com> <20160715100703.GQ30154@twins.programming.kicks-ass.net> <20160715163556.GA7141@twins.programming.kicks-ass.net> <20160716011657.GA29702@tardis.cn.ibm.com> <578C0FAA.5030503@hpe.com> In-Reply-To: <578C0FAA.5030503@hpe.com> Content-Type: text/plain; charset="ISO-8859-1"; format=flowed Content-Transfer-Encoding: 7bit X-Originating-IP: [72.71.243.249] X-ClientProxiedBy: SN2PR17CA0004.namprd17.prod.outlook.com (10.169.188.142) To TU4PR84MB0317.NAMPRD84.PROD.OUTLOOK.COM (10.162.186.27) X-MS-Office365-Filtering-Correlation-Id: 41b191f8-6d66-4cbd-d16e-08d3ae979eb0 X-Microsoft-Exchange-Diagnostics: 1;TU4PR84MB0317;2:tAUbnoy1DlH8S/uiHR1T9+8mql9dQ7KfOoE1eMgEGz1SOVUxfA/8uU2/4xRrIOCl3354tWn1YGcZylRgeFRV6tW5/sbk7ZBG/c1Ejk2ibLQBPBAESI6kdPqOry+vH8SG/slsbxdb2fbo+NnTEHDIh5vNaAlaqnGlxNISI5lnAjq0sV3ADQWwuQ5mNhzDs7/V;3:TRqHZc6ESvaIpE1W/08jBR0DbQkmGJrsExkO1qB4vFui/+wb8S1GIgMYpEKbZ+7P0gpytFJMnf0T2jdtYNJtIFvrIgjE0U7JeAB7XrVZ15Qy1Vxylolp3OnysIBpj03x X-Microsoft-Antispam: UriScan:;BCL:0;PCL:0;RULEID:;SRVR:TU4PR84MB0317; X-Microsoft-Exchange-Diagnostics: 1;TU4PR84MB0317;25:t+6S66k/ABj/K1f3nBoeFHPsO5bhqyz2UMc1jUNSRFhf2LrwVshgzT1+c+EsUJYgRCdW6uyG37eI+QqfZC3jmqfPDf5EkVz3ibCdF8qjrsvOr6L3OOH3ORq1z4LncibWFPOk3hE74iFxfoKKT//gSgl0zUkquKaUjxGIY3As2eHkQMunqCBpowWldo/7BId1I97bbgOMj6dKwvfaEKuCiML7yRLZfowkLu3hCfj82hnTjapcenPdgYRsfRET6KxIcCKHfFxjRHp7ovpXHsRKwA4pUzARbyIcpiCECL8XOQFTLLfFmZdQXNK5uYiROKbvZjXE32FjuNp+UJUMBVnLTmGJQ7IZc2e7iYjZDdHHLiLfoHXsLomRMloR9tFLFDeixhrOBOsE7JEUQglZBNAZt0SmfCiZlJqJVLAkfi/3oaIflENLpBxY4o3wcry7YNAr7qRUewtoJtUoNzfnVBFACW16esosCFJxHEe+hf1/tWrFQwpIGAnl1uiSjbtts/Yp0Bf+Moui35r7LoGa5iWClaXQbFmQLcsdqVb2B6OixJDYmJrElk0WQX8S96jdSEeAy3IpPvqhUtb4XS/MhEarlSYygTQa3u5FhvFLBEwXtV1D3fCr9WHiKivoV/e6zAjksMvEWxNmVIcA7qwhn0urFCfkgsGWD10FXZc+G3QcBMYbgtITBsEM9doNmmvvEefS0pofBMPC19Vofnxhdzi3DMGvkQbbg/WFHNKHDoA5oiQccDfQX0IVbNetTTxwRZyw68YtdKOYSzRT+UO7TEBvTzEpmryYdRhtLZP2jVEOWn0= X-Microsoft-Exchange-Diagnostics: 1;TU4PR84MB0317;31:nk+zAURcE0yU0xwa0cTS7H09lSiN2NoeEBlBF69G/cud3BSrh3eWF36kN9OzxYpTduRdTAmo3ceErXBcUlNE9QSquRntvBAwm0kF61BptlvnWg7MQKh+dEjo7THljzG2N/8dzpIQTptBxxSSrtKzulrBsalYXrBG4LcreeQYuPJJniBBH7qrOzQl6sEyxmV7Ff2IOlDML557VMDaugntUQ==;20:UFV0K0ZCxYeW0njdpMiVOsi8GWvF2uhCQjqOswiKA5hriX4o4eDKWb1/vrpgM6gFczX2Vdjzkv5yBuzn4i715v3K0b0KS20XTZ7KjZuvEHpJ1cyPMrhjUNy2tBRE8ZwGVBjOXINXLpgq1pT4QdAI9JJ1wc/Xbm/wVWu+bvDynnWRcQy67cxdovJlLTanUwzIDJ770/jcnjG6NPrdnkxi4diXjUwfJSLgukpnf2CtjkuDkNyMRaCyTFeEyQ2QVSb7OB0YOjx+p7GEcT18oPQyu2xUfJtHbcdn5hHPvOE45kg9Qk+58Q8ODm+nwyqcDlI7rQySGc/2SZDY+rRev8rMATaDbmkWEFajuxxgNppDOoko+mYwVFkBbmqq7STFkSLldc4Tb6YCCaBiLnQw5NlvAypkSEiNo4ANmmXdUiUnOqLSPaLxzNp2sOIrIB833NNB2Cm9682EldOyL2DlUewfsbsXC90cJ5zPo9w47eE5KssRd1eFBGTBaOxjYfRj5Cfv X-Microsoft-Antispam-PRVS: X-Exchange-Antispam-Report-Test: UriScan:(42068640409301); X-Exchange-Antispam-Report-CFA-Test: BCL:0;PCL:0;RULEID:(601004)(2401047)(8121501046)(5005006)(3002001)(10201501046)(6055026);SRVR:TU4PR84MB0317;BCL:0;PCL:0;RULEID:;SRVR:TU4PR84MB0317; X-Microsoft-Exchange-Diagnostics: 1;TU4PR84MB0317;4:BXCJx9+WHnD9kNgT5jTeVpSzNgHDJnk4NEMa/p+eM+zBrRFA0K9I58yVduaYe9KXmJ79TRxt/6y/ZJRd6sDr0BDtiW1Ic2Lw4jdJAQP5fJwUPRcDcPcKVSlSeYmnF6N6XmCDtxsaeVW/guXNSvj7G19TPzsPF/fdJucvLeJAt3gYM5x5rSrMFlXf5t19FENsgJQGFUl/yIcpHGAzhD/XcJy0e/sw5J6dRw+GCCVqddS4kIVHC4bGbf5MXdoESyNNDzgpALNztHfvlncgmdsW2Dt90tw+hjwSeiE8UaRCHkBm183xVHHWw6GfBKMOa+qcNPI1c9IgyMvbnUS2JbmFne6T32ayzsHvzgqPowNYxsIjkWmO5JO3rpgYl5L+nEUuDKHW1M78i6BPZipD0WV1ba2K3ZBjXphgefZIKfOSqfPLWYqBZFll+TTvem189Xia X-Forefront-PRVS: 00064751B6 X-Forefront-Antispam-Report: SFV:NSPM;SFS:(10019020)(4630300001)(6049001)(6009001)(7916002)(199003)(24454002)(377454003)(189002)(47776003)(305945005)(230700001)(81156014)(81166006)(66066001)(50986999)(189998001)(97736004)(83506001)(8676002)(117156001)(68736007)(7736002)(110136002)(65956001)(19580405001)(19580395003)(65806001)(7846002)(4001350100001)(2950100001)(50466002)(86362001)(2906002)(4326007)(64126003)(23756003)(42186005)(101416001)(105586002)(92566002)(93886004)(36756003)(76176999)(54356999)(6116002)(65816999)(3846002)(586003)(33656002)(106356001)(15975445007)(77096005);DIR:OUT;SFP:1102;SCL:1;SRVR:TU4PR84MB0317;H:[192.168.142.169];FPR:;SPF:None;PTR:InfoNoRecords;A:1;MX:1;LANG:en; X-Microsoft-Exchange-Diagnostics: =?iso-8859-1?Q?1;TU4PR84MB0317;23:b9PU/QU2005CXBei47tndHZBs+paTzAgnCYFs6m?= =?iso-8859-1?Q?62ysg4vZevx/+RN48Z2XIU8XUE53g+D946cIeXNanbxCbLKxujD5qVj6kj?= =?iso-8859-1?Q?vjwyvkHxXJ7whWP2MMV/Pow+cwwQg/bmwTkQo1VkoiZ15DpMatmviUoKO/?= =?iso-8859-1?Q?oGAGQtsyVRj05JbB7OoSVF+RT5ffubjswXfPMUHLBwq0Lx3Mf1b4wYYJmL?= =?iso-8859-1?Q?yntmOHPcimYxt+XPeiXsOa1FWQDpeOgzk0tH9GRb44UrUBiRXZSth+nzZA?= =?iso-8859-1?Q?/HVapOtyF8U2452sklTeZF1lsLygcF19g62bm21CpWGHbHoyW1rV0UZOME?= =?iso-8859-1?Q?8X8jZ/2+A/WIxFR7ttjL2u0XBTxLK6f90/wwYMuzRsjM6BPWnTcSqW8fGn?= =?iso-8859-1?Q?99DxKsB2xeGQhf4dSfI4mIfivLsZarmpCD7A5VGxiq1ilZtLtRXYsA09zi?= =?iso-8859-1?Q?RyuqC5NnQOFF+u49vfR9WytxQFZ/vPFybKx6ZBW1NxfsxZ4vkplEpEWDK2?= =?iso-8859-1?Q?ErJXI6PHGjxV8FcfvwGhGA5skeXlLTjX4ShZjHI6uPi7gb9AZ2UYk3Bb1R?= =?iso-8859-1?Q?agEnwKMRAYjeJKvmg4Ehh+3+O7iWSqf4xRZqpq3wg4P3yonZMujN8lSCuh?= =?iso-8859-1?Q?Ci5ODn3A3FVMunuZ9GSxbRL58xUouClq73wY8u5C7KpkPDlXrFM6b1u9Xp?= =?iso-8859-1?Q?MV+3kfhnycyo85YS20Ru3/a1nD/er5HNpdQWM6m4E1r3V1UF8L6wyYa94m?= =?iso-8859-1?Q?N8KOOloJdsxp4nrEnJzOlUaag1ORwiTFd76KgNhOBFRCJCuZRUP9rstYXc?= =?iso-8859-1?Q?YPi1JKTcyApb0eqQokWBQ9USLkT8D0k0TiJ9MJh6/aETVlYdn2lkqSPIC2?= =?iso-8859-1?Q?tW56Pufx+o90/XdV+/v5z2cV8IDze8mGjjdgkNcXyv7f9A/qAItRxwpGkB?= =?iso-8859-1?Q?j6/woXxQ6YRI3wy57mqpG/n48c58jKp47jN1vYpbmvmQ07U1ge2NjfLWLh?= =?iso-8859-1?Q?HzDt1j+4ZO75wpmObsHR1R7DuaOmDnkYOxP1Dj25GMaUsDw7i0hDOpAY/X?= =?iso-8859-1?Q?jKP24+VK7y5CXlCb6OyH+tM/VQSUq4pzhfHnO07of609IBNMf9yUtmSxc4?= =?iso-8859-1?Q?E4IbcQrDKPLOY+cB0+phwgj+LYwI3QTre6BuxG6WZdkTUTOJs7JjdVqdKT?= =?iso-8859-1?Q?IZZlz16KZJDOTL+gCkk71nUa5bx6DPB0miM63b7MA72bX+uGQXvOL3htRL?= =?iso-8859-1?Q?XVYkGZ68TKxma2ml5VKb2il4kkErFUfOqethhB0FKYaoGfaNLuPrCHfpuA?= =?iso-8859-1?Q?FMTzNmqdgrRvruj41ikWD9geQWlEjbLjhhLmXkAXd7TeQ=3D=3D?= X-Microsoft-Exchange-Diagnostics: 1;TU4PR84MB0317;6:zoz1NusUSzHXfEYcwnACqJHLHpqzuerv9GJCpSRfakF9lM1iilXaxmSh6ZEvMpaKVa+qMLbwhtcnY//EzwimhwHUm/spTqUueRG46kPN+W8k/ri+SC1BZmKcWyridGGtsAB4kozQmjGAcEa52ymOY1unztYH3n40MPrpFryHo9QhuJIo4usTzfOcbRqJwoL/BezQGNjQ9kKiqnsIEG6TYDU4FY3CnrXsyBeBxjrbpK+uF+2Ad5uYaCO5uHBioEr4uzstT+dWinwjVM1Xx+Cxk1ceR+2JyZV8lI5mWWTVgTTBgUkyuiGZeUcEToB5c2dOd1Y2qTyWyRnzgUfV9pN3BA==;5:PXSH9u3dEtKm5GvXXRvm3Pm5CKq+CsCR6QTR3W+LM3w8Su84aUdvxpp++XtZBR197Mvq8Uci2z1Jz66UWuUO7aUZqRkS5Ql8EnZrL/9beR+i+zxFZt7UClkBEmp6mUfn4BJ1Yyq88tIp/269830zVw==;24:kFULWRYVbew7dQL9Tw+nmUdbYY2WYApRNMzgbeX+aTro13bUA6V+UJt3WkIqEJmcpRRd9c6eg6uWMXUVVmkNgRgBQ7MVkZEnC5aaKgl4MdQ=;7:UevmR/Rvab1Rj5thb4MZ4SvQ/BM+y9DEsmzmYgrMmOdk+Q4fEL2tXBFtMwcDe7+C70isRKjGMTQ4b1dUw/LthA4eQVNGjdfXJhcMpNrzZmp/oJ2e2lifYoo/ugbJglLd6ybCefGkt5pB2ndDMcW232cbwuJhSmUrQP+BmOoZZhd7ZTxNhDSwpTiooGBTlAn+DJw1dTYMHSCBvgTl7hOXU/IfXGywkV3FxnR/gnmaslBoaSZBov7SSgWCThT6EDZ9 SpamDiagnosticOutput: 1:99 SpamDiagnosticMetadata: NSPM X-OriginatorOrg: hpe.com X-MS-Exchange-CrossTenant-OriginalArrivalTime: 17 Jul 2016 23:11:00.7087 (UTC) X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted X-MS-Exchange-Transport-CrossTenantHeadersStamped: TU4PR84MB0317 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 07/17/2016 07:07 PM, Waiman Long wrote: > On 07/15/2016 09:16 PM, Boqun Feng wrote: >> On Fri, Jul 15, 2016 at 06:35:56PM +0200, Peter Zijlstra wrote: >>> On Fri, Jul 15, 2016 at 12:07:03PM +0200, Peter Zijlstra wrote: >>>>> So if we are kicked by the unlock_slowpath, and the lock is >>>>> stealed by >>>>> someone else, we need hash its node again and set l->locked to >>>>> _Q_SLOW_VAL, then enter pv_wait. >>>> Right, let me go think about this a bit. >>> Urgh, brain hurt. >>> >>> So I _think_ the below does for it but I could easily have missed yet >>> another case. >>> >>> Waiman's patch has the problem that it can have two pv_hash() calls for >>> the same lock in progress and I'm thinking that means we can hit the >>> BUG() in pv_hash() due to that. >>> >> I think Waiman's patch does have the problem of two pv_hash() calls for >> the same lock in progress. As I mentioned in the first version: >> >> http://lkml.kernel.org/g/20160527074331.GB8096@insomnia >> >> And he tried to address this in the patch #3 of this series. However, >> I think what is proper here is either to reorder patch 2 and 3 or to >> merge patch 2 and 3, otherwise, we are introducing a bug in the middle >> of this series. >> >> Thoughts, Waiman? > > Patches 2 and 3 can be reversed. > >> >> That said, I found Peter's way is much simpler and easier to understand >> ;-) > > I agree. Peter's patch is better than mine. > >>> If we can't, it still has a problem because its not telling us either. >>> >>> >>> >>> --- a/kernel/locking/qspinlock_paravirt.h >>> +++ b/kernel/locking/qspinlock_paravirt.h >>> @@ -20,7 +20,8 @@ >>> * native_queued_spin_unlock(). >>> */ >>> >>> -#define _Q_SLOW_VAL (3U<< _Q_LOCKED_OFFSET) >>> +#define _Q_HASH_VAL (3U<< _Q_LOCKED_OFFSET) >>> +#define _Q_SLOW_VAL (7U<< _Q_LOCKED_OFFSET) >>> >>> /* >>> * Queue Node Adaptive Spinning >>> @@ -36,14 +37,11 @@ >>> */ >>> #define PV_PREV_CHECK_MASK 0xff >>> >>> -/* >>> - * Queue node uses: vcpu_running& vcpu_halted. >>> - * Queue head uses: vcpu_running& vcpu_hashed. >>> - */ >>> enum vcpu_state { >>> - vcpu_running = 0, >>> - vcpu_halted, /* Used only in pv_wait_node */ >>> - vcpu_hashed, /* = pv_hash'ed + vcpu_halted */ >>> + vcpu_node_running = 0, >>> + vcpu_node_halted, >>> + vcpu_head_running, >> We actually don't need this extra running state, right? Because nobody >> cares about the difference between two running states right now. > > That addresses the problem in Xinhui patch that changed the state from > halted to hashed. With that separation, that change is no longer > necessary. Oh, I meant Wanpeng's double hash race patch, not Xinhui's patch. Cheers, Longman