From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753431AbcBPDat (ORCPT ); Mon, 15 Feb 2016 22:30:49 -0500 Received: from mail-bn1on0119.outbound.protection.outlook.com ([157.56.110.119]:7224 "EHLO na01-bn1-obe.outbound.protection.outlook.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1752759AbcBPDas (ORCPT ); Mon, 15 Feb 2016 22:30:48 -0500 Authentication-Results: hp.com; dkim=none (message not signed) header.d=none;hp.com; dmarc=none action=none header.from=hpe.com; Message-ID: <56C297DA.7060505@hpe.com> Date: Mon, 15 Feb 2016 22:30:34 -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: Jason Low CC: Peter Zijlstra , Ingo Molnar , , Linus Torvalds , Ding Tianhong , Jason Low , Davidlohr Bueso , "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> <20160212204027.GZ6357@twins.programming.kicks-ass.net> <56C2655C.9030707@hpe.com> <1455591654.2276.64.camel@j-VirtualBox> In-Reply-To: <1455591654.2276.64.camel@j-VirtualBox> Content-Type: text/plain; charset="UTF-8"; format=flowed Content-Transfer-Encoding: 7bit X-Originating-IP: [71.168.64.167] X-ClientProxiedBy: BY2PR05CA033.namprd05.prod.outlook.com (10.141.250.23) To AT5PR84MB0132.NAMPRD84.PROD.OUTLOOK.COM (25.162.137.26) X-Microsoft-Exchange-Diagnostics: 1;AT5PR84MB0132;2:9gzKYLqp9N06ccNNR1GkdDRJawGyEbdSOA1zROAWO9PcaXawACky5vrX4/AKkKo48h8kY0SiyxXgHguVzqgrTCExTjLHDVR7AtlEhn2cCd4c2ph/xEEEP+CeTyJ7iasuNtYvOXXkajIi2aZcwKFBsw==;3:IkoKxHyZISMit04GaVDydOiG06iaX68ncdmzmO/6+vaMKWS6EqsOA/tWDGA+8Yq1GpcP6AXKGpbaoX7pIa+SL9mlTjM8s/iIJrbYkOsoSKpRHjHsMiOA1FbkHq+OWzN6;25:YUuumICYDNWna+qsQ84AWQmP5s/0YWAaaCp+HeClL8/xnJXd+fAi3lae4NCXFsPURxuQus0aOr/hYf1wX+vhTOwZcedaA3K4j8cdNyQghvnZKFP8RBQSdi8cATXlhHXRpSY7gvjpROb6wxtBjItpj9RyECHJ12Ya+asnSo/8mop/YJqRvHzOg2rQbDrFqrJLqjxRyUasCWQNF5bubmWo3NpCmKRlv5o6y4NHAO+oXteLbFanNXlksM0O8DoMD65HocCUboQeyhmEnj50yR/3fEodkPIO3BQnOZxwVm9nIHNABs9oe1QUBDydUsYr723v;20:R/bGQGPIm/Xk6dC/8Z+7n45+0LvX0jU7ziUXeZ41vh+I3yrHv3vPgwsA9J6ipiH5RlKb17WCzWyFeUuQ3YUvyRReTYi64B3VnmleDX1JPkrlima4XItqhNNWlQrWip8KkqRZ9dR5ek1N0aMd+56kccyBCpZYXz1GWAkvHRl848jgOj2yMYOCZz+M8HTEEjfFrC0TtR3Ju2z386lhGLU4z5okfn0C76aWg+Vv2fH/ozmqeIxxVD2DspGuLRmz8wR4 X-Microsoft-Antispam: UriScan:;BCL:0;PCL:0;RULEID:;SRVR:AT5PR84MB0132; X-MS-Office365-Filtering-Correlation-Id: 7cb342e9-ba2d-43d3-7e75-08d336818eaa 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:AT5PR84MB0132;BCL:0;PCL:0;RULEID:;SRVR:AT5PR84MB0132; X-Microsoft-Exchange-Diagnostics: 1;AT5PR84MB0132;4:ib3qF0wVd420tBsCQu1GE0klqd7VQaoJAd01gft6ENDKo+ramdthw971cPHN1D6PHi8QMxNWSCu34LHXu5xvQfee2P0JgkGor8222C8oclCzF/7XipnA2LQ0zF41NiYwwyXTL2gaZi8YCv1Nz5SJgT25IrOst0jmtR0yTS+TL5SDX+Gv0cH5+eRBrzCmZPBunaxn2pRymdfFQxe7vfowLB/ydtxAMxRpyB0oFTDGt0kJpmGBFKt/U2C7cflbuFhnLISnXFd7L6X1XlvVGgGoEbS/YMiax5k1aSV/ZoR1lVvgNdDh8zmU63K7GpoRCPQUuEGnSOVnFG3w+iFqyI+BFPH20suze2kU6eomlsm0P7jpWNT6wwDrwJ3FLkH2q5zP X-Forefront-PRVS: 0854128AF0 X-Forefront-Antispam-Report: SFV:NSPM;SFS:(10019020)(4630300001)(6049001)(6009001)(24454002)(377424004)(479174004)(377454003)(80316001)(40100003)(23676002)(83506001)(42186005)(5004730100002)(86362001)(65956001)(87976001)(36756003)(47776003)(92566002)(66066001)(77096005)(2950100001)(54356999)(33656002)(586003)(6116002)(5001960100002)(4001350100001)(87266999)(230700001)(65816999)(50986999)(3846002)(76176999)(117156001)(4326007)(110136002)(50466002)(2906002)(1096002)(93886004)(189998001)(5008740100001)(7059030);DIR:OUT;SFP:1102;SCL:1;SRVR:AT5PR84MB0132;H:[192.168.142.186];FPR:;SPF:None;MLV:sfv;LANG:en; X-Microsoft-Exchange-Diagnostics: =?utf-8?B?MTtBVDVQUjg0TUIwMTMyOzIzOmZzTlRYcXIwenRGK015TEUrWTlhRXVhSURK?= =?utf-8?B?OTV3WHJEcCtVQ0F5NElTQUlqVUxZbTBpSUhLS2loOHVpWmRYd0tLQnRTeS9o?= =?utf-8?B?TklRa3ZodktMQVh3K2NudmlId3d1a2NMUjI2YlREL2kzcGJHMjcwZnJETjNn?= =?utf-8?B?VmJkakhjcGpVOXFaRWx0ZGdTSTRjWDc3OHFyTmlNSGJLZ3R5S1RUd0ZxZldk?= =?utf-8?B?K2YrUXdXM0hLcDQzNmUzQjh5aVp1cUI4MjFEZDJEU3A1UUE4QU52dWxBQnhP?= =?utf-8?B?d2JBSElFWXZ1ZDZTYTN1eHB2YzJxbC9VcUZzZElnak1ienYzVUswQndjOGRY?= =?utf-8?B?Rm5vK0NYaWFKQXhHY3JUTWdadlNUOG5zZUxsMm53U3FlTUk4SzdqV2RRVzU3?= =?utf-8?B?RG5aNHoxZE1zN0w4ckJ0dHp6aGE1ZGF3K2tGT1V6UnQ1ak5sU25BK2ViVmE5?= =?utf-8?B?cFBlTDFJc1FjanBpVVBSWGNkYmpyR1k0OUtLRVZoOENjU2J3YklVQnZJY004?= =?utf-8?B?bFN1NHMxcFE0U29rTW5JR2t5TndLdmNPcFZXZTNmUDR5eVZ4aVROeVA2VG13?= =?utf-8?B?UXpaeVN5bU9KeHdoSnJRakFPajRQaElXdDJKT1VSRk5ySGRRdDJVdzRBUS9u?= =?utf-8?B?d2JON21MemU1NitzNGswbWhHRXZqdFVXN3dXVUM5dzFlQVpRNkhjMFlBSWFJ?= =?utf-8?B?S1hFbjNwWTYxdzJSUkJZQUdkZVBkaGFNaGpQWkpqYnUwSUtySmxxK0dEZU5W?= =?utf-8?B?ai9pRlA5MGxEWklxMDdadmhFQjFHVmJmVEZPSUNHUUJDV3JvWWxpdUFYYTI2?= =?utf-8?B?RjBhR0hta2doalZDczk4N1J1ZlFHb2tlcDhwSkE2NkxQTmJGR3dXdUdJRlhE?= =?utf-8?B?eVJ0ZGpsbldRZHZ0VXBhem8zclFTU0d2bit4ZU13b0owQzZkYkluOGREcjlo?= =?utf-8?B?WDhzZElUb2RXWnI2NmRRRjA5RTdvMWtqa1FsSzd0Z0ZCcmxkZTNLeldMNExB?= =?utf-8?B?NTJ1TWRlUlNCM0ZBbFdEZ3FGaE1PRTc3ckVMSUZaejNYSGtSYUY0VmUwYVlr?= =?utf-8?B?YnZ2bzU4d21SRXdkL2N1ZUdJUE1uVE1nWWJtSU5TRFZnYVJUVlJqRHl3SFE1?= =?utf-8?B?WkozSEgxRFZ5Sk1mY1VZZExzWXpheFUyaFFNcEUrUnFDb2lkTDV4SjZ1M2NV?= =?utf-8?B?RHJOUDY0V0dsM2JWemZJWG44QmdBa3cwcnNZcFV3Ukd0N1YxY2xFYkYrSUUw?= =?utf-8?B?cmY0V3YzZ05GalRoSVR3bjdNQW9oNitKd0NEV2l4SWpmelg3eUVwS1JreGpM?= =?utf-8?B?NEk1Uk9WOWZ0V3RIekhJS1ZRTnRMdjR2Qk5OM1lOTmJOYmF1S2FUQ3h6eWdQ?= =?utf-8?B?VVpmU2ZNZlVoZ0YvNlVyVC9MWk1BT3JSVk5VTDVXWkpUbDNzc25Kd1YrZXAy?= =?utf-8?B?SEtVYjJMMjdBSlNWMC9kNU9FV2VoWEV4UjdWVWg1blpKd0pnOENKOC9xdlhn?= =?utf-8?B?TFlGN0xXbFVDbThvTzN0TUUxa0hENkVualUwSmtML0pBcE9veUlJZzdESzNJ?= =?utf-8?B?dld6d1BTV251bkNFcG1HWVpaQkZvS1E9PQ==?= X-Microsoft-Exchange-Diagnostics: 1;AT5PR84MB0132;5:Kgk4rhW5eCSuWfgVpwGDYg0Zc+cDHay1jEHWHDpSLRmzSNrmrjQE8roAZyVLUKOOhhwgc1GA/n4vuo+Y6p+mqLFnvKajcOpyt7C7Wyg1hqPJcs/8qX4fobyFQ3qOQryu5qNvgsS3oS7r/4uAEwRRMQ==;24:mPfCoIXmwqcoLdEKQXxn4dVZqgLgqYqmaxBsn7BrSVdbWTe7WplmUVIzjnMVlUArgKTJYMej7RQFZYvol6S1gUQLa44/XKFK15ChPTYjNA8= SpamDiagnosticOutput: 1:23 SpamDiagnosticMetadata: NSPM X-OriginatorOrg: hpe.com X-MS-Exchange-CrossTenant-OriginalArrivalTime: 16 Feb 2016 03:30:43.2996 (UTC) X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted X-MS-Exchange-Transport-CrossTenantHeadersStamped: AT5PR84MB0132 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 02/15/2016 10:00 PM, Jason Low wrote: > On Mon, 2016-02-15 at 18:55 -0500, Waiman Long wrote: >> On 02/12/2016 03:40 PM, Peter Zijlstra wrote: >>> On Fri, Feb 12, 2016 at 12:32:12PM -0500, Waiman Long wrote: >>>> @@ -358,8 +373,8 @@ static bool mutex_optimistic_spin(struct mutex *lock, >>>> } >>>> >>>> mutex_set_owner(lock); >>>> - osq_unlock(&lock->osq); >>>> - return true; >>>> + acquired = true; >>>> + break; >>>> } >>>> >>>> /* >>>> @@ -380,7 +395,10 @@ static bool mutex_optimistic_spin(struct mutex *lock, >>>> cpu_relax_lowlatency(); >>>> } >>>> >>>> - osq_unlock(&lock->osq); >>>> + if (!waiter) >>>> + osq_unlock(&lock->osq); >>>> + if (acquired || waiter) >>>> + return acquired; >>>> done: >>>> /* >>>> * If we fell out of the spin path because of need_resched(), >>> Is there a reason to not also preempt in the wait-loop? Surely the same >>> reason is still valid there too? >> The waiter does check for need_sched(). So it will break out of the loop >> and return false in this case. This causes the waiter to loop back and >> goes to sleep if the lock can't be acquired. That is why I don't think >> we need to do another schedule_preempt_disabled() here. > The purpose of the additional reschedule point is to avoid delaying > preemption, which still applies if the spinner is a waiter. If it is a > waiter, the difference is that the delay isn't as long since it doesn't > need to be added to the wait_list. Nonetheless, preemption delays can > still occur, so I think the additional preemption point should also be > there in the waiter case. You are right. Taking the wait lock can introduce arbitrary delay. So I will modify the patch to fall through and check for preemption. Cheers, Longman