From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751873AbcGOQpE (ORCPT ); Fri, 15 Jul 2016 12:45:04 -0400 Received: from mail-bn3nam01on0127.outbound.protection.outlook.com ([104.47.33.127]:64032 "EHLO NAM01-BN3-obe.outbound.protection.outlook.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1751615AbcGOQpA (ORCPT ); Fri, 15 Jul 2016 12:45:00 -0400 Authentication-Results: spf=none (sender IP is ) smtp.mailfrom=waiman.long@hpe.com; Message-ID: <57891301.8040809@hpe.com> Date: Fri, 15 Jul 2016 12:44:49 -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: Peter Zijlstra , "linux-kernel@vger.kernel.org" , Wanpeng Li , Ingo Molnar , Davidlohr Bueso Subject: Re: [PATCH v3] locking/pvqspinlock: restore/set vcpu_hashed state after failing adaptive locking spinning References: <1468496357-6400-1-git-send-email-wanpeng.li@hotmail.com> <5787A718.5020901@hpe.com> <20160715070925.GO30154@twins.programming.kicks-ass.net> In-Reply-To: Content-Type: text/plain; charset="UTF-8"; format=flowed Content-Transfer-Encoding: 7bit X-Originating-IP: [72.71.243.182] X-ClientProxiedBy: BY2PR1001CA0071.namprd10.prod.outlook.com (10.164.163.39) To DF4PR84MB0314.NAMPRD84.PROD.OUTLOOK.COM (10.162.193.28) X-MS-Office365-Filtering-Correlation-Id: 5c3cc63b-cb75-4184-2e00-08d3accf5b04 X-Microsoft-Exchange-Diagnostics: 1;DF4PR84MB0314;2:caZYDzs1CFMUdw6W7gSSnzghzf34wDPwh9YNtzKZhhJdTgTknhQDJ/zcgtNPCBTuocOsKlSUtZZ1rZOWT1EUgN5Sdcwvxx0H8XZjy3lmsCkFl8HaCmfftQEpeJ5fh18Ogpfwy/hRucPY+Tlmot1khTde7w93WNAH93WN0rK3kYVOO5CGPnpBlap3aahO+g69;3:JgXB53QSe0WIwrmYLfk2jWc1IGdAxO5Me4t45Ju1oTu7vRZDyOfRrEZI3pELoO35XvwiYdPp/vmBC3/HuXZ7grqOOoOIto/8b6rFlNjWv18dUEKnYb8Xra/cj/p9Gl1m X-Microsoft-Antispam: UriScan:;BCL:0;PCL:0;RULEID:;SRVR:DF4PR84MB0314; X-Microsoft-Exchange-Diagnostics: 1;DF4PR84MB0314;25:Pr2Yfh8kked3yZmpwU2aWiGOZ97vTf+Gx/9zGxNah5a8OfYFbGrYXHSVMLjude/qm26HgH4+a//8JG1VHuHDs7GKW5Sq6N1H5MbxILAUAQf8yrTvUuRfoP9kP/rKxF2rTNrtxgCMIe5oh5574BwG9A/tChk5Toa+jqMw3VvByofPE1AOK1TiV+9zgo7THVg4UMXk2Ss6gakSy41mSXbB4wWG9wPfSq+2pxQ2GsEc11ygFtQ7G5Tbg96h9VVyMVUDvxTWu2eDTqTbXe4x3y+aD9t3PxeYEXlMPjmTOZkt/nbj9ej7W6FcpxFo107rCDB6jdOkgQcLudqumSrxqA7bYZuFRnlK1NHpk/076RZaPerH+iH0KU31KpZF8cztPz1vLM6iwgu1bIcx/QFtjfCehxQNdFYt0AhHXzAjoR4CK08SvRvr7OwXW7JxT/0ZlpOfQ7rfYsa0mRvJ8gb4/Wa5P7Ep0HfXgkp2GthyavVmAmjvNOz6/beHeB39aYaj+sYnfRsI5kApYVtu6fTthvlgi+UYz9tCAOY2dyZneHIJ4YtLkv7X/b1r1K0Lr13SKReDMCCjDkiHjeiXjVt++TD4LywUtNUkige0rhki7yswApKQ8VFGDHnN6663lJDcnAjpxguvt4qTuX0RsdCeKQOqb7AwpT3szwnYVEs9LoH2f5I4kdDPQX9dhVpyc+5pXxuO2hcwAUwzFKjiOmo0vJtATlj7krEtRFo8giEYsA6Z7f4=;31:7zGaHg6C35ITT+1D/RTf5UEdYvx4lyb125muFKrZ6zkpPWKSlIx2aSYjhMjKBaN9aYDDBkzaBh15NH77un9Lw+PtXKDxLIZRTqwlkW4zjbfo2g4pZfnD2h1aleDRWqT4SN4932ngUiSTowM52R9BXlrbgX60pS56b7oJQv1cVF2+lIwj22nK0nMwX/lHIon2brkNQ4JfAXEfhIPNKwFT2Q== X-Microsoft-Exchange-Diagnostics: 1;DF4PR84MB0314;20:5YlWdxiX3pGE73jCogrgOhuAPwZ1GWp8jDAQ9K3G0/vbLvudwxkRno8S72Ul9CyoSE+82WLBkP035uZz3MIJKieTKZew3bWw8pTbQiJSafHXssdhWfBi0ZtPg8LnQw/zNrsP/PnGcWEy+uzpDeVa7YAAgbGV4H1J9cl9f8hWvAjlTrzWePYHr+ljFwaB/1Lh0I/t/x6HuUjjSMUITJp2TChAsJfVzxjgPVqRh4uhpu+SsD/gnimsPR5cuoUNCOIu6nDDvi5fJWzSTN2vamaBwTZBcj2LqEw7GVGVswHTPpWQVL/dUmzIn9zuoAAr3hndDjhwgfDZIkmEYRXQDnz3xoCfr0WfZUEVgM32DQunMO4GmvetnjsBRxNsl7IF9bfwqVrUEyhJKcBeXf0D6ByzFADVf4wEYq9fWo1zDaa2TNTP901JvrRCU6yVWECXosMZGH7sFk0cetOPyLr0EbBm2S20UrYXnTbvzyPD4qt9cm47SUtErmqbisQ8VuLU3AGG;4:wqn6PacWxANia96LM1keZGOVgcEHv/j7bStekA1poKXLnFxvA2rJsNWohCHtsGav5Z03bD/iR4HqPFFC7T3IUj6+EUc6cEQ8L/KXhRoE+Z7gZ3U5BA5uD+vIPEEqB4l7jhmg8+4ogUmudcBjP9f+DexoHuDenqmLdsLe/xwqLDSTog2D8PrFbQSMzRv5fDqFoEVYPb6EU5dpi4Xih2unJXZ1GGCvuqFS9xAjSDYxBA9ztgQ3UdLFbZbiNrUMVXwRyU7sPm9n0C+taR4GqbaCIeGAEFiuwQveLqP6F2WoDfWA5F2CsAqttqF3iz+SZk+w8Q2cJyWIegRHgqenEg3M46WtGBHlsESlSOpRTg3pzsAGCuMkruIFSFknpKwM3uhL1+wYaKmyDILev8fVCqtUe3DrM0+m2Ts/GcIzOSOnk5LSY5blf0EL5zmXzQIO9dR3 X-Microsoft-Antispam-PRVS: X-Exchange-Antispam-Report-Test: UriScan:(227479698468861); X-Exchange-Antispam-Report-CFA-Test: BCL:0;PCL:0;RULEID:(601004)(2401047)(8121501046)(5005006)(3002001)(10201501046)(6055026);SRVR:DF4PR84MB0314;BCL:0;PCL:0;RULEID:;SRVR:DF4PR84MB0314; X-Forefront-PRVS: 00046D390F X-Forefront-Antispam-Report: SFV:NSPM;SFS:(10019020)(4630300001)(6049001)(6009001)(7916002)(377454003)(189002)(199003)(377424004)(24454002)(97736004)(47776003)(586003)(42186005)(4001350100001)(3846002)(110136002)(1411001)(7846002)(68736007)(6116002)(189998001)(105586002)(4326007)(65816999)(19580395003)(23676002)(65956001)(2906002)(65806001)(19580405001)(66066001)(50466002)(77096005)(64126003)(230700001)(99136001)(81166006)(83506001)(76176999)(7736002)(122286003)(86362001)(93886004)(81156014)(54356999)(50986999)(117156001)(92566002)(101416001)(117636001)(87266999)(106356001)(36756003)(8676002)(305945005)(59896002)(2950100001)(62816006);DIR:OUT;SFP:1102;SCL:1;SRVR:DF4PR84MB0314;H:[192.168.142.168];FPR:;SPF:None;PTR:InfoNoRecords;MX:1;A:1;LANG:en; X-Microsoft-Exchange-Diagnostics: =?utf-8?B?MTtERjRQUjg0TUIwMzE0OzIzOjJISEhFYmN0Nno5UC9TTHVzYUlzUGFINDVn?= =?utf-8?B?SkQwbXJmeFlKNHhkVUtaK0xmY0JPTzlaQVdYV09QN3A0dUkvS21aUXc3VnQy?= =?utf-8?B?enY3a3JiK216cXVlYjJHZVdRVVFBVWxPYk44YVF4UGRDUnRobDBlUWE2S3BH?= =?utf-8?B?dlNHOFYxLzZpcjdxQ0t1VWFrSWNlZEs4SDU0dDdMZjZGeDFNK1FZd1h4eGZM?= =?utf-8?B?RVpzdFFPM084U01iUE9aMkFiZWw1bG0vc3VQcSsvL3RoS0JsL1VEMUYvYmh3?= =?utf-8?B?UHJhQTcxdktYeVVPL0ZjUG9VRVNDbHBGaGZKOEFrazVmK0dBMzBSY1c4WVhL?= =?utf-8?B?bE1PeTdIcUlNVU5JYUJFRkQ0WXVaRWlBNFVYakNqMk54YnduNktCaXJwMDA1?= =?utf-8?B?c3gxUkZkKzRIMHVzc1czL1NyNHV4RmpZQnJhMjkrRzdpbUhOKytJNW10UE1K?= =?utf-8?B?UjJhUmNYZzBKelUxRWUzR3pwdmdXd09FeVFiMjFXbGNwVGQrVGFnZ082dmgw?= =?utf-8?B?SGxBaXhwSEhSWGVpcExic2dEOUltZ1E2YWFkQ0R2c1I4Znl6WUhqMkdFSFRy?= =?utf-8?B?TlBnM0dSTkFuV1VObWhpY0piUlNZdGRlZEMyK2FWUlFtMmhQRzlNK3llU25m?= =?utf-8?B?YS82S0ZCbjY2Q3ZENmRCQ2U5NmJGTFFOSzlIbC9tQWQ2aFhpWXNuWFNNZXJ4?= =?utf-8?B?MnRidWFPTDdmZEU0MjlwSHFaSUpka210Rk9WdTc4c0FrMUFPZ0lVMHdkUWxi?= =?utf-8?B?Wnl3b3BGV3hGZEJqVU5xTEhmYTNqMHFNUTRVQ3hSUDR4Vll1SUl1dEVzY05V?= =?utf-8?B?RTJWdFhpVGQ3azFKSlJxUDVEcllhQ04vUkVXalVZejVid2tLYTc5MkVaNys3?= =?utf-8?B?UW0yUnNPc3B1YWtMcU1tYll6OUF0T2JPVktSSWhCVHdOZjd5U3ZZL21xZmVj?= =?utf-8?B?Skd4THdHVEJ1aHNrRWVEYi9YWFF1ck9IeVc4cUs5NzhKRnZsMG1RWnc1K3FE?= =?utf-8?B?WDdXcGl0WEZTbm1uajAza1FNSnI5ZWVxRDk3cUsvRFZXTmgyMGNtbGdiS1NE?= =?utf-8?B?ZFczMnAxajc4eEI5VEtFUFo0OEtEM01xNlVRUVpXRGM2TjExb2Fza1RUQ1E0?= =?utf-8?B?YnI5eEFkVGJkTW8wdlc0NDI3T3cyUmJHRGg3c2psWVJWSmQ5TjVMdTZMemVO?= =?utf-8?B?K2x5bG9tblNDQ1oyS0FGbStGMGh3NlBwYXlGdG1aK1FaR3pEZ0JVcWNCa2dy?= =?utf-8?B?Ums4Wnk3c1A4eWlLUHZ6azY1dHY0emVocHlhMWk4RXVzeGhwTmZydENsdit3?= =?utf-8?B?d1VtcFZiNWJ2dTFJU2hRUHB3UmtoSWZ4MlNXZFR6SitYQ0RmWlkyeEhRVGM1?= =?utf-8?B?NXJHa1haZW9QVlN5MFRzMTIwbFZRdzN6YWJRWkhJK1MwZ2Vvc0ZHYXdobXRs?= =?utf-8?B?ckFIS2VBQ3FrdEdNK1EydHZzVlFwUlkxeWpkUnRQUjdHZEl6TnBudUZFQVMw?= =?utf-8?B?aUVCSGFYSTI3SFlzem9jWkVncjVGaTUzMGZQcHRCYVBENU5kTCtVWnYxNUhm?= =?utf-8?B?MFhOY2VXc014VFRJejlvb2pTc21hZk8rMVNjeERva2FtbTY4SmNxUVZOVHRj?= =?utf-8?B?UXRmd1ROSFlaUEVFMnZ6S0c0d1FOUmVlTnJ5eURQU3k3dk9TUVdZb01nSjlK?= =?utf-8?B?bHBmbStadzFwR2swaUFKTUEyMXVrNGZRRzEvM1o0RDhmM2NCUXV3MEdxcVdH?= =?utf-8?B?WVI1bnh1NTBqYVNLL3BLMG5aaGsvVWc4T1JZazFBb2lHa3ZLa0Vrb3RHUmVk?= =?utf-8?B?VkZITkFqWklkdXE4MmkvWTZCSVpPdFpIRkVNaWRMdEIvdFBHU3ZwRGNkcjJw?= =?utf-8?B?VTY0bGVjZzlhY2JpWVY0bWVZVlRFell2S0RrQ0t0a1ljM3dnNVlWWThsRU9J?= =?utf-8?B?Qm8vZm9ad2t3PT0=?= X-Microsoft-Exchange-Diagnostics: 1;DF4PR84MB0314;6:5heNbfbdYdDc3Yb/9nce5AysIUNQNgMyySRi7fPBY5UMBzzIh4KGhRZyfGZ5wl2Gf+GdDJt1woSgS3ozAu3krO6Ls0qwlOiQQQ9gRNQ7O4h6yzMBJUkcdjNhYN5QymtC/nlXqYea23ZKvg6auvBMI9ZJ4IZ6IM0+5OAigKk6drUls6w7jvLZVsBbZNUCGoNmsflBa83A1uc4oO54dIBY4U5ajsE5arMesDk4vF9qqGltw8t/ZjEPiUuEU0DiLBF4TNtPunz41GArr2xUKQsTVmXZwNGorsMI8yvadMpf0/bEoflWogagzCp+Q7jng1SZLoTPTRFidszrd/vMC9V8Zw==;5:XXnAwPED4nt9yqY6peAzJVEBqDlWWSARdOSyMPkEBECiueS6NtIEBudwBMJiOWdvVl8XqHfq52e5y7qH3h6WevfgcnN9S8qEpHy/qhhfHzWbpjVFYZGtbwtvoyFLYa0udNHtq1l7w1W1w1nOrBf6XA==;24:oBDz0xBw5uWcyVtv77+fq4jubkIfsmrj/2jwfSaZd6EzcKNHeC5pnt7T/5IfgEns7BOvbNj9n1XBL0+0vCSNcmDkWiPzzY6P0CpY7IP/LpY=;7:vXFi/ihiPAhWu34+UwuPiA3wPrptCbqfRj4cIWxT+0icxJ7bnC+OCJEQjKScVMh2MDDsBZD5ppblKTjSUQChIZVz+7SVoK+DPK71CP9HJz8SGOvACDiNmkyhkQJNR8gZdwgOU4KdycCgEHkZxnpWZorjzF0gifdnB0mu/dgN9Z6TBLP2+qAyO73mXaiVzjRhpT6Z/8gZ+FGmsxgbT7F1Si+hHbJBA2iVJRyAUNogc44mbePbxVie325pdZWTxH9p SpamDiagnosticOutput: 1:99 SpamDiagnosticMetadata: NSPM X-OriginatorOrg: hpe.com X-MS-Exchange-CrossTenant-OriginalArrivalTime: 15 Jul 2016 16:44:56.3109 (UTC) X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted X-MS-Exchange-Transport-CrossTenantHeadersStamped: DF4PR84MB0314 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 07/15/2016 03:45 AM, Wanpeng Li wrote: > 2016-07-15 15:09 GMT+08:00 Peter Zijlstra: >> On Fri, Jul 15, 2016 at 05:26:40AM +0800, Wanpeng Li wrote: >>> 2016-07-14 22:52 GMT+08:00 Waiman Long: >>> [...] >>>> As pv_kick_node() is called immediately after designating the next node as >>>> the queue head, the chance of this racing is possible, but is not likely >>>> unless the lock holder vCPU gets preempted for a long time at that right >>>> moment. This change does not do any harm though, so I am OK with that. >>>> However, I do want you to add a comment about the possible race in the code >>>> as it isn't that obvious or likely. >>> How about something like: >>> >>> /* >>> * If the lock holder vCPU gets preempted for a long time, pv_kick_node will >>> * advance its state and hash the lock, restore/set the vcpu_hashed state to >>> * avoid the race. >>> */ >> So I'm not sure. Yes it was a bug, but its fairly 'obvious' it should be > I believe Waiman can give a better comments. :) Yes, setting the state to vcpu_hashed is the more obvious choice. What I said is not obvious is that there can be a race between the new lock holder in pv_kick_node() and the new queue head trying to call pv_wait(). And it is what I want to document it. Maybe something more graphical can help: /* * 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. */ Cheers, Longman