diff mbox series

[-next,v3,4/5] ext4: simplify read_mmp_block fucntion

Message ID 20211019064959.625557-5-yebin10@huawei.com
State New
Headers show
Series Fix some issues about mmp | expand

Commit Message

Ye Bin Oct. 19, 2021, 6:49 a.m. UTC 
This patch is according to Jan Kara's suggestion:
I guess I would just get rid of sb_getblk() in read_mmp_block() and always
expect valid bh passed. The only place that passes NULL bh after this
patch is one case in ext4_multi_mount_protect() and that can call
sb_getblk() on its own. That way we can also simplify read_mmp_block()
prototype to:

static int read_mmp_block(struct super_block *sb, struct buffer_head *bh);

Signed-off-by: Ye Bin <yebin10@huawei.com>
---
 fs/ext4/mmp.c | 40 +++++++++++++++-------------------------
 1 file changed, 15 insertions(+), 25 deletions(-)

Comments

kernel test robot Oct. 19, 2021, 1:13 p.m. UTC | #1 
Hi Ye,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on next-20211018]

url:    https://github.com/0day-ci/linux/commits/Ye-Bin/Fix-some-issues-about-mmp/20211019-143859
base:    60e8840126bdcb60bccef74c3f962742183c681f
config: i386-randconfig-a001-20211019 (attached as .config)
compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project b37efed957ed0a0193d80020aefd55cb587dfc1f)
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/0day-ci/linux/commit/0f118633b71dacebbf7b01cd9ce4a2ed5d3aad0e
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Ye-Bin/Fix-some-issues-about-mmp/20211019-143859
        git checkout 0f118633b71dacebbf7b01cd9ce4a2ed5d3aad0e
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 ARCH=i386 

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

All warnings (new ones prefixed by >>):

>> fs/ext4/mmp.c:124:15: warning: variable 'mmp_block' set but not used [-Wunused-but-set-variable]
           ext4_fsblk_t mmp_block;
                        ^
   1 warning generated.


vim +/mmp_block +124 fs/ext4/mmp.c

c5e06d101aaf72 Johann Lombardi   2011-05-24  114  
c5e06d101aaf72 Johann Lombardi   2011-05-24  115  /*
c5e06d101aaf72 Johann Lombardi   2011-05-24  116   * kmmpd will update the MMP sequence every s_mmp_update_interval seconds
c5e06d101aaf72 Johann Lombardi   2011-05-24  117   */
c5e06d101aaf72 Johann Lombardi   2011-05-24  118  static int kmmpd(void *data)
c5e06d101aaf72 Johann Lombardi   2011-05-24  119  {
618f003199c618 Pavel Skripkin    2021-04-30  120  	struct super_block *sb = (struct super_block *) data;
c5e06d101aaf72 Johann Lombardi   2011-05-24  121  	struct ext4_super_block *es = EXT4_SB(sb)->s_es;
618f003199c618 Pavel Skripkin    2021-04-30  122  	struct buffer_head *bh = EXT4_SB(sb)->s_mmp_bh;
c5e06d101aaf72 Johann Lombardi   2011-05-24  123  	struct mmp_struct *mmp;
c5e06d101aaf72 Johann Lombardi   2011-05-24 @124  	ext4_fsblk_t mmp_block;
2b19579c262011 Ye Bin            2021-10-19  125  	u32 seq;
c5e06d101aaf72 Johann Lombardi   2011-05-24  126  	unsigned long failed_writes = 0;
c5e06d101aaf72 Johann Lombardi   2011-05-24  127  	int mmp_update_interval = le16_to_cpu(es->s_mmp_update_interval);
c5e06d101aaf72 Johann Lombardi   2011-05-24  128  	unsigned mmp_check_interval;
c5e06d101aaf72 Johann Lombardi   2011-05-24  129  	unsigned long last_update_time;
c5e06d101aaf72 Johann Lombardi   2011-05-24  130  	unsigned long diff;
70bae8f45abfe9 Ye Bin            2021-10-19  131  	char nodename[EXT4_MMP_NODENAME_LEN];
b66541422824cf Ye Bin            2021-07-13  132  	int retval = 0;
c5e06d101aaf72 Johann Lombardi   2011-05-24  133  
c5e06d101aaf72 Johann Lombardi   2011-05-24  134  	mmp_block = le64_to_cpu(es->s_mmp_block);
c5e06d101aaf72 Johann Lombardi   2011-05-24  135  	mmp = (struct mmp_struct *)(bh->b_data);
af123b3718592a Arnd Bergmann     2018-07-29  136  	mmp->mmp_time = cpu_to_le64(ktime_get_real_seconds());
2b19579c262011 Ye Bin            2021-10-19  137  	seq = le32_to_cpu(mmp->mmp_seq);
c5e06d101aaf72 Johann Lombardi   2011-05-24  138  	/*
c5e06d101aaf72 Johann Lombardi   2011-05-24  139  	 * Start with the higher mmp_check_interval and reduce it if
c5e06d101aaf72 Johann Lombardi   2011-05-24  140  	 * the MMP block is being updated on time.
c5e06d101aaf72 Johann Lombardi   2011-05-24  141  	 */
c5e06d101aaf72 Johann Lombardi   2011-05-24  142  	mmp_check_interval = max(EXT4_MMP_CHECK_MULT * mmp_update_interval,
c5e06d101aaf72 Johann Lombardi   2011-05-24  143  				 EXT4_MMP_MIN_CHECK_INTERVAL);
c5e06d101aaf72 Johann Lombardi   2011-05-24  144  	mmp->mmp_check_interval = cpu_to_le16(mmp_check_interval);
14c9ca0583eee8 Andreas Dilger    2020-01-26  145  	BUILD_BUG_ON(sizeof(mmp->mmp_bdevname) < BDEVNAME_SIZE);
c5e06d101aaf72 Johann Lombardi   2011-05-24  146  	bdevname(bh->b_bdev, mmp->mmp_bdevname);
c5e06d101aaf72 Johann Lombardi   2011-05-24  147  
70bae8f45abfe9 Ye Bin            2021-10-19  148  	memcpy(nodename, init_utsname()->nodename, sizeof(nodename));
70bae8f45abfe9 Ye Bin            2021-10-19  149  	memcpy(mmp->mmp_nodename, nodename, sizeof(mmp->mmp_nodename));
c5e06d101aaf72 Johann Lombardi   2011-05-24  150  
61bb4a1c417e5b Theodore Ts'o     2021-07-02  151  	while (!kthread_should_stop() && !sb_rdonly(sb)) {
61bb4a1c417e5b Theodore Ts'o     2021-07-02  152  		if (!ext4_has_feature_mmp(sb)) {
61bb4a1c417e5b Theodore Ts'o     2021-07-02  153  			ext4_warning(sb, "kmmpd being stopped since MMP feature"
61bb4a1c417e5b Theodore Ts'o     2021-07-02  154  				     " has been disabled.");
61bb4a1c417e5b Theodore Ts'o     2021-07-02  155  			goto wait_to_exit;
61bb4a1c417e5b Theodore Ts'o     2021-07-02  156  		}
c5e06d101aaf72 Johann Lombardi   2011-05-24  157  		if (++seq > EXT4_MMP_SEQ_MAX)
c5e06d101aaf72 Johann Lombardi   2011-05-24  158  			seq = 1;
c5e06d101aaf72 Johann Lombardi   2011-05-24  159  
c5e06d101aaf72 Johann Lombardi   2011-05-24  160  		mmp->mmp_seq = cpu_to_le32(seq);
af123b3718592a Arnd Bergmann     2018-07-29  161  		mmp->mmp_time = cpu_to_le64(ktime_get_real_seconds());
c5e06d101aaf72 Johann Lombardi   2011-05-24  162  		last_update_time = jiffies;
c5e06d101aaf72 Johann Lombardi   2011-05-24  163  
5c359a47e7d999 Darrick J. Wong   2012-04-29  164  		retval = write_mmp_block(sb, bh);
c5e06d101aaf72 Johann Lombardi   2011-05-24  165  		/*
c5e06d101aaf72 Johann Lombardi   2011-05-24  166  		 * Don't spew too many error messages. Print one every
c5e06d101aaf72 Johann Lombardi   2011-05-24  167  		 * (s_mmp_update_interval * 60) seconds.
c5e06d101aaf72 Johann Lombardi   2011-05-24  168  		 */
bdfc230f33a9da Nikitas Angelinas 2011-10-18  169  		if (retval) {
878520ac45f9f6 Theodore Ts'o     2019-11-19  170  			if ((failed_writes % 60) == 0) {
54d3adbc29f0c7 Theodore Ts'o     2020-03-28  171  				ext4_error_err(sb, -retval,
54d3adbc29f0c7 Theodore Ts'o     2020-03-28  172  					       "Error writing to MMP block");
878520ac45f9f6 Theodore Ts'o     2019-11-19  173  			}
c5e06d101aaf72 Johann Lombardi   2011-05-24  174  			failed_writes++;
c5e06d101aaf72 Johann Lombardi   2011-05-24  175  		}
c5e06d101aaf72 Johann Lombardi   2011-05-24  176  
c5e06d101aaf72 Johann Lombardi   2011-05-24  177  		diff = jiffies - last_update_time;
c5e06d101aaf72 Johann Lombardi   2011-05-24  178  		if (diff < mmp_update_interval * HZ)
c5e06d101aaf72 Johann Lombardi   2011-05-24  179  			schedule_timeout_interruptible(mmp_update_interval *
c5e06d101aaf72 Johann Lombardi   2011-05-24  180  						       HZ - diff);
c5e06d101aaf72 Johann Lombardi   2011-05-24  181  
c5e06d101aaf72 Johann Lombardi   2011-05-24  182  		/*
c5e06d101aaf72 Johann Lombardi   2011-05-24  183  		 * We need to make sure that more than mmp_check_interval
c5e06d101aaf72 Johann Lombardi   2011-05-24  184  		 * seconds have not passed since writing. If that has happened
c5e06d101aaf72 Johann Lombardi   2011-05-24  185  		 * we need to check if the MMP block is as we left it.
c5e06d101aaf72 Johann Lombardi   2011-05-24  186  		 */
c5e06d101aaf72 Johann Lombardi   2011-05-24  187  		diff = jiffies - last_update_time;
c5e06d101aaf72 Johann Lombardi   2011-05-24  188  		if (diff > mmp_check_interval * HZ) {
c5e06d101aaf72 Johann Lombardi   2011-05-24  189  			struct buffer_head *bh_check = NULL;
c5e06d101aaf72 Johann Lombardi   2011-05-24  190  			struct mmp_struct *mmp_check;
c5e06d101aaf72 Johann Lombardi   2011-05-24  191  
0f118633b71dac Ye Bin            2021-10-19  192  			retval = read_mmp_block(sb, bh_check);
c5e06d101aaf72 Johann Lombardi   2011-05-24  193  			if (retval) {
54d3adbc29f0c7 Theodore Ts'o     2020-03-28  194  				ext4_error_err(sb, -retval,
54d3adbc29f0c7 Theodore Ts'o     2020-03-28  195  					       "error reading MMP data: %d",
c5e06d101aaf72 Johann Lombardi   2011-05-24  196  					       retval);
61bb4a1c417e5b Theodore Ts'o     2021-07-02  197  				goto wait_to_exit;
c5e06d101aaf72 Johann Lombardi   2011-05-24  198  			}
c5e06d101aaf72 Johann Lombardi   2011-05-24  199  
c5e06d101aaf72 Johann Lombardi   2011-05-24  200  			mmp_check = (struct mmp_struct *)(bh_check->b_data);
70bae8f45abfe9 Ye Bin            2021-10-19  201  			if (seq != mmp_check->mmp_seq ||
70bae8f45abfe9 Ye Bin            2021-10-19  202  			    memcmp(nodename, mmp_check->mmp_nodename,
c5e06d101aaf72 Johann Lombardi   2011-05-24  203  				   sizeof(mmp->mmp_nodename))) {
c5e06d101aaf72 Johann Lombardi   2011-05-24  204  				dump_mmp_msg(sb, mmp_check,
c5e06d101aaf72 Johann Lombardi   2011-05-24  205  					     "Error while updating MMP info. "
c5e06d101aaf72 Johann Lombardi   2011-05-24  206  					     "The filesystem seems to have been"
c5e06d101aaf72 Johann Lombardi   2011-05-24  207  					     " multiply mounted.");
54d3adbc29f0c7 Theodore Ts'o     2020-03-28  208  				ext4_error_err(sb, EBUSY, "abort");
0304688676bdfc vikram.jadhav07   2016-03-13  209  				put_bh(bh_check);
0304688676bdfc vikram.jadhav07   2016-03-13  210  				retval = -EBUSY;
61bb4a1c417e5b Theodore Ts'o     2021-07-02  211  				goto wait_to_exit;
c5e06d101aaf72 Johann Lombardi   2011-05-24  212  			}
c5e06d101aaf72 Johann Lombardi   2011-05-24  213  			put_bh(bh_check);
c5e06d101aaf72 Johann Lombardi   2011-05-24  214  		}
c5e06d101aaf72 Johann Lombardi   2011-05-24  215  
c5e06d101aaf72 Johann Lombardi   2011-05-24  216  		 /*
c5e06d101aaf72 Johann Lombardi   2011-05-24  217  		 * Adjust the mmp_check_interval depending on how much time
c5e06d101aaf72 Johann Lombardi   2011-05-24  218  		 * it took for the MMP block to be written.
c5e06d101aaf72 Johann Lombardi   2011-05-24  219  		 */
c5e06d101aaf72 Johann Lombardi   2011-05-24  220  		mmp_check_interval = max(min(EXT4_MMP_CHECK_MULT * diff / HZ,
c5e06d101aaf72 Johann Lombardi   2011-05-24  221  					     EXT4_MMP_MAX_CHECK_INTERVAL),
c5e06d101aaf72 Johann Lombardi   2011-05-24  222  					 EXT4_MMP_MIN_CHECK_INTERVAL);
c5e06d101aaf72 Johann Lombardi   2011-05-24  223  		mmp->mmp_check_interval = cpu_to_le16(mmp_check_interval);
c5e06d101aaf72 Johann Lombardi   2011-05-24  224  	}
c5e06d101aaf72 Johann Lombardi   2011-05-24  225  
c5e06d101aaf72 Johann Lombardi   2011-05-24  226  	/*
c5e06d101aaf72 Johann Lombardi   2011-05-24  227  	 * Unmount seems to be clean.
c5e06d101aaf72 Johann Lombardi   2011-05-24  228  	 */
c5e06d101aaf72 Johann Lombardi   2011-05-24  229  	mmp->mmp_seq = cpu_to_le32(EXT4_MMP_SEQ_CLEAN);
af123b3718592a Arnd Bergmann     2018-07-29  230  	mmp->mmp_time = cpu_to_le64(ktime_get_real_seconds());
c5e06d101aaf72 Johann Lombardi   2011-05-24  231  
5c359a47e7d999 Darrick J. Wong   2012-04-29  232  	retval = write_mmp_block(sb, bh);
c5e06d101aaf72 Johann Lombardi   2011-05-24  233  
61bb4a1c417e5b Theodore Ts'o     2021-07-02  234  wait_to_exit:
61bb4a1c417e5b Theodore Ts'o     2021-07-02  235  	while (!kthread_should_stop()) {
61bb4a1c417e5b Theodore Ts'o     2021-07-02  236  		set_current_state(TASK_INTERRUPTIBLE);
61bb4a1c417e5b Theodore Ts'o     2021-07-02  237  		if (!kthread_should_stop())
61bb4a1c417e5b Theodore Ts'o     2021-07-02  238  			schedule();
61bb4a1c417e5b Theodore Ts'o     2021-07-02  239  	}
61bb4a1c417e5b Theodore Ts'o     2021-07-02  240  	set_current_state(TASK_RUNNING);
c5e06d101aaf72 Johann Lombardi   2011-05-24  241  	return retval;
c5e06d101aaf72 Johann Lombardi   2011-05-24  242  }
c5e06d101aaf72 Johann Lombardi   2011-05-24  243  

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
diff mbox series

Patch

diff --git a/fs/ext4/mmp.c b/fs/ext4/mmp.c
index 6ac6aacd8fa5..61c765c249b9 100644
--- a/fs/ext4/mmp.c
+++ b/fs/ext4/mmp.c
@@ -64,33 +64,26 @@  static int write_mmp_block(struct super_block *sb, struct buffer_head *bh)
 /*
  * Read the MMP block. It _must_ be read from disk and hence we clear the
  * uptodate flag on the buffer.
+ * Caller must ensure pass valid 'bh'.
  */
-static int read_mmp_block(struct super_block *sb, struct buffer_head **bh,
-			  ext4_fsblk_t mmp_block)
+static int read_mmp_block(struct super_block *sb, struct buffer_head *bh)
 {
 	struct mmp_struct *mmp;
 	int ret;
 
-	if (*bh)
-		clear_buffer_uptodate(*bh);
-
-	/* This would be sb_bread(sb, mmp_block), except we need to be sure
-	 * that the MD RAID device cache has been bypassed, and that the read
-	 * is not blocked in the elevator. */
-	if (!*bh) {
-		*bh = sb_getblk(sb, mmp_block);
-		if (!*bh) {
-			ret = -ENOMEM;
-			goto warn_exit;
-		}
+	if (!bh) {
+		ret = -EINVAL;
+		goto warn_exit;
 	}
 
-	lock_buffer(*bh);
-	ret = ext4_read_bh(*bh, REQ_META | REQ_PRIO, NULL);
+	clear_buffer_uptodate(bh);
+
+	lock_buffer(bh);
+	ret = ext4_read_bh(bh, REQ_META | REQ_PRIO, NULL);
 	if (ret)
 		goto warn_exit;
 
-	mmp = (struct mmp_struct *)((*bh)->b_data);
+	mmp = (struct mmp_struct *)((bh)->b_data);
 	if (le32_to_cpu(mmp->mmp_magic) != EXT4_MMP_MAGIC) {
 		ret = -EFSCORRUPTED;
 		goto warn_exit;
@@ -101,10 +94,7 @@  static int read_mmp_block(struct super_block *sb, struct buffer_head **bh,
 	}
 	return 0;
 warn_exit:
-	brelse(*bh);
-	*bh = NULL;
-	ext4_warning(sb, "Error %d while reading MMP block %llu",
-		     ret, mmp_block);
+	ext4_warning(sb, "Error %d while reading MMP block", ret);
 	return ret;
 }
 
@@ -199,7 +189,7 @@  static int kmmpd(void *data)
 			struct buffer_head *bh_check = NULL;
 			struct mmp_struct *mmp_check;
 
-			retval = read_mmp_block(sb, &bh_check, mmp_block);
+			retval = read_mmp_block(sb, bh_check);
 			if (retval) {
 				ext4_error_err(sb, -retval,
 					       "error reading MMP data: %d",
@@ -299,7 +289,7 @@  int ext4_multi_mount_protect(struct super_block *sb,
 	if (bh)
 		goto failed;
 
-	retval = read_mmp_block(sb, &bh, mmp_block);
+	retval = read_mmp_block(sb, bh);
 	if (retval)
 		goto failed;
 
@@ -337,7 +327,7 @@  int ext4_multi_mount_protect(struct super_block *sb,
 		goto failed;
 	}
 
-	retval = read_mmp_block(sb, &bh, mmp_block);
+	retval = read_mmp_block(sb, bh);
 	if (retval)
 		goto failed;
 	mmp = (struct mmp_struct *)(bh->b_data);
@@ -366,7 +356,7 @@  int ext4_multi_mount_protect(struct super_block *sb,
 		goto failed;
 	}
 
-	retval = read_mmp_block(sb, &bh, mmp_block);
+	retval = read_mmp_block(sb, bh);
 	if (retval)
 		goto failed;
 	mmp = (struct mmp_struct *)(bh->b_data);