From mboxrd@z Thu Jan 1 00:00:00 1970 From: Harvey Harrison Subject: Re: [PATCH] UBIFS: endian handling fixes and annotations Date: Mon, 24 Nov 2008 08:46:01 -0800 Message-ID: <1227545161.5511.2.camel@brick> References: <1227287970-14684-1-git-send-email-dedekind@infradead.org> <1227287970-14684-3-git-send-email-dedekind@infradead.org> <20081122192747.GB21433@Chamillionaire.breakpoint.cc> <492AB7F3.5020406@nokia.com> Mime-Version: 1.0 Content-Type: text/plain Content-Transfer-Encoding: 7bit Cc: Sebastian Andrzej Siewior , Artem Bityutskiy , linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org To: Adrian Hunter Return-path: Received: from qw-out-2122.google.com ([74.125.92.27]:47708 "EHLO qw-out-2122.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753355AbYKXQqG (ORCPT ); Mon, 24 Nov 2008 11:46:06 -0500 Received: by qw-out-2122.google.com with SMTP id 3so372273qwe.37 for ; Mon, 24 Nov 2008 08:46:04 -0800 (PST) In-Reply-To: <492AB7F3.5020406@nokia.com> Sender: linux-fsdevel-owner@vger.kernel.org List-ID: On Mon, 2008-11-24 at 16:19 +0200, Adrian Hunter wrote: > Sebastian Andrzej Siewior wrote: > > * Artem Bityutskiy | 2008-11-21 19:19:24 [+0200]: > > > >> index 9ee6508..3f1f16b 100644 > >> --- a/fs/ubifs/key.h > >> +++ b/fs/ubifs/key.h > >> @@ -345,7 +345,7 @@ static inline int key_type_flash(const struct ubifs_info *c, const void *k) > >> { > >> const union ubifs_key *key = k; > >> > >> - return le32_to_cpu(key->u32[1]) >> UBIFS_S_KEY_BLOCK_BITS; > >> + return le32_to_cpu(key->j32[1]) >> UBIFS_S_KEY_BLOCK_BITS; > > > > If you would change such references to something like > > |return le32_to_cpup(&key->j32[1]) >> UBIFS_S_KEY_BLOCK_BITS; > > then on powerpc > > > > text data bss dec hex filename > > 155384 1284 24 156692 26414 ubifs-b4.ko > > 155372 1284 24 156680 26408 ubifs-after.ko > > > > because now it is possible to load the value as LE from memory instead > > of loading it BE and swapping it afterwads. > > Wouldn't that be true for every le32_to_cpu of an lvalue? Shame you can't > do: > > is_lvalue(x) ? le32_to_cpup(&(x)) : le32_to_cpu(x) > No, you wouldn't want to do the above if the lvalue was on the stack as most of the time the extra code to setup a pointer to a stack variable ends up being more expensive than just using cpu_to_le32. Cheers, Harvey