[Top] [All Lists]

Re: [PATCH 23/28] xfs: byteswap constants instead of variables

To: Christoph Hellwig <hch@xxxxxxxxxxxxx>
Subject: Re: [PATCH 23/28] xfs: byteswap constants instead of variables
From: Alex Elder <aelder@xxxxxxx>
Date: Thu, 7 Jul 2011 11:24:20 -0500
Cc: <xfs@xxxxxxxxxxx>
In-reply-to: <20110707110645.046387162@xxxxxxxxxxxxxxxxxxxxxx>
References: <20110707110535.205001532@xxxxxxxxxxxxxxxxxxxxxx> <20110707110645.046387162@xxxxxxxxxxxxxxxxxxxxxx>
Reply-to: <aelder@xxxxxxx>
On Thu, 2011-07-07 at 07:05 -0400, Christoph Hellwig wrote:
> Micro-optimize various comparisms by always byteswapping the constant
> instead of the variable, which allows to do the swap at compile instead
> of runtime.
> Signed-off-by: Christoph Hellwig <hch@xxxxxx>

Ha!!!  You missed one!  File "fs/xfs/xfs_dir2_node.c"
lines 412-413 (after applying this patch):

  ASSERT(be32_to_cpu(free->hdr.magic) == XFS_DIR2_FREE_MAGIC);

Also, xfs_magics[] in "fs/xfs/xfs_btree.c" could be
initialized with the pre-byte-swapped versions of
the constants (and the array should be given static
scope as well).

There are a few other things remaining which are
compared against constant values and they could
get the same treatment at some point. For example,
"fs/xfs/xfs_dir2_block.c" line 142:
  if (be16_to_cpu(enddup->freetag) != XFS_DIR2_DATA_FREE_TAG)

But I've glanced all of them below and they seem
to have been done correctly.  I scanned for matching
bit counts and the appropriate change from using
be.._to_cpu() on the left to cpu_to_be..() on
the right.  I notice that you dropped the byte
conversion in a comparison with 0 in one case
but not in all of them.

To be clear, I do *not* expect you to re-post
this one (and if you do, I'm not going to review
it again)...

Reviewed-by: Alex Elder <aelder@xxxxxxx>

<Prev in Thread] Current Thread [Next in Thread>