From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S933625AbcBQB4i (ORCPT ); Tue, 16 Feb 2016 20:56:38 -0500 Received: from mail-bn1bon0110.outbound.protection.outlook.com ([157.56.111.110]:30030 "EHLO na01-bn1-obe.outbound.protection.outlook.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S933358AbcBQB4h (ORCPT ); Tue, 16 Feb 2016 20:56:37 -0500 Authentication-Results: infradead.org; dkim=none (message not signed) header.d=none;infradead.org; dmarc=none action=none header.from=hpe.com; Message-ID: <56C3CFA3.80902@hpe.com> Date: Tue, 16 Feb 2016 20:40:51 -0500 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: Jason Low , Davidlohr Bueso , Ingo Molnar , , Linus Torvalds , Ding Tianhong , Jason Low , "Paul E. McKenney" , Thomas Gleixner , Will Deacon , Tim Chen , Waiman Long Subject: Re: [PATCH v2 1/4] locking/mutex: Add waiter parameter to mutex_optimistic_spin() References: <1455298335-53229-1-git-send-email-Waiman.Long@hpe.com> <1455298335-53229-2-git-send-email-Waiman.Long@hpe.com> <20160212202355.GY6357@twins.programming.kicks-ass.net> <20160212221444.GC16417@linux-uzut.site> <1455588930.2276.36.camel@j-VirtualBox> <1455589334.2276.39.camel@j-VirtualBox> <20160216085322.GS6357@twins.programming.kicks-ass.net> In-Reply-To: <20160216085322.GS6357@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.212] X-ClientProxiedBy: CY1PR08CA0014.namprd08.prod.outlook.com (25.163.94.152) To AT5PR84MB0131.NAMPRD84.PROD.OUTLOOK.COM (25.162.137.25) X-Microsoft-Exchange-Diagnostics: 1;AT5PR84MB0131;2:IIYVLQikhyzPtFIDfj0t/GtcDZXqkxP0j1U0HtV6st/Tz6i4jG7DJCYs0q+GzQrTUJ4I2zNQKYR3fKbvqHU0KEytWd9BLNn5mdPkj0dFjGM8Drc2GmpJdTcXUcGWU9yoVGNE13j4kHeXOY+ay7b8xA==;3:UyJuQ3pxKK0q/xeIW+ffolmUvgi62WjBI0qaPffuy5ZiVIdRVljn1zW/SFMAgO3pfbEzzfNQt2qiccrLHvEFngTyjmqBS6zsQb3ivk8CNJ5aK+1UgeqddFPbrM2IMY4y;25:2d0fs2n00D9/1SvoBR2De9dhMadyfhtR9eWMiYmRRykmFqCAE7zigpEnsCjuiN4C9zYygWawZurlqVt2RbGG/BMLnvKKv8fVayeOdhQxB/Eq2/1ugq6KUrWSi9n8a0iNfqi1b0pO8K+qkHpkHbkX5+0eiW15kY+ZMVMsqemwnoIhtbxEENKvGDiJtLhOej57fOxsH38dEFk0MhabuyE2C4LbuxRCeQlm04oNgeehO86bpffh9cRvDOUdu3W1pdN28W/Vytaiz9Q0Z6nr1Rgs2zV94uKfiXrj6uZsG89Pk80tiF+gPoMNrY4WnfGjZaby;20:Iq4ccN3ORR57mDRRfIQBdShpOj7Gfxf2F8lwVSV+QeKCyZ3ykMLi58wDp+OHhAPgrqsF8jV2OxFEeAa1xweI679xY6cIJjdLLFmlUZTiIVd8oulAzQX3IuQ4coAUM6oAj0iQA8HfZJQSd/T0XiJdt3hkRz3aKQ0HlQBcIOGG7i/759SO1tMZjSKC8uJz7UIQXHMLrO6WQUe/p2WUFlY43gQMX542EWyOn7fUiw4t0wLfMz4Sgq/n1wfD6pDBEKXx X-Microsoft-Antispam: UriScan:;BCL:0;PCL:0;RULEID:;SRVR:AT5PR84MB0131; X-MS-Office365-Filtering-Correlation-Id: 160451db-b514-41dd-3edc-08d3373b649b X-LD-Processed: 105b2061-b669-4b31-92ac-24d304d195dc,ExtAddr 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:AT5PR84MB0131;BCL:0;PCL:0;RULEID:;SRVR:AT5PR84MB0131; X-Microsoft-Exchange-Diagnostics: 1;AT5PR84MB0131;4:2MAkYCsGw6lD9z8Sec7gAQAmY8XeRrCii0pRkbdqrn95rU7mLeGYVdCaayFX44PRaLTRGs2sGzEHNZneisYzYl+Kzfr45xFMBFkkZXaqesHjNwaZmvknt+mQdoaKoxs+5J5jBOqkcOlxahCA3CX//QfoUB4kqLbs3Il/vWZS/6LkSt5o8Iqf6uXWxfG8JJ5BosiDGpHjZR7Dm8lPs0TOY4ZNacWB6eWBqZiJ/WghhqFogQGVoKEgs0G87Ak8U3usc56CiVl8anJjUjpNFE+4F63g6UDEOxrjRjHip4CMKwtqoLqjUkcYDKpVw7gNNIH4xvHLdWzkM0Q/SbyuAN0B8MFHm/5YtvyeJDpE75/xy/NhyJ/ep/zirqrPeBPm9E3h X-Forefront-PRVS: 085551F5A8 X-Forefront-Antispam-Report: SFV:NSPM;SFS:(10019020)(4630300001)(6049001)(6009001)(377424004)(479174004)(377454003)(24454002)(42186005)(93886004)(189998001)(4001350100001)(230700001)(86362001)(83506001)(5001960100002)(33656002)(5008740100001)(117156001)(87976001)(77096005)(110136002)(7520500002)(92566002)(47776003)(1096002)(6116002)(2950100001)(50986999)(3846002)(23756003)(65816999)(36756003)(66066001)(54356999)(2906002)(5004730100002)(40100003)(65806001)(65956001)(586003)(50466002)(76176999)(4326007)(7059030);DIR:OUT;SFP:1102;SCL:1;SRVR:AT5PR84MB0131;H:[192.168.142.187];FPR:;SPF:None;MLV:sfv;LANG:en; X-Microsoft-Exchange-Diagnostics: =?iso-8859-1?Q?1;AT5PR84MB0131;23:p8q4/F844eEMqFRiSuTjtO6UznXN3kF27wBPjpf?= =?iso-8859-1?Q?848GcYFXA6GbRCjiiRBdC8LIzCzcPKPbrHNke2EaYdzk4zC3rsIPvtAX5x?= =?iso-8859-1?Q?dtAoW8R4311+52dW69CVYPjHeiFRr5eIfyQ9AysJc290muffMocRVteMR2?= =?iso-8859-1?Q?icXrirUpNCAwPG7ndqVkw8eGIwBiqaxxdCzPlnhQmOfWt/i6OW7yMoNUDU?= =?iso-8859-1?Q?fl6KRv96mm6VXAZMbXk+c4TqfvCfy7/h4Odz8ttwvYyNa3g7fXLx2SfLty?= =?iso-8859-1?Q?Db3L2A1jsLA+cXJb5s6WkBrp0XGhIZYe0QiPcgS3yqMqIlyGgHVuaDRFlJ?= =?iso-8859-1?Q?QeAbjjdcCT9wG4GkASI15Q8VgBo3TSnXZ1Wim4IBY007hU9appAv878dKy?= =?iso-8859-1?Q?+Im/Y/ctrtg60TQjzGnZh+v8L5Okz95PtzalChPPHVXwAQv5SDO49f02M2?= =?iso-8859-1?Q?Oi04m+efE5B4Mk8gTfH+aBHaTt77I4/xAmLmuHyHo9NYV+JVLRgA2zjjhR?= =?iso-8859-1?Q?1xqKLb2j01PXhAbZ3TR8w2bokdFbqPjsw12sL/b5R1lUH5If+x7oxJiY5d?= =?iso-8859-1?Q?MPmpr+pEBSaYQcu5BV0AQxsSMO+Ql7+2Br+8yVyWi+LXTMhdHQk/dEBCx4?= =?iso-8859-1?Q?eJnO+PhfDsgliOMn1hK27eMwj5QrL5U7wSjfzk81LnKiWZ84trG1lmm5Ee?= =?iso-8859-1?Q?6C+90j/Zs5p/GzR4y/8OahSlydKh+/KstW6tduToKwlyLp89ZtTrsKxU4x?= =?iso-8859-1?Q?caPltWF27tUiIyFHA6KC/DTSXf4nO3DLZ9an2fBEACi1nKmjTvXkxQBCl/?= =?iso-8859-1?Q?dxIUCaPp9FAByw7PKT+DgKKda0g/JqFma96rcbbMkBs31DnTwjoJcvZltI?= =?iso-8859-1?Q?I6gt8cYICNd0Ai0OlaTawSHRpa/kiABtMKol8jLP5DUEbYR4DUH+GouP89?= =?iso-8859-1?Q?fXAieyT+yRgHcZe2Purw9ZRt1fZlxH0NDgjLdXWYK3LXTbb7TyJbRVq38+?= =?iso-8859-1?Q?MxczWb08tl2+Kbh0UHobNQJNM39QizAwqqBjYPY2ZEnddnhQnsfYFTINqY?= =?iso-8859-1?Q?fR5vmdO+MO2WqS5CTCrzzMf+yz1DnYGnhnTbXnbMxY4ONIKixTBx5fLvGC?= =?iso-8859-1?Q?+/ZsAq/bwRvC/ph3cRNepzFHz+MAD9u4jHSBH/R7pG+GUE+Qo3TOdbND46?= =?iso-8859-1?Q?fN6ExqqPkKx02Zah0KkaBYJVqZ8QOzS9g=3D=3D?= X-Microsoft-Exchange-Diagnostics: 1;AT5PR84MB0131;5:6/iEcfzKK2QkLbDQv0xRf0x6Rpc3rJCW/6mST65qoftQKIMRgMr94s0Ncr3ViqhphTyjv0gbBz0Z69Cfm31RpBgZJvw3//kdXf9syRq0nZ6gyPtC+52mQSyY41wLQeUvK1GkmOggzCEUgOlGd53KRQ==;24:45tB4Dk2Fx522ABZzOST+n5ZTI9w934SGbvn0BUrJQI6UCQhllJFvHoEEK0FJ4YI2eo4+GMkzu810SLEySc1R3QaURibMCYF8irrZf1mLOk= SpamDiagnosticOutput: 1:23 SpamDiagnosticMetadata: NSPM X-OriginatorOrg: hpe.com X-MS-Exchange-CrossTenant-OriginalArrivalTime: 17 Feb 2016 01:40:59.5489 (UTC) X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted X-MS-Exchange-Transport-CrossTenantHeadersStamped: AT5PR84MB0131 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 02/16/2016 03:53 AM, Peter Zijlstra wrote: > On Mon, Feb 15, 2016 at 06:22:14PM -0800, Jason Low wrote: >> On Mon, 2016-02-15 at 18:15 -0800, Jason Low wrote: >>> On Fri, 2016-02-12 at 14:14 -0800, Davidlohr Bueso wrote: >>>> On Fri, 12 Feb 2016, Peter Zijlstra wrote: >>>> >>>>> On Fri, Feb 12, 2016 at 12:32:12PM -0500, Waiman Long wrote: >>>>>> static bool mutex_optimistic_spin(struct mutex *lock, >>>>>> + struct ww_acquire_ctx *ww_ctx, >>>>>> + const bool use_ww_ctx, int waiter) >>>>>> { >>>>>> struct task_struct *task = current; >>>>>> + bool acquired = false; >>>>>> >>>>>> + if (!waiter) { >>>>>> + if (!mutex_can_spin_on_owner(lock)) >>>>>> + goto done; >>>>> Why doesn't the waiter have to check mutex_can_spin_on_owner() ? >>>> afaict because mutex_can_spin_on_owner() fails immediately when the counter >>>> is -1, which is a nono for the waiters case. >>> mutex_can_spin_on_owner() returns false if the task needs to reschedule >>> or if the lock owner is not on_cpu. In either case, the task will end up >>> not spinning when it enters the spin loop. So it makes sense if the >>> waiter also checks mutex_can_spin_on_owner() so that the optimistic spin >>> queue overhead can be avoided in those cases. >> Actually, since waiters bypass the optimistic spin queue, that means the >> the mutex_can_spin_on_owner() isn't really beneficial. So Waiman is >> right in that it's fine to skip this in the waiter case. > My concern was the 'pointless' divergence between the code-paths. The > less they diverge the easier it is to understand and review. > > If it doesn't hurt, please keep it the same. If it does need to diverge, > include a comment on why. I will keep the preemption, but will still leave out the mutex_can_spin_on_owner() check for waiter. I will add a comment to document that. Cheers, Longman