All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH -next v3 0/5] Fix some issues about mmp
@ 2021-10-19  6:49 Ye Bin
  2021-10-19  6:49 ` [PATCH -next v3 1/5] ext4: init 'seq' with the value which set in 'ext4_multi_mount_protect' Ye Bin
                   ` (4 more replies)
  0 siblings, 5 replies; 12+ messages in thread
From: Ye Bin @ 2021-10-19  6:49 UTC (permalink / raw)
  To: tytso, adilger.kernel, linux-ext4; +Cc: linux-kernel, jack, Ye Bin

I test mmp function as follow steps:
1. Inject delay 5s in ext4_multi_mount_protect function after
"skip:" label.
2. Share HostA block device(sda) with HostB(nbd0) by NBD.
3. Enable mmp feature when mkfs.ext4 sda.
4. Mount sda and nbd0 at the same time.

I found kmmpd never trigger detect multi-mount. Reason is as follow:
1. Kmmpd init seq with 0, if two host have same nodename, will lead to
detect confliction very slow, even never detect confliction.
2. When detect confliction in kmmpd, we get 'check_bh' is same with 'bh'.
so we compare data with itself.
3. We only trigger detect when ”diff > mmp_check_interval * HZ“,
'mmp_check_interval' is double of 'mmp_update_interval', 'diff' is
about 'mmp_update_interval'. So 'diff' is little than 'mmp_check_interval * HZ'
normaly. As Jan Kara explain as follows:
"I think the check is there only for the case where write_mmp_block() +
sleep took longer than mmp_check_interval. I agree that should rarely
happen but on a really busy system it is possible and in that case we would
miss updating mmp block for too long and so another node could have started
using the filesystem. "

v1->v2:
Fix 'last_check_time' not initialized before checking.

v3->v2:
1. drop commit "ext4: introduce last_check_time record previous check time"
As Ted explain as follows:
"I'd like Andreas to comment here.  My understanding is that MMP
originally intended as a safety mechanism which would be used as part
of a primary/backup high availability system, but not as the *primary*
system where you might try to have two servers simultaneously try to
mount the file system and use MMP as the "election" mechanism to
decide which server is going to be the primary system, and which would
be the backup system.

The cost of being able to handle this particular race is it would slow
down the mounts of cleanly unmounted systems.

There *are* better systems to implement leader elections[1] than using
MMP.  Most of these more efficient leader elections assume that you
have a working IP network, and so if you have a separate storage
network (including a shared SCSI bus) from your standard IP network,
then MMP is a useful failsafe in the face of a network partition of
your IP network.  The question is whether MMP should be useful for
more than that.  And if it isn't, then we should probably document
what MMP is and isn't good for, and give advice in the form of an
application note for how MMP should be used in the context of a larger
system."
2. drop commit "ext4: fix possible store wrong check interval value in disk when umount"
3. simplify read_mmp_block fucntion to avoid UAF


Ye Bin (5):
  ext4: init 'seq' with the value which set in
    'ext4_multi_mount_protect'
  ext4: compare to local seq and nodename when check conflict
  ext4: get buffer head before read_mmp_block
  ext4: simplify read_mmp_block fucntion
  ext4: avoid to re-read mmp check data get from page cache

 fs/ext4/ext4.h |  5 +++-
 fs/ext4/mmp.c  | 67 ++++++++++++++++++++++----------------------------
 2 files changed, 33 insertions(+), 39 deletions(-)

-- 
2.31.1


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH -next v3 1/5] ext4: init 'seq' with the value which set in 'ext4_multi_mount_protect'
  2021-10-19  6:49 [PATCH -next v3 0/5] Fix some issues about mmp Ye Bin
@ 2021-10-19  6:49 ` Ye Bin
  2021-10-19  8:53   ` Jan Kara
  2021-10-19  6:49 ` [PATCH -next v3 2/5] ext4: compare to local seq and nodename when check conflict Ye Bin
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 12+ messages in thread
From: Ye Bin @ 2021-10-19  6:49 UTC (permalink / raw)
  To: tytso, adilger.kernel, linux-ext4; +Cc: linux-kernel, jack, Ye Bin

If two host have the same nodename, and seq start from 0, May cause the
detection mechanism to fail.
So init 'seq' with the value which set in 'ext4_multi_mount_protect' to
accelerate conflict detection.

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

diff --git a/fs/ext4/mmp.c b/fs/ext4/mmp.c
index cebea4270817..11627ff002d3 100644
--- a/fs/ext4/mmp.c
+++ b/fs/ext4/mmp.c
@@ -132,7 +132,7 @@ static int kmmpd(void *data)
 	struct buffer_head *bh = EXT4_SB(sb)->s_mmp_bh;
 	struct mmp_struct *mmp;
 	ext4_fsblk_t mmp_block;
-	u32 seq = 0;
+	u32 seq;
 	unsigned long failed_writes = 0;
 	int mmp_update_interval = le16_to_cpu(es->s_mmp_update_interval);
 	unsigned mmp_check_interval;
@@ -143,6 +143,7 @@ static int kmmpd(void *data)
 	mmp_block = le64_to_cpu(es->s_mmp_block);
 	mmp = (struct mmp_struct *)(bh->b_data);
 	mmp->mmp_time = cpu_to_le64(ktime_get_real_seconds());
+	seq = le32_to_cpu(mmp->mmp_seq);
 	/*
 	 * Start with the higher mmp_check_interval and reduce it if
 	 * the MMP block is being updated on time.
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH -next v3 2/5] ext4: compare to local seq and nodename when check conflict
  2021-10-19  6:49 [PATCH -next v3 0/5] Fix some issues about mmp Ye Bin
  2021-10-19  6:49 ` [PATCH -next v3 1/5] ext4: init 'seq' with the value which set in 'ext4_multi_mount_protect' Ye Bin
@ 2021-10-19  6:49 ` Ye Bin
  2021-10-19  8:54   ` Jan Kara
  2021-10-19  6:49 ` [PATCH -next v3 3/5] ext4: get buffer head before read_mmp_block Ye Bin
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 12+ messages in thread
From: Ye Bin @ 2021-10-19  6:49 UTC (permalink / raw)
  To: tytso, adilger.kernel, linux-ext4; +Cc: linux-kernel, jack, Ye Bin

As mmp and check_mmp is point to the same data, so there will never
detect conflict.
To solve this issue just compare to local data.

Signed-off-by: Ye Bin <yebin10@huawei.com>
---
 fs/ext4/ext4.h | 5 ++++-
 fs/ext4/mmp.c  | 9 +++++----
 2 files changed, 9 insertions(+), 5 deletions(-)

diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
index 404dd50856e5..9a487a558787 100644
--- a/fs/ext4/ext4.h
+++ b/fs/ext4/ext4.h
@@ -2601,6 +2601,8 @@ struct ext4_features {
 #define EXT4_MMP_SEQ_FSCK  0xE24D4D50U /* mmp_seq value when being fscked */
 #define EXT4_MMP_SEQ_MAX   0xE24D4D4FU /* maximum valid mmp_seq value */
 
+#define EXT4_MMP_NODENAME_LEN   64 /* mmp_nodename length */
+
 struct mmp_struct {
 	__le32	mmp_magic;		/* Magic number for MMP */
 	__le32	mmp_seq;		/* Sequence no. updated periodically */
@@ -2610,7 +2612,8 @@ struct mmp_struct {
 	 * purposes and do not affect the correctness of the algorithm
 	 */
 	__le64	mmp_time;		/* Time last updated */
-	char	mmp_nodename[64];	/* Node which last updated MMP block */
+	/* Node which last updated MMP block */
+	char	mmp_nodename[EXT4_MMP_NODENAME_LEN];
 	char	mmp_bdevname[32];	/* Bdev which last updated MMP block */
 
 	/*
diff --git a/fs/ext4/mmp.c b/fs/ext4/mmp.c
index 11627ff002d3..4af8b99ade84 100644
--- a/fs/ext4/mmp.c
+++ b/fs/ext4/mmp.c
@@ -138,6 +138,7 @@ static int kmmpd(void *data)
 	unsigned mmp_check_interval;
 	unsigned long last_update_time;
 	unsigned long diff;
+	char nodename[EXT4_MMP_NODENAME_LEN];
 	int retval = 0;
 
 	mmp_block = le64_to_cpu(es->s_mmp_block);
@@ -154,8 +155,8 @@ static int kmmpd(void *data)
 	BUILD_BUG_ON(sizeof(mmp->mmp_bdevname) < BDEVNAME_SIZE);
 	bdevname(bh->b_bdev, mmp->mmp_bdevname);
 
-	memcpy(mmp->mmp_nodename, init_utsname()->nodename,
-	       sizeof(mmp->mmp_nodename));
+	memcpy(nodename, init_utsname()->nodename, sizeof(nodename));
+	memcpy(mmp->mmp_nodename, nodename, sizeof(mmp->mmp_nodename));
 
 	while (!kthread_should_stop() && !sb_rdonly(sb)) {
 		if (!ext4_has_feature_mmp(sb)) {
@@ -207,8 +208,8 @@ static int kmmpd(void *data)
 			}
 
 			mmp_check = (struct mmp_struct *)(bh_check->b_data);
-			if (mmp->mmp_seq != mmp_check->mmp_seq ||
-			    memcmp(mmp->mmp_nodename, mmp_check->mmp_nodename,
+			if (seq != mmp_check->mmp_seq ||
+			    memcmp(nodename, mmp_check->mmp_nodename,
 				   sizeof(mmp->mmp_nodename))) {
 				dump_mmp_msg(sb, mmp_check,
 					     "Error while updating MMP info. "
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH -next v3 3/5] ext4: get buffer head before read_mmp_block
  2021-10-19  6:49 [PATCH -next v3 0/5] Fix some issues about mmp Ye Bin
  2021-10-19  6:49 ` [PATCH -next v3 1/5] ext4: init 'seq' with the value which set in 'ext4_multi_mount_protect' Ye Bin
  2021-10-19  6:49 ` [PATCH -next v3 2/5] ext4: compare to local seq and nodename when check conflict Ye Bin
@ 2021-10-19  6:49 ` Ye Bin
  2021-10-19  9:12   ` Jan Kara
  2021-10-19  6:49 ` [PATCH -next v3 4/5] ext4: simplify read_mmp_block fucntion Ye Bin
  2021-10-19  6:49 ` [PATCH -next v3 5/5] ext4: avoid to re-read mmp check data get from page cache Ye Bin
  4 siblings, 1 reply; 12+ messages in thread
From: Ye Bin @ 2021-10-19  6:49 UTC (permalink / raw)
  To: tytso, adilger.kernel, linux-ext4; +Cc: linux-kernel, jack, Ye Bin

There is only pass NULL 'bh' in ext4_multi_mount_protect,
So just call sb_getblk get buffer head fisrt, and we will
simplify read_mmp_block function.

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

diff --git a/fs/ext4/mmp.c b/fs/ext4/mmp.c
index 4af8b99ade84..6ac6aacd8fa5 100644
--- a/fs/ext4/mmp.c
+++ b/fs/ext4/mmp.c
@@ -295,6 +295,10 @@ int ext4_multi_mount_protect(struct super_block *sb,
 		goto failed;
 	}
 
+	bh = sb_getblk(sb, mmp_block);
+	if (bh)
+		goto failed;
+
 	retval = read_mmp_block(sb, &bh, mmp_block);
 	if (retval)
 		goto failed;
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH -next v3 4/5] ext4: simplify read_mmp_block fucntion
  2021-10-19  6:49 [PATCH -next v3 0/5] Fix some issues about mmp Ye Bin
                   ` (2 preceding siblings ...)
  2021-10-19  6:49 ` [PATCH -next v3 3/5] ext4: get buffer head before read_mmp_block Ye Bin
@ 2021-10-19  6:49 ` Ye Bin
  2021-10-19 13:13     ` kernel test robot
  2021-10-19  6:49 ` [PATCH -next v3 5/5] ext4: avoid to re-read mmp check data get from page cache Ye Bin
  4 siblings, 1 reply; 12+ messages in thread
From: Ye Bin @ 2021-10-19  6:49 UTC (permalink / raw)
  To: tytso, adilger.kernel, linux-ext4; +Cc: linux-kernel, jack, Ye Bin

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(-)

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);
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH -next v3 5/5] ext4: avoid to re-read mmp check data get from page cache
  2021-10-19  6:49 [PATCH -next v3 0/5] Fix some issues about mmp Ye Bin
                   ` (3 preceding siblings ...)
  2021-10-19  6:49 ` [PATCH -next v3 4/5] ext4: simplify read_mmp_block fucntion Ye Bin
@ 2021-10-19  6:49 ` Ye Bin
  2021-10-19  9:10   ` Jan Kara
  4 siblings, 1 reply; 12+ messages in thread
From: Ye Bin @ 2021-10-19  6:49 UTC (permalink / raw)
  To: tytso, adilger.kernel, linux-ext4; +Cc: linux-kernel, jack, Ye Bin

As call read_mmp_block pass bh_check which value is NULL, then call
sb_getblk to get buffer_head. But mmp_block's buffer_head is already exist
 which also is uptodate. Lead to compare the same data.

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

diff --git a/fs/ext4/mmp.c b/fs/ext4/mmp.c
index 61c765c249b9..8018f6fb6ac2 100644
--- a/fs/ext4/mmp.c
+++ b/fs/ext4/mmp.c
@@ -186,10 +186,7 @@ static int kmmpd(void *data)
 		 */
 		diff = jiffies - last_update_time;
 		if (diff > mmp_check_interval * HZ) {
-			struct buffer_head *bh_check = NULL;
-			struct mmp_struct *mmp_check;
-
-			retval = read_mmp_block(sb, bh_check);
+			retval = read_mmp_block(sb, bh);
 			if (retval) {
 				ext4_error_err(sb, -retval,
 					       "error reading MMP data: %d",
@@ -197,20 +194,18 @@ static int kmmpd(void *data)
 				goto wait_to_exit;
 			}
 
-			mmp_check = (struct mmp_struct *)(bh_check->b_data);
-			if (seq != mmp_check->mmp_seq ||
-			    memcmp(nodename, mmp_check->mmp_nodename,
-				   sizeof(mmp->mmp_nodename))) {
-				dump_mmp_msg(sb, mmp_check,
+			mmp = (struct mmp_struct *)(bh->b_data);
+			if (seq != le32_to_cpu(mmp->mmp_seq) ||
+			    memcmp(nodename, mmp->mmp_nodename,
+				    sizeof(nodename))) {
+				dump_mmp_msg(sb, mmp,
 					     "Error while updating MMP info. "
 					     "The filesystem seems to have been"
 					     " multiply mounted.");
 				ext4_error_err(sb, EBUSY, "abort");
-				put_bh(bh_check);
 				retval = -EBUSY;
 				goto wait_to_exit;
 			}
-			put_bh(bh_check);
 		}
 
 		 /*
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH -next v3 1/5] ext4: init 'seq' with the value which set in 'ext4_multi_mount_protect'
  2021-10-19  6:49 ` [PATCH -next v3 1/5] ext4: init 'seq' with the value which set in 'ext4_multi_mount_protect' Ye Bin
@ 2021-10-19  8:53   ` Jan Kara
  0 siblings, 0 replies; 12+ messages in thread
From: Jan Kara @ 2021-10-19  8:53 UTC (permalink / raw)
  To: Ye Bin; +Cc: tytso, adilger.kernel, linux-ext4, linux-kernel, jack

On Tue 19-10-21 14:49:55, Ye Bin wrote:
> If two host have the same nodename, and seq start from 0, May cause the
> detection mechanism to fail.
> So init 'seq' with the value which set in 'ext4_multi_mount_protect' to
> accelerate conflict detection.
> 
> Signed-off-by: Ye Bin <yebin10@huawei.com>

...

> @@ -143,6 +143,7 @@ static int kmmpd(void *data)
>  	mmp_block = le64_to_cpu(es->s_mmp_block);
>  	mmp = (struct mmp_struct *)(bh->b_data);
>  	mmp->mmp_time = cpu_to_le64(ktime_get_real_seconds());
> +	seq = le32_to_cpu(mmp->mmp_seq);
>  	/*
>  	 * Start with the higher mmp_check_interval and reduce it if
>  	 * the MMP block is being updated on time.

Thanks for the patch. After discussing what MMP guards against and what it
does not protect, I don't think this change is actually needed. Under
normal conditions we expect kmmpd() to only write to MMP block, checking of
MMP block is done only in ext4_multi_mount_protect(). And for checking
there to trigger, using 'seq' starting from 1 in kmmpd is enough...

								Honza

-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH -next v3 2/5] ext4: compare to local seq and nodename when check conflict
  2021-10-19  6:49 ` [PATCH -next v3 2/5] ext4: compare to local seq and nodename when check conflict Ye Bin
@ 2021-10-19  8:54   ` Jan Kara
  0 siblings, 0 replies; 12+ messages in thread
From: Jan Kara @ 2021-10-19  8:54 UTC (permalink / raw)
  To: Ye Bin; +Cc: tytso, adilger.kernel, linux-ext4, linux-kernel, jack

On Tue 19-10-21 14:49:56, Ye Bin wrote:
> As mmp and check_mmp is point to the same data, so there will never
> detect conflict.
> To solve this issue just compare to local data.
> 
> Signed-off-by: Ye Bin <yebin10@huawei.com>

Looks good. Feel free to add:

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

								Honza

> diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
> index 404dd50856e5..9a487a558787 100644
> --- a/fs/ext4/ext4.h
> +++ b/fs/ext4/ext4.h
> @@ -2601,6 +2601,8 @@ struct ext4_features {
>  #define EXT4_MMP_SEQ_FSCK  0xE24D4D50U /* mmp_seq value when being fscked */
>  #define EXT4_MMP_SEQ_MAX   0xE24D4D4FU /* maximum valid mmp_seq value */
>  
> +#define EXT4_MMP_NODENAME_LEN   64 /* mmp_nodename length */
> +
>  struct mmp_struct {
>  	__le32	mmp_magic;		/* Magic number for MMP */
>  	__le32	mmp_seq;		/* Sequence no. updated periodically */
> @@ -2610,7 +2612,8 @@ struct mmp_struct {
>  	 * purposes and do not affect the correctness of the algorithm
>  	 */
>  	__le64	mmp_time;		/* Time last updated */
> -	char	mmp_nodename[64];	/* Node which last updated MMP block */
> +	/* Node which last updated MMP block */
> +	char	mmp_nodename[EXT4_MMP_NODENAME_LEN];
>  	char	mmp_bdevname[32];	/* Bdev which last updated MMP block */
>  
>  	/*
> diff --git a/fs/ext4/mmp.c b/fs/ext4/mmp.c
> index 11627ff002d3..4af8b99ade84 100644
> --- a/fs/ext4/mmp.c
> +++ b/fs/ext4/mmp.c
> @@ -138,6 +138,7 @@ static int kmmpd(void *data)
>  	unsigned mmp_check_interval;
>  	unsigned long last_update_time;
>  	unsigned long diff;
> +	char nodename[EXT4_MMP_NODENAME_LEN];
>  	int retval = 0;
>  
>  	mmp_block = le64_to_cpu(es->s_mmp_block);
> @@ -154,8 +155,8 @@ static int kmmpd(void *data)
>  	BUILD_BUG_ON(sizeof(mmp->mmp_bdevname) < BDEVNAME_SIZE);
>  	bdevname(bh->b_bdev, mmp->mmp_bdevname);
>  
> -	memcpy(mmp->mmp_nodename, init_utsname()->nodename,
> -	       sizeof(mmp->mmp_nodename));
> +	memcpy(nodename, init_utsname()->nodename, sizeof(nodename));
> +	memcpy(mmp->mmp_nodename, nodename, sizeof(mmp->mmp_nodename));
>  
>  	while (!kthread_should_stop() && !sb_rdonly(sb)) {
>  		if (!ext4_has_feature_mmp(sb)) {
> @@ -207,8 +208,8 @@ static int kmmpd(void *data)
>  			}
>  
>  			mmp_check = (struct mmp_struct *)(bh_check->b_data);
> -			if (mmp->mmp_seq != mmp_check->mmp_seq ||
> -			    memcmp(mmp->mmp_nodename, mmp_check->mmp_nodename,
> +			if (seq != mmp_check->mmp_seq ||
> +			    memcmp(nodename, mmp_check->mmp_nodename,
>  				   sizeof(mmp->mmp_nodename))) {
>  				dump_mmp_msg(sb, mmp_check,
>  					     "Error while updating MMP info. "
> -- 
> 2.31.1
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH -next v3 5/5] ext4: avoid to re-read mmp check data get from page cache
  2021-10-19  6:49 ` [PATCH -next v3 5/5] ext4: avoid to re-read mmp check data get from page cache Ye Bin
@ 2021-10-19  9:10   ` Jan Kara
  0 siblings, 0 replies; 12+ messages in thread
From: Jan Kara @ 2021-10-19  9:10 UTC (permalink / raw)
  To: Ye Bin; +Cc: tytso, adilger.kernel, linux-ext4, linux-kernel, jack

On Tue 19-10-21 14:49:59, Ye Bin wrote:
> As call read_mmp_block pass bh_check which value is NULL, then call
> sb_getblk to get buffer_head. But mmp_block's buffer_head is already exist
>  which also is uptodate. Lead to compare the same data.
> 
> Signed-off-by: Ye Bin <yebin10@huawei.com>

This patch needs to go earlier in the series - just after patch 2.
Otherwise the read_mmp_block() cleanup is not correct. Otherwise it looks
good, just some language corrections:

ext4: remove useless bh_check variable

Since we initialize 'bh_check' to NULL and pass it to read_mmp_block(), that
function will just call sb_getblk() which will just return the buffer_head
we have in 'bh'. So just remove the pointless 'bh_check' variable and use
'bh' directly.

With this fixed feel free to add:

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

								Honza

> ---
>  fs/ext4/mmp.c | 17 ++++++-----------
>  1 file changed, 6 insertions(+), 11 deletions(-)
> 
> diff --git a/fs/ext4/mmp.c b/fs/ext4/mmp.c
> index 61c765c249b9..8018f6fb6ac2 100644
> --- a/fs/ext4/mmp.c
> +++ b/fs/ext4/mmp.c
> @@ -186,10 +186,7 @@ static int kmmpd(void *data)
>  		 */
>  		diff = jiffies - last_update_time;
>  		if (diff > mmp_check_interval * HZ) {
> -			struct buffer_head *bh_check = NULL;
> -			struct mmp_struct *mmp_check;
> -
> -			retval = read_mmp_block(sb, bh_check);
> +			retval = read_mmp_block(sb, bh);
>  			if (retval) {
>  				ext4_error_err(sb, -retval,
>  					       "error reading MMP data: %d",
> @@ -197,20 +194,18 @@ static int kmmpd(void *data)
>  				goto wait_to_exit;
>  			}
>  
> -			mmp_check = (struct mmp_struct *)(bh_check->b_data);
> -			if (seq != mmp_check->mmp_seq ||
> -			    memcmp(nodename, mmp_check->mmp_nodename,
> -				   sizeof(mmp->mmp_nodename))) {
> -				dump_mmp_msg(sb, mmp_check,
> +			mmp = (struct mmp_struct *)(bh->b_data);
> +			if (seq != le32_to_cpu(mmp->mmp_seq) ||
> +			    memcmp(nodename, mmp->mmp_nodename,
> +				    sizeof(nodename))) {
> +				dump_mmp_msg(sb, mmp,
>  					     "Error while updating MMP info. "
>  					     "The filesystem seems to have been"
>  					     " multiply mounted.");
>  				ext4_error_err(sb, EBUSY, "abort");
> -				put_bh(bh_check);
>  				retval = -EBUSY;
>  				goto wait_to_exit;
>  			}
> -			put_bh(bh_check);
>  		}
>  
>  		 /*
> -- 
> 2.31.1
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH -next v3 3/5] ext4: get buffer head before read_mmp_block
  2021-10-19  6:49 ` [PATCH -next v3 3/5] ext4: get buffer head before read_mmp_block Ye Bin
@ 2021-10-19  9:12   ` Jan Kara
  0 siblings, 0 replies; 12+ messages in thread
From: Jan Kara @ 2021-10-19  9:12 UTC (permalink / raw)
  To: Ye Bin; +Cc: tytso, adilger.kernel, linux-ext4, linux-kernel, jack

On Tue 19-10-21 14:49:57, Ye Bin wrote:
> There is only pass NULL 'bh' in ext4_multi_mount_protect,
> So just call sb_getblk get buffer head fisrt, and we will
> simplify read_mmp_block function.
> 
> Signed-off-by: Ye Bin <yebin10@huawei.com>

I don't think there's a need to separate this into a special patch. Just
fold this change into patch 4. With that feel free to add:

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

The combined change looks good to me.

								Honza

> ---
>  fs/ext4/mmp.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/fs/ext4/mmp.c b/fs/ext4/mmp.c
> index 4af8b99ade84..6ac6aacd8fa5 100644
> --- a/fs/ext4/mmp.c
> +++ b/fs/ext4/mmp.c
> @@ -295,6 +295,10 @@ int ext4_multi_mount_protect(struct super_block *sb,
>  		goto failed;
>  	}
>  
> +	bh = sb_getblk(sb, mmp_block);
> +	if (bh)
> +		goto failed;
> +
>  	retval = read_mmp_block(sb, &bh, mmp_block);
>  	if (retval)
>  		goto failed;
> -- 
> 2.31.1
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH -next v3 4/5] ext4: simplify read_mmp_block fucntion
  2021-10-19  6:49 ` [PATCH -next v3 4/5] ext4: simplify read_mmp_block fucntion Ye Bin
@ 2021-10-19 13:13     ` kernel test robot
  0 siblings, 0 replies; 12+ messages in thread
From: kernel test robot @ 2021-10-19 13:13 UTC (permalink / raw)
  To: Ye Bin, tytso, adilger.kernel, linux-ext4
  Cc: llvm, kbuild-all, linux-kernel, jack, Ye Bin

[-- Attachment #1: Type: text/plain, Size: 11822 bytes --]

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

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 37352 bytes --]

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH -next v3 4/5] ext4: simplify read_mmp_block fucntion
@ 2021-10-19 13:13     ` kernel test robot
  0 siblings, 0 replies; 12+ messages in thread
From: kernel test robot @ 2021-10-19 13:13 UTC (permalink / raw)
  To: kbuild-all

[-- Attachment #1: Type: text/plain, Size: 11991 bytes --]

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(a)lists.01.org

[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 37352 bytes --]

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2021-10-19 13:46 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-10-19  6:49 [PATCH -next v3 0/5] Fix some issues about mmp Ye Bin
2021-10-19  6:49 ` [PATCH -next v3 1/5] ext4: init 'seq' with the value which set in 'ext4_multi_mount_protect' Ye Bin
2021-10-19  8:53   ` Jan Kara
2021-10-19  6:49 ` [PATCH -next v3 2/5] ext4: compare to local seq and nodename when check conflict Ye Bin
2021-10-19  8:54   ` Jan Kara
2021-10-19  6:49 ` [PATCH -next v3 3/5] ext4: get buffer head before read_mmp_block Ye Bin
2021-10-19  9:12   ` Jan Kara
2021-10-19  6:49 ` [PATCH -next v3 4/5] ext4: simplify read_mmp_block fucntion Ye Bin
2021-10-19 13:13   ` kernel test robot
2021-10-19 13:13     ` kernel test robot
2021-10-19  6:49 ` [PATCH -next v3 5/5] ext4: avoid to re-read mmp check data get from page cache Ye Bin
2021-10-19  9:10   ` Jan Kara

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.