From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753008AbcELTmr (ORCPT ); Thu, 12 May 2016 15:42:47 -0400 Received: from mail-bn1bn0107.outbound.protection.outlook.com ([157.56.110.107]:9147 "EHLO na01-bn1-obe.outbound.protection.outlook.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1752657AbcELTmq (ORCPT ); Thu, 12 May 2016 15:42:46 -0400 Authentication-Results: kernel.org; dkim=none (message not signed) header.d=none;kernel.org; dmarc=none action=none header.from=hpe.com; Message-ID: <5734DC9C.9060105@hpe.com> Date: Thu, 12 May 2016 15:42:20 -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: Michal Hocko CC: Peter Zijlstra , Tetsuo Handa , LKML , Ingo Molnar , Thomas Gleixner , "H. Peter Anvin" , "David S. Miller" , Tony Luck , Andrew Morton , Chris Zankel , Max Filippov , Davidlohr Bueso Subject: Re: [PATCH 03/11] locking, rwsem: introduce basis for down_write_killable References: <20160511072357.GC16677@dhcp22.suse.cz> <20160511082853.GF16677@dhcp22.suse.cz> <20160511084401.GH3193@twins.programming.kicks-ass.net> <20160511090442.GH16677@dhcp22.suse.cz> <20160511091733.GC3192@twins.programming.kicks-ass.net> <20160511093127.GI16677@dhcp22.suse.cz> <20160511094128.GB3190@twins.programming.kicks-ass.net> <20160511135938.GA19577@dhcp22.suse.cz> <20160511180345.GA27728@dhcp22.suse.cz> <20160512121204.GQ3192@twins.programming.kicks-ass.net> <20160512121907.GG4200@dhcp22.suse.cz> In-Reply-To: <20160512121907.GG4200@dhcp22.suse.cz> Content-Type: text/plain; charset="ISO-8859-1"; format=flowed Content-Transfer-Encoding: 7bit X-Originating-IP: [72.71.243.221] X-ClientProxiedBy: CO2PR06CA019.namprd06.prod.outlook.com (10.141.242.19) To AT5PR84MB0308.NAMPRD84.PROD.OUTLOOK.COM (10.162.138.30) X-MS-Office365-Filtering-Correlation-Id: 2d09dc89-7152-4ad3-2234-08d37a9d927f X-Microsoft-Exchange-Diagnostics: 1;AT5PR84MB0308;2:dGC5Sep52xJQaosL0SCsUBQF/SmQEXsUjw/ZpaFHr3FB0hMKiTCTtt0nIspcWRNL4CdnnmTt++agUr71ZwPJAcnCQPgF/MTUSOmUf9U/56vuUum5OYeGHSp0+V0vyL+jxnGjLt/WKxFkx5NYxUo8ZGBcQHBX9K+t/66gT9uNOYgWN1Glv0yg+qKPQLjvRv8e;3:KrcCcFHY7dQDduf6dSCvMIa9Y54sZwKaeWj5Y99xSz1v7MdAbFaAQPEomI8APKWZNOG8r/ecQ8reJsYIMgaV5IZ6MV9u98bRkCmZL/qitHcq1PAbTVnN9KRFVT2WIqBe;25:EoM3++ME2X8JXPr8OJeEvnLxq8bpqi+vqFG8fWLW8GEgw2RgRT3gR+awPL7k/ybbWPYNilX33oHdaVCPUjk88a0dEl7A2jmJXDs8i97pzSwi8nNMgj9xUKq9/Cu5MDPu4+AU88fUqH1LFbhSPfApQwUVT2AFoHoLHpB8TE8zS+7TywYSAygThwQHcYGWq+DOsAwsA0wL/DniDrS0i3tQixnnzilI5rFH2SC1m7vthsTY9xV+KaKbe2rk22YDgTeOvJ0GVF1PbdzriD7jjUbW4thPbKADtqM3rDsDn3kgp/hGR9AtRo37gGK0HZs1hC/TI5dT2OFLVLaYc9f3BHAgnA6P5QE3KAWj8y427hGYLF4qua6FB8PbPPxoWrQ+LIKGmQHLPskQfFl+je/3Ml4SG7rgdU36HKyTranXNtOxy0Q= X-Microsoft-Antispam: UriScan:;BCL:0;PCL:0;RULEID:;SRVR:AT5PR84MB0308; X-Microsoft-Exchange-Diagnostics: 1;AT5PR84MB0308;20:pe2FBn24Yw7MGs6WOorQVCqBy3TdXZQbltEmd1/En0SL/Zb63XG/jF8NJkKIFYp/w9nErRj3zGIT84jWRqDYjBsAH1Tb6rPndMfWnlDejkkTrr9h0h1+QWjTfZftZLOZPB4lZAOTXbUaNbdToXp9bsR/qxvz7gNPJYx1dZy5DJoKfoxg2M8B67ShxCAYU5K5P5e6RUoZXuVkzO4eozm3+X/gpe2KW5gcNvRChnBR1UxWWPTnuFuE3zCIFdUyuHq6N1JruGPeoI1xfYu6jqTqmqUV+Q0aHPETHuUTKCjL0PqXdxsptwDOsb9OlLQ9gTfIm2sM7BwxUp/vu3KEzoi/1Q==;4:f588kOy5Iju8RiRrrckn7bkH10S4TKi3G2URUDwRXAUYwE3Wh2LqbJXOkasIiG9aoP2eeuVXXymqq5/T/2yE7eREnUOOxfX7Kf1ANEzjLpwGPqDjjF/xue40tzP3xrDHJ0YDJ8Cik/PWIHGNPHn9STMkYq1OxaUkFGZmta6sTMfHfHQ7zyi/9W6Y+7r6hYQAb7wrjOC2+r9N0qJgyjfrLcHmbnWrl/wAUl9U3XrBTmF7v921GwMy1h4nb7bO4ZEgpCbEPgiR0dAe40zMhWU/imhdBhWKSFul6XGYa6ga/mj6+uIMX7syOdR744W+vicfYH4CLdc/NsN3aQ8maGPkSYAjjNhUg2Iwt/4i7IsSnGqQi5kaP3IOm0b76mk3K0fb X-Microsoft-Antispam-PRVS: X-Exchange-Antispam-Report-Test: UriScan:; X-Exchange-Antispam-Report-CFA-Test: BCL:0;PCL:0;RULEID:(601004)(2401047)(5005006)(8121501046)(10201501046)(3002001);SRVR:AT5PR84MB0308;BCL:0;PCL:0;RULEID:;SRVR:AT5PR84MB0308; X-Forefront-PRVS: 0940A19703 X-Forefront-Antispam-Report: SFV:NSPM;SFS:(10019020)(4630300001)(6009001)(6049001)(377454003)(24454002)(377424004)(76176999)(50986999)(65816999)(87266999)(50466002)(54356999)(83506001)(59896002)(117156001)(92566002)(6116002)(4001350100001)(81166006)(93886004)(110136002)(189998001)(77096005)(2950100001)(64126003)(33656002)(230700001)(86362001)(23756003)(2906002)(4326007)(586003)(36756003)(42186005)(5008740100001)(5004730100002)(65806001)(3846002)(47776003)(66066001)(65956001);DIR:OUT;SFP:1102;SCL:1;SRVR:AT5PR84MB0308;H:[192.168.142.129];FPR:;SPF:None;MLV:sfv;LANG:en; X-Microsoft-Exchange-Diagnostics: =?iso-8859-1?Q?1;AT5PR84MB0308;23:BRdM5dvNZybcWyvHuVWZxl6BlsjZ9Qwkv8FL6Fs?= =?iso-8859-1?Q?ScUGmcEPuXrypGU9nemKtrZIZKLerwlezGwlGmAw+Y1GW+5sZeiG09jx52?= =?iso-8859-1?Q?2VMJXxz3bsfMP5pGDViJlzIam3GW6DLnYcYkCobVmbt+IeHIYTC3b3toZb?= =?iso-8859-1?Q?+pkBbk/pQVhhH+7hlg/w0w2W65O0uQ4tyJPtTAsskmsH/g3h41orR1rIhI?= =?iso-8859-1?Q?cXXu/3zRlNs+hzsArGKT7jKDtYJupt3XymjGhDReSHtP5UzaXN5FOWc2Kb?= =?iso-8859-1?Q?3GAKzInMzB02PSAi1F92aLXw7/ZjW9tLjq3sRUX/PpIb2ipQryhV5bjwsg?= =?iso-8859-1?Q?PYEmAYgeEsrtvrm01hl7R/5g7a4OLzhJE7MnEwoGYkEAvrhXxBu+J4wU9U?= =?iso-8859-1?Q?cgNZOOmPTkNBpcVU6/Iw1LJBySSa9UhbJSkkEGBYhG+IxpcO7HamrPLrKy?= =?iso-8859-1?Q?eUvopXqoCUWYC3vWuwNlR+oE2M1Z3SXVD3qYzWqDGI/uoflUc6026WGIFd?= =?iso-8859-1?Q?QZ+omYe0gOJBj7huei/hY6QUELBT9vNYf53kj+0TdX3GsIhP3vKhWL6O+u?= =?iso-8859-1?Q?sbZWuHOF2vTzo1MKgLcO+0tcrVSspCwtNNi0N8i+wEgj2JNXjpQqMcdVJO?= =?iso-8859-1?Q?Nl3o2XZBgJMl3if4tELYjGA7WhlagXIMtEmkN7IGubj/NoE7XPIrl1YsZP?= =?iso-8859-1?Q?f0XI7g+BokS0ehXP3fnECdUWet0swxPKIZ9TjHUbIykochKT5kv9WGTbkn?= =?iso-8859-1?Q?vMjATk/9A+THZccrvXWzIdAA7Bgw6OE7waJaBCXyAywN4UHhMHBxZWGtSq?= =?iso-8859-1?Q?WKjuS0Ln3cKXDzJ/kVhg1zXgF6qy8sO7zlAOSE/bhrblUhN3C1bEhgC83Z?= =?iso-8859-1?Q?bnUt9YiTyWMg7VJYbQjeA30CHwSXyw2EPCq1Sb3klY7qvhpgm7gYPBKeIf?= =?iso-8859-1?Q?v2oxeYmqz8ypYLlIlVjFMNhqCkzaRKwuZul8wsudShJQICxnAcRDL2cUhO?= =?iso-8859-1?Q?zJGn4IlVsxalcutgKzElFc7f/yHHkIITPgR9NgDTXllChz76NGNTAkdbdJ?= =?iso-8859-1?Q?KbVkXdXG68hCRkytQL7Cp893WthZ/rFODF10nGvLaSQRczsnzKhYi7ssoc?= =?iso-8859-1?Q?jjFXn?= X-Microsoft-Exchange-Diagnostics: 1;AT5PR84MB0308;5:ufD9UGkYUGk7nGzotTAfRzXNaKCwhR020ngG6ZYNA/ZU+ogbA+kC74m+FSQtRsoBv5lbBE0F7j3GT2D/wc6CjzfbzJaLonE3q+9QkSUsy5ey5WV4GksqB487bYNXa8+ICpzd44azfcep5D6m2xJVNQ==;24:BekmNDzi1cEQnJ7/I8Ylcx7N2IymuizmRurARDHRglYAY6l/cD4G87hOyA3BljdTGqDm78TiJdqaVRZcucrc/T7ihC7mGQJ++QhAdYy+vz8=;7:gaBcN337f9x2s1xhYLjJI5N6FId1sjSRrd0UZWQNXRbym6pz5vdOWrWT56MS0CrZ2himB/78dhQdP1i5olggEGVg0xrkg+X8de3iRKdrps2qmEHvmROvGznTSEgNzUq+ZRFV9WlY9Yfjfvgu7jq00ea7sceXQQcuBcAfonsyVJ/sWCfhu9fZ1m9uwc/NTVJD SpamDiagnosticOutput: 1:23 SpamDiagnosticMetadata: NSPM X-OriginatorOrg: hpe.com X-MS-Exchange-CrossTenant-OriginalArrivalTime: 12 May 2016 19:42:34.9536 (UTC) X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted X-MS-Exchange-Transport-CrossTenantHeadersStamped: AT5PR84MB0308 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 05/12/2016 08:19 AM, Michal Hocko wrote: > On Thu 12-05-16 14:12:04, Peter Zijlstra wrote: >> On Wed, May 11, 2016 at 08:03:46PM +0200, Michal Hocko wrote: >>> I still cannot say I would understand why the pending >>> RWSEM_WAITING_BIAS matters but I would probably need to look at the code >>> again with a clean head, __rwsem_wake is quite tricky... >> Ah, you're asking why an unconditional __rwsem_wake(ANY) isn't enough? >> >> Because; if at that point there's nobody waiting, we're left with an >> empty list and WAITER_BIAS set. This in turn will make all fast paths >> fail. >> >> Look at rwsem_down_read_failed() for instance; if we enter that we'll >> unconditionally queue ourself, with nobody left to come wake us. > This is still not clear to me because rwsem_down_read_failed will call > __rwsem_do_wake if the count is RWSEM_WAITING_BIAS so we shouldn't go to > sleep and get the lock. So you are right that we would force everybody > to the slow path which is not great but shouldn't cause incorrect > behavior. I guess I must be missing something obvious here... Because of writer lock stealing, having a count of RWSEM_WAITING_BIAS doesn't mean the reader can surely get the lock even if it is the first one in the queue. Calling __rwsem_do_wake() will take care of all the locking and queue checking work. Yes, I think it is a bit odd for the possibility that a task may wake up itself. Maybe we can add code like: --- a/kernel/locking/rwsem-xadd.c +++ b/kernel/locking/rwsem-xadd.c @@ -202,7 +202,8 @@ __rwsem_do_wake(struct rw_semaphore *sem, enum rwsem_wake_type wake_type) */ smp_mb(); waiter->task = NULL; - wake_up_process(tsk); + if (tsk != current) + wake_up_process(tsk); put_task_struct(tsk); } while (--loop); Cheers, Longman