[PATCH 0/5] some cleanup and refactor for jbd2 journal recover

[PATCH 0/5] some cleanup and refactor for jbd2 journal recover

[Ye Bin]

From: Ye Bin <yebin10@huawei.com>

Ye Bin (5):
  jbd2: remove redundant judgments for check v1 checksum
  jbd2: unified release of buffer_head in do_one_pass()
  jbd2: refactor JBD2_COMMIT_BLOCK process in do_one_pass()
  jbd2: factor out jbd2_do_replay()
  jbd2: remove useless 'block_error' variable

 fs/jbd2/recovery.c | 310 +++++++++++++++++++++------------------------
 1 file changed, 146 insertions(+), 164 deletions(-)

-- 
2.31.1

[PATCH 1/5] jbd2: remove redundant judgments for check v1 checksum

[Ye Bin]

From: Ye Bin <yebin10@huawei.com>

'need_check_commit_time' is only used by v2/v3 checksum, so there isn't
need to add 'need_check_commit_time' judegement for v1 checksum logic.

Signed-off-by: Ye Bin <yebin10@huawei.com>
---
 fs/jbd2/recovery.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/fs/jbd2/recovery.c b/fs/jbd2/recovery.c
index 667f67342c52..5efbca6a98c4 100644
--- a/fs/jbd2/recovery.c
+++ b/fs/jbd2/recovery.c
@@ -619,7 +619,6 @@ static int do_one_pass(journal_t *journal,
 			if (pass != PASS_REPLAY) {
 				if (pass == PASS_SCAN &&
 				    jbd2_has_feature_checksum(journal) &&
-				    !need_check_commit_time &&
 				    !info->end_transaction) {
 					if (calc_chksums(journal, bh,
 							&next_log_block,
-- 
2.31.1

[PATCH 2/5] jbd2: unified release of buffer_head in do_one_pass()

[Ye Bin]

From: Ye Bin <yebin10@huawei.com>

Now buffer_head free is very fragmented in do_one_pass(), unified release
of buffer_head in do_one_pass()

Signed-off-by: Ye Bin <yebin10@huawei.com>
---
 fs/jbd2/recovery.c | 34 +++++++++-------------------------
 1 file changed, 9 insertions(+), 25 deletions(-)

diff --git a/fs/jbd2/recovery.c b/fs/jbd2/recovery.c
index 5efbca6a98c4..0adf0cb31a03 100644
--- a/fs/jbd2/recovery.c
+++ b/fs/jbd2/recovery.c
@@ -493,7 +493,7 @@ static int do_one_pass(journal_t *journal,
 	int			err, success = 0;
 	journal_superblock_t *	sb;
 	journal_header_t *	tmp;
-	struct buffer_head *	bh;
+	struct buffer_head	*bh = NULL;
 	unsigned int		sequence;
 	int			blocktype;
 	int			tag_bytes = journal_tag_bytes(journal);
@@ -552,6 +552,8 @@ static int do_one_pass(journal_t *journal,
 		 * record. */
 
 		jbd2_debug(3, "JBD2: checking block %ld\n", next_log_block);
+		brelse(bh);
+		bh = NULL;
 		err = jread(&bh, journal, next_log_block);
 		if (err)
 			goto failed;
@@ -567,20 +569,16 @@ static int do_one_pass(journal_t *journal,
 
 		tmp = (journal_header_t *)bh->b_data;
 
-		if (tmp->h_magic != cpu_to_be32(JBD2_MAGIC_NUMBER)) {
-			brelse(bh);
+		if (tmp->h_magic != cpu_to_be32(JBD2_MAGIC_NUMBER))
 			break;
-		}
 
 		blocktype = be32_to_cpu(tmp->h_blocktype);
 		sequence = be32_to_cpu(tmp->h_sequence);
 		jbd2_debug(3, "Found magic %d, sequence %d\n",
 			  blocktype, sequence);
 
-		if (sequence != next_commit_ID) {
-			brelse(bh);
+		if (sequence != next_commit_ID)
 			break;
-		}
 
 		/* OK, we have a valid descriptor block which matches
 		 * all of the sequence number checks.  What are we going
@@ -603,7 +601,6 @@ static int do_one_pass(journal_t *journal,
 					pr_err("JBD2: Invalid checksum recovering block %lu in log\n",
 					       next_log_block);
 					err = -EFSBADCRC;
-					brelse(bh);
 					goto failed;
 				}
 				need_check_commit_time = true;
@@ -622,16 +619,12 @@ static int do_one_pass(journal_t *journal,
 				    !info->end_transaction) {
 					if (calc_chksums(journal, bh,
 							&next_log_block,
-							&crc32_sum)) {
-						put_bh(bh);
+							&crc32_sum))
 						break;
-					}
-					put_bh(bh);
 					continue;
 				}
 				next_log_block += count_tags(journal, bh);
 				wrap(journal, next_log_block);
-				put_bh(bh);
 				continue;
 			}
 
@@ -701,7 +694,6 @@ static int do_one_pass(journal_t *journal,
 						       "JBD2: Out of memory "
 						       "during recovery.\n");
 						err = -ENOMEM;
-						brelse(bh);
 						brelse(obh);
 						goto failed;
 					}
@@ -733,7 +725,6 @@ static int do_one_pass(journal_t *journal,
 					break;
 			}
 
-			brelse(bh);
 			continue;
 
 		case JBD2_COMMIT_BLOCK:
@@ -781,7 +772,6 @@ static int do_one_pass(journal_t *journal,
 					pr_err("JBD2: Invalid checksum found in transaction %u\n",
 					       next_commit_ID);
 					err = -EFSBADCRC;
-					brelse(bh);
 					goto failed;
 				}
 			ignore_crc_mismatch:
@@ -791,7 +781,6 @@ static int do_one_pass(journal_t *journal,
 				 */
 				jbd2_debug(1, "JBD2: Invalid checksum ignored in transaction %u, likely stale data\n",
 					  next_commit_ID);
-				brelse(bh);
 				goto done;
 			}
 
@@ -811,7 +800,6 @@ static int do_one_pass(journal_t *journal,
 				if (info->end_transaction) {
 					journal->j_failed_commit =
 						info->end_transaction;
-					brelse(bh);
 					break;
 				}
 
@@ -847,7 +835,6 @@ static int do_one_pass(journal_t *journal,
 				if (!jbd2_has_feature_async_commit(journal)) {
 					journal->j_failed_commit =
 						next_commit_ID;
-					brelse(bh);
 					break;
 				}
 			}
@@ -856,7 +843,6 @@ static int do_one_pass(journal_t *journal,
 				last_trans_commit_time = commit_time;
 				head_block = next_log_block;
 			}
-			brelse(bh);
 			next_commit_ID++;
 			continue;
 
@@ -875,14 +861,11 @@ static int do_one_pass(journal_t *journal,
 
 			/* If we aren't in the REVOKE pass, then we can
 			 * just skip over this block. */
-			if (pass != PASS_REVOKE) {
-				brelse(bh);
+			if (pass != PASS_REVOKE)
 				continue;
-			}
 
 			err = scan_revoke_records(journal, bh,
 						  next_commit_ID, info);
-			brelse(bh);
 			if (err)
 				goto failed;
 			continue;
@@ -890,12 +873,12 @@ static int do_one_pass(journal_t *journal,
 		default:
 			jbd2_debug(3, "Unrecognised magic %d, end of scan.\n",
 				  blocktype);
-			brelse(bh);
 			goto done;
 		}
 	}
 
  done:
+	brelse(bh);
 	/*
 	 * We broke out of the log scan loop: either we came to the
 	 * known end of the log or we found an unexpected block in the
@@ -931,6 +914,7 @@ static int do_one_pass(journal_t *journal,
 	return success;
 
  failed:
+	brelse(bh);
 	return err;
 }
 
-- 
2.31.1

[PATCH 3/5] jbd2: refactor JBD2_COMMIT_BLOCK process in do_one_pass()

[Ye Bin]

From: Ye Bin <yebin10@huawei.com>

To make JBD2_COMMIT_BLOCK process more clean, no functional change.

Signed-off-by: Ye Bin <yebin10@huawei.com>
---
 fs/jbd2/recovery.c | 55 ++++++++++++++++++++++++----------------------
 1 file changed, 29 insertions(+), 26 deletions(-)

diff --git a/fs/jbd2/recovery.c b/fs/jbd2/recovery.c
index 0adf0cb31a03..0d697979d83e 100644
--- a/fs/jbd2/recovery.c
+++ b/fs/jbd2/recovery.c
@@ -728,6 +728,11 @@ static int do_one_pass(journal_t *journal,
 			continue;
 
 		case JBD2_COMMIT_BLOCK:
+			if (pass != PASS_SCAN) {
+				next_commit_ID++;
+				continue;
+			}
+
 			/*     How to differentiate between interrupted commit
 			 *               and journal corruption ?
 			 *
@@ -790,8 +795,7 @@ static int do_one_pass(journal_t *journal,
 			 * much to do other than move on to the next sequence
 			 * number.
 			 */
-			if (pass == PASS_SCAN &&
-			    jbd2_has_feature_checksum(journal)) {
+			if (jbd2_has_feature_checksum(journal)) {
 				struct commit_header *cbh =
 					(struct commit_header *)bh->b_data;
 				unsigned found_chksum =
@@ -815,34 +819,33 @@ static int do_one_pass(journal_t *journal,
 					goto chksum_error;
 
 				crc32_sum = ~0;
+				goto chksum_ok;
 			}
-			if (pass == PASS_SCAN &&
-			    !jbd2_commit_block_csum_verify(journal,
-							   bh->b_data)) {
-				if (jbd2_commit_block_csum_verify_partial(
-								  journal,
-								  bh->b_data)) {
-					pr_notice("JBD2: Find incomplete commit block in transaction %u block %lu\n",
-						  next_commit_ID, next_log_block);
-					goto chksum_ok;
-				}
-			chksum_error:
-				if (commit_time < last_trans_commit_time)
-					goto ignore_crc_mismatch;
-				info->end_transaction = next_commit_ID;
-				info->head_block = head_block;
 
-				if (!jbd2_has_feature_async_commit(journal)) {
-					journal->j_failed_commit =
-						next_commit_ID;
-					break;
-				}
+			if (jbd2_commit_block_csum_verify(journal, bh->b_data))
+				goto chksum_ok;
+
+			if (jbd2_commit_block_csum_verify_partial(journal,
+								  bh->b_data)) {
+				pr_notice("JBD2: Find incomplete commit block in transaction %u block %lu\n",
+					  next_commit_ID, next_log_block);
+				goto chksum_ok;
 			}
-			if (pass == PASS_SCAN) {
-			chksum_ok:
-				last_trans_commit_time = commit_time;
-				head_block = next_log_block;
+
+chksum_error:
+			if (commit_time < last_trans_commit_time)
+				goto ignore_crc_mismatch;
+			info->end_transaction = next_commit_ID;
+			info->head_block = head_block;
+
+			if (!jbd2_has_feature_async_commit(journal)) {
+				journal->j_failed_commit = next_commit_ID;
+				break;
 			}
+
+chksum_ok:
+			last_trans_commit_time = commit_time;
+			head_block = next_log_block;
 			next_commit_ID++;
 			continue;
 
-- 
2.31.1

[PATCH 4/5] jbd2: factor out jbd2_do_replay()

[Ye Bin]

From: Ye Bin <yebin10@huawei.com>

Factor out jbd2_do_replay() no funtional change.

Signed-off-by: Ye Bin <yebin10@huawei.com>
---
 fs/jbd2/recovery.c | 219 +++++++++++++++++++++++----------------------
 1 file changed, 110 insertions(+), 109 deletions(-)

diff --git a/fs/jbd2/recovery.c b/fs/jbd2/recovery.c
index 0d697979d83e..05ea449b95c4 100644
--- a/fs/jbd2/recovery.c
+++ b/fs/jbd2/recovery.c
@@ -485,6 +485,105 @@ static int jbd2_block_tag_csum_verify(journal_t *j, journal_block_tag_t *tag,
 		return tag->t_checksum == cpu_to_be16(csum32);
 }
 
+static __always_inline int jbd2_do_replay(journal_t *journal,
+					  struct recovery_info *info,
+					  struct buffer_head *bh,
+					  unsigned long *next_log_block,
+					  unsigned int next_commit_ID,
+					  int *success, int *block_error)
+{
+	char *tagp;
+	int flags;
+	int err;
+	int tag_bytes = journal_tag_bytes(journal);
+	int descr_csum_size = 0;
+	unsigned long io_block;
+	journal_block_tag_t tag;
+	struct buffer_head *obh;
+	struct buffer_head *nbh;
+
+	if (jbd2_journal_has_csum_v2or3(journal))
+		descr_csum_size = sizeof(struct jbd2_journal_block_tail);
+
+	tagp = &bh->b_data[sizeof(journal_header_t)];
+	while ((tagp - bh->b_data + tag_bytes) <=
+	       journal->j_blocksize - descr_csum_size) {
+
+		memcpy(&tag, tagp, sizeof(tag));
+		flags = be16_to_cpu(tag.t_flags);
+
+		io_block = (*next_log_block)++;
+		wrap(journal, *next_log_block);
+		err = jread(&obh, journal, io_block);
+		if (err) {
+			/* Recover what we can, but report failure at the end. */
+			*success = err;
+			pr_err("JBD2: IO error %d recovering block %lu in log\n",
+			      err, io_block);
+		} else {
+			unsigned long long blocknr;
+
+			J_ASSERT(obh != NULL);
+			blocknr = read_tag_block(journal, &tag);
+
+			/* If the block has been revoked, then we're all done here. */
+			if (jbd2_journal_test_revoke(journal, blocknr,
+						     next_commit_ID)) {
+				brelse(obh);
+				++info->nr_revoke_hits;
+				goto skip_write;
+			}
+
+			/* Look for block corruption */
+			if (!jbd2_block_tag_csum_verify(journal, &tag,
+				(journal_block_tag3_t *)tagp, obh->b_data,
+				next_commit_ID)) {
+				brelse(obh);
+				*success = -EFSBADCRC;
+				pr_err("JBD2: Invalid checksum recovering data block %llu in journal block %lu\n",
+				      blocknr, io_block);
+				*block_error = 1;
+				goto skip_write;
+			}
+
+			/* Find a buffer for the new data being restored */
+			nbh = __getblk(journal->j_fs_dev, blocknr,
+				       journal->j_blocksize);
+			if (nbh == NULL) {
+				pr_err("JBD2: Out of memory during recovery.\n");
+				brelse(obh);
+				return -ENOMEM;
+			}
+
+			lock_buffer(nbh);
+			memcpy(nbh->b_data, obh->b_data, journal->j_blocksize);
+			if (flags & JBD2_FLAG_ESCAPE) {
+				*((__be32 *)nbh->b_data) =
+				cpu_to_be32(JBD2_MAGIC_NUMBER);
+			}
+
+			BUFFER_TRACE(nbh, "marking dirty");
+			set_buffer_uptodate(nbh);
+			mark_buffer_dirty(nbh);
+			BUFFER_TRACE(nbh, "marking uptodate");
+			++info->nr_replays;
+			unlock_buffer(nbh);
+			brelse(obh);
+			brelse(nbh);
+		}
+
+skip_write:
+		tagp += tag_bytes;
+		if (!(flags & JBD2_FLAG_SAME_UUID))
+			tagp += 16;
+
+		if (flags & JBD2_FLAG_LAST_TAG)
+			break;
+	}
+
+	return 0;
+}
+
 static int do_one_pass(journal_t *journal,
 			struct recovery_info *info, enum passtype pass)
 {
@@ -496,9 +595,7 @@ static int do_one_pass(journal_t *journal,
 	struct buffer_head	*bh = NULL;
 	unsigned int		sequence;
 	int			blocktype;
-	int			tag_bytes = journal_tag_bytes(journal);
 	__u32			crc32_sum = ~0; /* Transactional Checksums */
-	int			descr_csum_size = 0;
 	int			block_error = 0;
 	bool			need_check_commit_time = false;
 	__u64			last_trans_commit_time = 0, commit_time;
@@ -528,12 +625,6 @@ static int do_one_pass(journal_t *journal,
 	 */
 
 	while (1) {
-		int			flags;
-		char *			tagp;
-		journal_block_tag_t	tag;
-		struct buffer_head *	obh;
-		struct buffer_head *	nbh;
-
 		cond_resched();
 
 		/* If we already know where to stop the log traversal,
@@ -587,11 +678,7 @@ static int do_one_pass(journal_t *journal,
 		switch(blocktype) {
 		case JBD2_DESCRIPTOR_BLOCK:
 			/* Verify checksum first */
-			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,
+			if (!jbd2_descriptor_block_csum_verify(journal,
 							       bh->b_data)) {
 				/*
 				 * PASS_SCAN can see stale blocks due to lazy
@@ -628,102 +715,16 @@ static int do_one_pass(journal_t *journal,
 				continue;
 			}
 
-			/* A descriptor block: we can now write all of
-			 * the data blocks.  Yay, useful work is finally
-			 * getting done here! */
-
-			tagp = &bh->b_data[sizeof(journal_header_t)];
-			while ((tagp - bh->b_data + tag_bytes)
-			       <= journal->j_blocksize - descr_csum_size) {
-				unsigned long io_block;
-
-				memcpy(&tag, tagp, sizeof(tag));
-				flags = be16_to_cpu(tag.t_flags);
-
-				io_block = next_log_block++;
-				wrap(journal, next_log_block);
-				err = jread(&obh, journal, io_block);
-				if (err) {
-					/* Recover what we can, but
-					 * report failure at the end. */
-					success = err;
-					printk(KERN_ERR
-						"JBD2: IO error %d recovering "
-						"block %lu in log\n",
-						err, io_block);
-				} else {
-					unsigned long long blocknr;
-
-					J_ASSERT(obh != NULL);
-					blocknr = read_tag_block(journal,
-								 &tag);
-
-					/* If the block has been
-					 * revoked, then we're all done
-					 * here. */
-					if (jbd2_journal_test_revoke
-					    (journal, blocknr,
-					     next_commit_ID)) {
-						brelse(obh);
-						++info->nr_revoke_hits;
-						goto skip_write;
-					}
-
-					/* Look for block corruption */
-					if (!jbd2_block_tag_csum_verify(
-			journal, &tag, (journal_block_tag3_t *)tagp,
-			obh->b_data, be32_to_cpu(tmp->h_sequence))) {
-						brelse(obh);
-						success = -EFSBADCRC;
-						printk(KERN_ERR "JBD2: Invalid "
-						       "checksum recovering "
-						       "data block %llu in "
-						       "journal block %lu\n",
-						       blocknr, io_block);
-						block_error = 1;
-						goto skip_write;
-					}
-
-					/* Find a buffer for the new
-					 * data being restored */
-					nbh = __getblk(journal->j_fs_dev,
-							blocknr,
-							journal->j_blocksize);
-					if (nbh == NULL) {
-						printk(KERN_ERR
-						       "JBD2: Out of memory "
-						       "during recovery.\n");
-						err = -ENOMEM;
-						brelse(obh);
-						goto failed;
-					}
-
-					lock_buffer(nbh);
-					memcpy(nbh->b_data, obh->b_data,
-							journal->j_blocksize);
-					if (flags & JBD2_FLAG_ESCAPE) {
-						*((__be32 *)nbh->b_data) =
-						cpu_to_be32(JBD2_MAGIC_NUMBER);
-					}
-
-					BUFFER_TRACE(nbh, "marking dirty");
-					set_buffer_uptodate(nbh);
-					mark_buffer_dirty(nbh);
-					BUFFER_TRACE(nbh, "marking uptodate");
-					++info->nr_replays;
-					unlock_buffer(nbh);
-					brelse(obh);
-					brelse(nbh);
-				}
-
-			skip_write:
-				tagp += tag_bytes;
-				if (!(flags & JBD2_FLAG_SAME_UUID))
-					tagp += 16;
-
-				if (flags & JBD2_FLAG_LAST_TAG)
-					break;
-			}
+			/*
+			 * A descriptor block: we can now write all of the
+			 * data blocks. Yay, useful work is finally getting
+			 * done here!
+			 */
+			err = jbd2_do_replay(journal, info, bh, &next_log_block,
+					     next_commit_ID, &success,
+					     &block_error);
+			if (err)
+				goto failed;
 
 			continue;
 
-- 
2.31.1

[PATCH 5/5] jbd2: remove useless 'block_error' variable

[Ye Bin]

From: Ye Bin <yebin10@huawei.com>

The judgement 'if (block_error && success == 0)' is never valid. Just
remove useless 'block_error' variable.

Signed-off-by: Ye Bin <yebin10@huawei.com>
---
 fs/jbd2/recovery.c | 9 ++-------
 1 file changed, 2 insertions(+), 7 deletions(-)

diff --git a/fs/jbd2/recovery.c b/fs/jbd2/recovery.c
index 05ea449b95c4..0bcbb58d634b 100644
--- a/fs/jbd2/recovery.c
+++ b/fs/jbd2/recovery.c
@@ -490,7 +490,7 @@ static __always_inline int jbd2_do_replay(journal_t *journal,
 					  struct buffer_head *bh,
 					  unsigned long *next_log_block,
 					  unsigned int next_commit_ID,
-					  int *success, int *block_error)
+					  int *success)
 {
 	char *tagp;
 	int flags;
@@ -542,7 +542,6 @@ static __always_inline int jbd2_do_replay(journal_t *journal,
 				*success = -EFSBADCRC;
 				pr_err("JBD2: Invalid checksum recovering data block %llu in journal block %lu\n",
 				      blocknr, io_block);
-				*block_error = 1;
 				goto skip_write;
 			}
 
@@ -596,7 +595,6 @@ static int do_one_pass(journal_t *journal,
 	unsigned int		sequence;
 	int			blocktype;
 	__u32			crc32_sum = ~0; /* Transactional Checksums */
-	int			block_error = 0;
 	bool			need_check_commit_time = false;
 	__u64			last_trans_commit_time = 0, commit_time;
 
@@ -721,8 +719,7 @@ static int do_one_pass(journal_t *journal,
 			 * done here!
 			 */
 			err = jbd2_do_replay(journal, info, bh, &next_log_block,
-					     next_commit_ID, &success,
-					     &block_error);
+					     next_commit_ID, &success);
 			if (err)
 				goto failed;
 
@@ -913,8 +910,6 @@ static int do_one_pass(journal_t *journal,
 			success = err;
 	}
 
-	if (block_error && success == 0)
-		success = -EIO;
 	return success;
 
  failed:
-- 
2.31.1

Re: [PATCH 5/5] jbd2: remove useless 'block_error' variable

[Zhang Yi]

On 2024/9/18 19:36, Ye Bin wrote:
> From: Ye Bin <yebin10@huawei.com>
> 
> The judgement 'if (block_error && success == 0)' is never valid. Just
> remove useless 'block_error' variable.
> 
> Signed-off-by: Ye Bin <yebin10@huawei.com>
> ---
>  fs/jbd2/recovery.c | 9 ++-------
>  1 file changed, 2 insertions(+), 7 deletions(-)
> 
> diff --git a/fs/jbd2/recovery.c b/fs/jbd2/recovery.c
> index 05ea449b95c4..0bcbb58d634b 100644
> --- a/fs/jbd2/recovery.c
> +++ b/fs/jbd2/recovery.c
> @@ -490,7 +490,7 @@ static __always_inline int jbd2_do_replay(journal_t *journal,
>  					  struct buffer_head *bh,
>  					  unsigned long *next_log_block,
>  					  unsigned int next_commit_ID,
> -					  int *success, int *block_error)
> +					  int *success)

I wonder an unrelated question, why do we need this 'success' variable? Now we
keep on replaying later log blocks even if some bad things happened(read I/O
error or checksum error) instead of just quit, is it to replay as many log
blocks as possible, and then to reduce the data lose and also reduce e2fsck's
workload?

Thanks,
Yi.

>  {
>  	char *tagp;
>  	int flags;
> @@ -542,7 +542,6 @@ static __always_inline int jbd2_do_replay(journal_t *journal,
>  				*success = -EFSBADCRC;
>  				pr_err("JBD2: Invalid checksum recovering data block %llu in journal block %lu\n",
>  				      blocknr, io_block);
> -				*block_error = 1;
>  				goto skip_write;
>  			}
>  
> @@ -596,7 +595,6 @@ static int do_one_pass(journal_t *journal,
>  	unsigned int		sequence;
>  	int			blocktype;
>  	__u32			crc32_sum = ~0; /* Transactional Checksums */
> -	int			block_error = 0;
>  	bool			need_check_commit_time = false;
>  	__u64			last_trans_commit_time = 0, commit_time;
>  
> @@ -721,8 +719,7 @@ static int do_one_pass(journal_t *journal,
>  			 * done here!
>  			 */
>  			err = jbd2_do_replay(journal, info, bh, &next_log_block,
> -					     next_commit_ID, &success,
> -					     &block_error);
> +					     next_commit_ID, &success);
>  			if (err)
>  				goto failed;
>  
> @@ -913,8 +910,6 @@ static int do_one_pass(journal_t *journal,
>  			success = err;
>  	}
>  
> -	if (block_error && success == 0)
> -		success = -EIO;
>  	return success;
>  
>   failed:

Re: [PATCH 1/5] jbd2: remove redundant judgments for check v1 checksum

[Jan Kara]

On Wed 18-09-24 19:36:00, Ye Bin wrote:
> From: Ye Bin <yebin10@huawei.com>
> 
> 'need_check_commit_time' is only used by v2/v3 checksum, so there isn't
> need to add 'need_check_commit_time' judegement for v1 checksum logic.
> 
> Signed-off-by: Ye Bin <yebin10@huawei.com>

Looks good. Feel free to add:

Reviewed-by: Jan Kara <jack@suse.cz>

								Honza

> ---
>  fs/jbd2/recovery.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/fs/jbd2/recovery.c b/fs/jbd2/recovery.c
> index 667f67342c52..5efbca6a98c4 100644
> --- a/fs/jbd2/recovery.c
> +++ b/fs/jbd2/recovery.c
> @@ -619,7 +619,6 @@ static int do_one_pass(journal_t *journal,
>  			if (pass != PASS_REPLAY) {
>  				if (pass == PASS_SCAN &&
>  				    jbd2_has_feature_checksum(journal) &&
> -				    !need_check_commit_time &&
>  				    !info->end_transaction) {
>  					if (calc_chksums(journal, bh,
>  							&next_log_block,
> -- 
> 2.31.1
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

Re: [PATCH 2/5] jbd2: unified release of buffer_head in do_one_pass()

[Jan Kara]

On Wed 18-09-24 19:36:01, Ye Bin wrote:
> From: Ye Bin <yebin10@huawei.com>
> 
> Now buffer_head free is very fragmented in do_one_pass(), unified release
> of buffer_head in do_one_pass()
> 
> Signed-off-by: Ye Bin <yebin10@huawei.com>

Indeed there are many places releasing bh. The patch looks good. Feel free
to add:

Reviewed-by: Jan Kara <jack@suse.cz>

								Honza

> ---
>  fs/jbd2/recovery.c | 34 +++++++++-------------------------
>  1 file changed, 9 insertions(+), 25 deletions(-)
> 
> diff --git a/fs/jbd2/recovery.c b/fs/jbd2/recovery.c
> index 5efbca6a98c4..0adf0cb31a03 100644
> --- a/fs/jbd2/recovery.c
> +++ b/fs/jbd2/recovery.c
> @@ -493,7 +493,7 @@ static int do_one_pass(journal_t *journal,
>  	int			err, success = 0;
>  	journal_superblock_t *	sb;
>  	journal_header_t *	tmp;
> -	struct buffer_head *	bh;
> +	struct buffer_head	*bh = NULL;
>  	unsigned int		sequence;
>  	int			blocktype;
>  	int			tag_bytes = journal_tag_bytes(journal);
> @@ -552,6 +552,8 @@ static int do_one_pass(journal_t *journal,
>  		 * record. */
>  
>  		jbd2_debug(3, "JBD2: checking block %ld\n", next_log_block);
> +		brelse(bh);
> +		bh = NULL;
>  		err = jread(&bh, journal, next_log_block);
>  		if (err)
>  			goto failed;
> @@ -567,20 +569,16 @@ static int do_one_pass(journal_t *journal,
>  
>  		tmp = (journal_header_t *)bh->b_data;
>  
> -		if (tmp->h_magic != cpu_to_be32(JBD2_MAGIC_NUMBER)) {
> -			brelse(bh);
> +		if (tmp->h_magic != cpu_to_be32(JBD2_MAGIC_NUMBER))
>  			break;
> -		}
>  
>  		blocktype = be32_to_cpu(tmp->h_blocktype);
>  		sequence = be32_to_cpu(tmp->h_sequence);
>  		jbd2_debug(3, "Found magic %d, sequence %d\n",
>  			  blocktype, sequence);
>  
> -		if (sequence != next_commit_ID) {
> -			brelse(bh);
> +		if (sequence != next_commit_ID)
>  			break;
> -		}
>  
>  		/* OK, we have a valid descriptor block which matches
>  		 * all of the sequence number checks.  What are we going
> @@ -603,7 +601,6 @@ static int do_one_pass(journal_t *journal,
>  					pr_err("JBD2: Invalid checksum recovering block %lu in log\n",
>  					       next_log_block);
>  					err = -EFSBADCRC;
> -					brelse(bh);
>  					goto failed;
>  				}
>  				need_check_commit_time = true;
> @@ -622,16 +619,12 @@ static int do_one_pass(journal_t *journal,
>  				    !info->end_transaction) {
>  					if (calc_chksums(journal, bh,
>  							&next_log_block,
> -							&crc32_sum)) {
> -						put_bh(bh);
> +							&crc32_sum))
>  						break;
> -					}
> -					put_bh(bh);
>  					continue;
>  				}
>  				next_log_block += count_tags(journal, bh);
>  				wrap(journal, next_log_block);
> -				put_bh(bh);
>  				continue;
>  			}
>  
> @@ -701,7 +694,6 @@ static int do_one_pass(journal_t *journal,
>  						       "JBD2: Out of memory "
>  						       "during recovery.\n");
>  						err = -ENOMEM;
> -						brelse(bh);
>  						brelse(obh);
>  						goto failed;
>  					}
> @@ -733,7 +725,6 @@ static int do_one_pass(journal_t *journal,
>  					break;
>  			}
>  
> -			brelse(bh);
>  			continue;
>  
>  		case JBD2_COMMIT_BLOCK:
> @@ -781,7 +772,6 @@ static int do_one_pass(journal_t *journal,
>  					pr_err("JBD2: Invalid checksum found in transaction %u\n",
>  					       next_commit_ID);
>  					err = -EFSBADCRC;
> -					brelse(bh);
>  					goto failed;
>  				}
>  			ignore_crc_mismatch:
> @@ -791,7 +781,6 @@ static int do_one_pass(journal_t *journal,
>  				 */
>  				jbd2_debug(1, "JBD2: Invalid checksum ignored in transaction %u, likely stale data\n",
>  					  next_commit_ID);
> -				brelse(bh);
>  				goto done;
>  			}
>  
> @@ -811,7 +800,6 @@ static int do_one_pass(journal_t *journal,
>  				if (info->end_transaction) {
>  					journal->j_failed_commit =
>  						info->end_transaction;
> -					brelse(bh);
>  					break;
>  				}
>  
> @@ -847,7 +835,6 @@ static int do_one_pass(journal_t *journal,
>  				if (!jbd2_has_feature_async_commit(journal)) {
>  					journal->j_failed_commit =
>  						next_commit_ID;
> -					brelse(bh);
>  					break;
>  				}
>  			}
> @@ -856,7 +843,6 @@ static int do_one_pass(journal_t *journal,
>  				last_trans_commit_time = commit_time;
>  				head_block = next_log_block;
>  			}
> -			brelse(bh);
>  			next_commit_ID++;
>  			continue;
>  
> @@ -875,14 +861,11 @@ static int do_one_pass(journal_t *journal,
>  
>  			/* If we aren't in the REVOKE pass, then we can
>  			 * just skip over this block. */
> -			if (pass != PASS_REVOKE) {
> -				brelse(bh);
> +			if (pass != PASS_REVOKE)
>  				continue;
> -			}
>  
>  			err = scan_revoke_records(journal, bh,
>  						  next_commit_ID, info);
> -			brelse(bh);
>  			if (err)
>  				goto failed;
>  			continue;
> @@ -890,12 +873,12 @@ static int do_one_pass(journal_t *journal,
>  		default:
>  			jbd2_debug(3, "Unrecognised magic %d, end of scan.\n",
>  				  blocktype);
> -			brelse(bh);
>  			goto done;
>  		}
>  	}
>  
>   done:
> +	brelse(bh);
>  	/*
>  	 * We broke out of the log scan loop: either we came to the
>  	 * known end of the log or we found an unexpected block in the
> @@ -931,6 +914,7 @@ static int do_one_pass(journal_t *journal,
>  	return success;
>  
>   failed:
> +	brelse(bh);
>  	return err;
>  }
>  
> -- 
> 2.31.1
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

Re: [PATCH 3/5] jbd2: refactor JBD2_COMMIT_BLOCK process in do_one_pass()

[Jan Kara]

On Wed 18-09-24 19:36:02, Ye Bin wrote:
> From: Ye Bin <yebin10@huawei.com>
> 
> To make JBD2_COMMIT_BLOCK process more clean, no functional change.
> 
> Signed-off-by: Ye Bin <yebin10@huawei.com>

Looks good. Feel free to add:

Reviewed-by: Jan Kara <jack@suse.cz>

								Honza

> ---
>  fs/jbd2/recovery.c | 55 ++++++++++++++++++++++++----------------------
>  1 file changed, 29 insertions(+), 26 deletions(-)
> 
> diff --git a/fs/jbd2/recovery.c b/fs/jbd2/recovery.c
> index 0adf0cb31a03..0d697979d83e 100644
> --- a/fs/jbd2/recovery.c
> +++ b/fs/jbd2/recovery.c
> @@ -728,6 +728,11 @@ static int do_one_pass(journal_t *journal,
>  			continue;
>  
>  		case JBD2_COMMIT_BLOCK:
> +			if (pass != PASS_SCAN) {
> +				next_commit_ID++;
> +				continue;
> +			}
> +
>  			/*     How to differentiate between interrupted commit
>  			 *               and journal corruption ?
>  			 *
> @@ -790,8 +795,7 @@ static int do_one_pass(journal_t *journal,
>  			 * much to do other than move on to the next sequence
>  			 * number.
>  			 */
> -			if (pass == PASS_SCAN &&
> -			    jbd2_has_feature_checksum(journal)) {
> +			if (jbd2_has_feature_checksum(journal)) {
>  				struct commit_header *cbh =
>  					(struct commit_header *)bh->b_data;
>  				unsigned found_chksum =
> @@ -815,34 +819,33 @@ static int do_one_pass(journal_t *journal,
>  					goto chksum_error;
>  
>  				crc32_sum = ~0;
> +				goto chksum_ok;
>  			}
> -			if (pass == PASS_SCAN &&
> -			    !jbd2_commit_block_csum_verify(journal,
> -							   bh->b_data)) {
> -				if (jbd2_commit_block_csum_verify_partial(
> -								  journal,
> -								  bh->b_data)) {
> -					pr_notice("JBD2: Find incomplete commit block in transaction %u block %lu\n",
> -						  next_commit_ID, next_log_block);
> -					goto chksum_ok;
> -				}
> -			chksum_error:
> -				if (commit_time < last_trans_commit_time)
> -					goto ignore_crc_mismatch;
> -				info->end_transaction = next_commit_ID;
> -				info->head_block = head_block;
>  
> -				if (!jbd2_has_feature_async_commit(journal)) {
> -					journal->j_failed_commit =
> -						next_commit_ID;
> -					break;
> -				}
> +			if (jbd2_commit_block_csum_verify(journal, bh->b_data))
> +				goto chksum_ok;
> +
> +			if (jbd2_commit_block_csum_verify_partial(journal,
> +								  bh->b_data)) {
> +				pr_notice("JBD2: Find incomplete commit block in transaction %u block %lu\n",
> +					  next_commit_ID, next_log_block);
> +				goto chksum_ok;
>  			}
> -			if (pass == PASS_SCAN) {
> -			chksum_ok:
> -				last_trans_commit_time = commit_time;
> -				head_block = next_log_block;
> +
> +chksum_error:
> +			if (commit_time < last_trans_commit_time)
> +				goto ignore_crc_mismatch;
> +			info->end_transaction = next_commit_ID;
> +			info->head_block = head_block;
> +
> +			if (!jbd2_has_feature_async_commit(journal)) {
> +				journal->j_failed_commit = next_commit_ID;
> +				break;
>  			}
> +
> +chksum_ok:
> +			last_trans_commit_time = commit_time;
> +			head_block = next_log_block;
>  			next_commit_ID++;
>  			continue;
>  
> -- 
> 2.31.1
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

Re: [PATCH 5/5] jbd2: remove useless 'block_error' variable

[Jan Kara]

On Wed 18-09-24 19:36:04, Ye Bin wrote:
> From: Ye Bin <yebin10@huawei.com>
> 
> The judgement 'if (block_error && success == 0)' is never valid. Just
> remove useless 'block_error' variable.
> 
> Signed-off-by: Ye Bin <yebin10@huawei.com>

Looks good. Feel free to add:

Reviewed-by: Jan Kara <jack@suse.cz>

								Honza

> ---
>  fs/jbd2/recovery.c | 9 ++-------
>  1 file changed, 2 insertions(+), 7 deletions(-)
> 
> diff --git a/fs/jbd2/recovery.c b/fs/jbd2/recovery.c
> index 05ea449b95c4..0bcbb58d634b 100644
> --- a/fs/jbd2/recovery.c
> +++ b/fs/jbd2/recovery.c
> @@ -490,7 +490,7 @@ static __always_inline int jbd2_do_replay(journal_t *journal,
>  					  struct buffer_head *bh,
>  					  unsigned long *next_log_block,
>  					  unsigned int next_commit_ID,
> -					  int *success, int *block_error)
> +					  int *success)
>  {
>  	char *tagp;
>  	int flags;
> @@ -542,7 +542,6 @@ static __always_inline int jbd2_do_replay(journal_t *journal,
>  				*success = -EFSBADCRC;
>  				pr_err("JBD2: Invalid checksum recovering data block %llu in journal block %lu\n",
>  				      blocknr, io_block);
> -				*block_error = 1;
>  				goto skip_write;
>  			}
>  
> @@ -596,7 +595,6 @@ static int do_one_pass(journal_t *journal,
>  	unsigned int		sequence;
>  	int			blocktype;
>  	__u32			crc32_sum = ~0; /* Transactional Checksums */
> -	int			block_error = 0;
>  	bool			need_check_commit_time = false;
>  	__u64			last_trans_commit_time = 0, commit_time;
>  
> @@ -721,8 +719,7 @@ static int do_one_pass(journal_t *journal,
>  			 * done here!
>  			 */
>  			err = jbd2_do_replay(journal, info, bh, &next_log_block,
> -					     next_commit_ID, &success,
> -					     &block_error);
> +					     next_commit_ID, &success);
>  			if (err)
>  				goto failed;
>  
> @@ -913,8 +910,6 @@ static int do_one_pass(journal_t *journal,
>  			success = err;
>  	}
>  
> -	if (block_error && success == 0)
> -		success = -EIO;
>  	return success;
>  
>   failed:
> -- 
> 2.31.1
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

Re: [PATCH 4/5] jbd2: factor out jbd2_do_replay()

[Jan Kara]

On Wed 18-09-24 19:36:03, Ye Bin wrote:
> From: Ye Bin <yebin10@huawei.com>
> 
> Factor out jbd2_do_replay() no funtional change.
> 
> Signed-off-by: Ye Bin <yebin10@huawei.com>

The patch looks good but I have a few comments below:

> +static __always_inline int jbd2_do_replay(journal_t *journal,
> +					  struct recovery_info *info,
> +					  struct buffer_head *bh,
> +					  unsigned long *next_log_block,
> +					  unsigned int next_commit_ID,
> +					  int *success, int *block_error)

So you remove block_error in the later patch so that is good. But I also
wonder if we need the 'success' parameter. I'd think that jbd2_do_replay()
can just keep success internally to track if any error happened and then
return it at the end? do_one_pass() will then incorporate the returned
value in its 'success' variable. That way the error handling will be more
standard...

> +{
> +	char *tagp;
> +	int flags;
> +	int err;
> +	int tag_bytes = journal_tag_bytes(journal);
> +	int descr_csum_size = 0;
> +	unsigned long io_block;
> +	journal_block_tag_t tag;
> +	struct buffer_head *obh;
> +	struct buffer_head *nbh;
> +
> +	if (jbd2_journal_has_csum_v2or3(journal))
> +		descr_csum_size = sizeof(struct jbd2_journal_block_tail);
> +
> +	tagp = &bh->b_data[sizeof(journal_header_t)];
> +	while ((tagp - bh->b_data + tag_bytes) <=
	       ^^ no need for braces here...

> +	       journal->j_blocksize - descr_csum_size) {
		^^ when the indentation level is this close to the code
block indentation, I actually prefer indenting it one tab more like:

			journal->j_blocksize - descr_csum_size) {

But this is just a personal preference so feel free to ignore it :)

> +
> +		memcpy(&tag, tagp, sizeof(tag));
> +		flags = be16_to_cpu(tag.t_flags);
> +
> +		io_block = (*next_log_block)++;
> +		wrap(journal, *next_log_block);
> +		err = jread(&obh, journal, io_block);
> +		if (err) {
> +			/* Recover what we can, but report failure at the end. */
> +			*success = err;
> +			pr_err("JBD2: IO error %d recovering block %lu in log\n",
> +			      err, io_block);
> +		} else {
> +			unsigned long long blocknr;
> +
> +			J_ASSERT(obh != NULL);
> +			blocknr = read_tag_block(journal, &tag);
> +
> +			/* If the block has been revoked, then we're all done here. */
> +			if (jbd2_journal_test_revoke(journal, blocknr,
> +						     next_commit_ID)) {
> +				brelse(obh);
> +				++info->nr_revoke_hits;
> +				goto skip_write;
> +			}
> +
> +			/* Look for block corruption */
> +			if (!jbd2_block_tag_csum_verify(journal, &tag,
> +				(journal_block_tag3_t *)tagp, obh->b_data,
> +				next_commit_ID)) {
				^^^
Indentation like this is really confusing. I'd add one more tab here.

Otherwise the patch looks good.

									Honza


> +				brelse(obh);
> +				*success = -EFSBADCRC;
> +				pr_err("JBD2: Invalid checksum recovering data block %llu in journal block %lu\n",
> +				      blocknr, io_block);
> +				*block_error = 1;
> +				goto skip_write;
> +			}
> +
> +			/* Find a buffer for the new data being restored */
> +			nbh = __getblk(journal->j_fs_dev, blocknr,
> +				       journal->j_blocksize);
> +			if (nbh == NULL) {
> +				pr_err("JBD2: Out of memory during recovery.\n");
> +				brelse(obh);
> +				return -ENOMEM;
> +			}
> +
> +			lock_buffer(nbh);
> +			memcpy(nbh->b_data, obh->b_data, journal->j_blocksize);
> +			if (flags & JBD2_FLAG_ESCAPE) {
> +				*((__be32 *)nbh->b_data) =
> +				cpu_to_be32(JBD2_MAGIC_NUMBER);
> +			}
> +
> +			BUFFER_TRACE(nbh, "marking dirty");
> +			set_buffer_uptodate(nbh);
> +			mark_buffer_dirty(nbh);
> +			BUFFER_TRACE(nbh, "marking uptodate");
> +			++info->nr_replays;
> +			unlock_buffer(nbh);
> +			brelse(obh);
> +			brelse(nbh);
> +		}
> +
> +skip_write:
> +		tagp += tag_bytes;
> +		if (!(flags & JBD2_FLAG_SAME_UUID))
> +			tagp += 16;
> +
> +		if (flags & JBD2_FLAG_LAST_TAG)
> +			break;
> +	}
> +
> +	return 0;
> +}
> +
>  static int do_one_pass(journal_t *journal,
>  			struct recovery_info *info, enum passtype pass)
>  {
> @@ -496,9 +595,7 @@ static int do_one_pass(journal_t *journal,
>  	struct buffer_head	*bh = NULL;
>  	unsigned int		sequence;
>  	int			blocktype;
> -	int			tag_bytes = journal_tag_bytes(journal);
>  	__u32			crc32_sum = ~0; /* Transactional Checksums */
> -	int			descr_csum_size = 0;
>  	int			block_error = 0;
>  	bool			need_check_commit_time = false;
>  	__u64			last_trans_commit_time = 0, commit_time;
> @@ -528,12 +625,6 @@ static int do_one_pass(journal_t *journal,
>  	 */
>  
>  	while (1) {
> -		int			flags;
> -		char *			tagp;
> -		journal_block_tag_t	tag;
> -		struct buffer_head *	obh;
> -		struct buffer_head *	nbh;
> -
>  		cond_resched();
>  
>  		/* If we already know where to stop the log traversal,
> @@ -587,11 +678,7 @@ static int do_one_pass(journal_t *journal,
>  		switch(blocktype) {
>  		case JBD2_DESCRIPTOR_BLOCK:
>  			/* Verify checksum first */
> -			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,
> +			if (!jbd2_descriptor_block_csum_verify(journal,
>  							       bh->b_data)) {
>  				/*
>  				 * PASS_SCAN can see stale blocks due to lazy
> @@ -628,102 +715,16 @@ static int do_one_pass(journal_t *journal,
>  				continue;
>  			}
>  
> -			/* A descriptor block: we can now write all of
> -			 * the data blocks.  Yay, useful work is finally
> -			 * getting done here! */
> -
> -			tagp = &bh->b_data[sizeof(journal_header_t)];
> -			while ((tagp - bh->b_data + tag_bytes)
> -			       <= journal->j_blocksize - descr_csum_size) {
> -				unsigned long io_block;
> -
> -				memcpy(&tag, tagp, sizeof(tag));
> -				flags = be16_to_cpu(tag.t_flags);
> -
> -				io_block = next_log_block++;
> -				wrap(journal, next_log_block);
> -				err = jread(&obh, journal, io_block);
> -				if (err) {
> -					/* Recover what we can, but
> -					 * report failure at the end. */
> -					success = err;
> -					printk(KERN_ERR
> -						"JBD2: IO error %d recovering "
> -						"block %lu in log\n",
> -						err, io_block);
> -				} else {
> -					unsigned long long blocknr;
> -
> -					J_ASSERT(obh != NULL);
> -					blocknr = read_tag_block(journal,
> -								 &tag);
> -
> -					/* If the block has been
> -					 * revoked, then we're all done
> -					 * here. */
> -					if (jbd2_journal_test_revoke
> -					    (journal, blocknr,
> -					     next_commit_ID)) {
> -						brelse(obh);
> -						++info->nr_revoke_hits;
> -						goto skip_write;
> -					}
> -
> -					/* Look for block corruption */
> -					if (!jbd2_block_tag_csum_verify(
> -			journal, &tag, (journal_block_tag3_t *)tagp,
> -			obh->b_data, be32_to_cpu(tmp->h_sequence))) {
> -						brelse(obh);
> -						success = -EFSBADCRC;
> -						printk(KERN_ERR "JBD2: Invalid "
> -						       "checksum recovering "
> -						       "data block %llu in "
> -						       "journal block %lu\n",
> -						       blocknr, io_block);
> -						block_error = 1;
> -						goto skip_write;
> -					}
> -
> -					/* Find a buffer for the new
> -					 * data being restored */
> -					nbh = __getblk(journal->j_fs_dev,
> -							blocknr,
> -							journal->j_blocksize);
> -					if (nbh == NULL) {
> -						printk(KERN_ERR
> -						       "JBD2: Out of memory "
> -						       "during recovery.\n");
> -						err = -ENOMEM;
> -						brelse(obh);
> -						goto failed;
> -					}
> -
> -					lock_buffer(nbh);
> -					memcpy(nbh->b_data, obh->b_data,
> -							journal->j_blocksize);
> -					if (flags & JBD2_FLAG_ESCAPE) {
> -						*((__be32 *)nbh->b_data) =
> -						cpu_to_be32(JBD2_MAGIC_NUMBER);
> -					}
> -
> -					BUFFER_TRACE(nbh, "marking dirty");
> -					set_buffer_uptodate(nbh);
> -					mark_buffer_dirty(nbh);
> -					BUFFER_TRACE(nbh, "marking uptodate");
> -					++info->nr_replays;
> -					unlock_buffer(nbh);
> -					brelse(obh);
> -					brelse(nbh);
> -				}
> -
> -			skip_write:
> -				tagp += tag_bytes;
> -				if (!(flags & JBD2_FLAG_SAME_UUID))
> -					tagp += 16;
> -
> -				if (flags & JBD2_FLAG_LAST_TAG)
> -					break;
> -			}
> +			/*
> +			 * A descriptor block: we can now write all of the
> +			 * data blocks. Yay, useful work is finally getting
> +			 * done here!
> +			 */
> +			err = jbd2_do_replay(journal, info, bh, &next_log_block,
> +					     next_commit_ID, &success,
> +					     &block_error);
> +			if (err)
> +				goto failed;
>  
>  			continue;
>  
> -- 
> 2.31.1
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR