From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S933891AbcJQXRL (ORCPT ); Mon, 17 Oct 2016 19:17:11 -0400 Received: from mail-by2nam01on0112.outbound.protection.outlook.com ([104.47.34.112]:44447 "EHLO NAM01-BY2-obe.outbound.protection.outlook.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1754909AbcJQXRC (ORCPT ); Mon, 17 Oct 2016 19:17:02 -0400 Authentication-Results: spf=none (sender IP is ) smtp.mailfrom=waiman.long@hpe.com; Message-ID: <58055BE2.1040908@hpe.com> Date: Mon, 17 Oct 2016 19:16:50 -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: Linus Torvalds , Jason Low , Ding Tianhong , Thomas Gleixner , Will Deacon , Ingo Molnar , Imre Deak , Linux Kernel Mailing List , Davidlohr Bueso , Tim Chen , Terry Rudd , "Paul E. McKenney" , Jason Low , Chris Wilson , Daniel Vetter Subject: Re: [PATCH -v4 6/8] locking/mutex: Restructure wait loop References: <20161007145243.361481786@infradead.org> <20161007150211.271490994@infradead.org> In-Reply-To: <20161007150211.271490994@infradead.org> Content-Type: text/plain; charset="UTF-8"; format=flowed Content-Transfer-Encoding: 7bit X-Originating-IP: [72.71.243.218] X-ClientProxiedBy: CO2PR06CA038.namprd06.prod.outlook.com (10.141.242.38) To CS1PR84MB0312.NAMPRD84.PROD.OUTLOOK.COM (10.162.190.30) X-MS-Office365-Filtering-Correlation-Id: 10bdb8d9-7efd-4b87-08eb-08d3f6e3b2a4 X-Microsoft-Exchange-Diagnostics: 1;CS1PR84MB0312;2:1Nyt0MdPiwMiUEaRXbZS2rO13WuRSwz6JA7Z37f54TgxbTjs5XaamIuDDWwSBc8Q3UEIuYedTw4OzG59Btlzv76vGk80rC2lH64TfddXD5LABab7HkSCLv+jhETXAFjkPl2xTYSrwT5yggXsXzEj+paqYqCOI2gx8ozXNJmeVvDhClJpmt9jurHEULkO1Xy8YthSYhDqTN5+r1LMp3Xg5A==;3:xmeO18vAqhZzuQ+UPrqjxG6dRcjtrfuICzFnw+qe71cOV0PA5Ei01Ozd3gxGD0yC7uFTW+ymex04EMKMrgm2LS8gdNcrXVZ9sSVSWQhBbyQTmueBBF4mhjFJrkRyFd6sU3MjxzjsWAfO9+0kG3mfiw== X-Microsoft-Antispam: UriScan:;BCL:0;PCL:0;RULEID:;SRVR:CS1PR84MB0312; X-Microsoft-Exchange-Diagnostics: 1;CS1PR84MB0312;25:8Uxd1tCiqDC4UTSteSCRE8mz4PSL/6HQ6q/mycOiA2fONG/f82iitBlpiuzknx8EpwALR4GDLsXGw1oL5PXoXO486iMZ1U1n5WRg7QiZ2L44ySSw+MG2DSywCyTgD/5+Hge3vHtIR/41deTS8Nr72JxiO1Q1vH3h5Vf/9qk1HaUzAvEYwpXHrMkm0gPtIKIm8WT5uVXoPh1d8lqPOJOYZoNBdJASlPDrd22bAhIgY7iFzBRhnlAFWJdaISy7n6AIYIbZEqk7ROQiLJ91lV4Fxu+jC/Dxa58jL64gcRJJ0tHvso1Blkm8wKg4/t/jr3VwdcqFHHlZMXj1n41Sa+V0YjDI2EkSuXW7jvu8eQnJMKZWwYQt3hlJUUJwpkaVUqu8v8s7+BrEjzCKvOEWg8Y9V0V1Kmw3Hm1jVevxMqL+gJIrj92md/trC9hY4OrqQCjQoE5Y4XArnvyLFE9zVOec4jZ3oaH2u+g28pQzpnwp1l5xEEPZhp64AbnQnbT0VwMLdzJlagaU4p9ZKExhroWUD1M7wcZfYGWspK6K/MCZc4fqnuvDWeDoTNxCTHEGvbRrzRyWJzdsW2FzzOXN1MAlC6xIlf3nGylvemma+03q0d3QsKUWrVEJMef3pk66SCjQDO9EOr2LAAmJu2RyeOXcYrwIWiy3fEr8DdZCa5AfElbGPkhWBHqMNtGYxqPNcGDoyWmrUQ4fDI6jPznF5aLdUQ== X-Microsoft-Exchange-Diagnostics: 1;CS1PR84MB0312;31:t6AZBHBw36HweKqbNWWImuMitSjP42XErXwvtwLfIya+ZNgQcttzhDUKDLS+LN9t88fgdQUmSGLsFe6D9r+wpWsRSHehH+ZeiQso1K0B13CDD2YyMl7nFdJKHGGFZOMfpjyViDjQULGWSKg0aVfVQYr9rQq2nXaWQg0mAzstRea0rwIEyoaY7hu5HJk32rtfo3pD48Sedgdx0P8YVW5vcTVVaI2VHcUdFEHA4BuTxP9y59sBY+kobIcEvNpVa/9ZwC4dZon8hwBPcIYH5ccjfA==;20:JabkNaF0zv3O8RLSe/VLLiOK7yDa8F881ns/+3zWkcio/SRPn5ypsMId8GNbYO+UliB1vU99eW4TgQWZR2WzRveLekytTUrZrJzk6OFMcPG3YvGdhGVaEL4j1q43a+bGWyFVAycUlI1FBrq05/QjbSpFCLRwO2XpnwIQMhBJL6XZ0eCykDJbmZeSe23GorRBUkxn+GRwUdrt31yy0OVHTbeWkQ6YqURUmqOiMNMIKy2WDzpmyVtJKfPn3EQghzU0ZVt0PYWuTS8sFUry+K0Yfb7IGih5+SAY/D4E+h5nzH85SC4br4Dxb/sWVojfFsf1zpwL+p06hHyrcqQDFnGvnONRLdeKgr7NLkTgKX4QiTwQkUls7sJSj7eWQ/QyDx3rmDwfc3nPO/N1gzkMTKoLA16ydOUsNJZxyt/iPd0Y87eo37cMEZrK+FdLfL3ulGC8NitRCilWRilY+NfgNBzuFm7iuoUsRQDlz8rIqxaTXEunTzr6VMgUAwmWHWVoVV8K X-Microsoft-Antispam-PRVS: X-Exchange-Antispam-Report-Test: UriScan:(227479698468861); X-Exchange-Antispam-Report-CFA-Test: BCL:0;PCL:0;RULEID:(6040176)(601004)(2401047)(8121501046)(5005006)(3002001)(10201501046)(6055026);SRVR:CS1PR84MB0312;BCL:0;PCL:0;RULEID:;SRVR:CS1PR84MB0312; X-Microsoft-Exchange-Diagnostics: 1;CS1PR84MB0312;4:wto0Zf/rwjxKyxfAfH8emIICU1144KCw2rlJxfoo6HSssyE5O5WngMaq5338B4cxjcUSOK0vRNv0/fG5cmcvnjOnuyoFvX50VqA26OFcfHNvv9aXbD4dQltZOVk2tPAmXIDhD28106lK7iidy9nDvQxUf9X8yh5kImUcisfkQ7wq6NDDlmbmbTRQFKrmanSfFWqNmR799lhuGmjLHv7LpgzNMZI7+M3fPZf63e6ClU6w5fY/VSpfW3mT1xdpwLGLoBTzE0eOqxPtHQi+aImcSIfObFphgSLpMnpfZQHO4Sr+zd1hcuCr1EXTMGOJSI9N4TzYv59ZrkWqilg6ApXPY1HarSJ4NvhXSKfu10aKA7NecqrxJ1D/oG3v3xe5UAClnJSfU+qR+g9W4GLyOLmTfensPDmFq5KI0vOOrsIk6MDu96xPqPAa/Gy2CcOql8WVvPpo8Ep06D0yAhgrTZ6+JA== X-Forefront-PRVS: 0098BA6C6C X-Forefront-Antispam-Report: SFV:NSPM;SFS:(10019020)(4630300001)(6049001)(6009001)(7916002)(189002)(377454003)(199003)(24454002)(86362001)(4326007)(2950100002)(65806001)(586003)(23676002)(6666003)(110136003)(92566002)(65956001)(65816999)(66066001)(47776003)(6916009)(50986999)(80316001)(189998001)(77096005)(87266999)(8666005)(64126003)(50466002)(5660300001)(7416002)(101416001)(99136001)(2906002)(19580395003)(19580405001)(305945005)(117156001)(59896002)(42186005)(97736004)(36756003)(7846002)(83506001)(230700001)(33656002)(54356999)(6116002)(76176999)(105586002)(68736007)(3846002)(7736002)(106356001)(81156014)(81166006)(4001350100001)(8676002)(7059030);DIR:OUT;SFP:1102;SCL:1;SRVR:CS1PR84MB0312;H:[192.168.142.146];FPR:;SPF:None;PTR:InfoNoRecords;MX:1;A:1;LANG:en; X-Microsoft-Exchange-Diagnostics: =?utf-8?B?MTtDUzFQUjg0TUIwMzEyOzIzOmdGaXNzYVhpTGdlMldRMHVOS0MwcGg4MlNz?= =?utf-8?B?YklVck1pNGNHYklHanZxSTI4U29CdXJXVURuMzkvS3NVQ1VKWElGSGJ6MzFS?= =?utf-8?B?ZU94UzhFUUp3ck9keTlhNWpEVHJsYUFENUR2QWFaVzMxeWpaNDRRUGNjcjNj?= =?utf-8?B?aDY2ekRGdUJ1MGNlb1F0TjNJOHAxcGV5dmd5V1JKcW5TcjAxMm90MWxnbzd3?= =?utf-8?B?UEdkOXI0dFV4ekhDaUIyRFVrcmV6NytsdysvdGw1d2wvSmVYeUQvbzY3U2pT?= =?utf-8?B?NDRDUUpYbEdySGlSUiszSHRiV09UcEF3eDJLRkMvdncxZnp3dmczc1VFcEFy?= =?utf-8?B?bUdXT3Q0R1FUckdka25ya1NSUFdCTHg1cjd3eU9TMmZDbHlKK1RyV2RZNkFW?= =?utf-8?B?NGMvaVA3QlVLcGpsYUpjM2JNS3lCdUIyMFpzLzlqR0FlQXFRVWtza0syMTV1?= =?utf-8?B?VnJTVmE1ZnJHQXVnWjA3NkZyM3VtS3hydkp0czluK0t2cVBuQ3pnY0FrVGsz?= =?utf-8?B?OFFnZ1ZnbmorUnpPM3VxZytES2R6RnhHU3RxNjBBeGFWTjlkSFhrVW8wS2tB?= =?utf-8?B?bGZSbSttWE5kNzBzZFBLWkp3bHJHaVdzcEVPQzdzRzFWaTc1c2ZVOExLeHBj?= =?utf-8?B?Zmd2MExiOTVyOS9zZTEwelUzbTFvNExJUGNmMmorT05mYnloVE1tdC9tRmdD?= =?utf-8?B?TXh5Q0hkSG14ODUxMjdiV2tzSFh2RzVzd2pOOUpJTnJuTzQ5VFRpcnNFYnJM?= =?utf-8?B?aFRQWEMyUTdFdGdNaVE5WWlwYkY3bE5iSnphM0RLbVZ6Q1l0N1lZbEZseUtk?= =?utf-8?B?NnlFVnMwd05PM0RlTVJPTllBUURicUlvV2JwelpGZ0h6SmY2TkFPZ2IrbVF5?= =?utf-8?B?TmgwNkxEbys4aDJHZEs0bVhYWTZqRGRaaEhRQmkzTVpJcHJ5V1o4bnJvNmIw?= =?utf-8?B?dk0yUm5XeS92WHJkdjYzU3BRMTE4RUlwOHZZZFA4S2dLanRqTElkNHgreE1I?= =?utf-8?B?MzZ0b25PNHI2NzRTKzRFckdSdXF6b0cwKzRORFJOcVk3aWdLUDkxNTBxY2h3?= =?utf-8?B?MjVJY1ZBNndnWmVYMU5uUlhaRjIzSkF0R0x0S2c1cXprT2VxeWpyTExHQ0FC?= =?utf-8?B?RDU0NFlkRy9ZTFhINnkzb0NrSmFKZFIwQ28zLzFwd2FNN3RFczR3ZGc1MTVj?= =?utf-8?B?Y2N5YzlxbVVtVjhxdXIrTUZPY1NyYVlsaEQwMUVVdUwrUDZES3J6NlBsQXJa?= =?utf-8?B?VVFROE1CQlc2MFAwb2duWVBsanFKQktzaVBLZ3dQM1JpSjQxdVFZcEpDbnhw?= =?utf-8?B?UmdoUC9JcEphajV6b2MwRVBaNVY0VHZIZHFNZ0UzK1g0ZHV1WHI4aUdIRGxL?= =?utf-8?B?bXFZb2UwRStReXBxUVBkYU82ajkzb1FxenBNbEhJYU8yKzBNOVJ0TzBxenhY?= =?utf-8?B?VnNaL0NrcHlmdzQyTjdhalRjVVQ2YWg2dnRxaXNqMldMNWNUUEZLSjhEZWRV?= =?utf-8?B?bSs1U21uenliUVpqYmpmdS9obEE5eVhLamw1U2VSVWpBT0Z4cXgySDZjdFVj?= =?utf-8?B?UkpINForTkowZ1RSVjQvK3B2WVBjeCtUc09iWTB1OCt4ZWtvUisxRFN4dUJD?= =?utf-8?B?YUlybldMWTE0TGdqSWpGaElpVFZoTkVtVW9zVm96dkhrejdsZ2dleHhwTHFh?= =?utf-8?B?UnpFdnRSdFNkTnhhZjJoaTNnQVZoY3Z6S1lQTEN3RTlRb1E2SlN6ajg2SFlW?= =?utf-8?B?R3lKWmI3S3lrNHJ1SDV2OURDN1Qxc0NxOE5rWDFkeUJmY2pDRUFYN1Y0eHZE?= =?utf-8?B?cTBKa0M4eTZkc2VPeW9wcmZiNGdJczFNOHA5TW5Mak0zTnhpOXcvOUt2MndY?= =?utf-8?B?aXNwazlCR244SjgxcTdDcFQvZldid1RST2VTQVdseGt2ZG5vUDFMdUQ5Qnp6?= =?utf-8?Q?mSWnbynAED/Wpccd+GqKzWtY1VtSNE=3D?= X-Microsoft-Exchange-Diagnostics: 1;CS1PR84MB0312;6:/KwN5YFyGG+jgB+lr0nXjYDp5DJOZ3QSOrRUvXqQr2+VdTC8pVjc9BmZ/ZA9JFv69wB9RI4pVVbFJVisSCp2xsFm+E97m/VFNxrRBI5Xjvy4JDEJVtb0E/5YrXIIaxKHYIBt4uYJluGgrKFsHvt7sXwgzAa6srLJCP0ShLnLzJPycJk2vcA8k3uRA5duLQeispqvCcL7ruWmxD/eFvVAU9rWhNWNykuQLaVXcUQCG0E+nxWja/fdZmXG7eh54+CUvLbofgUztm6KQctnrmIzj8BYXTlteLzS5AITr+Ena1UnRnkLtU0+IcF/e6kiZtgjgOckK/wlR1OKagpKW7UO6w==;5:kdfiFmsrDYu5/TMHMvO6HkJerLzOU9lSy+v3sIZHzusdBiKS7oiTWT8bWaBN+K9BppmpzW2M8x6GxrchLEWqvdNvUZriw8DrUPhUOTQNo/4N9cvGP7aZBrKXsn/nPZqiX55uVOp3KMdG5Bx1nkAScA==;24:6+5KTYtaif+KREXb3oiIt698HXXpDAJLOwL09yo5xpgAK3mBWtgIX3e2gkR+ldDqg/UkwgisEU8zZtgprSzfyqnifjjGG4jpEQwJJQACrg4= SpamDiagnosticOutput: 1:99 SpamDiagnosticMetadata: NSPM X-Microsoft-Exchange-Diagnostics: 1;CS1PR84MB0312;7:jU/WhJU+a5/m9nX/nBhpa8SyoPkbwHv7e1VgKWazfCCmxEpAs/kF5aZM+C1xK0rFcd7890kl7s74oyWPLJ7tmwobdfEEm9lX4txQXPgB1OaJKXebU/Hwr+5A+cn+BoFx7Jmv28AkQzakdwCWHIJ0jiq3MbUJ6AGaL6lN1N9dQCnoD94a7tiUDCYXGFz5pycz7f1qLR81Y573X/TvxxbDrWDmGbEKAk2EUIZpMqTFdey2qt4pK6HdSKYgpREUT52QNzYwPqx0L6gRTp8pZ0Kt6Whc8xVxG0r9SJI9uSTvvIMxk8gGC5ekB5xGeYD94K3fpaiCFHKo8FkHVSUDn6zofb798F/OlXiL6O38uyQNM6A= X-OriginatorOrg: hpe.com X-MS-Exchange-CrossTenant-OriginalArrivalTime: 17 Oct 2016 23:16:58.1997 (UTC) X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted X-MS-Exchange-Transport-CrossTenantHeadersStamped: CS1PR84MB0312 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 10/07/2016 10:52 AM, Peter Zijlstra wrote: > Doesn't really matter yet, but pull the HANDOFF and trylock out from > under the wait_lock. > > The intention is to add an optimistic spin loop here, which requires > we do not hold the wait_lock, so shuffle code around in preparation. > > Also clarify the purpose of taking the wait_lock in the wait loop, its > tempting to want to avoid it altogether, but the cancellation cases > need to to avoid losing wakeups. > > Suggested-by: Waiman Long > Signed-off-by: Peter Zijlstra (Intel) > --- > kernel/locking/mutex.c | 30 +++++++++++++++++++++++++----- > 1 file changed, 25 insertions(+), 5 deletions(-) > --- a/kernel/locking/mutex.c > +++ b/kernel/locking/mutex.c > @@ -631,13 +631,21 @@ __mutex_lock_common(struct mutex *lock, > > lock_contended(&lock->dep_map, ip); > > + set_task_state(task, state); Do we want to set the state here? I am not sure if it is OK to set the task state without ever calling schedule(). > for (;;) { > + /* > + * Once we hold wait_lock, we're serialized against > + * mutex_unlock() handing the lock off to us, do a trylock > + * before testing the error conditions to make sure we pick up > + * the handoff. > + */ > if (__mutex_trylock(lock, first)) > - break; > + goto acquired; > > /* > - * got a signal? (This code gets eliminated in the > - * TASK_UNINTERRUPTIBLE case.) > + * Check for signals and wound conditions while holding > + * wait_lock. This ensures the lock cancellation is ordered > + * against mutex_unlock() and wake-ups do not go missing. > */ > if (unlikely(signal_pending_state(state, task))) { > ret = -EINTR; > @@ -650,16 +658,27 @@ __mutex_lock_common(struct mutex *lock, > goto err; > } > > - __set_task_state(task, state); > spin_unlock_mutex(&lock->wait_lock, flags); > schedule_preempt_disabled(); > - spin_lock_mutex(&lock->wait_lock, flags); > > if (!first&& __mutex_waiter_is_first(lock,&waiter)) { > first = true; > __mutex_set_flag(lock, MUTEX_FLAG_HANDOFF); > } > + > + set_task_state(task, state); I would suggest keep the __set_task_state() above and change set_task_state(task, state) to set_task_state(task, TASK_RUNNING) to provide the memory barrier. Then we don't need adding __set_task_state() calls below. > + /* > + * Here we order against unlock; we must either see it change > + * state back to RUNNING and fall through the next schedule(), > + * or we must see its unlock and acquire. > + */ > + if (__mutex_trylock(lock, first)) > + break; > + I don't think we need a trylock here since we are going to do it at the top of the loop within wait_lock anyway. > + spin_lock_mutex(&lock->wait_lock, flags); > } > + spin_lock_mutex(&lock->wait_lock, flags); > +acquired: > __set_task_state(task, TASK_RUNNING); > > mutex_remove_waiter(lock,&waiter, task); > @@ -682,6 +701,7 @@ __mutex_lock_common(struct mutex *lock, > return 0; > > err: > + __set_task_state(task, TASK_RUNNING); > mutex_remove_waiter(lock,&waiter, task); > spin_unlock_mutex(&lock->wait_lock, flags); > debug_mutex_free_waiter(&waiter); > > Cheers, Longman