public inbox for linux-xfs@vger.kernel.org
 help / color / mirror / Atom feed
From: Dave Chinner <david@fromorbit.com>
To: Christoph Hellwig <hch@infradead.org>
Cc: xfs@oss.sgi.com
Subject: Re: [PATCH 18/27] xfs: avoid usage of struct xfs_dir2_data
Date: Wed, 6 Jul 2011 13:02:28 +1000	[thread overview]
Message-ID: <20110706030228.GM1026@dastard> (raw)
In-Reply-To: <20110701094606.003170984@bombadil.infradead.org>

On Fri, Jul 01, 2011 at 05:43:39AM -0400, Christoph Hellwig wrote:
> In most places we can simply pass around and use the struct xfs_dir2_data_hdr,
> which is the first and most important member of struct xfs_dir2_data instead
> of the full structure.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Only a couple of minor things.

....

> @@ -251,12 +258,13 @@ xfs_dir2_data_freeinsert(
>  	xfs_dir2_data_free_t	new;		/* new bestfree entry */
>  
>  #ifdef __KERNEL__
> -	ASSERT(be32_to_cpu(d->hdr.magic) == XFS_DIR2_DATA_MAGIC ||
> -	       be32_to_cpu(d->hdr.magic) == XFS_DIR2_BLOCK_MAGIC);
> +	ASSERT(be32_to_cpu(hdr->magic) == XFS_DIR2_DATA_MAGIC ||
> +	       be32_to_cpu(hdr->magic) == XFS_DIR2_BLOCK_MAGIC);
>  #endif

You kill the ifdef __KERNEL__ there.

> @@ -286,36 +294,36 @@ xfs_dir2_data_freeinsert(
>   */
>  STATIC void
>  xfs_dir2_data_freeremove(
> -	xfs_dir2_data_t		*d,		/* data block pointer */
> +	xfs_dir2_data_hdr_t	*hdr,		/* data block header */
>  	xfs_dir2_data_free_t	*dfp,		/* bestfree entry pointer */
>  	int			*loghead)	/* out: log data header */
>  {
>  #ifdef __KERNEL__
> -	ASSERT(be32_to_cpu(d->hdr.magic) == XFS_DIR2_DATA_MAGIC ||
> -	       be32_to_cpu(d->hdr.magic) == XFS_DIR2_BLOCK_MAGIC);
> +	ASSERT(be32_to_cpu(hdr->magic) == XFS_DIR2_DATA_MAGIC ||
> +	       be32_to_cpu(hdr->magic) == XFS_DIR2_BLOCK_MAGIC);
>  #endif

And there.

> @@ -335,23 +344,23 @@ xfs_dir2_data_freescan(
>  	char			*p;		/* current entry pointer */
>  
>  #ifdef __KERNEL__
> -	ASSERT(be32_to_cpu(d->hdr.magic) == XFS_DIR2_DATA_MAGIC ||
> -	       be32_to_cpu(d->hdr.magic) == XFS_DIR2_BLOCK_MAGIC);
> +	ASSERT(hdr->magic == cpu_to_be32(XFS_DIR2_DATA_MAGIC) ||
> +	       hdr->magic == cpu_to_be32(XFS_DIR2_BLOCK_MAGIC));
>  #endif

I'll stop commenting on this now ;)

However, I have noticed that you've converted some of the magic
number compares to cpu_to_be32(XFS_DIR2_DATA_MAGIC) form and not
others. I'm not so concerned about the ASSERT()s, but some of the
real runtime checks are touched but not then not changed around.
Anyway, this can probably be done later as a separate cleanup.

>  			if (!needscan) {
> -				xfs_dir2_data_freeremove(d, dfp, needlogp);
> -				(void)xfs_dir2_data_freeinsert(d, newdup,
> +				xfs_dir2_data_freeremove(hdr, dfp, needlogp);
> +				(void)xfs_dir2_data_freeinsert(hdr, newdup,
>  					needlogp);
> -				(void)xfs_dir2_data_freeinsert(d, newdup2,
> +				(void)xfs_dir2_data_freeinsert(hdr, newdup2,
>  					needlogp);
>  			}
>  		}

Kill the (void) casts?

Otherwise looks good.

Reviewed-by: Dave Chinner <dchinner@redhat.com>
-- 
Dave Chinner
david@fromorbit.com

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

  reply	other threads:[~2011-07-06  3:02 UTC|newest]

Thread overview: 87+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-07-01  9:43 [PATCH 00/27] patch queue for Linux 3.1, V2 Christoph Hellwig
2011-07-01  9:43 ` [PATCH 01/27] xfs: PF_FSTRANS should never be set in ->writepage Christoph Hellwig
2011-07-01  9:43 ` [PATCH 02/27] xfs: re-enable non-blocking behaviour in xfs_map_blocks Christoph Hellwig
2011-07-05 22:35   ` Alex Elder
2011-07-06  6:37     ` Christoph Hellwig
2011-07-06 13:36       ` Alex Elder
2011-07-01  9:43 ` [PATCH 03/27] xfs: work around bogus gcc warning in xfs_allocbt_init_cursor Christoph Hellwig
2011-07-01  9:43 ` [PATCH 04/27] xfs: split xfs_setattr Christoph Hellwig
2011-07-01  9:43 ` [PATCH 06/27] xfs: kill xfs_itruncate_start Christoph Hellwig
2011-07-01  9:43 ` [PATCH 07/27] xfs: split xfs_itruncate_finish Christoph Hellwig
2011-07-06  4:35   ` Alex Elder
2011-07-06  8:11     ` Christoph Hellwig
2011-07-06 14:05       ` Alex Elder
2011-07-01  9:43 ` [PATCH 08/27] xfs: improve sync behaviour in the fact of aggressive dirtying Christoph Hellwig
2011-07-05 22:36   ` Alex Elder
2011-07-06  8:15     ` Christoph Hellwig
2011-07-06 14:59       ` Alex Elder
2011-07-01  9:43 ` [PATCH 09/27] xfs: fix filesystsem freeze race in xfs_trans_alloc Christoph Hellwig
2011-07-05 22:36   ` Alex Elder
2011-07-01  9:43 ` [PATCH 10/27] xfs: remove i_transp Christoph Hellwig
2011-07-05 22:36   ` Alex Elder
2011-07-01  9:43 ` [PATCH 11/27] xfs: kill the unused struct xfs_sync_work Christoph Hellwig
2011-07-05 22:36   ` Alex Elder
2011-07-01  9:43 ` [PATCH 12/27] xfs: factor out xfs_dir2_leaf_find_entry Christoph Hellwig
2011-07-05 22:36   ` Alex Elder
2011-07-01  9:43 ` [PATCH 13/27] xfs: cleanup shortform directory inode number handling Christoph Hellwig
2011-07-05 22:36   ` Alex Elder
2011-07-01  9:43 ` [PATCH 14/27] xfs: kill struct xfs_dir2_sf Christoph Hellwig
2011-07-06  1:57   ` Dave Chinner
2011-07-06  8:28     ` Christoph Hellwig
2011-07-06  3:24   ` Alex Elder
2011-07-06  8:33     ` Christoph Hellwig
2011-07-06 15:05       ` Alex Elder
2011-07-01  9:43 ` [PATCH 15/27] xfs: cleanup the defintion of struct xfs_dir2_sf_entry Christoph Hellwig
2011-07-06  2:00   ` Dave Chinner
2011-07-06  3:33   ` Alex Elder
2011-07-06  8:34     ` Christoph Hellwig
2011-07-01  9:43 ` [PATCH 16/27] xfs: avoid usage of struct xfs_dir2_block Christoph Hellwig
2011-07-06  2:19   ` Dave Chinner
2011-07-06  8:35     ` Christoph Hellwig
2011-07-06  3:36   ` Alex Elder
2011-07-01  9:43 ` [PATCH 17/27] xfs: kill " Christoph Hellwig
2011-07-06  2:31   ` Dave Chinner
2011-07-06  8:37     ` Christoph Hellwig
2011-07-06 15:11       ` Alex Elder
2011-07-06  3:36   ` Alex Elder
2011-07-01  9:43 ` [PATCH 18/27] xfs: avoid usage of struct xfs_dir2_data Christoph Hellwig
2011-07-06  3:02   ` Dave Chinner [this message]
2011-07-06  8:43     ` Christoph Hellwig
2011-07-06  3:38   ` Alex Elder
2011-07-06  8:45     ` Christoph Hellwig
2011-07-01  9:43 ` [PATCH 19/27] xfs: kill " Christoph Hellwig
2011-07-06  3:05   ` Dave Chinner
2011-07-06  3:38   ` Alex Elder
2011-07-01  9:43 ` [PATCH 20/27] xfs: cleanup the defintion of struct xfs_dir2_data_entry Christoph Hellwig
2011-07-06  3:06   ` Dave Chinner
2011-07-06  3:44   ` Alex Elder
2011-07-06  8:48     ` Christoph Hellwig
2011-07-01  9:43 ` [PATCH 21/27] xfs: cleanup struct xfs_dir2_leaf Christoph Hellwig
2011-07-06  3:13   ` Dave Chinner
2011-07-06  3:44   ` Alex Elder
2011-07-01  9:43 ` [PATCH 22/27] xfs: use generic get_unaligned_beXX helpers Christoph Hellwig
2011-07-06  3:44   ` Dave Chinner
2011-07-06  9:07     ` Christoph Hellwig
2011-07-07  8:00       ` Christoph Hellwig
2011-07-06  3:47   ` Alex Elder
2011-07-01  9:43 ` [PATCH 23/27] xfs: remove the unused xfs_bufhash structure Christoph Hellwig
2011-07-06  3:44   ` Dave Chinner
2011-07-06  3:49   ` Alex Elder
2011-07-01  9:43 ` [PATCH 24/27] xfs: clean up buffer locking helpers Christoph Hellwig
2011-07-06  3:47   ` Dave Chinner
2011-07-06  3:55   ` Alex Elder
2011-07-01  9:43 ` [PATCH 25/27] xfs: return the buffer locked from xfs_buf_get_uncached Christoph Hellwig
2011-07-06  3:48   ` Dave Chinner
2011-07-06  3:57   ` Alex Elder
2011-07-01  9:43 ` [PATCH 26/27] xfs: cleanup I/O-related buffer flags Christoph Hellwig
2011-07-06  3:54   ` Dave Chinner
2011-07-06  9:11     ` Christoph Hellwig
2011-07-06  4:09   ` Alex Elder
2011-07-06  9:11     ` Christoph Hellwig
2011-07-01  9:43 ` [PATCH 27/27] xfs: avoid a few disk cache flushes Christoph Hellwig
2011-07-06  3:55   ` Dave Chinner
2011-07-06  4:11   ` Alex Elder
2011-07-06  4:40 ` [PATCH 00/27] patch queue for Linux 3.1, V2 Alex Elder
2011-07-06  6:42   ` Christoph Hellwig
2011-07-06 13:32     ` Alex Elder
2011-07-06 13:43       ` 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=20110706030228.GM1026@dastard \
    --to=david@fromorbit.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox