2022-08-04 09:57:28

by Alexey Lyahkov

[permalink] [raw]
Subject: [PATCH 2/5] move a journal checksum code into common place

e2fsck and debugfs have own copy a journal checksum functions,
kill duplicates and move into support library.
---
debugfs/journal.c | 50 ---------------------------------
e2fsck/journal.c | 60 ++++-----------------------------------
lib/support/Android.bp | 1 +
lib/support/Makefile.in | 7 +++--
lib/support/jfs_user.c | 62 +++++++++++++++++++++++++++++++++++++++++
lib/support/jfs_user.h | 6 ++++
6 files changed, 79 insertions(+), 107 deletions(-)
create mode 100644 lib/support/jfs_user.c

diff --git a/debugfs/journal.c b/debugfs/journal.c
index 095fff00..dac17800 100644
--- a/debugfs/journal.c
+++ b/debugfs/journal.c
@@ -43,56 +43,6 @@ static int bh_count = 0;
*/
#undef USE_INODE_IO

-/* Checksumming functions */
-static int ext2fs_journal_verify_csum_type(journal_t *j,
- journal_superblock_t *jsb)
-{
- if (!jbd2_journal_has_csum_v2or3(j))
- return 1;
-
- return jsb->s_checksum_type == JBD2_CRC32C_CHKSUM;
-}
-
-static __u32 ext2fs_journal_sb_csum(journal_superblock_t *jsb)
-{
- __u32 crc, old_crc;
-
- old_crc = jsb->s_checksum;
- jsb->s_checksum = 0;
- crc = ext2fs_crc32c_le(~0, (unsigned char *)jsb,
- sizeof(journal_superblock_t));
- jsb->s_checksum = old_crc;
-
- return crc;
-}
-
-static int ext2fs_journal_sb_csum_verify(journal_t *j,
- journal_superblock_t *jsb)
-{
- __u32 provided, calculated;
-
- if (!jbd2_journal_has_csum_v2or3(j))
- return 1;
-
- provided = ext2fs_be32_to_cpu(jsb->s_checksum);
- calculated = ext2fs_journal_sb_csum(jsb);
-
- return provided == calculated;
-}
-
-static errcode_t ext2fs_journal_sb_csum_set(journal_t *j,
- journal_superblock_t *jsb)
-{
- __u32 crc;
-
- if (!jbd2_journal_has_csum_v2or3(j))
- return 0;
-
- crc = ext2fs_journal_sb_csum(jsb);
- jsb->s_checksum = ext2fs_cpu_to_be32(crc);
- return 0;
-}
-
/* Kernel compatibility functions for handling the journal. These allow us
* to use the recovery.c file virtually unchanged from the kernel, so we
* don't have to do much to keep kernel and user recovery in sync.
diff --git a/e2fsck/journal.c b/e2fsck/journal.c
index d3002a62..46a9bcb7 100644
--- a/e2fsck/journal.c
+++ b/e2fsck/journal.c
@@ -38,56 +38,6 @@ static int bh_count = 0;
*/
#undef USE_INODE_IO

-/* Checksumming functions */
-static int e2fsck_journal_verify_csum_type(journal_t *j,
- journal_superblock_t *jsb)
-{
- if (!jbd2_journal_has_csum_v2or3(j))
- return 1;
-
- return jsb->s_checksum_type == JBD2_CRC32C_CHKSUM;
-}
-
-static __u32 e2fsck_journal_sb_csum(journal_superblock_t *jsb)
-{
- __u32 crc, old_crc;
-
- old_crc = jsb->s_checksum;
- jsb->s_checksum = 0;
- crc = ext2fs_crc32c_le(~0, (unsigned char *)jsb,
- sizeof(journal_superblock_t));
- jsb->s_checksum = old_crc;
-
- return crc;
-}
-
-static int e2fsck_journal_sb_csum_verify(journal_t *j,
- journal_superblock_t *jsb)
-{
- __u32 provided, calculated;
-
- if (!jbd2_journal_has_csum_v2or3(j))
- return 1;
-
- provided = ext2fs_be32_to_cpu(jsb->s_checksum);
- calculated = e2fsck_journal_sb_csum(jsb);
-
- return provided == calculated;
-}
-
-static errcode_t e2fsck_journal_sb_csum_set(journal_t *j,
- journal_superblock_t *jsb)
-{
- __u32 crc;
-
- if (!jbd2_journal_has_csum_v2or3(j))
- return 0;
-
- crc = e2fsck_journal_sb_csum(jsb);
- jsb->s_checksum = ext2fs_cpu_to_be32(crc);
- return 0;
-}
-
/* Kernel compatibility functions for handling the journal. These allow us
* to use the recovery.c file virtually unchanged from the kernel, so we
* don't have to do much to keep kernel and user recovery in sync.
@@ -1330,8 +1280,8 @@ static errcode_t e2fsck_journal_load(journal_t *journal)
jbd2_has_feature_checksum(journal))
return EXT2_ET_CORRUPT_JOURNAL_SB;

- if (!e2fsck_journal_verify_csum_type(journal, jsb) ||
- !e2fsck_journal_sb_csum_verify(journal, jsb))
+ if (!ext2fs_journal_verify_csum_type(journal, jsb) ||
+ !ext2fs_journal_sb_csum_verify(journal, jsb))
return EXT2_ET_CORRUPT_JOURNAL_SB;

if (jbd2_journal_has_csum_v2or3(journal))
@@ -1419,7 +1369,7 @@ static void e2fsck_journal_reset_super(e2fsck_t ctx, journal_superblock_t *jsb,
for (i = 0; i < 4; i ++)
new_seq ^= u.val[i];
jsb->s_sequence = htonl(new_seq);
- e2fsck_journal_sb_csum_set(journal, jsb);
+ ext2fs_journal_sb_csum_set(journal, jsb);

mark_buffer_dirty(journal->j_sb_buffer);
ll_rw_block(REQ_OP_WRITE, 0, 1, &journal->j_sb_buffer);
@@ -1459,7 +1409,7 @@ static void e2fsck_journal_release(e2fsck_t ctx, journal_t *journal,
jsb->s_sequence = htonl(journal->j_tail_sequence);
if (reset)
jsb->s_start = 0; /* this marks the journal as empty */
- e2fsck_journal_sb_csum_set(journal, jsb);
+ ext2fs_journal_sb_csum_set(journal, jsb);
mark_buffer_dirty(journal->j_sb_buffer);
}
brelse(journal->j_sb_buffer);
@@ -1602,7 +1552,7 @@ no_has_journal:
ctx->fs->super->s_state |= EXT2_ERROR_FS;
ext2fs_mark_super_dirty(ctx->fs);
journal->j_superblock->s_errno = 0;
- e2fsck_journal_sb_csum_set(journal, journal->j_superblock);
+ ext2fs_journal_sb_csum_set(journal, journal->j_superblock);
mark_buffer_dirty(journal->j_sb_buffer);
}

diff --git a/lib/support/Android.bp b/lib/support/Android.bp
index a0b064dd..efa0f955 100644
--- a/lib/support/Android.bp
+++ b/lib/support/Android.bp
@@ -29,6 +29,7 @@ cc_library {
"quotaio.c",
"quotaio_tree.c",
"quotaio_v2.c",
+ "jfs_user.c"
],
shared_libs: [
"libext2fs",
diff --git a/lib/support/Makefile.in b/lib/support/Makefile.in
index f3c7981e..04fbcf31 100644
--- a/lib/support/Makefile.in
+++ b/lib/support/Makefile.in
@@ -23,7 +23,8 @@ OBJS= cstring.o \
quotaio.o \
quotaio_v2.o \
quotaio_tree.o \
- dict.o
+ dict.o \
+ jfs_user.o

SRCS= $(srcdir)/argv_parse.c \
$(srcdir)/cstring.c \
@@ -36,7 +37,9 @@ SRCS= $(srcdir)/argv_parse.c \
$(srcdir)/quotaio.c \
$(srcdir)/quotaio_tree.c \
$(srcdir)/quotaio_v2.c \
- $(srcdir)/dict.c
+ $(srcdir)/dict.c \
+ $(srcdir)/jfs_user.c
+

LIBRARY= libsupport
LIBDIR= support
diff --git a/lib/support/jfs_user.c b/lib/support/jfs_user.c
new file mode 100644
index 00000000..4ff1b5c1
--- /dev/null
+++ b/lib/support/jfs_user.c
@@ -0,0 +1,62 @@
+#define DEBUGFS
+#include "jfs_user.h"
+
+/*
+ * Define USE_INODE_IO to use the inode_io.c / fileio.c codepaths.
+ * This creates a larger static binary, and a smaller binary using
+ * shared libraries. It's also probably slightly less CPU-efficient,
+ * which is why it's not on by default. But, it's a good way of
+ * testing the functions in inode_io.c and fileio.c.
+ */
+#undef USE_INODE_IO
+
+/* Checksumming functions */
+int ext2fs_journal_verify_csum_type(journal_t *j,
+ journal_superblock_t *jsb)
+{
+ if (!jbd2_journal_has_csum_v2or3(j))
+ return 1;
+
+ return jsb->s_checksum_type == JBD2_CRC32C_CHKSUM;
+}
+
+__u32 ext2fs_journal_sb_csum(journal_superblock_t *jsb)
+{
+ __u32 crc, old_crc;
+
+ old_crc = jsb->s_checksum;
+ jsb->s_checksum = 0;
+ crc = ext2fs_crc32c_le(~0, (unsigned char *)jsb,
+ sizeof(journal_superblock_t));
+ jsb->s_checksum = old_crc;
+
+ return crc;
+}
+
+int ext2fs_journal_sb_csum_verify(journal_t *j,
+ journal_superblock_t *jsb)
+{
+ __u32 provided, calculated;
+
+ if (!jbd2_journal_has_csum_v2or3(j))
+ return 1;
+
+ provided = ext2fs_be32_to_cpu(jsb->s_checksum);
+ calculated = ext2fs_journal_sb_csum(jsb);
+
+ return provided == calculated;
+}
+
+errcode_t ext2fs_journal_sb_csum_set(journal_t *j,
+ journal_superblock_t *jsb)
+{
+ __u32 crc;
+
+ if (!jbd2_journal_has_csum_v2or3(j))
+ return 0;
+
+ crc = ext2fs_journal_sb_csum(jsb);
+ jsb->s_checksum = ext2fs_cpu_to_be32(crc);
+ return 0;
+}
+
diff --git a/lib/support/jfs_user.h b/lib/support/jfs_user.h
index 4ad2005a..8bdbf85b 100644
--- a/lib/support/jfs_user.h
+++ b/lib/support/jfs_user.h
@@ -212,6 +212,12 @@ _INLINE_ void jbd2_descriptor_block_csum_set(journal_t *j,
#undef _INLINE_
#endif

+/* Checksumming functions */
+int ext2fs_journal_verify_csum_type(journal_t *j, journal_superblock_t *jsb);
+__u32 ext2fs_journal_sb_csum(journal_superblock_t *jsb);
+int ext2fs_journal_sb_csum_verify(journal_t *j, journal_superblock_t *jsb);
+errcode_t ext2fs_journal_sb_csum_set(journal_t *j, journal_superblock_t *jsb);
+
/*
* Kernel compatibility functions are defined in journal.c
*/
--
2.31.1



2023-09-18 23:21:17

by Andreas Dilger

[permalink] [raw]
Subject: Re: [PATCH 2/5] move a journal checksum code into common place

On Aug 4, 2022, at 3:56 AM, Alexey Lyashkov <[email protected]> wrote:
>
> e2fsck and debugfs have own copy a journal checksum functions,
> kill duplicates and move into support library.

Missing a Signed-off-by: line.

It would be better to name the new file "jbd2_user.c" if patch is refreshed,
or Ted can rename and amend the commit locally.

Reviewed-by: Andreas Dilger <[email protected]>

> ---
> debugfs/journal.c | 50 ---------------------------------
> e2fsck/journal.c | 60 ++++-----------------------------------
> lib/support/Android.bp | 1 +
> lib/support/Makefile.in | 7 +++--
> lib/support/jfs_user.c | 62 +++++++++++++++++++++++++++++++++++++++++
> lib/support/jfs_user.h | 6 ++++
> 6 files changed, 79 insertions(+), 107 deletions(-)
> create mode 100644 lib/support/jfs_user.c
>
> diff --git a/debugfs/journal.c b/debugfs/journal.c
> index 095fff00..dac17800 100644
> --- a/debugfs/journal.c
> +++ b/debugfs/journal.c
> @@ -43,56 +43,6 @@ static int bh_count = 0;
> */
> #undef USE_INODE_IO
>
> -/* Checksumming functions */
> -static int ext2fs_journal_verify_csum_type(journal_t *j,
> - journal_superblock_t *jsb)
> -{
> - if (!jbd2_journal_has_csum_v2or3(j))
> - return 1;
> -
> - return jsb->s_checksum_type == JBD2_CRC32C_CHKSUM;
> -}
> -
> -static __u32 ext2fs_journal_sb_csum(journal_superblock_t *jsb)
> -{
> - __u32 crc, old_crc;
> -
> - old_crc = jsb->s_checksum;
> - jsb->s_checksum = 0;
> - crc = ext2fs_crc32c_le(~0, (unsigned char *)jsb,
> - sizeof(journal_superblock_t));
> - jsb->s_checksum = old_crc;
> -
> - return crc;
> -}
> -
> -static int ext2fs_journal_sb_csum_verify(journal_t *j,
> - journal_superblock_t *jsb)
> -{
> - __u32 provided, calculated;
> -
> - if (!jbd2_journal_has_csum_v2or3(j))
> - return 1;
> -
> - provided = ext2fs_be32_to_cpu(jsb->s_checksum);
> - calculated = ext2fs_journal_sb_csum(jsb);
> -
> - return provided == calculated;
> -}
> -
> -static errcode_t ext2fs_journal_sb_csum_set(journal_t *j,
> - journal_superblock_t *jsb)
> -{
> - __u32 crc;
> -
> - if (!jbd2_journal_has_csum_v2or3(j))
> - return 0;
> -
> - crc = ext2fs_journal_sb_csum(jsb);
> - jsb->s_checksum = ext2fs_cpu_to_be32(crc);
> - return 0;
> -}
> -
> /* Kernel compatibility functions for handling the journal. These allow us
> * to use the recovery.c file virtually unchanged from the kernel, so we
> * don't have to do much to keep kernel and user recovery in sync.
> diff --git a/e2fsck/journal.c b/e2fsck/journal.c
> index d3002a62..46a9bcb7 100644
> --- a/e2fsck/journal.c
> +++ b/e2fsck/journal.c
> @@ -38,56 +38,6 @@ static int bh_count = 0;
> */
> #undef USE_INODE_IO
>
> -/* Checksumming functions */
> -static int e2fsck_journal_verify_csum_type(journal_t *j,
> - journal_superblock_t *jsb)
> -{
> - if (!jbd2_journal_has_csum_v2or3(j))
> - return 1;
> -
> - return jsb->s_checksum_type == JBD2_CRC32C_CHKSUM;
> -}
> -
> -static __u32 e2fsck_journal_sb_csum(journal_superblock_t *jsb)
> -{
> - __u32 crc, old_crc;
> -
> - old_crc = jsb->s_checksum;
> - jsb->s_checksum = 0;
> - crc = ext2fs_crc32c_le(~0, (unsigned char *)jsb,
> - sizeof(journal_superblock_t));
> - jsb->s_checksum = old_crc;
> -
> - return crc;
> -}
> -
> -static int e2fsck_journal_sb_csum_verify(journal_t *j,
> - journal_superblock_t *jsb)
> -{
> - __u32 provided, calculated;
> -
> - if (!jbd2_journal_has_csum_v2or3(j))
> - return 1;
> -
> - provided = ext2fs_be32_to_cpu(jsb->s_checksum);
> - calculated = e2fsck_journal_sb_csum(jsb);
> -
> - return provided == calculated;
> -}
> -
> -static errcode_t e2fsck_journal_sb_csum_set(journal_t *j,
> - journal_superblock_t *jsb)
> -{
> - __u32 crc;
> -
> - if (!jbd2_journal_has_csum_v2or3(j))
> - return 0;
> -
> - crc = e2fsck_journal_sb_csum(jsb);
> - jsb->s_checksum = ext2fs_cpu_to_be32(crc);
> - return 0;
> -}
> -
> /* Kernel compatibility functions for handling the journal. These allow us
> * to use the recovery.c file virtually unchanged from the kernel, so we
> * don't have to do much to keep kernel and user recovery in sync.
> @@ -1330,8 +1280,8 @@ static errcode_t e2fsck_journal_load(journal_t *journal)
> jbd2_has_feature_checksum(journal))
> return EXT2_ET_CORRUPT_JOURNAL_SB;
>
> - if (!e2fsck_journal_verify_csum_type(journal, jsb) ||
> - !e2fsck_journal_sb_csum_verify(journal, jsb))
> + if (!ext2fs_journal_verify_csum_type(journal, jsb) ||
> + !ext2fs_journal_sb_csum_verify(journal, jsb))
> return EXT2_ET_CORRUPT_JOURNAL_SB;
>
> if (jbd2_journal_has_csum_v2or3(journal))
> @@ -1419,7 +1369,7 @@ static void e2fsck_journal_reset_super(e2fsck_t ctx, journal_superblock_t *jsb,
> for (i = 0; i < 4; i ++)
> new_seq ^= u.val[i];
> jsb->s_sequence = htonl(new_seq);
> - e2fsck_journal_sb_csum_set(journal, jsb);
> + ext2fs_journal_sb_csum_set(journal, jsb);
>
> mark_buffer_dirty(journal->j_sb_buffer);
> ll_rw_block(REQ_OP_WRITE, 0, 1, &journal->j_sb_buffer);
> @@ -1459,7 +1409,7 @@ static void e2fsck_journal_release(e2fsck_t ctx, journal_t *journal,
> jsb->s_sequence = htonl(journal->j_tail_sequence);
> if (reset)
> jsb->s_start = 0; /* this marks the journal as empty */
> - e2fsck_journal_sb_csum_set(journal, jsb);
> + ext2fs_journal_sb_csum_set(journal, jsb);
> mark_buffer_dirty(journal->j_sb_buffer);
> }
> brelse(journal->j_sb_buffer);
> @@ -1602,7 +1552,7 @@ no_has_journal:
> ctx->fs->super->s_state |= EXT2_ERROR_FS;
> ext2fs_mark_super_dirty(ctx->fs);
> journal->j_superblock->s_errno = 0;
> - e2fsck_journal_sb_csum_set(journal, journal->j_superblock);
> + ext2fs_journal_sb_csum_set(journal, journal->j_superblock);
> mark_buffer_dirty(journal->j_sb_buffer);
> }
>
> diff --git a/lib/support/Android.bp b/lib/support/Android.bp
> index a0b064dd..efa0f955 100644
> --- a/lib/support/Android.bp
> +++ b/lib/support/Android.bp
> @@ -29,6 +29,7 @@ cc_library {
> "quotaio.c",
> "quotaio_tree.c",
> "quotaio_v2.c",
> + "jfs_user.c"
> ],
> shared_libs: [
> "libext2fs",
> diff --git a/lib/support/Makefile.in b/lib/support/Makefile.in
> index f3c7981e..04fbcf31 100644
> --- a/lib/support/Makefile.in
> +++ b/lib/support/Makefile.in
> @@ -23,7 +23,8 @@ OBJS= cstring.o \
> quotaio.o \
> quotaio_v2.o \
> quotaio_tree.o \
> - dict.o
> + dict.o \
> + jfs_user.o
>
> SRCS= $(srcdir)/argv_parse.c \
> $(srcdir)/cstring.c \
> @@ -36,7 +37,9 @@ SRCS= $(srcdir)/argv_parse.c \
> $(srcdir)/quotaio.c \
> $(srcdir)/quotaio_tree.c \
> $(srcdir)/quotaio_v2.c \
> - $(srcdir)/dict.c
> + $(srcdir)/dict.c \
> + $(srcdir)/jfs_user.c
> +
>
> LIBRARY= libsupport
> LIBDIR= support
> diff --git a/lib/support/jfs_user.c b/lib/support/jfs_user.c
> new file mode 100644
> index 00000000..4ff1b5c1
> --- /dev/null
> +++ b/lib/support/jfs_user.c
> @@ -0,0 +1,62 @@
> +#define DEBUGFS
> +#include "jfs_user.h"
> +
> +/*
> + * Define USE_INODE_IO to use the inode_io.c / fileio.c codepaths.
> + * This creates a larger static binary, and a smaller binary using
> + * shared libraries. It's also probably slightly less CPU-efficient,
> + * which is why it's not on by default. But, it's a good way of
> + * testing the functions in inode_io.c and fileio.c.
> + */
> +#undef USE_INODE_IO
> +
> +/* Checksumming functions */
> +int ext2fs_journal_verify_csum_type(journal_t *j,
> + journal_superblock_t *jsb)
> +{
> + if (!jbd2_journal_has_csum_v2or3(j))
> + return 1;
> +
> + return jsb->s_checksum_type == JBD2_CRC32C_CHKSUM;
> +}
> +
> +__u32 ext2fs_journal_sb_csum(journal_superblock_t *jsb)
> +{
> + __u32 crc, old_crc;
> +
> + old_crc = jsb->s_checksum;
> + jsb->s_checksum = 0;
> + crc = ext2fs_crc32c_le(~0, (unsigned char *)jsb,
> + sizeof(journal_superblock_t));
> + jsb->s_checksum = old_crc;
> +
> + return crc;
> +}
> +
> +int ext2fs_journal_sb_csum_verify(journal_t *j,
> + journal_superblock_t *jsb)
> +{
> + __u32 provided, calculated;
> +
> + if (!jbd2_journal_has_csum_v2or3(j))
> + return 1;
> +
> + provided = ext2fs_be32_to_cpu(jsb->s_checksum);
> + calculated = ext2fs_journal_sb_csum(jsb);
> +
> + return provided == calculated;
> +}
> +
> +errcode_t ext2fs_journal_sb_csum_set(journal_t *j,
> + journal_superblock_t *jsb)
> +{
> + __u32 crc;
> +
> + if (!jbd2_journal_has_csum_v2or3(j))
> + return 0;
> +
> + crc = ext2fs_journal_sb_csum(jsb);
> + jsb->s_checksum = ext2fs_cpu_to_be32(crc);
> + return 0;
> +}
> +
> diff --git a/lib/support/jfs_user.h b/lib/support/jfs_user.h
> index 4ad2005a..8bdbf85b 100644
> --- a/lib/support/jfs_user.h
> +++ b/lib/support/jfs_user.h
> @@ -212,6 +212,12 @@ _INLINE_ void jbd2_descriptor_block_csum_set(journal_t *j,
> #undef _INLINE_
> #endif
>
> +/* Checksumming functions */
> +int ext2fs_journal_verify_csum_type(journal_t *j, journal_superblock_t *jsb);
> +__u32 ext2fs_journal_sb_csum(journal_superblock_t *jsb);
> +int ext2fs_journal_sb_csum_verify(journal_t *j, journal_superblock_t *jsb);
> +errcode_t ext2fs_journal_sb_csum_set(journal_t *j, journal_superblock_t *jsb);
> +
> /*
> * Kernel compatibility functions are defined in journal.c
> */
> --
> 2.31.1
>


Cheers, Andreas






Attachments:
signature.asc (890.00 B)
Message signed with OpenPGP