From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S934807AbbIVUfU (ORCPT ); Tue, 22 Sep 2015 16:35:20 -0400 Received: from mail-by2on0133.outbound.protection.outlook.com ([207.46.100.133]:6669 "EHLO na01-by2-obe.outbound.protection.outlook.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S934320AbbIVUfQ (ORCPT ); Tue, 22 Sep 2015 16:35:16 -0400 Authentication-Results: spf=none (sender IP is ) smtp.mailfrom=scottwood@freescale.com; Message-ID: <1442954101.19102.286.camel@freescale.com> Subject: Re: [PATCH v2 22/25] powerpc32: move xxxxx_dcache_range() functions inline From: Scott Wood To: Joakim Tjernlund CC: "christophe.leroy@c-s.fr" , "paulus@samba.org" , "mpe@ellerman.id.au" , "benh@kernel.crashing.org" , "linux-kernel@vger.kernel.org" , "linuxppc-dev@lists.ozlabs.org" Date: Tue, 22 Sep 2015 15:35:01 -0500 In-Reply-To: <1442953971.29498.76.camel@transmode.se> References: <1442945547.29498.50.camel@transmode.se> <1442948339.19102.270.camel@freescale.com> <1442950473.29498.54.camel@transmode.se> <1442950926.19102.280.camel@freescale.com> <1442951752.29498.58.camel@transmode.se> <1442952852.19102.281.camel@freescale.com> <1442953971.29498.76.camel@transmode.se> Content-Type: text/plain; charset="UTF-8" X-Mailer: Evolution 3.16.0-fta1 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Originating-IP: [2601:448:8100:f9f:d59:7025:7bba:8c10] X-ClientProxiedBy: BLUPR11CA0060.namprd11.prod.outlook.com (10.141.30.28) To BN3PR03MB1479.namprd03.prod.outlook.com (25.163.35.142) X-Microsoft-Exchange-Diagnostics: 1;BN3PR03MB1479;2:lLgbEk8vQILGxA9h/R/dYgR8iZ1reb0x/v+K1aXBL8Z0SkFwcAWVIfNj7giRYNBuxZsE1FRS+uEqpJSZykL7qDaE4OQl0LKZkDHad/rtwXwhNNINshIVKuUsTVSn4rA1zpCRq7lqbbPByRcFbxiyVypYTUlBAbtDhhrO5egWtno=;3:JS7rT2DvZH9tLM8qmidP/SL69//asUpwpxpdid3PkOY5OGipa+ihsSdsTKG6T2bn5p8QEv1IkoBMr4xRgJFISl+w1PyEElFRmsfhIYsVoWF6OolUMtzjN9WBVVfHW1vAGGloC5wktb5e+EkRHPUPhw==;25:oGhHl7NrOoc1Eznfk7ky03d9ZeFQC2Vz29fUiW70IHUHoIa4hxwfbrgl5pP45og+FIrgZo8cJL7Je9r0pxpuIcsGprp467ROafx18iNKMEPskil13jle7OURNuqicxzKOqbSBagkjl0m8BYln2VHSzP6QhyPrOCNnvVTRdubBOal3XIWr0qLEA3rFErcCHVur1ZDBK8i+j4u6j6MNgZP+rrrtW5PgzKo9h6OJw8pJvpU2gjmpGgfbeD/G2lIcGtZnHSlSMpBQpdUoJeQmpgVUw== X-Microsoft-Antispam: UriScan:;BCL:0;PCL:0;RULEID:;SRVR:BN3PR03MB1479; X-Microsoft-Exchange-Diagnostics: 1;BN3PR03MB1479;20:wmN5cKqqE0FTCmTDLSX/vqso7jy0nLbB9+JlGVbGDe8hrDBJpbM9T7D5CziDZ7ZQ2RgLT8kXB+PeeN9qVh0oyCekvdTQXLXmHH3EZ9lFYyydl96Z84ILuylR0W9rYw0uLSOO9AuHysWmQeIcU04rCjgQ7a/VUQHLb9su8fnJCU2VOYR0RaM2sNOs9sTjWZrzgf7CJ2creuYq2tbWIWgsdTfunM943UMA+D6MKFApmvUBh4ZMtStzrm3yw9d9m4dRFJ9I2Ua+sDJHShUGj2o2x9YW3CZtwlA+lZxn8ABsIRhdIFfwE5AS11mQn4sDxXf13vACX4WYplV0fGkYIYjyKEm15dvZZ6xgVv+3IvAcRKCJF8iLMNHzMFI6YJpNDy62dKXXMqua6URJtzlzz2bVLA11/1WNSpA2maPvB+05bYRhgyPuQ1KV8U/E+7gu1gtZCi/K/g3VkybJ8JyP1PQhrq3K0tjSahZR3syns7ShgAeGlMcb5iKfJuQOA9XS58Ha;4:qMMwALzDhnhOjtNaEbLyLYRL1pXmitR72J07W8tkg0F8N3jNB1fcvm4tfvNJ06HCc3krjXxRw++EvfxrelZ39pHfu6p+iAA8+/EXBd1+rc2MVjdXOG1MhfyHRnO3FvgNgAhMC4NJtKqeufCX/Cu35lDOx7l+NGSPL4hMje6NLMYP4ZZiqEHC749Mw6QJZoQkxuTnEae47ITt2Oih36Pe59l1rAq4bMGRu0qDeJ2qSJrrKc/uC/VEsAYOegjaxmuhjkWeEvuEhR/Ra8+x2PaS5NqgH0ky2F+CyrzeUNiorTX1g/UrXMzOtiG32oeg3JE42gWa3U8SR5MNWSD1QDZvo1q3gO1XEEhbyWRCQ4fwGfU= 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)(520078)(3002001);SRVR:BN3PR03MB1479;BCL:0;PCL:0;RULEID:;SRVR:BN3PR03MB1479; X-Forefront-PRVS: 0707248B64 X-Forefront-Antispam-Report: SFV:NSPM;SFS:(10019020)(979002)(6009001)(199003)(377424004)(189002)(24454002)(101416001)(47776003)(97736004)(5820100001)(86362001)(5004730100002)(105586002)(92566002)(122386002)(189998001)(40100003)(110136002)(5001830100001)(5001860100001)(64706001)(106356001)(5001960100002)(81156007)(50466002)(4001540100001)(76176999)(50986999)(50226001)(2950100001)(42186005)(77156002)(5007970100001)(68736005)(36756003)(93886004)(46102003)(33646002)(19580405001)(23676002)(103116003)(87976001)(62966003)(77096005)(99106002)(3826002)(5001840100002)(969003)(989001)(999001)(1009001)(1019001);DIR:OUT;SFP:1102;SCL:1;SRVR:BN3PR03MB1479;H:[IPv6:2601:448:8100:f9f:d59:7025:7bba:8c10];FPR:;SPF:None;PTR:InfoNoRecords;A:1;MX:3;LANG:en; X-Microsoft-Exchange-Diagnostics: =?utf-8?B?MTtCTjNQUjAzTUIxNDc5OzIzOkpJUVBGd3JGbTh5LzdiV2RqRytkNnVwMzNU?= =?utf-8?B?Z1p2U2RwNUtBWk5kRXMzTDUyL0gxRnVVTmJsQmJvUzhBVUNrZVZOSThDNVp0?= =?utf-8?B?NThtM2Foa1hDV0UyZ3p4eWRFaGlyRjBZeWdyUW9zY1lDSnBNWUM1RVZrV2pG?= =?utf-8?B?b3V6eTl5UUZzZ3UxNmRTTHhFTXAyYkVEQStXbEpUWXZLdERkSlBhSlF4VnJx?= =?utf-8?B?YzVaakR1RjBPWTM2aGw5ZHJnUmt2Q3lXZVNKKzgzY0NUSEg3MUhWZlo3U0dx?= =?utf-8?B?TjNXeWhpUmZrYVROcVpRT2xkTkRPbjhQRmlFY1F5cWV5ZzRxQ0FRdzNGOGdl?= =?utf-8?B?dmVmM1I1VW9vOUxCRmdrUlJ2aUFsMmRPRTMvOFEzeU1XL1RmMVdsQlozMTJX?= =?utf-8?B?dWs3eFNCVHRZTE95M095SFhSSHB5Znh1Q21JaHFPR2VnZXp6VXlCRnRNOEdH?= =?utf-8?B?djUxYXV3K1hIMm41TzVpWm9relM4aFhEakZnbG5tRkVJc0I1M0dMSXJ1TVE5?= =?utf-8?B?QzFnd2RtNm1TN0tpVk9pWnN1OWFjMlhleGNaMGJkT2cxZSsyekF5M2hHNjVj?= =?utf-8?B?OENoaVZ6ekVrazQwekFwNms0Q0tEY3VsNVhYNDhpbEhVTjMzUFhHZXYySGlD?= =?utf-8?B?cXMrZVhDZGRxYXBLc3hhTmpTcVlyT2FzR1hFZlNaUFcyTXo2bWE1NXBiZS9i?= =?utf-8?B?Ulpjb2ZkNnEwT05rNUttb2MrOFJoanhtdWlqQkl5WWxyL3BMemRyc0lnbGQ4?= =?utf-8?B?ZWpMRUV0SWxTenB1S2k1aE9sUnF5ODJQSGx1YjhkMWFqODJQdXZjazJPZFBj?= =?utf-8?B?MXJEbGR4MVpIM3E5RzZjdmx2eG5LQU82YmxZZSs4ckV2dW51anIvRDZ2Q2o1?= =?utf-8?B?ZVVPYnpGajZmOC9HYTRCcEg3a3lRRFJ6SGxyVndIdSs1aWpnWXRTZ01NRzZH?= =?utf-8?B?Yy9abW1BalV4RGl0d3A5Y2ZMc3VnZG1iNXFaQzYvcjRFOGNiWlZNclIyRm5w?= =?utf-8?B?bVIzdnRIV3hZSXpRMkEvVVRGeTdtTVFrWWUrdzJyMzROWU1zTUpmbUVJdkZs?= =?utf-8?B?RWtHNHVUSGtlVWErdVNuZERCQlR1YjhIenMxM2k2UUNyejcyWklPYVVxSWln?= =?utf-8?B?YUdvS1QrQm9WRGpuMUsyN2xkR3J1RW5ESVFoN3BEUmFGaDlSUDMxUEVQZnp4?= =?utf-8?B?UDNTalRFTHZzTGhXY2g4VmFaYy9DQ0tpb1ZGaWN2OG8zL3gxYURBeFNQYlNE?= =?utf-8?B?U2FyYjNMU3JkOXdrMyt1MTlEN3NrTE1nMDNNN281K3FXVmo2ZjBNQ3JySGMw?= =?utf-8?B?YU9aK1BhL0JHYnpGZDBoR1poQ1RiWmhPUEJSWHljZkhIcXQvWGNuKzR3RWc5?= =?utf-8?B?UDVtNDJ3TWw1K2dOVHZXcVVaVGZ0TEVjb2dNekRpaTl5enoza2x0WDdNZWts?= =?utf-8?B?U24rTHNvUXhWMTV0S0pFRGVjcXBBOWwzUEtrMTZaNlZ0WUNpRDVyT3pqekFt?= =?utf-8?B?WTR4NnpGMTlOWjVlRlkwb0F3ZlR6b2FVdG1oRXJ6aTBrMUUwY3VhN0JibzQ4?= =?utf-8?B?VW53RUM2SkpRZjk3UGJ3Y3dmaFI0YWpDL0V0VGFxd0dPWnhXWndhS3JCdWMx?= =?utf-8?B?ak5KNGNzQWNDa2czOHZaTVFOcmlVM0JWdmdVbllveVZVeXQ4MzhLNmhCVlBQ?= =?utf-8?B?MXVuTWt1VDVPNTJEQXdsN20rT1pydEZ5M3cxRU5oSFlVTlBVeGloeTNuMktD?= =?utf-8?B?V1Jlemg2MnR2QXFWTEllQVhxVkZSRTBzcVZPV0Ria1RGSVBFYWordm1la05X?= =?utf-8?Q?58dGN838PS0Tc?= X-Microsoft-Exchange-Diagnostics: 1;BN3PR03MB1479;5:RqWSx0CwzZX5CI+nE8Jz71nT2eX8iiXgPtKrOsei/GwjDkfWWIzDzuZ3OZAZiu6q/SL/W/0HdqW+3cRazv+Fns4dGnUq5cMcKCw98NCVNAweIRHME6UI34X3xLrvyjEZlQGAArhoZ6CRtvo7hOmmlQ==;24:Q4rb6pIRCahdEbvVGouEMSm4k0IwGUlOz/q9elZ5HXD0hUy6RpauY9jH0gDO/d0K1p8/tFo0JSM5kXAVSYZs0FyjxV0HzX0uqVJBmbvlxG0=;20:okxgwqpD0MqqXXyPsDHLy8oBoBVc/eWtMhLGuzYzwn5MpK4uoWV+Fb8Fj3FrbDXcN6JCtPFdym/fwUdtBQI1Lg== SpamDiagnosticOutput: 1:23 SpamDiagnosticMetadata: NSPM X-OriginatorOrg: freescale.com X-MS-Exchange-CrossTenant-OriginalArrivalTime: 22 Sep 2015 20:35:06.8575 (UTC) X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted X-MS-Exchange-Transport-CrossTenantHeadersStamped: BN3PR03MB1479 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, 2015-09-22 at 20:32 +0000, Joakim Tjernlund wrote: > On Tue, 2015-09-22 at 15:14 -0500, Scott Wood wrote: > > On Tue, 2015-09-22 at 19:55 +0000, Joakim Tjernlund wrote: > > > On Tue, 2015-09-22 at 14:42 -0500, Scott Wood wrote: > > > > On Tue, 2015-09-22 at 19:34 +0000, Joakim Tjernlund wrote: > > > > > On Tue, 2015-09-22 at 13:58 -0500, Scott Wood wrote: > > > > > > On Tue, 2015-09-22 at 18:12 +0000, Joakim Tjernlund wrote: > > > > > > > On Tue, 2015-09-22 at 18:51 +0200, Christophe Leroy wrote: > > > > > > > > flush/clean/invalidate _dcache_range() functions are all very > > > > > > > > similar and are quite short. They are mainly used in > > > > > > > > __dma_sync() > > > > > > > > perf_event locate them in the top 3 consumming functions > > > > > > > > during > > > > > > > > heavy ethernet activity > > > > > > > > > > > > > > > > They are good candidate for inlining, as __dma_sync() does > > > > > > > > almost nothing but calling them > > > > > > > > > > > > > > > > Signed-off-by: Christophe Leroy > > > > > > > > --- > > > > > > > > New in v2 > > > > > > > > > > > > > > > > arch/powerpc/include/asm/cacheflush.h | 55 > > > > > > > > +++++++++++++++++++++++++++-- > > > > > > > > arch/powerpc/kernel/misc_32.S | 65 ------------------ > > > > > > > > ---- > > > > > > > > ---- > > > > > > > > ---- > > > > > > > > ----- > > > > > > > > arch/powerpc/kernel/ppc_ksyms.c | 2 ++ > > > > > > > > 3 files changed, 54 insertions(+), 68 deletions(-) > > > > > > > > > > > > > > > > diff --git a/arch/powerpc/include/asm/cacheflush.h > > > > > > > > b/arch/powerpc/include/asm/cacheflush.h > > > > > > > > index 6229e6b..6169604 100644 > > > > > > > > --- a/arch/powerpc/include/asm/cacheflush.h > > > > > > > > +++ b/arch/powerpc/include/asm/cacheflush.h > > > > > > > > @@ -47,12 +47,61 @@ static inline void > > > > > > > > __flush_dcache_icache_phys(unsigned long physaddr) > > > > > > > > } > > > > > > > > #endif > > > > > > > > > > > > > > > > -extern void flush_dcache_range(unsigned long start, unsigned > > > > > > > > long > > > > > > > > stop); > > > > > > > > #ifdef CONFIG_PPC32 > > > > > > > > -extern void clean_dcache_range(unsigned long start, unsigned > > > > > > > > long > > > > > > > > stop); > > > > > > > > -extern void invalidate_dcache_range(unsigned long start, > > > > > > > > unsigned > > > > > > > > long > > > > > > > > stop); > > > > > > > > +/* > > > > > > > > + * Write any modified data cache blocks out to memory and > > > > > > > > invalidate > > > > > > > > them. > > > > > > > > + * Does not invalidate the corresponding instruction cache > > > > > > > > blocks. > > > > > > > > + */ > > > > > > > > +static inline void flush_dcache_range(unsigned long start, > > > > > > > > unsigned > > > > > > > > long > > > > > > > > stop) > > > > > > > > +{ > > > > > > > > + void *addr = (void *)(start & ~(L1_CACHE_BYTES - 1)); > > > > > > > > + unsigned int size = stop - (unsigned long)addr + > > > > > > > > (L1_CACHE_BYTES - > > > > > > > > 1); > > > > > > > > + unsigned int i; > > > > > > > > + > > > > > > > > + for (i = 0; i < size >> L1_CACHE_SHIFT; i++, addr += > > > > > > > > L1_CACHE_BYTES) > > > > > > > > + dcbf(addr); > > > > > > > > + if (i) > > > > > > > > + mb(); /* sync */ > > > > > > > > +} > > > > > > > > > > > > > > This feels optimized for the uncommon case when there is no > > > > > > > invalidation. > > > > > > > > > > > > If you mean the "if (i)", yes, that looks odd. > > > > > > > > > > Yes. > > > > > > > > > > > > > > > > > > I THINK it would be better to bail early > > > > > > > > > > > > Bail under what conditions? > > > > > > > > > > test for "i = 0" and return. > > > > > > > > Why bother? > > > > > > I usally find it better to dela with special cases upfront så the rest > > > doesn't need to > > > bother. i=0 is a NOP and it is clearer to show that upfront. > > > > No, I mean why bother special casing this at all? > > I just said why? > You to found the if(i) mb() a bit odd and it took a little time to see why > it was there. > Ahh, you mean just skip the if(i) and have mb() unconditionally? Yes. > That changes the semantics a little from the ASM version but perhaps that > doesn't matter? Adding more barriers than strictly necessary is a performance concern, not a semantic change. How often are we really calling this function over an empty range? Not that it matters much one way or another... -Scott