From: Ben Myers <bpm@sgi.com>
To: Dave Chinner <david@fromorbit.com>
Cc: xfs@oss.sgi.com
Subject: Re: [PATCH 13/21] xfs: add CRC checking to dir2 data blocks
Date: Wed, 3 Apr 2013 17:13:45 -0500 [thread overview]
Message-ID: <20130403221345.GB22182@sgi.com> (raw)
In-Reply-To: <1363091454-8852-14-git-send-email-david@fromorbit.com>
Dave,
On Tue, Mar 12, 2013 at 11:30:46PM +1100, Dave Chinner wrote:
> From: Dave Chinner <dchinner@redhat.com>
>
> This addition follows the same pattern as the dir2 block CRCs.
>
> Signed-off-by: Dave Chinner <dchinner@redhat.com>
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@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
next prev parent reply other threads:[~2013-04-03 22:13 UTC|newest]
Thread overview: 51+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-03-12 12:30 [PATCH 00/21] xfs: metadata CRCs, third version Dave Chinner
2013-03-12 12:30 ` [PATCH 01/21] xfs: ensure we capture IO errors correctly Dave Chinner
2013-03-12 12:30 ` [PATCH 02/21] xfs: increase hexdump output in xfs_corruption_error Dave Chinner
2013-03-14 21:18 ` Ben Myers
2013-03-15 1:13 ` Dave Chinner
2013-03-12 12:30 ` [PATCH 03/21] xfs: take inode version into account in XFS_LITINO Dave Chinner
2013-03-12 12:30 ` [PATCH 04/21] xfs: add support for large btree blocks Dave Chinner
2013-03-12 12:30 ` [PATCH 05/21] xfs: add CRC checks to the AGF Dave Chinner
2013-03-12 12:30 ` [PATCH 06/21] xfs: add CRC checks to the AGFL Dave Chinner
2013-03-12 12:30 ` [PATCH 07/21] xfs: add CRC checks to the AGI Dave Chinner
2013-03-12 12:30 ` [PATCH 08/21] xfs: add CRC checks for quota blocks Dave Chinner
2013-03-12 12:30 ` [PATCH 09/21] xfs: add version 3 inode format with CRCs Dave Chinner
2013-03-14 16:03 ` Ben Myers
2013-03-14 19:01 ` Ben Myers
2013-03-15 1:11 ` Dave Chinner
2013-03-26 22:56 ` Dave Chinner
2013-03-27 0:53 ` Ben Myers
2013-03-27 1:48 ` Dave Chinner
2013-04-02 22:44 ` Ben Myers
2013-04-03 4:08 ` Dave Chinner
2013-04-02 22:49 ` Ben Myers
2013-03-12 12:30 ` [PATCH 10/21] xfs: add CRC checks to remote symlinks Dave Chinner
2013-03-20 21:14 ` Ben Myers
2013-03-21 1:22 ` Dave Chinner
2013-03-21 14:59 ` Ben Myers
2013-03-20 22:03 ` Ben Myers
2013-03-21 1:32 ` Dave Chinner
2013-03-12 12:30 ` [PATCH 11/21] xfs: add CRC checks to block format directory blocks Dave Chinner
2013-03-26 18:39 ` Ben Myers
2013-03-26 21:40 ` Dave Chinner
2013-03-12 12:30 ` [PATCH 12/21] xfs: add CRC checking to dir2 free blocks Dave Chinner
2013-03-28 23:40 ` Ben Myers
2013-03-29 3:13 ` Dave Chinner
2013-03-12 12:30 ` [PATCH 13/21] xfs: add CRC checking to dir2 data blocks Dave Chinner
2013-04-03 22:13 ` Ben Myers [this message]
2013-03-12 12:30 ` [PATCH 14/21] xfs: add CRC checking to dir2 leaf blocks Dave Chinner
2013-03-12 12:30 ` [PATCH 15/21] xfs: shortform directory offsets change for dir3 format Dave Chinner
2013-03-12 12:30 ` [PATCH 16/21] xfs: add CRCs to dir2/da node blocks Dave Chinner
2013-03-12 12:30 ` [PATCH 17/21] xfs: add CRCs to attr leaf blocks Dave Chinner
2013-03-12 12:30 ` [PATCH 18/21] xfs: split remote attribute code out Dave Chinner
2013-03-12 12:30 ` [PATCH 19/21] xfs: add CRC protection to remote attributes Dave Chinner
2013-03-12 12:30 ` [PATCH 20/21] xfs: add buffer types to directory and attribute buffers Dave Chinner
2013-03-12 12:30 ` [PATCH 21/21] xfs: add CRC checks to the superblock Dave Chinner
2013-03-26 20:58 ` Chandra Seetharaman
2013-03-27 1:06 ` Dave Chinner
2013-03-27 23:07 ` Chandra Seetharaman
2013-03-28 1:36 ` Dave Chinner
2013-03-12 12:43 ` [PATCH 22/21] xfs: Fix magic number assert in xfs_dir3_leaf_log_bests Dave Chinner
2013-03-13 0:29 ` [PATCH 23/21] xfs: fix endian issues reported by sparse Dave Chinner
2013-03-13 1:34 ` [PATCH 24/21] xfs: buffer type overruns blf_flags field Dave Chinner
2013-03-14 21:41 ` [PATCH 00/21] xfs: metadata CRCs, third version Ben Myers
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=20130403221345.GB22182@sgi.com \
--to=bpm@sgi.com \
--cc=david@fromorbit.com \
--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