From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754152AbbKLSRd (ORCPT ); Thu, 12 Nov 2015 13:17:33 -0500 Received: from mail-by2on0055.outbound.protection.outlook.com ([207.46.100.55]:6432 "EHLO na01-by2-obe.outbound.protection.outlook.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1751769AbbKLSRb (ORCPT ); Thu, 12 Nov 2015 13:17:31 -0500 Authentication-Results: spf=none (sender IP is ) smtp.mailfrom=David.Daney@caviumnetworks.com; Subject: Re: [RFC][PATCH] mips: Fix arch_spin_unlock() To: =?UTF-8?B?TcOlbnMgUnVsbGfDpXJk?= References: <20151112123123.GZ17308@twins.programming.kicks-ass.net> <5644D08D.4080206@caviumnetworks.com> CC: Peter Zijlstra , , , Paul McKenney , Will Deacon , , From: David Daney Message-ID: <5644D7B5.6020009@caviumnetworks.com> Date: Thu, 12 Nov 2015 10:17:25 -0800 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.3.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset="windows-1252"; format=flowed Content-Transfer-Encoding: 8bit X-Originating-IP: [64.2.3.194] X-ClientProxiedBy: BLUPR07CA0047.namprd07.prod.outlook.com (10.255.223.160) To SN1PR07MB2144.namprd07.prod.outlook.com (25.164.47.14) X-Microsoft-Exchange-Diagnostics: 1;SN1PR07MB2144;2:UB635oZCEjCVoOxogTk5XdOb3yS5D+Yp/4spESLb4v/bS4hCtU7tebBRSBalCdIL4+zNbjM8bMw47e77sa+yWruPsX3oikcXN4ZEWn60LWBaSfD8t1fIDi2uNh7fPkQkKPrevVXdtBXTDQjyrx4v7WEO/i4oLv0CoCGrC4swzY8=;3:noW++/cO1ihmdwVTXRUY99Cz0Jk/1lKcd1fHkIDndr6ZpgYNctZkDwuvXidm+FJfu7ZT3j9v14SZ75GR8A8tDCHlV+m9IsijWbtMmGaFEIwP1Et93mVlob78wXWF9M7BOT9Z0gHB41JVsTH96VfAoQ==;25:uj98l3Xfvuf8sOQVMrNLlGMnnjFRbIQyBZkbtI2XXDBzBsqLLj2RPlc81oaBO48awJbY+SomEKmN/eanMTfOuXLD7IGU+tXGA4qh6vS1Pm1mh0/Z9il7H6DpQqnttm1CoCcDznFd0hqnRWpeW7mRbHaPR6NTbyGF0xby3R9Wvwfu5FPuUkotbQ1aai3GabJeGWtRQAMYsLYDy3UqJDClSW/UQTeVLnSS341zxpIWCFFBjPNW0VdrN78zGFKyekNkISAICuIZgBXWR91jwQQcTw== X-Microsoft-Antispam: UriScan:;BCL:0;PCL:0;RULEID:;SRVR:SN1PR07MB2144; X-Microsoft-Exchange-Diagnostics: 1;SN1PR07MB2144;20:gCmovZHxsRDfYUtzTSnIbCpQaziGkFN+flwoOc2EWqakILQPYYesywyS+MSniqNQ8V3JQgIgupURsu+rxR+R8nF/5/79qQVfoq1aFFnHkfWmPYQUbLc1e7MjfYyvjhEPbeUBUt+vSJG4W9XyItj094jeEg6dUVHKYma+EcfUf8h0uTsQUspsTV44ZHRSCZMqvL/ce656RbuhOIRNFgnES4MUFINNkckQGEZoCcFaj+6GtkhzTz5eGI8r9efKsv8DkOhETFl+o0dYLgGKJMBWT5oI3bNUfWnBDnhfCsnBxvRFowTaoXCoTJ73KxmLfHcljTNUaKvlzvajo2yMhDNVAGt/glM3XlXVS1zooGz+7BFV8EdWcOMquj043DZtnoYEebg4cMl6Oz5s0kvQuE73vZ/sJ1/X/CEtFVKmX22doSgfY93w6hIQoT2vdLkrT0uJjID/fu0Uaydt+GswpNW+mIUL8s44w8DrFk9iG3a9I09Lqt47YxofuzdF+rI4faPQLxiadfbCRQxk/f+UvlB/a3eEms87KhGS9Iie5N+mBJ85rN5sCdCzcewemF6S42hSPsGi93iHur5I4R7ScfnAreXNMSf3WIsR8lENf/ss29s= X-Microsoft-Antispam-PRVS: X-Exchange-Antispam-Report-Test: UriScan:(236414709691187); X-Exchange-Antispam-Report-CFA-Test: BCL:0;PCL:0;RULEID:(601004)(2401047)(8121501046)(520078)(5005006)(10201501046)(3002001);SRVR:SN1PR07MB2144;BCL:0;PCL:0;RULEID:;SRVR:SN1PR07MB2144; X-Microsoft-Exchange-Diagnostics: 1;SN1PR07MB2144;4:m2OyN9h7WrsEiYNLnxYHcFOyKY99QtSYhcVvBoJsKSVYkBKrk6xF+dxtqgst7KM8p0pq9P2a07xM6ioU0BlflUC4+P56S88K6I3SgvvFxJizhEiz6063uM9WAiG914WXf7JR5ihV7LeHqGMYurGmoZITHtWxoRBpSZrtvLxycjma6p6Rza7qiP6gQtpIIuTfjJ87H65mP72s4QochQ4Vx4Gqz2znQLRkb9OaxZ5VuUwjgAog3OtiLMJWYOHJ+SAzal0mQbo2GnO0WVM7iRH6eSczwDYqL4DAO4QpYNeGKGy6iYdb73XjowebXDKrPpXGGkELzOD0bEgOeovrabn0fZNrI4sYjGZt1hryWFxIGeanA6r6tZUjMpqsAdm/q90F X-Forefront-PRVS: 07584EDBCD X-Forefront-Antispam-Report: SFV:NSPM;SFS:(10009020)(6069001)(6009001)(24454002)(199003)(189002)(479174004)(377454003)(19580405001)(36756003)(65816999)(105586002)(5004730100002)(87266999)(92566002)(80316001)(76506005)(54356999)(19580395003)(5008740100001)(64126003)(2950100001)(101416001)(69596002)(33656002)(53416004)(106356001)(110136002)(65956001)(65806001)(87976001)(5007970100001)(83506001)(77096005)(5001960100002)(4001350100001)(76176999)(189998001)(66066001)(81156007)(50466002)(23746002)(59896002)(47776003)(40100003)(122386002)(42186005)(97736004)(50986999);DIR:OUT;SFP:1101;SCL:1;SRVR:SN1PR07MB2144;H:localhost.caveonetworks.com;FPR:;SPF:None;PTR:InfoNoRecords;A:1;MX:1;LANG:en; X-Microsoft-Exchange-Diagnostics: =?Windows-1252?Q?1;SN1PR07MB2144;23:8XBHl4h+POFw7tglcA5IA2aamvi7SR+ZcnEWg?= =?Windows-1252?Q?j1gEToQ+UyHbozCDVl1cb+zyIWJqUzS5pNAFBK2AnyAyZ2pSppSeM2EJ?= =?Windows-1252?Q?5rRjK+HNCi18sNGtOcwbl0w3g6j8Q78RGd6uEmm8Eln31T1HVUqZ+1zQ?= =?Windows-1252?Q?Rs/1w3yTbq9trHdq6cCOaNYEZ6FnO8jDA3eSIdE4G8pUg8ulXjXv5iT1?= =?Windows-1252?Q?ysgmqp6NhAvQnqcapqNHuXSj6kqWaeUA40fAAUnJIDCs+nl0odICldg5?= =?Windows-1252?Q?IY7Y85aZ9NTseZvbHszQBQAYvBYpI8AFHMRt1eyIRbboHcjXHve00aFH?= =?Windows-1252?Q?wb6BN8bCwwdfuQE/oZPzGA9DxNsjU5HWLyXTEvybBeUn9fnR6JO3b/or?= =?Windows-1252?Q?IvfWsAoVJ7dXLm/M8Q5bDjq+NBePQY5ulEiJZYme1oLNiekd6KDeM6fY?= =?Windows-1252?Q?hiYfx64D7JxbJSZnJcMtvGh1Hf6yuc49k5g+HomAx0ivXjIKQ28tkA4f?= =?Windows-1252?Q?Lkko6cUjhKRCJz8BsRUDHcEg+g9/UVTxolf9x24hYdCvre2ypkZqQPal?= =?Windows-1252?Q?0M0bspAehCwTU4nZsXkH7V80U5CfAPkYYe1Dkx3b/w9baB3msaw1c2Re?= =?Windows-1252?Q?o1rikUyJul/Ea/RwuCH/bPb72w25mciDjdd9+Eucwx02ruCfKxSPllaJ?= =?Windows-1252?Q?QLm6coiuQcqxN0kSYeIz3r7XQBlmg0HJTlvpYrggn5jqRMKqnT1IpsUN?= =?Windows-1252?Q?IDW30duuoWaCcnwU6VnDR/Iql3BEdkP+V6O5t4I9vuYdunul2mSd4kBn?= =?Windows-1252?Q?ncn2CqIDsEIf5oiuapF0zTLY/LL37619+TA/0oSDP7dde3dQXXe4yCms?= =?Windows-1252?Q?nKlSX9V9grd8ZepAPFeU9UlScrxPnjZvPv/e+ycvSg6uT3JcJZmcPdMP?= =?Windows-1252?Q?OWXWkGBPAAc48sjLX+MDGs9XocX7Ar2QFYfYtxeYtDIisfeCOc6vskcS?= =?Windows-1252?Q?LMpqYcO9sW/SQrASuJYA9L4UkzEhtbhVEZU7EMR8FFkNwEPY9DoM+CtU?= =?Windows-1252?Q?j2IXtKejyEJht5qN8gCmffjtYlV2movkBq9zupSKu9AV2yc2Xp+AqKQw?= =?Windows-1252?Q?nB9LfkVoC1m+gEQQBar+hI9c5oN53WUpVa1ru5Y19Cw1eBO0mgO41LlU?= =?Windows-1252?Q?vI/TiKPy+VOh9EpaJO4G7JNav3z5wOkEpP8WK5NzfxYRXQOTL54xfueB?= =?Windows-1252?Q?zWbjy1DpfPnS3YIxPyORMboNCEFGm6pllP64EcduI/hAu//Uf5x4QMOu?= =?Windows-1252?Q?a+o0T+UTnpLbx6UpxoxCIxZ4Eq/4xNOmrzb5aoCkS0ZYGhsybe1jOGs4?= =?Windows-1252?Q?yZhFyTJ4HQp?= X-Microsoft-Exchange-Diagnostics: 1;SN1PR07MB2144;5:6NelkIQ2n8nzjmU/qyZP2IwnRJTKm8e4Ru/NoAelRKbJahhQNAC/jomInpT4wJyWyuvjy04ZJDJbTgRBiSt5fQU6MVZG5kbxgbF3fuDarGE5gdsXExFeEwPOkmUX7+5BE2X/4RMabegkiw8WtNNJiA==;24:UpMGSqlWDjjtzranTqfEVhbyhtTfK9iVWajmQoy72NsHGIdxAoa0ImtkLw6awu0oc4GzNnOeOSWVygs/qEGQ6u6wCauDE6bk+ZSz/0kjF9E=;20:dHCf9DnHuUPUGN5OTRdFhL5JumziIWJo0l5/6U/M69Vls28oC5QvKUBUZuRsumlWZSrrKQjIpEgnqxzJ6aq0yQ== SpamDiagnosticOutput: 1:23 SpamDiagnosticMetadata: NSPM X-OriginatorOrg: caviumnetworks.com X-MS-Exchange-CrossTenant-OriginalArrivalTime: 12 Nov 2015 18:17:28.9011 (UTC) X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted X-MS-Exchange-Transport-CrossTenantHeadersStamped: SN1PR07MB2144 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 11/12/2015 10:13 AM, Måns Rullgård wrote: > David Daney writes: > >> On 11/12/2015 04:31 AM, Peter Zijlstra wrote: >>> Hi >>> >>> I think the MIPS arch_spin_unlock() is borken. >>> >>> spin_unlock() must have RELEASE semantics, these require that no LOADs >>> nor STOREs leak out from the critical section. >>> >>> From what I know MIPS has a relaxed memory model which allows reads to >>> pass stores, and as implemented arch_spin_unlock() only issues a wmb >>> which doesn't order prior reads vs later stores. >>> >>> Therefore upgrade the wmb() to smp_mb(). >>> >>> (Also, why the unconditional wmb, as opposed to smp_wmb() ?) >> >> asm/spinlock.h is only used for !CONFIG_SMP. So, smp_wmb() would >> imply that special handling for non-SMP is needed, when this is >> already only used for the SMP build case. >> >>> >>> Maybe-Signed-off-by: Peter Zijlstra (Intel) >>> --- >>> diff --git a/arch/mips/include/asm/spinlock.h b/arch/mips/include/asm/spinlock.h >>> index 40196bebe849..b2ca13f06152 100644 >>> --- a/arch/mips/include/asm/spinlock.h >>> +++ b/arch/mips/include/asm/spinlock.h >>> @@ -140,7 +140,7 @@ static inline void arch_spin_lock(arch_spinlock_t *lock) >>> static inline void arch_spin_unlock(arch_spinlock_t *lock) >>> { >>> unsigned int serving_now = lock->h.serving_now + 1; >>> - wmb(); >>> + smp_mb(); >> >> That is too heavy. >> >> It implies a full MIPS "SYNC" operation which stalls execution until >> all previous writes are committed and globally visible. >> >> We really want just release semantics, and there is no standard named >> primitive that gives us that. >> >> For CONFIG_CPU_CAVIUM_OCTEON the proper thing would be: >> >> smp_wmb(); >> smp_rmb(); >> >> Which expands to exactly the same thing as wmb() because smp_rmb() >> expands to nothing. >> >> For CPUs that have out-of-order loads, smp_rmb() should expand to >> something lighter weight than "SYNC" >> >> Certainly we can load up the code with "SYNC" all over the place, but >> it will kill performance on SMP systems. So, my vote would be to make >> it as light weight as possible, but no lighter. That will mean >> inventing the proper barrier primitives. > > It seems to me that the proper barrier here is a "SYNC 18" aka > SYNC_RELEASE instruction, at least on CPUs that implement that variant. > Yes, unfortunately very few CPUs implement that. It is an instruction that MIPS invented only recently, so older CPUs need a different solution. David Daney