diff mbox series

[3/3] add support to check a journal checksum v1 while journal dump.

Message ID 20181119091650.81803-4-alexey.lyashkov@gmail.com
State New, archived
Headers show
Series debugfs fixes | expand

Commit Message

Alexey Lyahkov Nov. 19, 2018, 9:16 a.m. UTC 
journal checksum v1 cover an all blocks in journal descriptor.
This checksum stored into commit block and check with commit
block reading.

Signed-off-by: Alexey Lyashkov <alexey.lyashkov@gmail.com>
---
 debugfs/logdump.c | 69 ++++++++++++++++++++++++++++++++++++++---------
 e2fsck/jfs_user.h |  5 ++++
 2 files changed, 62 insertions(+), 12 deletions(-)

Comments

Theodore Ts'o Nov. 22, 2018, 12:07 a.m. UTC | #1 
On Mon, Nov 19, 2018 at 12:16:50PM +0300, alexey.lyashkov@gmail.com wrote:
> 
> journal checksum v1 cover an all blocks in journal descriptor.
> This checksum stored into commit block and check with commit
> block reading.

I'm really curious --- why do you care about journal_checksum v1 at
all?  It rarely used, and the journal_checksum v3 is much superior.

      	 	       	     	     		      - Ted
Alexey Lyahkov Nov. 22, 2018, 5:08 a.m. UTC | #2 
it’s typical option for Sonexion storages.

journal checksum is enough to cover a corruptions for external journal, 
but provide a less overhead than full metadata checksums.
In our’s workload - any checksums is source of huge latency, so minimal option had enabled.
from other sides it option can enabled without disk format change.

I was have plan to add v2/v3 checksums to patch series but images need to prepared for it.

In this particular usage it was used to find a bug in jbd2 code, with back porting commit
de92c8caf16ca84926fa31b7a5590c0fb9c0d5ca
    jbd2: speedup jbd2_journal_get_[write|undo]_access()

to rhel7 code.

that commits introduce a race window while frozen buffer had modified in parallel thread.
this race likely to be fixed by
ee57aba159a5c329dc78c181a3ae0549e59f0925
    jbd2: simplify code flow in do_get_write_access()

which described as cleanup.


> 22 нояб. 2018 г., в 3:07, Theodore Y. Ts'o <tytso@mit.edu> написал(а):
> 
> On Mon, Nov 19, 2018 at 12:16:50PM +0300, alexey.lyashkov@gmail.com wrote:
>> 
>> journal checksum v1 cover an all blocks in journal descriptor.
>> This checksum stored into commit block and check with commit
>> block reading.
> 
> I'm really curious --- why do you care about journal_checksum v1 at
> all?  It rarely used, and the journal_checksum v3 is much superior.
> 
>      	 	       	     	     		      - Ted
>
diff mbox series

Patch

diff --git a/debugfs/logdump.c b/debugfs/logdump.c
index c88f6f9c..3dd04789 100644
--- a/debugfs/logdump.c
+++ b/debugfs/logdump.c
@@ -55,7 +55,7 @@  static void dump_journal(char *, FILE *, struct journal_source *);
 
 static void dump_descriptor_block(FILE *, struct journal_source *,
 				  char *, journal_superblock_t *,
-				  unsigned int *, int, tid_t);
+				  unsigned int *, int, tid_t, __u32 *);
 
 static void dump_revoke_block(FILE *, char *, journal_superblock_t *,
 				  blk64_t, int, tid_t);
@@ -63,10 +63,14 @@  static void dump_revoke_block(FILE *, char *, journal_superblock_t *,
 static void dump_metadata_block(FILE *, struct journal_source *,
 				journal_superblock_t*,
 				unsigned int, blk64_t, unsigned int,
-				int, tid_t);
+				int, tid_t, __u32 *);
 
 static void do_hexdump (FILE *, char *, int);
 
+static void jbd2_check_commit_cksum(FILE *, struct journal_source *,
+				  journal_superblock_t *,
+				  unsigned int , int , __u32);
+
 #define WRAP(jsb, blocknr)					\
 	if (blocknr >= be32_to_cpu((jsb)->s_maxlen))		\
 		blocknr -= (be32_to_cpu((jsb)->s_maxlen) -	\
@@ -353,6 +357,7 @@  static void dump_journal(char *cmdname, FILE *out_file,
 	journal_header_t	*header;
 	tid_t			transaction;
 	unsigned int		blocknr = 0;
+	__u32			crc32_sum = ~0;
 
 	/* First, check to see if there's an ext2 superblock header */
 	retval = read_journal_block(cmdname, source, 0, buf, 2048);
@@ -453,12 +458,16 @@  static void dump_journal(char *cmdname, FILE *out_file,
 		case JFS_DESCRIPTOR_BLOCK:
 			dump_descriptor_block(out_file, source, buf, jsb,
 					      &blocknr, blocksize,
-					      transaction);
+					      transaction, &crc32_sum);
 			continue;
 
 		case JFS_COMMIT_BLOCK:
+			jbd2_check_commit_cksum(out_file, source,
+						jsb, blocknr, blocksize,
+						crc32_sum);
 			transaction++;
 			blocknr++;
+			crc32_sum = ~0;
 			WRAP(jsb, blocknr);
 			continue;
 
@@ -511,7 +520,7 @@  static void dump_descriptor_block(FILE *out_file,
 				  char *buf,
 				  journal_superblock_t *jsb,
 				  unsigned int *blockp, int blocksize,
-				  tid_t transaction)
+				  tid_t transaction, __u32 *crc32_sum)
 {
 	int			offset, tag_size, csum_size = 0;
 	char			*tagp;
@@ -535,6 +544,9 @@  static void dump_descriptor_block(FILE *out_file,
 	++blocknr;
 	WRAP(jsb, blocknr);
 
+	if (JSB_HAS_COMPAT_FEATURE(jsb, JFS_FEATURE_COMPAT_CHECKSUM))
+		*crc32_sum = ext2fs_crc32_be(*crc32_sum, buf, blocksize);
+
 	do {
 		/* Work out the location of the current tag, and skip to
 		 * the next one... */
@@ -555,7 +567,7 @@  static void dump_descriptor_block(FILE *out_file,
 
 		dump_metadata_block(out_file, source, jsb,
 				    blocknr, tag_block, tag_flags, blocksize,
-				    transaction);
+				    transaction, crc32_sum);
 
 		++blocknr;
 		WRAP(jsb, blocknr);
@@ -630,7 +642,7 @@  static void dump_metadata_block(FILE *out_file, struct journal_source *source,
 				blk64_t fs_blocknr,
 				unsigned int log_tag_flags,
 				int blocksize,
-				tid_t transaction)
+				tid_t transaction, __u32 *crc32_sum)
 {
 	int		retval;
 	char 		buf[8192];
@@ -659,18 +671,21 @@  static void dump_metadata_block(FILE *out_file, struct journal_source *source,
 	 * structure symbolically.
 	 */
 
-	if (!(dump_contents && dump_all)
-	    && fs_blocknr != block_to_dump
-	    && fs_blocknr != bitmap_to_dump
-	    && fs_blocknr != inode_block_to_dump)
-		return;
-
 	retval = read_journal_block("logdump", source,
 				    ((ext2_loff_t) log_blocknr) * blocksize,
 				    buf, blocksize);
 	if (retval)
 		return;
 
+	if (JSB_HAS_COMPAT_FEATURE(jsb, JFS_FEATURE_COMPAT_CHECKSUM))
+		*crc32_sum = ext2fs_crc32_be(*crc32_sum, buf, blocksize);
+
+	if (!(dump_contents && dump_all)
+	    && fs_blocknr != block_to_dump
+	    && fs_blocknr != bitmap_to_dump
+	    && fs_blocknr != inode_block_to_dump)
+		return;
+
 	if (fs_blocknr == bitmap_to_dump) {
 		struct ext2_super_block *super;
 		int offset;
@@ -751,3 +766,33 @@  static void do_hexdump (FILE *out_file, char *buf, int blocksize)
 	}
 }
 
+static void jbd2_check_commit_cksum(FILE *out_file, struct journal_source *source,
+				  journal_superblock_t *jsb,
+				  unsigned int blocknr, int blocksize,
+				  __u32 crc32_sum)
+{
+	char 		buf[8192];
+	struct commit_header *h = (void *)&buf[0];
+	int retval;
+
+	if (!dump_all)
+		return;
+
+	retval = read_journal_block("logdump", source,
+				    ((ext2_loff_t) blocknr) * blocksize,
+				    buf, blocksize);
+	if (retval)
+		return;
+
+	if (JSB_HAS_COMPAT_FEATURE(jsb, JFS_FEATURE_COMPAT_CHECKSUM)) {
+		if (h->h_chksum_type != JBD2_CRC32_CHKSUM)
+			fprintf(out_file, "Unknow commit checksum %u\n",
+				h->h_chksum);
+		if (h->h_chksum_size != JBD2_CRC32_CHKSUM_SIZE)
+			fprintf(out_file, "Unknown checksum size %u\n",
+				h->h_chksum_size);
+		if (h->h_chksum[0] != cpu_to_be32(crc32_sum))
+			fprintf(out_file, "Wrong checksum %u <> %u\n",
+				h->h_chksum[0], cpu_to_be32(crc32_sum));
+	}
+}
diff --git a/e2fsck/jfs_user.h b/e2fsck/jfs_user.h
index a1c6951c..6e42a2a6 100644
--- a/e2fsck/jfs_user.h
+++ b/e2fsck/jfs_user.h
@@ -204,6 +204,11 @@  void wait_on_buffer(struct buffer_head *bh);
 #define JSB_HAS_INCOMPAT_FEATURE(jsb, mask)				\
 	((jsb)->s_header.h_blocktype == ext2fs_cpu_to_be32(JFS_SUPERBLOCK_V2) &&	\
 	 ((jsb)->s_feature_incompat & ext2fs_cpu_to_be32((mask))))
+
+#define JSB_HAS_COMPAT_FEATURE(jsb, mask)				\
+	((jsb)->s_header.h_blocktype == ext2fs_cpu_to_be32(JFS_SUPERBLOCK_V2) &&	\
+	 ((jsb)->s_feature_compat & ext2fs_cpu_to_be32((mask))))
+
 #else  /* !DEBUGFS */
 
 extern e2fsck_t e2fsck_global_ctx;  /* Try your very best not to use this! */