All of lore.kernel.org
 help / color / mirror / Atom feed
From: Alex Elder <aelder@sgi.com>
To: Christoph Hellwig <hch@infradead.org>
Cc: xfs@oss.sgi.com
Subject: Re: [PATCH 23/28] xfs: byteswap constants instead of variables
Date: Thu, 7 Jul 2011 11:24:20 -0500	[thread overview]
Message-ID: <1310055860.1980.23.camel@doink> (raw)
In-Reply-To: <20110707110645.046387162@bombadil.infradead.org>

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@lst.de>

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@sgi.com>



_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

  reply	other threads:[~2011-07-07 16:24 UTC|newest]

Thread overview: 38+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-07-07 11:05 [PATCH 00/28] patch queue for Linux 3.1, V3 Christoph Hellwig
2011-07-07 11:05 ` [PATCH 01/28] xfs: PF_FSTRANS should never be set in ->writepage Christoph Hellwig
2011-07-07 11:05 ` [PATCH 02/28] xfs: re-enable non-blocking behaviour in xfs_map_blocks Christoph Hellwig
2011-07-08 10:19   ` Dave Chinner
2011-07-07 11:05 ` [PATCH 03/28] xfs: work around bogus gcc warning in xfs_allocbt_init_cursor Christoph Hellwig
2011-07-07 11:05 ` [PATCH 04/28] xfs: split xfs_setattr Christoph Hellwig
2011-07-07 11:05 ` [PATCH 06/28] xfs: kill xfs_itruncate_start Christoph Hellwig
2011-07-07 11:05 ` [PATCH 07/28] xfs: split xfs_itruncate_finish Christoph Hellwig
2011-07-07 11:05 ` [PATCH 08/28] xfs: improve sync behaviour in the face of aggressive dirtying Christoph Hellwig
2011-07-07 11:05 ` [PATCH 09/28] xfs: fix filesystsem freeze race in xfs_trans_alloc Christoph Hellwig
2011-07-07 11:05 ` [PATCH 10/28] xfs: remove i_transp Christoph Hellwig
2011-07-07 11:05 ` [PATCH 11/28] xfs: kill the unused struct xfs_sync_work Christoph Hellwig
2011-07-08 10:20   ` Dave Chinner
2011-07-07 11:05 ` [PATCH 12/28] xfs: factor out xfs_dir2_leaf_find_entry Christoph Hellwig
2011-07-07 11:05 ` [PATCH 13/28] xfs: cleanup shortform directory inode number handling Christoph Hellwig
2011-07-07 11:05 ` [PATCH 14/28] xfs: kill struct xfs_dir2_sf Christoph Hellwig
2011-07-07 11:05 ` [PATCH 15/28] xfs: cleanup the definition of struct xfs_dir2_sf_entry Christoph Hellwig
2011-07-07 11:05 ` [PATCH 16/28] xfs: avoid usage of struct xfs_dir2_block Christoph Hellwig
2011-07-07 11:05 ` [PATCH 17/28] xfs: kill " Christoph Hellwig
2011-07-08 10:20   ` Dave Chinner
2011-07-07 11:05 ` [PATCH 18/28] xfs: avoid usage of struct xfs_dir2_data Christoph Hellwig
2011-07-07 11:05 ` [PATCH 19/28] xfs: kill " Christoph Hellwig
2011-07-07 11:05 ` [PATCH 20/28] xfs: cleanup the definition of struct xfs_dir2_data_entry Christoph Hellwig
2011-07-07 11:05 ` [PATCH 21/28] xfs: cleanup struct xfs_dir2_leaf Christoph Hellwig
2011-07-08 10:21   ` Dave Chinner
2011-07-07 11:05 ` [PATCH 22/28] xfs: use generic get_unaligned_beXX helpers Christoph Hellwig
2011-07-08 10:22   ` Dave Chinner
2011-07-07 11:05 ` [PATCH 23/28] xfs: byteswap constants instead of variables Christoph Hellwig
2011-07-07 16:24   ` Alex Elder [this message]
2011-07-07 16:28     ` Christoph Hellwig
2011-07-08 10:26   ` Dave Chinner
2011-07-07 11:05 ` [PATCH 24/28] xfs: remove the unused xfs_bufhash structure Christoph Hellwig
2011-07-07 11:06 ` [PATCH 25/28] xfs: clean up buffer locking helpers Christoph Hellwig
2011-07-07 11:06 ` [PATCH 26/28] xfs: return the buffer locked from xfs_buf_get_uncached Christoph Hellwig
2011-07-07 11:06 ` [PATCH 27/28] xfs: cleanup I/O-related buffer flags Christoph Hellwig
2011-07-07 11:06 ` [PATCH 28/28] xfs: avoid a few disk cache flushes Christoph Hellwig
2011-07-08 10:18 ` [PATCH 00/28] patch queue for Linux 3.1, V3 Dave Chinner
2011-07-08 12:28   ` Christoph Hellwig

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1310055860.1980.23.camel@doink \
    --to=aelder@sgi.com \
    --cc=hch@infradead.org \
    --cc=xfs@oss.sgi.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.