On Sep 10, 2020, at 5:55 AM, changfengnan <changfengnan@xxxxxx> wrote:
>
> From: Fengnan Chang <changfengnan@xxxxxxxxxxxxx>
>
> When format ext4 with lazy_journal_init=1, the previous transaction is
> still on disk, it is possible that the previous transaction will be
> used again during jbd2 recovery.Because the seed is changed, the CRC
> check will fail.
>
> Signed-off-by: Fengnan Chang <changfengnan@xxxxxxxxxxxxx>
This version of the patch looks OK, with one trivial indentation problem
after the "jbd_debug(1, "JBD2: Invalid checksum" line that could be fixed.
Reviewed-by: Andreas Dilger <adilger@xxxxxxxxx>
> ---
> fs/jbd2/recovery.c | 60 +++++++++++++++++++++++++++++++++++++++++-----
> 1 file changed, 54 insertions(+), 6 deletions(-)
>
> diff --git a/fs/jbd2/recovery.c b/fs/jbd2/recovery.c
> index a4967b27ffb6..8a6bc322a06d 100644
> --- a/fs/jbd2/recovery.c
> +++ b/fs/jbd2/recovery.c
> @@ -33,6 +33,8 @@ struct recovery_info
> int nr_replays;
> int nr_revokes;
> int nr_revoke_hits;
> + unsigned long ri_commit_block;
> + __be64 last_trans_commit_time;
> };
>
> enum passtype {PASS_SCAN, PASS_REVOKE, PASS_REPLAY};
> @@ -412,7 +414,27 @@ static int jbd2_block_tag_csum_verify(journal_t *j, journal_block_tag_t *tag,
> else
> return tag->t_checksum == cpu_to_be16(csum32);
> }
> +/*
> + * We check the commit time and compare it with the commit time of
> + * the previous transaction, if it's smaller than previous,
> + * We think it's not belong to same journal.
> + */
> +static bool is_same_journal(journal_t *journal, struct buffer_head *bh, unsigned long blocknr, __u64 last_commit_sec)
> +{
> + unsigned long commit_block = blocknr + count_tags(journal, bh) + 1;
> + struct buffer_head *nbh;
> + struct commit_header *cbh;
> + __u64 commit_sec;
> + int err = jread(&nbh, journal, commit_block);
>
> + if (err)
> + return true;
> +
> + cbh = (struct commit_header *)nbh->b_data;
> + commit_sec = be64_to_cpu(cbh->h_commit_sec);
> +
> + return commit_sec >= last_commit_sec;
> +}
> static int do_one_pass(journal_t *journal,
> struct recovery_info *info, enum passtype pass)
> {
> @@ -514,18 +536,29 @@ static int do_one_pass(journal_t *journal,
> switch(blocktype) {
> case JBD2_DESCRIPTOR_BLOCK:
> /* Verify checksum first */
> + if (pass == PASS_SCAN)
> + info->ri_commit_block = 0;
> +
> if (jbd2_journal_has_csum_v2or3(journal))
> descr_csum_size =
> sizeof(struct jbd2_journal_block_tail);
> if (descr_csum_size > 0 &&
> !jbd2_descriptor_block_csum_verify(journal,
> bh->b_data)) {
> - printk(KERN_ERR "JBD2: Invalid checksum "
> - "recovering block %lu in log\n",
> + if (is_same_journal(journal, bh, next_log_block-1, info->last_trans_commit_time)) {
> + printk(KERN_ERR "JBD2: Invalid checksum recovering block %lu in log\n",
> next_log_block);
> - err = -EFSBADCRC;
> - brelse(bh);
> - goto failed;
> + err = -EFSBADCRC;
> + brelse(bh);
> + goto failed;
> + } else {
> + /*it's not belong to same journal, just end this recovery with success*/
> + jbd_debug(1, "JBD2: Invalid checksum found in block %lu in log, but not same journal %d\n",
> + next_log_block, next_commit_ID);
The continued line should be indented one more tab.
> + err = 0;
> + brelse(bh);
> + goto done;
> + }
> }
>
> /* If it is a valid descriptor block, replay it
> @@ -688,6 +721,17 @@ static int do_one_pass(journal_t *journal,
> * are present verify them in PASS_SCAN; else not
> * much to do other than move on to the next sequence
> * number. */
> + if (pass == PASS_SCAN) {
> + struct commit_header *cbh =
> + (struct commit_header *)bh->b_data;
> + if (info->ri_commit_block) {
> + jbd_debug(1, "invalid commit block found in %lu, stop here.\n", next_log_block);
> + brelse(bh);
> + goto done;
> + }
> + info->ri_commit_block = next_log_block;
> + info->last_trans_commit_time = be64_to_cpu(cbh->h_commit_sec);
> + }
> if (pass == PASS_SCAN &&
> jbd2_has_feature_checksum(journal)) {
> int chksum_err, chksum_seen;
> @@ -761,7 +805,11 @@ static int do_one_pass(journal_t *journal,
> brelse(bh);
> continue;
> }
> -
> + if (pass != PASS_SCAN && info->ri_commit_block) {
> + jbd_debug(1, "invalid revoke block found in %lu, stop here.\n", next_log_block);
> + brelse(bh);
> + goto done;
> + }
> err = scan_revoke_records(journal, bh,
> next_commit_ID, info);
> brelse(bh);
> --
> 2.27.0.windows.1
>
>
Cheers, Andreas
Attachment:
signature.asc
Description: Message signed with OpenPGP
