lkml.org 
[lkml]   [2011]   [Sep]   [2]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH 16/16] jbd2: Support CRC32C transaction checksums
    On Thu, Sep 01, 2011 at 02:11:26AM -0600, Andreas Dilger wrote:
    > On 2011-08-31, at 6:32 PM, Darrick J. Wong wrote:
    > > The CRC32c polynomial provides better error detection and can be hardware
    > > accelerated on certain machines. To that end, add support for it to jbd2.
    > >
    > > Signed-off-by: Darrick J. Wong <djwong@us.ibm.com>
    > > ---
    > > fs/jbd2/Kconfig | 1 +
    > > fs/jbd2/commit.c | 6 +++---
    > > fs/jbd2/recovery.c | 20 +++++++++++++++++---
    > > include/linux/jbd2.h | 1 +
    > > 4 files changed, 22 insertions(+), 6 deletions(-)
    > >
    > >
    > > diff --git a/fs/jbd2/Kconfig b/fs/jbd2/Kconfig
    > > index f32f346..40a126b 100644
    > > --- a/fs/jbd2/Kconfig
    > > +++ b/fs/jbd2/Kconfig
    > > @@ -1,6 +1,7 @@
    > > config JBD2
    > > tristate
    > > select CRC32
    > > + select LIBCRC32C
    > > help
    > > This is a generic journaling layer for block devices that support
    > > both 32-bit and 64-bit block numbers. It is currently used by
    > > diff --git a/fs/jbd2/commit.c b/fs/jbd2/commit.c
    > > index eef6979..00387be 100644
    > > --- a/fs/jbd2/commit.c
    > > +++ b/fs/jbd2/commit.c
    > > @@ -21,7 +21,7 @@
    > > #include <linux/mm.h>
    > > #include <linux/pagemap.h>
    > > #include <linux/jiffies.h>
    > > -#include <linux/crc32.h>
    > > +#include <linux/crc32c.h>
    > > #include <linux/writeback.h>
    > > #include <linux/backing-dev.h>
    > > #include <linux/bio.h>
    > > @@ -125,7 +125,7 @@ static int journal_submit_commit_record(journal_t *journal,
    > >
    > > if (JBD2_HAS_COMPAT_FEATURE(journal,
    > > JBD2_FEATURE_COMPAT_CHECKSUM)) {
    > > - tmp->h_chksum_type = JBD2_CRC32_CHKSUM;
    > > + tmp->h_chksum_type = JBD2_CRC32C_CHKSUM;
    > > tmp->h_chksum_size = JBD2_CRC32_CHKSUM_SIZE;
    > > tmp->h_chksum[0] = cpu_to_be32(crc32_sum);
    > > }
    > > @@ -287,7 +287,7 @@ static __u32 jbd2_checksum_data(__u32 crc32_sum, struct buffer_head *bh)
    > > __u32 checksum;
    > >
    > > addr = kmap_atomic(page, KM_USER0);
    > > - checksum = crc32_be(crc32_sum,
    > > + checksum = crc32c_le(crc32_sum,
    > > (void *)(addr + offset_in_page(bh->b_data)), bh->b_size);
    > > kunmap_atomic(addr, KM_USER0);
    > >
    > > diff --git a/fs/jbd2/recovery.c b/fs/jbd2/recovery.c
    > > index 1cad869..4bab4dd 100644
    > > --- a/fs/jbd2/recovery.c
    > > +++ b/fs/jbd2/recovery.c
    > > @@ -21,6 +21,7 @@
    > > #include <linux/jbd2.h>
    > > #include <linux/errno.h>
    > > #include <linux/crc32.h>
    > > +#include <linux/crc32c.h>
    > > #endif
    > >
    > > /*
    > > @@ -323,7 +324,8 @@ static inline unsigned long long read_tag_block(int tag_bytes, journal_block_tag
    > > * descriptor block.
    > > */
    > > static int calc_chksums(journal_t *journal, struct buffer_head *bh,
    > > - unsigned long *next_log_block, __u32 *crc32_sum)
    > > + unsigned long *next_log_block, __u32 *crc32_sum,
    > > + __u32 *crc32c_sum)
    > > {
    > > int i, num_blks, err;
    > > unsigned long io_block;
    > > @@ -332,6 +334,7 @@ static int calc_chksums(journal_t *journal, struct buffer_head *bh,
    > > num_blks = count_tags(journal, bh);
    > > /* Calculate checksum of the descriptor block. */
    > > *crc32_sum = crc32_be(*crc32_sum, (void *)bh->b_data, bh->b_size);
    > > + *crc32c_sum = crc32c_le(*crc32c_sum, (void *)bh->b_data, bh->b_size);
    >
    > We definitely do not want to compute both the crc32 and crc32c for every
    > block written to the journal. That would be needlessly expensive.
    >
    > It looks like the missing factor is not knowing the checksum type before
    > the commit block is accessed. This can be fixed by storing the checksum
    > type in the journal superblock (JBD2_CKSUM_TYPE_CRC32 = 0 for compatibility).
    >
    > During recovery, the journal superblock s_chksum_type should remain as
    > set by the previous kernel (so the existing commit block checksums can
    > be verified) and as soon as the journal recovery has completed and a new
    > block written to the journal the checksum type can be updated to the latest
    > default value.

    Okay, that does sound better than my current weird approach. :) I admit to
    being a bit timid about messing with jbd2 after proposing a lot of changes to
    ext4.

    > > for (i = 0; i < num_blks; i++) {
    > > io_block = (*next_log_block)++;
    > > @@ -344,6 +347,9 @@ static int calc_chksums(journal_t *journal, struct buffer_head *bh,
    > > } else {
    > > *crc32_sum = crc32_be(*crc32_sum, (void *)obh->b_data,
    > > obh->b_size);
    > > + *crc32c_sum = crc32c_le(*crc32c_sum,
    > > + (void *)obh->b_data,
    > > + obh->b_size);
    >
    > (style) the indenting here is funky. Continued lines should follow the
    > '(' on the previous line and not be gratuitously wrapped.

    Yeah, I wondered about that.

    --D

    > > @@ -617,7 +625,12 @@ static int do_one_pass(journal_t *journal,
    > > cbh->h_chksum_type == JBD2_CRC32_CHKSUM &&
    > > cbh->h_chksum_size ==
    > > JBD2_CRC32_CHKSUM_SIZE)
    > > - chksum_seen = 1;
    > > + chksum_seen = 1;
    > > + else if (crc32c_sum == found_chksum &&
    > > + cbh->h_chksum_type == JBD2_CRC32C_CHKSUM &&
    > > + cbh->h_chksum_size ==
    > > + JBD2_CRC32_CHKSUM_SIZE)
    > > + chksum_seen = 1;
    > > else if (!(cbh->h_chksum_type == 0 &&
    > > cbh->h_chksum_size == 0 &&
    > > found_chksum == 0 &&
    > > @@ -646,6 +659,7 @@ static int do_one_pass(journal_t *journal,
    > > }
    > > }
    > > crc32_sum = ~0;
    > > + crc32c_sum = ~0;
    > > }
    > > brelse(bh);
    > > next_commit_ID++;
    > > diff --git a/include/linux/jbd2.h b/include/linux/jbd2.h
    > > index 38f307b..de3ec23 100644
    > > --- a/include/linux/jbd2.h
    > > +++ b/include/linux/jbd2.h
    > > @@ -147,6 +147,7 @@ typedef struct journal_header_s
    > > #define JBD2_CRC32_CHKSUM 1
    > > #define JBD2_MD5_CHKSUM 2
    > > #define JBD2_SHA1_CHKSUM 3
    > > +#define JBD2_CRC32C_CHKSUM 4
    > >
    > > #define JBD2_CRC32_CHKSUM_SIZE 4
    > >
    > >
    >


    \
     
     \ /
      Last update: 2011-09-02 20:33    [W:0.033 / U:0.136 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site