From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932655AbcFOTnW (ORCPT ); Wed, 15 Jun 2016 15:43:22 -0400 Received: from mail-bn1on0119.outbound.protection.outlook.com ([157.56.110.119]:18352 "EHLO na01-bn1-obe.outbound.protection.outlook.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1752444AbcFOTnS (ORCPT ); Wed, 15 Jun 2016 15:43:18 -0400 X-Greylist: delayed 870 seconds by postgrey-1.27 at vger.kernel.org; Wed, 15 Jun 2016 15:43:17 EDT Authentication-Results: spf=none (sender IP is ) smtp.mailfrom=waiman.long@hpe.com; Message-ID: <5761AC43.5080707@hpe.com> Date: Wed, 15 Jun 2016 15:28:03 -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 , , , , , , , Davidlohr Bueso , Jason Low , Dave Chinner , Scott J Norton , Douglas Hatch Subject: Re: [RFC PATCH-tip v2 3/6] locking/rwsem: Enable count-based spinning on reader References: <1465944489-43440-1-git-send-email-Waiman.Long@hpe.com> <1465944489-43440-4-git-send-email-Waiman.Long@hpe.com> <20160615173830.GR30921@twins.programming.kicks-ass.net> In-Reply-To: <20160615173830.GR30921@twins.programming.kicks-ass.net> Content-Type: text/plain; charset="ISO-8859-1"; format=flowed Content-Transfer-Encoding: 7bit X-Originating-IP: [71.168.64.62] X-ClientProxiedBy: SN1PR0701CA0012.namprd07.prod.outlook.com (10.162.96.22) To AT5PR84MB0307.NAMPRD84.PROD.OUTLOOK.COM (10.162.138.29) X-MS-Office365-Filtering-Correlation-Id: e613d86c-b8f6-4357-d502-08d395532fa0 X-Microsoft-Exchange-Diagnostics: 1;AT5PR84MB0307;2:+vnQS6tPf9w1X5/jjH88Y+6oocqCJKvQWi+PcMVsizbN2P1Z4qoQYEZ6AJPfP1mOlMkN36RtQKEfayIfl+GDYVdeyIHaAXtxWCW+Y7AGzyzCbuamMFgvlzdPUB/xS/6hnJ5GhG02pfyrod1pdqfMSQVIGxGn//rgTCDB/9NAboR23tWzLaZ82xSYHXwa6Z+5;3:/XqBm/0F615t2aQ3vfIjUJLM5aPkUiCRXoLhUYd6QP4PKwOnv71GxOtule2AfgAcEeqeA0ReuyfC2N2qjUSYjg88vnADARyETUOc0G85SVvaUt7VtIJddvXM8k0BkRX7;25:lwNCRSceqU1qgaO8Oj0YE8m2dlnF6Jq23Ob9/17MaLH3HyylhvbvbR+xJTucEQmtZWkcTfTHBG8FiL38t0qiXYA3Ut2AvnN/y6P/F/d6wpVREVk+xCXSJpFaTeGOrlhv8oM2HJMk3Bu6rvseU+jzYT8q1dYEtzY7HJLRgNPTvJZH0827nfBhZj8MMOgoyo/wrmcD2as6nJjeiQFDpIbqYDseIcL0NNs7JPdAanRtR9JMXZAjju8cto3130rN0UgiWjBudFVadp2BsDnyaV9TB+LmKv5Tb8iuPl8N/bf/azRX19R+74fmYShGZhkYDJ+83OwPRvZ8CeNBjMzAoBwQ+UKgSLjR6s3BzbGDuKKwvTi+BZC8HyXOp5lDOqsrlWzXZAhySBbaeBOLmmB20iDoK5GL3ttkUnzRbCu5vt4Q+BI= X-Microsoft-Antispam: UriScan:;BCL:0;PCL:0;RULEID:;SRVR:AT5PR84MB0307; X-Microsoft-Exchange-Diagnostics: 1;AT5PR84MB0307;20:iRl3URP2dEZtpPuFG4guubi0bb4xPJx8PM2BSGA2+DGvt/lHYTMthJiTjxBkn3h2nyXHiudKJnuKlfAQMX3C8p4kVoAhd4bFJq9XnGR9NGmw4ixsMEkf7siMCctq8gS0YZoxrkrdpVULYkKuiPHayyZfg724xlIiDgXNmFQ61VuzNSg3NILz0NeCQvt8aw4PO2NUDC2aLMsIpRmVrWmOs6POMGVY0KE55UJrRF7lkLqGkMsBoL0dyhwq5OVs685qMcehWYRVvjpjHFuy9ZY4Jilto0NJuyOWla43ONb5g2nXR1+/yThGi2gI2m2729RVdlyLGpalkT2dduVsNuDH77E2ByGUpgfHrTo73QbDdKFZTc/CB0V/D6Q/BZTc/gSVWjdnP4TQquUtuA/+4+wfO76l5J1dXb+hhPsWMbYl1AnqO83XL14w7IQfUMXRf/XpRQAHQZJH7+5sVLCfhAhg4Tm1KrXoLOwAcNoFr9MX/ViCzYxw+UBcYrrrnuMNs5vR;4:K28eepx/6lprUjGlse9m5Jib1gprhtN0YRgC6v96+OH48Wm4YsesZTE4hELHfD4hczG4nxtTkkcMq7GR6XmydT0ZEEf/SyfCjAqru52ksSrfAGHyRXIhVE+4UL0DykYGwUYyYzM88m5HmOdUK2poqL7Ur2lAhvF77x2ZNyR2xTHkNffHg8NVQ+t1O6z0ABsgd9zkhR4OP40G4oK8ofcy3S1oeIvGWCAZAcHx7x3CW/IvgY74VRg/MMX36ete62JiqMv6YdEk03GzTBPk+aIaNbCroS07GxBirqZLkw1zdoulC5ejIzytop4uikxodIE+iwWea/zjVR71uG1CstT7GH0pB+DpxQKk32a2RbLPCf9FWfFE1CYwO37zP06caB0w 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)(3002001)(10201501046);SRVR:AT5PR84MB0307;BCL:0;PCL:0;RULEID:;SRVR:AT5PR84MB0307; X-Forefront-PRVS: 09749A275C X-Forefront-Antispam-Report: SFV:NSPM;SFS:(10019020)(4630300001)(6049001)(6009001)(7916002)(189002)(377454003)(24454002)(199003)(2950100001)(83506001)(86362001)(76176999)(5004730100002)(50986999)(4001350100001)(87266999)(80316001)(54356999)(77096005)(65956001)(65816999)(66066001)(65806001)(92566002)(189998001)(97736004)(110136002)(117156001)(33656002)(59896002)(47776003)(8666005)(42186005)(64126003)(50466002)(23756003)(105586002)(101416001)(106356001)(3846002)(2906002)(230700001)(4326007)(6116002)(8676002)(68736007)(81156014)(81166006)(5008740100001)(36756003)(586003)(7059030);DIR:OUT;SFP:1102;SCL:1;SRVR:AT5PR84MB0307;H:[192.168.142.154];FPR:;SPF:None;PTR:InfoNoRecords;MX:1;A:1;CAT:NONE;LANG:en;CAT:NONE; X-Microsoft-Exchange-Diagnostics: =?iso-8859-1?Q?1;AT5PR84MB0307;23:nn8s3r7iE9X34ohCOFRPzcR6DluVplNWG8mTA1a?= =?iso-8859-1?Q?wpsmHmldd4xL3d548LPV2ifcvYVfBN6/bVldg+hxVPFk6YUXC1dGZb/XNx?= =?iso-8859-1?Q?z5RccR14+Ffo8Z7cVz2QxRML59n4g7+WwNUGYwA9cpDwsA6dmVDhUF0rkp?= =?iso-8859-1?Q?TMmaDLmFAIm2RTv3lbyFWitv9qOrc/DircQ9QqJesNIV4FxtQ8kDUWo/dc?= =?iso-8859-1?Q?9OJ9DvFmz6bB59EREROK8GBKDX79XeUeldAa45GK9jLR6NAMTHZ8FrdSW5?= =?iso-8859-1?Q?ZPUTKkYB7f6AmS5Tnde9tBDoSBuTB+JpduPF73LnOhx4q3Otov0dI+sxpK?= =?iso-8859-1?Q?9FHymmZ8kKHXtPQEWyyzvyTxnFGraXTXiGntqvGofk0Hh3u45yPcJLgXus?= =?iso-8859-1?Q?G9gC7QkOy8PADD6zvtVvKPN5Qf5Zzg2r9P9XGEbzUFz2MCBVyOQHUV9NE1?= =?iso-8859-1?Q?6mnIjdKzkH7xrlK1VAInayeT8jBAmzyJC1Op1McKU8Yub5+5xX5H5GQPtl?= =?iso-8859-1?Q?V85cCRmmvpr2LQOxR5cNKmKue4W4HH+4a2aWyc/2VovV8bX3AkGxeACiEo?= =?iso-8859-1?Q?xptgS2NgLHMmUccPoViw4EipkBV/wgCvSjeZMFvwzKCdEvpTm4xjnEACwq?= =?iso-8859-1?Q?yONo8YEt58AX4Z7yqyRaJBFfOYsZCpAMk1e5gHS5ot8vU/BvUe9vxtNAZd?= =?iso-8859-1?Q?sbL8y8ntylXjlwJQZT2OH1Dr4Sy0VIw5C7PsNx6sAFxKyu0tF+MXK9U7BY?= =?iso-8859-1?Q?S0hPew60alevGI7r/IlPNENANvPPoQyWXDeOLU7riuuvrievoQVTsIgVQE?= =?iso-8859-1?Q?RRTQN9hrDg9DGkVsxPPblL0TPHLRw3OZGKuNM+CuCPHuasjTdXHt4aJcYU?= =?iso-8859-1?Q?8wt6yLsmeuEiiIa4yfOAZmUYi7sAl8mCknD9nVclkIvCtahXRfavohX3F6?= =?iso-8859-1?Q?ArTMUm7BvPSnvbjgUQv7ukSOn/c0myzqgxpcRzc4ar8symNCcE+luK3H2g?= =?iso-8859-1?Q?APHowoZlC1fIfJn9q+U3fN4mOV4GwapBe58yWaZTYyfbtWdN5NlzjpodPl?= =?iso-8859-1?Q?8kOdbRZXFGDer9pUly7vreTGll9YAoDFOLxTlzSK3VO6MUdfSkjEfeDSv9?= =?iso-8859-1?Q?/Elnw7FBh4klvEfPIp6hmVchlCtUEVqJX8zKy6qovBwV9IxXv9A3lYE7iV?= =?iso-8859-1?Q?wz+frMKlOM6TWL7lJMLxrAdvB2rqMsW/e0qIiG3uCvvBtYcMJ2N3LMqiSw?= =?iso-8859-1?Q?GLc8mhG6WMuF0MGuQVrFqOtTDjUKGnMKLq6ssxy/CuDu0hgmSRadvztOrB?= =?iso-8859-1?Q?MatrMKP+w5tEWWxU/YvOpburLhgC0goauVU7mC1WvPcQg=3D=3D?= X-Microsoft-Exchange-Diagnostics: 1;AT5PR84MB0307;6:MIodcuYTBpAkM1YkSCvd9QXABRdfzY5mKquTIqKzsQO/t2mBmX7v13vtgiWh6qnXMJ658QMHHzmkqM4dhn0ozgiKTBH6YZLSNDsL5PCJitSCJU6yEJ6YFTLP7GF211ygx3iR2ycqW75Df8QPiYjvw5PJAD/eJ6m9vLp6F9zSfUZELqf01+spEMhXLOTMNmLSXNlmJq0J0NrcHJpD0rQIn1nrhjIRB1xOJX13IcewHP/CpeKm+U0PTMlO8LzVdfDiHblZnpPFosJ84hlUJF9WvkcnSGgqtFoDcRpFzysHikk=;5:pxUW0WDR6dYd8UDN1sFBWmAWrwuXTDhKqlBkqVRnVME/uL/I0umthArAQzE474dae2k6gMEGbOEMgJ3BAqGP6YclYGogWK2PMV7Nl/famzBNrAA7quPH5XSdxitsy4DkOVZvibcmrLG2XkkTMFq+UQ==;24:cgnNlys71bAGT4jTNTnHqxegrb6ZGPKTQ2XcM4/9FRtK0jJPvBYpmJiDP3K0zCGvGY+HI3L57XqPonAX+6ZxYZ2ELL1OdEQhbnElEfbqHvk=;7:RyjrtSRo3R6faCKiZqUjMHpCGMbYvMlKoh3z6NnbvjQCE+Cia4cKH1jZ+JlqRrfP8CMIeZSkVnyukCkO/XRqyUr5+leKsUrd3ohrhTYAHdOy+fAmFppyrz0AXWZ9f87b4M0ThS+ByTq97hpOqfqP8Yvjk6gGVVYwRCK0c+YB8Jj6Ow5rHsOs4w00n00TVIqEO1E7SyqpjvwLoFqqVOVHu3QV5nQhedgOdod4S0SpOpI= SpamDiagnosticOutput: 1:99 SpamDiagnosticMetadata: NSPM X-OriginatorOrg: hpe.com X-MS-Exchange-CrossTenant-OriginalArrivalTime: 15 Jun 2016 19:28:08.7320 (UTC) X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted X-MS-Exchange-Transport-CrossTenantHeadersStamped: AT5PR84MB0307 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 06/15/2016 01:38 PM, Peter Zijlstra wrote: > On Tue, Jun 14, 2016 at 06:48:06PM -0400, Waiman Long wrote: >> static bool rwsem_optimistic_spin(struct rw_semaphore *sem) >> { >> - bool taken = false; >> + bool taken = false, can_spin; > I would place the variables without assignment first. Sure, easy change. > >> + int loopcnt; >> >> preempt_disable(); >> >> @@ -409,6 +412,8 @@ static bool rwsem_optimistic_spin(struct rw_semaphore *sem) >> if (!osq_lock(&sem->osq)) >> goto done; >> >> + loopcnt = sem->rspin_enabled ? RWSEM_RSPIN_THRESHOLD : 0; >> + >> /* >> * Optimistically spin on the owner field and attempt to acquire the >> * lock whenever the owner changes. Spinning will be stopped when: >> @@ -416,7 +421,7 @@ static bool rwsem_optimistic_spin(struct rw_semaphore *sem) >> * 2) readers own the lock as we can't determine if they are >> * actively running or not. >> */ >> - while (rwsem_spin_on_owner(sem)) { >> + while ((can_spin = rwsem_spin_on_owner(sem)) || loopcnt) { >> /* >> * Try to acquire the lock >> */ >> @@ -425,13 +430,16 @@ static bool rwsem_optimistic_spin(struct rw_semaphore *sem) >> break; >> } >> >> + if (!can_spin&& loopcnt) >> + loopcnt--; > This seems to suggest 'can_spin' is a bad name, because if we cannot > spin, we do in fact spin anyway? > > Maybe call it write_spin or something, which makes it clear that if its > not a write spin we'll do a read spin? I am fine with the write_spin name. > > Also, isn't this the wrong level to do loopcnt at? > rwsem_spin_on_owner() can have spend any amount of cycles spinning. So > you're not counting loops of similar unit. The loopcnt does not include time spinning on writer. It will be decremented only if the lock is owned by reader (can_spin == false). I will clarify that with additional comments. >> + /* >> + * Was owner a reader? >> + */ >> + if (rwsem_owner_is_reader(sem->owner)) { >> + /* >> + * Update rspin_enabled for reader spinning > full stop and newline? Sure. > >> + * Increment by 1 if successfully& decrement by 8 if >> + * unsuccessful. > This is bloody obvious from the code, explain why, not what the code > does. Will clarify the comment. >> The decrement amount is kind of arbitrary >> + * and can be adjusted if necessary. >> + */ >> + if (taken&& (sem->rspin_enabled< RWSEM_RSPIN_ENABLED_MAX)) >> + sem->rspin_enabled++; >> + else if (!taken) >> + sem->rspin_enabled = (sem->rspin_enabled>= 8) >> + ? sem->rspin_enabled - 8 : 0; > This is unreadable and against coding style. I will fix the coding style problem. Cheers, Longman