[PATCH 0/5] fix error flag covered by journal recovery

[PATCH 0/5] fix error flag covered by journal recovery

[Ye Bin]

From: Ye Bin <yebin10@huawei.com>

When do fault injection test, got issue as follows:
EXT4-fs (dm-5): warning: mounting fs with errors, running e2fsck is recommended
EXT4-fs (dm-5): Errors on filesystem, clearing orphan list.
EXT4-fs (dm-5): recovery complete
EXT4-fs (dm-5): mounted filesystem with ordered data mode. Opts: data_err=abort,errors=remount-ro

EXT4-fs (dm-5): recovery complete
EXT4-fs (dm-5): mounted filesystem with ordered data mode. Opts: data_err=abort,errors=remount-ro

Without do file system check, file system is clean when do second mount.
Theoretically, the kernel will not clear fs error flag. In errors=remount-ro
mode the last super block is commit directly. So super block in journal is
not uptodate. When do jounral recovery, the uptodate super block will be
covered by jounral data. If super block submit all failed after recover
journal, then file system error flag is lost. When do "fsck -a" couldn't
repair file system deeply.
To solve above issue we need to do extra handle when do super block journal
recovery.

Ye Bin (5):
  jbd2: introduce callback for recovery journal
  ext4: introudce helper for jounral recover handle
  ext4: fix error flag covered by journal recovery
  ext4: fix super block checksum error
  ext4: make sure fs error flag setted before clear journal error

 fs/ext4/ext4_jbd2.c  | 66 ++++++++++++++++++++++++++++++++++++++++++++
 fs/ext4/ext4_jbd2.h  |  2 ++
 fs/ext4/super.c      | 45 ++++++++++++++----------------
 fs/jbd2/recovery.c   | 14 ++++++++++
 include/linux/jbd2.h | 11 ++++++++
 5 files changed, 114 insertions(+), 24 deletions(-)

-- 
2.31.1

[PATCH 1/5] jbd2: introduce callback for recovery journal

[Ye Bin]

From: Ye Bin <yebin10@huawei.com>

EXT4 file system's super block may submited by journal, however it
maybe submited directly when do error handle and also other scene.
So super block isn't uptodate in journal. So there is need to do
some extra handle when recover journal.

Signed-off-by: Ye Bin <yebin10@huawei.com>
---
 include/linux/jbd2.h | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/include/linux/jbd2.h b/include/linux/jbd2.h
index 5962072a4b19..ab0e1a435a50 100644
--- a/include/linux/jbd2.h
+++ b/include/linux/jbd2.h
@@ -1308,6 +1308,17 @@ struct journal_s
 				    struct buffer_head *bh,
 				    enum passtype pass, int off,
 				    tid_t expected_commit_id);
+	/*
+	 * EXT4 file system's super block may submited by journal, however it
+	 * maybe submited directly when do error handle. So super block isn't
+	 * uptodate in journal. So there is need to do some extra handle when
+	 * recover journal.
+	 */
+	void *j_replay_private_data;
+	int (*j_replay_prepare_callback)(struct journal_s *journal);
+	int (*j_replay_callback)(struct journal_s *journal,
+				  struct buffer_head *bh);
+	void (*j_replay_end_callback)(struct journal_s *journal);
 };
 
 #define jbd2_might_wait_for_commit(j) \
-- 
2.31.1

[PATCH 2/5] ext4: introudce helper for jounral recover handle

[Ye Bin]

From: Ye Bin <yebin10@huawei.com>

Now, ext4 file system only need to handle super block when do
recover journal.

Signed-off-by: Ye Bin <yebin10@huawei.com>
---
 fs/ext4/ext4_jbd2.c | 65 +++++++++++++++++++++++++++++++++++++++++++++
 fs/ext4/ext4_jbd2.h |  2 ++
 fs/ext4/super.c     |  1 +
 3 files changed, 68 insertions(+)

diff --git a/fs/ext4/ext4_jbd2.c b/fs/ext4/ext4_jbd2.c
index 77f318ec8abb..7c0f2bed0ec4 100644
--- a/fs/ext4/ext4_jbd2.c
+++ b/fs/ext4/ext4_jbd2.c
@@ -395,3 +395,68 @@ int __ext4_handle_dirty_metadata(const char *where, unsigned int line,
 	}
 	return err;
 }
+
+void ext4_replay_end_callback(struct journal_s *journal)
+{
+	kfree(journal->j_replay_private_data);
+	journal->j_replay_private_data = NULL;
+	journal->j_replay_callback = NULL;
+	journal->j_replay_end_callback = NULL;
+}
+
+static int ext4_replay_callback(struct journal_s *journal,
+				struct buffer_head *bh)
+{
+	struct super_block *sb = journal->j_private;
+	struct ext4_sb_info *sbi = EXT4_SB(sb);
+	struct ext4_super_block *es = sbi->s_es;
+	struct ext4_super_block *nes;
+	unsigned long offset;
+
+	if (likely(sbi->s_sbh != bh))
+		return 0;
+
+	offset = (void*)es - (void*)sbi->s_sbh->b_data;
+	nes = (struct ext4_super_block*)(bh->b_data + offset);
+	/*
+	 * If super block has error flag in journal record, there isn't need to
+	 * cover error information, as in this case is errors=continue mode,
+	 * error handle submit super block through journal.
+	 */
+	if (le16_to_cpu(nes->s_state) & EXT4_ERROR_FS)
+		return 0;
+
+	memcpy(((char *)es) + EXT4_S_ERR_START,
+	       journal->j_replay_private_data, EXT4_S_ERR_LEN);
+	if (sbi->s_mount_state & EXT4_ERROR_FS)
+		es->s_state |= cpu_to_le16(EXT4_ERROR_FS);
+
+	return 0;
+}
+
+static int ext4_replay_prepare_callback(struct journal_s *journal)
+{
+	struct super_block *sb = journal->j_private;
+	struct ext4_sb_info *sbi = EXT4_SB(sb);
+	char *private;
+	struct ext4_super_block *es = sbi->s_es;
+
+	if (!(sbi->s_mount_state & EXT4_ERROR_FS))
+		return 0;
+
+	private = kmalloc(EXT4_S_ERR_LEN, GFP_KERNEL);
+	if (!private)
+		return -ENOMEM;
+	memcpy(private, ((char *)es) + EXT4_S_ERR_START, EXT4_S_ERR_LEN);
+
+	journal->j_replay_private_data = private;
+	journal->j_replay_callback = ext4_replay_callback;
+	journal->j_replay_end_callback = ext4_replay_end_callback;
+
+	return 0;
+}
+
+void ext4_init_replay(journal_t *journal)
+{
+	journal->j_replay_prepare_callback = ext4_replay_prepare_callback;
+}
diff --git a/fs/ext4/ext4_jbd2.h b/fs/ext4/ext4_jbd2.h
index 0c77697d5e90..8dcc7ef5028c 100644
--- a/fs/ext4/ext4_jbd2.h
+++ b/fs/ext4/ext4_jbd2.h
@@ -513,4 +513,6 @@ static inline int ext4_should_dioread_nolock(struct inode *inode)
 	return 1;
 }
 
+void ext4_init_replay(journal_t *journal);
+
 #endif	/* _EXT4_JBD2_H */
diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index dc3907dff13a..ea0fea04907c 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -5677,6 +5677,7 @@ static void ext4_init_journal_params(struct super_block *sb, journal_t *journal)
 	journal->j_commit_interval = sbi->s_commit_interval;
 	journal->j_min_batch_time = sbi->s_min_batch_time;
 	journal->j_max_batch_time = sbi->s_max_batch_time;
+	ext4_init_replay(journal);
 	ext4_fc_init(sb, journal);
 
 	write_lock(&journal->j_state_lock);
-- 
2.31.1

Re: [PATCH 2/5] ext4: introudce helper for jounral recover handle

[kernel test robot]

Hi Ye,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on tytso-ext4/dev]
[also build test WARNING on linus/master v6.2-rc6 next-20230201]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Ye-Bin/jbd2-introduce-callback-for-recovery-journal/20230201-192400
base:   https://git.kernel.org/pub/scm/linux/kernel/git/tytso/ext4.git dev
patch link:    https://lore.kernel.org/r/20230201114651.4090446-3-yebin%40huaweicloud.com
patch subject: [PATCH 2/5] ext4: introudce helper for jounral recover handle
config: m68k-allyesconfig (https://download.01.org/0day-ci/archive/20230201/202302012039.pe8277UH-lkp@intel.com/config)
compiler: m68k-linux-gcc (GCC) 12.1.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/intel-lab-lkp/linux/commit/316c2a97e3b3629813acbac07b6a03d836a00fa1
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Ye-Bin/jbd2-introduce-callback-for-recovery-journal/20230201-192400
        git checkout 316c2a97e3b3629813acbac07b6a03d836a00fa1
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=m68k olddefconfig
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=m68k SHELL=/bin/bash fs/

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

>> fs/ext4/ext4_jbd2.c:399:6: warning: no previous prototype for 'ext4_replay_end_callback' [-Wmissing-prototypes]
     399 | void ext4_replay_end_callback(struct journal_s *journal)
         |      ^~~~~~~~~~~~~~~~~~~~~~~~


vim +/ext4_replay_end_callback +399 fs/ext4/ext4_jbd2.c

   398	
 > 399	void ext4_replay_end_callback(struct journal_s *journal)
   400	{
   401		kfree(journal->j_replay_private_data);
   402		journal->j_replay_private_data = NULL;
   403		journal->j_replay_callback = NULL;
   404		journal->j_replay_end_callback = NULL;
   405	}
   406	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests

Re: [PATCH 2/5] ext4: introudce helper for jounral recover handle

[kernel test robot]

Hi Ye,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on tytso-ext4/dev]
[also build test WARNING on linus/master v6.2-rc6 next-20230201]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Ye-Bin/jbd2-introduce-callback-for-recovery-journal/20230201-192400
base:   https://git.kernel.org/pub/scm/linux/kernel/git/tytso/ext4.git dev
patch link:    https://lore.kernel.org/r/20230201114651.4090446-3-yebin%40huaweicloud.com
patch subject: [PATCH 2/5] ext4: introudce helper for jounral recover handle
config: i386-randconfig-a013-20230130 (https://download.01.org/0day-ci/archive/20230202/202302020346.Oh31U8Zk-lkp@intel.com/config)
compiler: clang version 14.0.6 (https://github.com/llvm/llvm-project f28c006a5895fc0e329fe15fead81e37457cb1d1)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/intel-lab-lkp/linux/commit/316c2a97e3b3629813acbac07b6a03d836a00fa1
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Ye-Bin/jbd2-introduce-callback-for-recovery-journal/20230201-192400
        git checkout 316c2a97e3b3629813acbac07b6a03d836a00fa1
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=i386 olddefconfig
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=i386 SHELL=/bin/bash fs/ext4/ lib/

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

>> fs/ext4/ext4_jbd2.c:399:6: warning: no previous prototype for function 'ext4_replay_end_callback' [-Wmissing-prototypes]
   void ext4_replay_end_callback(struct journal_s *journal)
        ^
   fs/ext4/ext4_jbd2.c:399:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
   void ext4_replay_end_callback(struct journal_s *journal)
   ^
   static 
   1 warning generated.


vim +/ext4_replay_end_callback +399 fs/ext4/ext4_jbd2.c

   398	
 > 399	void ext4_replay_end_callback(struct journal_s *journal)
   400	{
   401		kfree(journal->j_replay_private_data);
   402		journal->j_replay_private_data = NULL;
   403		journal->j_replay_callback = NULL;
   404		journal->j_replay_end_callback = NULL;
   405	}
   406	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests

[PATCH 3/5] ext4: fix error flag covered by journal recovery

[Ye Bin]

From: Ye Bin <yebin10@huawei.com>

Super block in journal maybe not uptodate, when file system has
error, we need set error information when do recover super block.

Signed-off-by: Ye Bin <yebin10@huawei.com>
---
 fs/ext4/super.c    | 38 ++++++++++++++++----------------------
 fs/jbd2/recovery.c | 14 ++++++++++++++
 2 files changed, 30 insertions(+), 22 deletions(-)

diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index ea0fea04907c..ab136d08d93d 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -5887,7 +5887,7 @@ static int ext4_load_journal(struct super_block *sb,
 		ext4_msg(sb, KERN_ERR,
 			 "journal device read-only, try mounting with '-o ro'");
 		err = -EROFS;
-		goto err_out;
+		goto out;
 	}
 
 	/*
@@ -5904,7 +5904,7 @@ static int ext4_load_journal(struct super_block *sb,
 					"unavailable, cannot proceed "
 					"(try mounting with noload)");
 				err = -EROFS;
-				goto err_out;
+				goto out;
 			}
 			ext4_msg(sb, KERN_INFO, "write access will "
 			       "be enabled during recovery");
@@ -5917,29 +5917,21 @@ static int ext4_load_journal(struct super_block *sb,
 	if (!ext4_has_feature_journal_needs_recovery(sb))
 		err = jbd2_journal_wipe(journal, !really_read_only);
 	if (!err) {
-		char *save = kmalloc(EXT4_S_ERR_LEN, GFP_KERNEL);
-		if (save)
-			memcpy(save, ((char *) es) +
-			       EXT4_S_ERR_START, EXT4_S_ERR_LEN);
-		err = jbd2_journal_load(journal);
-		if (save)
-			memcpy(((char *) es) + EXT4_S_ERR_START,
-			       save, EXT4_S_ERR_LEN);
-		kfree(save);
+		if (journal->j_replay_prepare_callback)
+			err = journal->j_replay_prepare_callback(journal);
+		if (!err)
+			err = jbd2_journal_load(journal);
 	}
 
 	if (err) {
 		ext4_msg(sb, KERN_ERR, "error loading journal");
-		goto err_out;
+		goto out;
 	}
 
 	EXT4_SB(sb)->s_journal = journal;
 	err = ext4_clear_journal_err(sb, es);
-	if (err) {
-		EXT4_SB(sb)->s_journal = NULL;
-		jbd2_journal_destroy(journal);
-		return err;
-	}
+	if (err)
+		goto out;
 
 	if (!really_read_only && journal_devnum &&
 	    journal_devnum != le32_to_cpu(es->s_journal_dev)) {
@@ -5948,11 +5940,13 @@ static int ext4_load_journal(struct super_block *sb,
 		/* Make sure we flush the recovery flag to disk. */
 		ext4_commit_super(sb);
 	}
-
-	return 0;
-
-err_out:
-	jbd2_journal_destroy(journal);
+out:
+	if (unlikely(journal->j_replay_end_callback))
+		journal->j_replay_end_callback(journal);
+	if (err) {
+		EXT4_SB(sb)->s_journal = NULL;
+		jbd2_journal_destroy(journal);
+	}
 	return err;
 }
 
diff --git a/fs/jbd2/recovery.c b/fs/jbd2/recovery.c
index 8286a9ec122f..8175d0a8d5f7 100644
--- a/fs/jbd2/recovery.c
+++ b/fs/jbd2/recovery.c
@@ -687,6 +687,20 @@ static int do_one_pass(journal_t *journal,
 						*((__be32 *)nbh->b_data) =
 						cpu_to_be32(JBD2_MAGIC_NUMBER);
 					}
+					if (unlikely(journal->j_replay_callback)) {
+						err = journal->j_replay_callback(
+							journal, nbh);
+						if (err) {
+							printk(KERN_ERR
+							       "JBD2: replay "
+							       "call back "
+							       "failed.\n");
+							unlock_buffer(nbh);
+							brelse(obh);
+							brelse(nbh);
+							goto failed;
+						}
+					}
 
 					BUFFER_TRACE(nbh, "marking dirty");
 					set_buffer_uptodate(nbh);
-- 
2.31.1

[PATCH 4/5] ext4: fix super block checksum error

[Ye Bin]

From: Ye Bin <yebin10@huawei.com>

As commit("ext4: fix error flag covered by journal recovery") update
error record when do journal recovery.There is need to recalculate
super block checksum after update error record or will lead to super
block checksum mismatch to data.

Signed-off-by: Ye Bin <yebin10@huawei.com>
---
 fs/ext4/ext4_jbd2.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/fs/ext4/ext4_jbd2.c b/fs/ext4/ext4_jbd2.c
index 7c0f2bed0ec4..21f4f00429a1 100644
--- a/fs/ext4/ext4_jbd2.c
+++ b/fs/ext4/ext4_jbd2.c
@@ -430,6 +430,7 @@ static int ext4_replay_callback(struct journal_s *journal,
 	       journal->j_replay_private_data, EXT4_S_ERR_LEN);
 	if (sbi->s_mount_state & EXT4_ERROR_FS)
 		es->s_state |= cpu_to_le16(EXT4_ERROR_FS);
+	ext4_superblock_csum_set(sb);
 
 	return 0;
 }
-- 
2.31.1

[PATCH 5/5] ext4: make sure fs error flag setted before clear journal error

[Ye Bin]

From: Ye Bin <yebin10@huawei.com>

Now, jounral error number maybe cleared even though ext4_commit_super()
failed. This may lead to error flag miss, then fsck will miss to check
file system deeply.

Signed-off-by: Ye Bin <yebin10@huawei.com>
---
 fs/ext4/super.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index ab136d08d93d..c36cc9708258 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -6146,11 +6146,13 @@ static int ext4_clear_journal_err(struct super_block *sb,
 		errstr = ext4_decode_error(sb, j_errno, nbuf);
 		ext4_warning(sb, "Filesystem error recorded "
 			     "from previous mount: %s", errstr);
-		ext4_warning(sb, "Marking fs in need of filesystem check.");
 
 		EXT4_SB(sb)->s_mount_state |= EXT4_ERROR_FS;
 		es->s_state |= cpu_to_le16(EXT4_ERROR_FS);
-		ext4_commit_super(sb);
+		j_errno = ext4_commit_super(sb);
+		if (j_errno)
+			return j_errno;
+		ext4_warning(sb, "Marked fs in need of filesystem check.");
 
 		jbd2_journal_clear_err(journal);
 		jbd2_journal_update_sb_errno(journal);
-- 
2.31.1