From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1758911AbbIVTmR (ORCPT ); Tue, 22 Sep 2015 15:42:17 -0400 Received: from mail-bn1on0131.outbound.protection.outlook.com ([157.56.110.131]:26291 "EHLO na01-bn1-obe.outbound.protection.outlook.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1758826AbbIVTmO (ORCPT ); Tue, 22 Sep 2015 15:42:14 -0400 Authentication-Results: spf=none (sender IP is ) smtp.mailfrom=scottwood@freescale.com; Message-ID: <1442950926.19102.280.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 14:42:06 -0500 In-Reply-To: <1442950473.29498.54.camel@transmode.se> References: <1442945547.29498.50.camel@transmode.se> <1442948339.19102.270.camel@freescale.com> <1442950473.29498.54.camel@transmode.se> Content-Type: text/plain; charset="UTF-8" X-Mailer: Evolution 3.16.0-fta1 MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Originating-IP: [2601:448:8100:f9f:d59:7025:7bba:8c10] X-ClientProxiedBy: BLUPR11CA0052.namprd11.prod.outlook.com (10.141.30.20) To BN3PR03MB1479.namprd03.prod.outlook.com (25.163.35.142) X-Microsoft-Exchange-Diagnostics: 1;BN3PR03MB1479;2:nKU2uqt7b5NdtiYYF0OMi1iBpNzZNEqN3MOz5RWN+3+zwea+rE0LHqLhAznpfg1qIXH0ACEbs1xQdYmCS2PcZfwo7CohmztXDnGzwORpz6dqcSFYlJdCxbAjOpnQ5EJU3sncjVnCWcVnU2uZgkcL2ap1YlzJtds1ewQFonWjXYg=;3:4Lz8txcXHtWCI9zW7Px2BQvZK0qHBppu/Kd9iRo/qhcrwWLk5XUX7lZ+XKwFDBAhEIY6e1yhsXYsPgvompXLIpYNvC+pKfcVWUSHt3/6X0agu8NKIgkNoO4adPGOJwU8HWKgzlYykEagRE05eHoSeA==;25:ybkshatcN2zN3eiY16DwLBseSGEW8iNbcMqQhF8AvBIQoXPDIrq2txHsWTo0bwRZIsMZVWUf+o69q1NpvAwR9+tCzOlXiBwz3wcgwc6MalkS+r6w4H0p73CcEK4i+OdZZk7RoTeiWicE8En+hx/pgZRraR5Cvun6YOd9PUMhIK1IZw+430CcCq3R0fvmE8Jj4t6zlX5SQMP0reuHDTs/FuCaV6SFWsYta1PeBh4UcgIlI+fAZqU34XKhYKatc8ZoplRvzzNi0qCtIXRBN/ZfjA== X-Microsoft-Antispam: UriScan:;BCL:0;PCL:0;RULEID:;SRVR:BN3PR03MB1479; X-Microsoft-Exchange-Diagnostics: 1;BN3PR03MB1479;20:2XZGvUq9p19InNBkuPZIgPrpZKUTvUfjitAUeoXh4gChhQYShSFwJ/fVTrEp6fL0EecW+jgwv8qFuuiaBW8FbbfAFWWv9eSUm22r/2xkmXZmEsS7xYBeCF5GhCTQK0D+bPGTu/mOc77ZOUQYk1iVIv6/1rmh03thRYACbJSi0mp4uzWEKmmjoXWlFQsbr3R2goCK7mc3xR1B4kxTCUx8d9cqASY0u2KsACAfa27i7IVNohk7YKxPn2g7ZhjElJ4KxLsFGo6ruYJ2jP/JuUuU0zSA67M/+KrEqjXd04Az1HGmBbAn8RrpaTY6Mcm7cWG+GejEtbrJRfMuAy6eVH/HQDqBqC2wZSO0sVyFlHMvfANhOcBk/mt0pQryV6D1GhRscw/zsPuiodGs29NnDg4nDBu8d/sqw9SOYLeQ9tF9+M4/uVZurPb8Q/VSruz1JXWNPwQNDbOwk8JQ1ViNNi5YY3a0SqFz40RKJilRzGCrShJ4uniAJ1t/K2DK8tlGCSAI;4:5wgPxSSPFsdJqPXlKCNgKRgisNyjQfaJr82OxqTCuRleoYrbBFbubgviFYZdtfzsP6HAB+geFMtwKuX09JdHgSLOzUNAIf+TdJzaBPovVUnYmHZC7eO6JmdbDyr08wnPqRYbCZLf/FpJUh6TgI32IKXnTHDkwwn6CiDFBJa8BJHmiu64XzcTNWkSnk7hRU41QvtDbJWhwD3JNsQlaL9nbpNpTHfmUJDlJk/h6tpaiKV9EFSdm++qWg1WbudoSOm6vNpkz7FDhOQ3Y2nTQg0lVVPu4eZaWjGnm/WEnlNgQEQaVHbWJqxTsxUQPtlNo5dGPJB8BPqGmED11vPxnd1/pjQHpiNwEKD1f0E9HBkagGo= X-Microsoft-Antispam-PRVS: X-Exchange-Antispam-Report-Test: UriScan:; X-Exchange-Antispam-Report-CFA-Test: BCL:0;PCL:0;RULEID:(601004)(2401047)(520078)(5005006)(8121501046)(3002001);SRVR:BN3PR03MB1479;BCL:0;PCL:0;RULEID:;SRVR:BN3PR03MB1479; X-Forefront-PRVS: 0707248B64 X-Forefront-Antispam-Report: SFV:NSPM;SFS:(10019020)(6009001)(377424004)(189002)(24454002)(199003)(5007970100001)(77156002)(68736005)(46102003)(36756003)(93886004)(50226001)(2950100001)(76176999)(50986999)(42186005)(87976001)(62966003)(77096005)(103116003)(33646002)(23676002)(19580405001)(40100003)(122386002)(189998001)(86362001)(5004730100002)(101416001)(47776003)(5820100001)(97736004)(92566002)(105586002)(81156007)(4001540100001)(50466002)(64706001)(106356001)(110136002)(5001860100001)(5001830100001)(5001960100002)(99106002)(3826002)(5001840100002);DIR:OUT;SFP:1102;SCL:1;SRVR:BN3PR03MB1479;H:[IPv6:2601:448:8100:f9f:d59:7025:7bba:8c10];FPR:;SPF:None;PTR:InfoNoRecords;MX:1;A:1;LANG:en; X-Microsoft-Exchange-Diagnostics: =?utf-8?B?MTtCTjNQUjAzTUIxNDc5OzIzOjFBcnVQV014d3NLeXY0NHdEQTdnQmJ2b2cr?= =?utf-8?B?SW9MQTRtUDdKTzhaQUdNSWhaSkxUbnNzbms3U0VXQXFwTDJ1cWN3R2Z0NU5B?= =?utf-8?B?ZWFGYW9FVjI3cUJoU1psdVB4ZTl4Uk5QZW4xNlQwa0Vqbi9JbFFidjc2N1BY?= =?utf-8?B?R211TFZYb095OXdGSWdoa3U3REZQWXRYWGF0UmgzU2RaSEJGK0M0N2NBSDBI?= =?utf-8?B?ODVmQnpkNjhjczZDUTV0ZldJSGtQSTBGbWRNeVlGYm5aT0xxMnJzSXMycXRa?= =?utf-8?B?TzYrdklnbkZQSzQ3QmlCUW1WT2pKTGZIV2lqVHlUUEljenRnQTNYSExjeUI3?= =?utf-8?B?dG9hbGN4NE9vOVFrV3hZNHFKZncyS0hHcUZuRnRuZjBUdjcvT3ExZ0t0eWV5?= =?utf-8?B?Z3hTWHlycmxNUzlBeUhNNnRFZEZCbDNxRVFVSGhsYjAvRE8rR2RTNlF3N25Q?= =?utf-8?B?T0RtNEoya0MxWk9lNUljYzViNVZkOWJNYmw3UnF5SldOQklITVdwcWJXYzl1?= =?utf-8?B?SnhreEtzdStpVk1POG5pN1ZpWURlWExWQUtORFZ5STdBYlFpOGV4cU5ab0pp?= =?utf-8?B?RnhRTGRrN0ZHUmxibDdoRm5vTjY4NEdLRTUwZVpqUUJFWDZkdG1BeVNnRkQ1?= =?utf-8?B?Ukk0UHNBQ1c5aVYvaytvaTBzRTJUdk9WK1ZFbDM4VXpFdktrUnNuZ1NZR0Fy?= =?utf-8?B?Tlp1akZaTFlWM3huZFc3Vkh1bjN6NDgyRTc0aFFtbVhSTzhOVWtmc3ZrUEhm?= =?utf-8?B?VHhIejI1UVNEVGlOekMwSXYrK1R1UFdLck9FQnpRdjJ5ZnQ0TFNPUFFNUVVP?= =?utf-8?B?QWVOVENhOUxKb1k0ZTl0bFdzaTMxSXdxSzIvODVNYVd4ZU9uNi9nY1F3eW5t?= =?utf-8?B?bkFqbFF2M0lvb0k1UVlURnV0bW9LeENXamlIcTNWbExOMm54N0Y1NGl5RzBx?= =?utf-8?B?czZPd3RkM3YrVkV0bzlHN3pWS2NhcVlJc21lM1pWeldreVcyaVVwcjJNdTVn?= =?utf-8?B?dGhWVXlKdk1TMDBtcndSaDB0MWtLQkpHTXI5OU5LV1ZVYUh3emNEZC90TnFR?= =?utf-8?B?UWc4cWtQb3d5UkxxaXJoOU5sRUpjZEladnFnZWNmR3hMZVZsUWJvSnRjSGxY?= =?utf-8?B?T0FkVW8wa01mMUZURWRhdFBhMmhQQm9PY0NDU2dsems2SGl3VlRPcXZacVRm?= =?utf-8?B?UUFkc3NObFI1OW1ydU5tbHdEU1VHQ3k2Ly9Sd0d0bjRCM0pheVhnZlBZUTh0?= =?utf-8?B?K1hyVzdZTHk5bGRCQ1ZjMEJEMDhtR0NKQ0ZjbE9rTjR5WEcwM0JWNWUyeXd5?= =?utf-8?B?ZEZ5QWc3M0hreXk0dWFNS3hKOVZnWVROQmE2aW1xbUxHTXBWUVA1K0w3UlVG?= =?utf-8?B?aFFKcENUV1ZnMFlKekdNREwxWHFQRThjQjE4WWNWUG9oaG1wQzlPekVFWWdh?= =?utf-8?B?TFNROHFuUjVocGZ6QXZvdFZFaXdwZW8zeGNOMXVFVjJrRWk4dlZ6bEFrSHRE?= =?utf-8?B?S2laRm1yQ24rN0luV0FHRTR4dEQySFUvVS8vWm96MGM2eGdCRzI1b2ZwRWRG?= =?utf-8?B?VzNlZUMra2tGd1NFZ3Bydm5DSWx0TnhnZ1p4TGtIQTlCM3BNNlUwbFZURkVq?= =?utf-8?B?RThXOCtJTzQ1RlVnWHhscWc1MlVDQm5BeDBvNkdYS0xPK2dBd3JoSjU2dFJM?= =?utf-8?Q?y5Zp80EgCqd/LW7KMY=3D?= X-Microsoft-Exchange-Diagnostics: 1;BN3PR03MB1479;5:Ypafm55F93mh3Mt1Cyl3aCbzMlRcIYVxabUl1uyvZrJ+AXBXSR8XHxXLf8SqZbBj3jc91XDQZLPb/evAuGe6vhUgCNrPUuNx/L56Fyp2tNU6apzMi9SI0IRW9WvKzstuqkFSd7I6WxLrxoCPM40iBw==;24:w2olcwWOJswI44BiliMasNDd165gA6oiKwuPmFMdLR6Im+KnKd7iLqiVlZord81ZpbhbdCKd8Uxvuq3OnD7mVNbGzt4caLPO4sjpNmLKM3o=;20:Z5JUa4jBhirvXN2yDEXfxkRYoxjKxxVzFFZBXbX5sSTNRPVymkvcbMMaDnK41JDCGecY5LuJr4o6YvTuqXg77A== SpamDiagnosticOutput: 1:23 SpamDiagnosticMetadata: NSPM X-OriginatorOrg: freescale.com X-MS-Exchange-CrossTenant-OriginalArrivalTime: 22 Sep 2015 19:42:11.5128 (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 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? > > > > > > and use do { .. } while(--i); instead. > > > > GCC knows how to optimize loops. Please don't make them less readable. > > Been a while since I checked but it used to be bad att transforming post > inc to pre inc/dec > I remain unconvinced until I have seen it. I would expect it to use bdnz for this loop, as the loop variable isn't referenced in the loop body. And generally the one proposing uglification-for-optimization should provide the evidence. :-) -Scott