Re: [PATCH 13/21] xfs: add CRC checking to dir2 data blocks

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Dave,

On Tue, Mar 12, 2013 at 11:30:46PM +1100, Dave Chinner wrote:
> From: Dave Chinner <dchinner@xxxxxxxxxx>
> 
> This addition follows the same pattern as the dir2 block CRCs.
> 
> Signed-off-by: Dave Chinner <dchinner@xxxxxxxxxx>

Just a few nits.

> @@ -908,7 +908,7 @@ xfs_dir2_block_removename(
>  		xfs_dir2_data_freescan(mp, hdr, &needlog);
>  	if (needlog)
>  		xfs_dir2_data_log_header(tp, bp);
> -	xfs_dir2_data_check(dp, bp);
> +	xfs_dir3_data_check(dp, bp);

In this debug code, maybe it is better check the blocks after making the
changes to them but before logging them?  Not sure.

> @@ -212,7 +221,7 @@ xfs_dir2_data_verify(
>   * format buffer or a data format buffer on readahead.
>   */
>  static void
> -xfs_dir2_data_reada_verify(
> +xfs_dir3_data_reada_verify(
>  	struct xfs_buf		*bp)
>  {
>  	struct xfs_mount	*mp = bp->b_target->bt_mount;
> @@ -225,7 +234,8 @@ xfs_dir2_data_reada_verify(
>  		bp->b_ops->verify_read(bp);
>  		return;
>  	case cpu_to_be32(XFS_DIR2_DATA_MAGIC):
> -		xfs_dir2_data_verify(bp);
> +	case cpu_to_be32(XFS_DIR3_DATA_MAGIC):
> +		xfs_dir3_data_verify(bp);

I think it might be a good idea to add a print in the situation where we have
an error in readahead.  It needn't be a forced shutdown.

> diff --git a/fs/xfs/xfs_dir2_format.h b/fs/xfs/xfs_dir2_format.h
> index bec058f..dfc8ccf 100644
> --- a/fs/xfs/xfs_dir2_format.h
> +++ b/fs/xfs/xfs_dir2_format.h
> @@ -283,7 +283,8 @@ struct xfs_dir3_data_hdr {
>  	static inline struct xfs_dir2_data_free *
>  xfs_dir3_data_bestfree_p(struct xfs_dir2_data_hdr *hdr)

Get rid of the tab.

> @@ -204,8 +204,12 @@ xfs_dir2_block_to_leaf(
>  	/*
>  	 * Fix up the block header, make it a data block.
>  	 */
> -	dbp->b_ops = &xfs_dir2_data_buf_ops;
> -	hdr->magic = cpu_to_be32(XFS_DIR2_DATA_MAGIC);
> +	dbp->b_ops = &xfs_dir3_data_buf_ops;
> +	if (hdr->magic == cpu_to_be32(XFS_DIR2_BLOCK_MAGIC))
> +		hdr->magic = cpu_to_be32(XFS_DIR2_DATA_MAGIC);
> +	else
> +		hdr->magic = cpu_to_be32(XFS_DIR3_DATA_MAGIC);
> +

	else {
		ASSERT(hdr->magic == XFS_DIR3_BLOCK_MAGIC));
		hdr->magic = cpu_to_be32(XFS_DIR3_DATA_MAGIC);
	}

Might be nice...

Else, looks good.  I'll switch over to reviewing your new rev.

Regards,
	Ben

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




[Index of Archives]     [Linux XFS Devel]     [Linux Filesystem Development]     [Filesystem Testing]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux