[PATCH RFC] f2fs: recovering broken superblock during mount

[PATCH RFC] f2fs: recovering broken superblock during mount

[hujianyang]

Hi Jaegeuk,

There are two superblocks in f2fs. I wrapped one of them by 'dd' in
a reliability test. After this, the polluted f2fs partition can be
mounted with an error message:

F2FS-fs (sdd3): Magic Mismatch, valid(0xf2f52010) - read(0x0)
F2FS-fs (sdd3): Can't find valid F2FS filesystem in 1th superblock

Seems the broken superblock can't be recovered by f2fs driver or
fsck.f2fs. Tell me how to recover if I was wrong.:)

So I'd like to provide a patch to recover broken f2fs superblock
during mount. But I'm not pleased with my patch. I wish you and
others could give me some suggestions.

Do you think it's necessary to recover the superblock? and can
we change 'f2fs_commit_super' a little to reuse this function
during superblock recovery?

Thanks,
Hu


diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
index ae14fc4..126f8fa 100644
--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -965,6 +965,33 @@ static void init_sb_info(struct f2fs_sb_info *sbi)
 	clear_sbi_flag(sbi, SBI_NEED_FSCK);
 }

+static int get_valid_super_block(struct super_block *sb,
+			struct buffer_head **buffer, int block)
+{
+	struct f2fs_super_block *super;
+
+	*buffer = sb_bread(sb, block);
+	if (!*buffer) {
+		f2fs_msg(sb, KERN_ERR, "Unable to read %dth superblock",
+				block + 1);
+		return -EIO;
+	}
+
+	super = (struct f2fs_super_block *)
+		((char *)(*buffer)->b_data + F2FS_SUPER_OFFSET);
+
+	/* sanity checking of raw super */
+	if (sanity_check_raw_super(sb, super)) {
+		brelse(*buffer);
+		f2fs_msg(sb, KERN_ERR,
+			"Can't find valid F2FS filesystem in %dth superblock",
+								block + 1);
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
 /*
  * Read f2fs raw super block.
  * Because we have two copies of super block, so read the first one at first,
@@ -975,37 +1002,41 @@ static int read_raw_super_block(struct super_block *sb,
 			struct buffer_head **raw_super_buf)
 {
 	int block = 0;
-
-retry:
-	*raw_super_buf = sb_bread(sb, block);
-	if (!*raw_super_buf) {
-		f2fs_msg(sb, KERN_ERR, "Unable to read %dth superblock",
-				block + 1);
-		if (block == 0) {
-			block++;
-			goto retry;
-		} else {
-			return -EIO;
-		}
+	struct buffer_head *buffer0 = NULL, *buffer1 = NULL;
+	bool readonly = f2fs_readonly(sb) || bdev_read_only(sb->s_bdev);
+	int err0, err1;
+
+	err0 = get_valid_super_block(sb, &buffer0, 0);
+	if (!err0 && readonly)
+		goto set_raw_super;
+
+	err1 = get_valid_super_block(sb, &buffer1, 1);
+	if (err0 && err1)
+		return err1;
+
+	if (err0 && !err1) {
+		buffer0 = buffer1;
+		if (readonly)
+			goto set_raw_super;
+	} else if (!err0 && !err1) {
+		brelse(buffer1);
+		goto set_raw_super;
 	}

+	f2fs_msg(sb, KERN_INFO, "Recover superblock");
+	block = buffer0->b_blocknr;
+	buffer0->b_blocknr = block ? 0 : 1;
+	mark_buffer_dirty(buffer0);
+	sync_dirty_buffer(buffer0);
+	buffer0->b_blocknr = block;
+	clear_buffer_write_io_error(buffer0);
+	set_buffer_uptodate(buffer0);
+
+set_raw_super:
+	*raw_super_buf = buffer0;
 	*raw_super = (struct f2fs_super_block *)
 		((char *)(*raw_super_buf)->b_data + F2FS_SUPER_OFFSET);

-	/* sanity checking of raw super */
-	if (sanity_check_raw_super(sb, *raw_super)) {
-		brelse(*raw_super_buf);
-		f2fs_msg(sb, KERN_ERR,
-			"Can't find valid F2FS filesystem in %dth superblock",
-								block + 1);
-		if (block == 0) {
-			block++;
-			goto retry;
-		} else {
-			return -EINVAL;
-		}
-	}
-
 	return 0;
 }


------------------------------------------------------------------------------
One dashboard for servers and applications across Physical-Virtual-Cloud 
Widest out-of-the-box monitoring support with 50+ applications
Performance metrics, stats and reports that give you Actionable Insights
Deep dive visibility with transaction tracing using APM Insight.
http://ad.doubleclick.net/ddm/clk/290420510;117567292;y

Re: [PATCH RFC] f2fs: recovering broken superblock during mount

[Jaegeuk Kim]

Hi Hu,

On Wed, May 20, 2015 at 03:01:04PM +0800, hujianyang wrote:
> Hi Jaegeuk,
> 
> There are two superblocks in f2fs. I wrapped one of them by 'dd' in
> a reliability test. After this, the polluted f2fs partition can be
> mounted with an error message:
> 
> F2FS-fs (sdd3): Magic Mismatch, valid(0xf2f52010) - read(0x0)
> F2FS-fs (sdd3): Can't find valid F2FS filesystem in 1th superblock
> 
> Seems the broken superblock can't be recovered by f2fs driver or
> fsck.f2fs. Tell me how to recover if I was wrong.:)
> 
> So I'd like to provide a patch to recover broken f2fs superblock
> during mount. But I'm not pleased with my patch. I wish you and
> others could give me some suggestions.
> 
> Do you think it's necessary to recover the superblock? and can
> we change 'f2fs_commit_super' a little to reuse this function
> during superblock recovery?

What is the real problem in the original code?
It's the message problem, isn't it?
Even though the first superblock is missing with that message, mount must be
done with the second superblock, right?

If so, it would be good to use f2fs_commit_super to recover that and leave some
messages.

Thanks,

> 
> Thanks,
> Hu
> 
> 
> diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
> index ae14fc4..126f8fa 100644
> --- a/fs/f2fs/super.c
> +++ b/fs/f2fs/super.c
> @@ -965,6 +965,33 @@ static void init_sb_info(struct f2fs_sb_info *sbi)
>  	clear_sbi_flag(sbi, SBI_NEED_FSCK);
>  }
> 
> +static int get_valid_super_block(struct super_block *sb,
> +			struct buffer_head **buffer, int block)
> +{
> +	struct f2fs_super_block *super;
> +
> +	*buffer = sb_bread(sb, block);
> +	if (!*buffer) {
> +		f2fs_msg(sb, KERN_ERR, "Unable to read %dth superblock",
> +				block + 1);
> +		return -EIO;
> +	}
> +
> +	super = (struct f2fs_super_block *)
> +		((char *)(*buffer)->b_data + F2FS_SUPER_OFFSET);
> +
> +	/* sanity checking of raw super */
> +	if (sanity_check_raw_super(sb, super)) {
> +		brelse(*buffer);
> +		f2fs_msg(sb, KERN_ERR,
> +			"Can't find valid F2FS filesystem in %dth superblock",
> +								block + 1);
> +		return -EINVAL;
> +	}
> +
> +	return 0;
> +}
> +
>  /*
>   * Read f2fs raw super block.
>   * Because we have two copies of super block, so read the first one at first,
> @@ -975,37 +1002,41 @@ static int read_raw_super_block(struct super_block *sb,
>  			struct buffer_head **raw_super_buf)
>  {
>  	int block = 0;
> -
> -retry:
> -	*raw_super_buf = sb_bread(sb, block);
> -	if (!*raw_super_buf) {
> -		f2fs_msg(sb, KERN_ERR, "Unable to read %dth superblock",
> -				block + 1);
> -		if (block == 0) {
> -			block++;
> -			goto retry;
> -		} else {
> -			return -EIO;
> -		}
> +	struct buffer_head *buffer0 = NULL, *buffer1 = NULL;
> +	bool readonly = f2fs_readonly(sb) || bdev_read_only(sb->s_bdev);
> +	int err0, err1;
> +
> +	err0 = get_valid_super_block(sb, &buffer0, 0);
> +	if (!err0 && readonly)
> +		goto set_raw_super;
> +
> +	err1 = get_valid_super_block(sb, &buffer1, 1);
> +	if (err0 && err1)
> +		return err1;
> +
> +	if (err0 && !err1) {
> +		buffer0 = buffer1;
> +		if (readonly)
> +			goto set_raw_super;
> +	} else if (!err0 && !err1) {
> +		brelse(buffer1);
> +		goto set_raw_super;
>  	}
> 
> +	f2fs_msg(sb, KERN_INFO, "Recover superblock");
> +	block = buffer0->b_blocknr;
> +	buffer0->b_blocknr = block ? 0 : 1;
> +	mark_buffer_dirty(buffer0);
> +	sync_dirty_buffer(buffer0);
> +	buffer0->b_blocknr = block;
> +	clear_buffer_write_io_error(buffer0);
> +	set_buffer_uptodate(buffer0);
> +
> +set_raw_super:
> +	*raw_super_buf = buffer0;
>  	*raw_super = (struct f2fs_super_block *)
>  		((char *)(*raw_super_buf)->b_data + F2FS_SUPER_OFFSET);
> 
> -	/* sanity checking of raw super */
> -	if (sanity_check_raw_super(sb, *raw_super)) {
> -		brelse(*raw_super_buf);
> -		f2fs_msg(sb, KERN_ERR,
> -			"Can't find valid F2FS filesystem in %dth superblock",
> -								block + 1);
> -		if (block == 0) {
> -			block++;
> -			goto retry;
> -		} else {
> -			return -EINVAL;
> -		}
> -	}
> -
>  	return 0;
>  }

------------------------------------------------------------------------------
One dashboard for servers and applications across Physical-Virtual-Cloud 
Widest out-of-the-box monitoring support with 50+ applications
Performance metrics, stats and reports that give you Actionable Insights
Deep dive visibility with transaction tracing using APM Insight.
http://ad.doubleclick.net/ddm/clk/290420510;117567292;y

Re: [PATCH RFC] f2fs: recovering broken superblock during mount

[hujianyang]

On 2015/5/21 9:07, Jaegeuk Kim wrote:
> Hi Hu,
> 
> On Wed, May 20, 2015 at 03:01:04PM +0800, hujianyang wrote:
>> Hi Jaegeuk,
>>
>> There are two superblocks in f2fs. I wrapped one of them by 'dd' in
>> a reliability test. After this, the polluted f2fs partition can be
>> mounted with an error message:
>>
>> F2FS-fs (sdd3): Magic Mismatch, valid(0xf2f52010) - read(0x0)
>> F2FS-fs (sdd3): Can't find valid F2FS filesystem in 1th superblock
>>
>> Seems the broken superblock can't be recovered by f2fs driver or
>> fsck.f2fs. Tell me how to recover if I was wrong.:)
>>
>> So I'd like to provide a patch to recover broken f2fs superblock
>> during mount. But I'm not pleased with my patch. I wish you and
>> others could give me some suggestions.
>>
>> Do you think it's necessary to recover the superblock? and can
>> we change 'f2fs_commit_super' a little to reuse this function
>> during superblock recovery?
> 
> What is the real problem in the original code?
> It's the message problem, isn't it?
> Even though the first superblock is missing with that message, mount must be
> done with the second superblock, right?
> 
> If so, it would be good to use f2fs_commit_super to recover that and leave some
> messages.
> 
> Thanks,
> 

Hi Jaegeuk,

Thanks for your suggestions. I've reworked the patch, how do you feel
about the following one?

Current f2fs doesn't check the 2th superblock if the first one is
valid, my patch changes to check both of superblocks during mount
because I think either of them may corrupted and we should detect the
corruption at once, then recover it.

Thanks,
Hu

Signed-off-by: hujianyang <hujianyang@huawei.com>
---
 fs/f2fs/super.c |   56 ++++++++++++++++++++++++++++++++++++++++++++----------
 1 files changed, 45 insertions(+), 11 deletions(-)

diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
index ae14fc4..8e5cd83 100644
--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -972,29 +972,36 @@ static void init_sb_info(struct f2fs_sb_info *sbi)
  */
 static int read_raw_super_block(struct super_block *sb,
 			struct f2fs_super_block **raw_super,
-			struct buffer_head **raw_super_buf)
+			struct buffer_head **raw_super_buf,
+			int *recovery)
 {
 	int block = 0;
+	struct buffer_head *buffer;
+	struct f2fs_super_block *super;
+	int err = 0;

 retry:
-	*raw_super_buf = sb_bread(sb, block);
-	if (!*raw_super_buf) {
+	buffer = sb_bread(sb, block);
+	if (!buffer) {
+		*recovery = 1;
 		f2fs_msg(sb, KERN_ERR, "Unable to read %dth superblock",
 				block + 1);
 		if (block == 0) {
 			block++;
 			goto retry;
 		} else {
-			return -EIO;
+			err = -EIO;
+			goto out;
 		}
 	}

-	*raw_super = (struct f2fs_super_block *)
-		((char *)(*raw_super_buf)->b_data + F2FS_SUPER_OFFSET);
+	super = (struct f2fs_super_block *)
+		((char *)(buffer)->b_data + F2FS_SUPER_OFFSET);

 	/* sanity checking of raw super */
-	if (sanity_check_raw_super(sb, *raw_super)) {
-		brelse(*raw_super_buf);
+	if (sanity_check_raw_super(sb, super)) {
+		brelse(buffer);
+		*recovery = 1;
 		f2fs_msg(sb, KERN_ERR,
 			"Can't find valid F2FS filesystem in %dth superblock",
 								block + 1);
@@ -1002,10 +1009,30 @@ retry:
 			block++;
 			goto retry;
 		} else {
-			return -EINVAL;
+			err = -EINVAL;
+			goto out;
 		}
 	}

+	if (!*raw_super) {
+		*raw_super_buf = buffer;
+		*raw_super = super;
+	} else {
+		/* already have a valid superblock */
+		brelse(buffer);
+	}
+
+	/* check the validity of the second superblock */
+	if (block == 0) {
+		block++;
+		goto retry;
+	}
+
+out:
+	/* No valid superblock */
+	if (!*raw_super)
+		return err;
+
 	return 0;
 }

@@ -1042,7 +1069,7 @@ static int f2fs_fill_super(struct super_block *sb, void *data, int silent)
 	long err = -EINVAL;
 	bool retry = true, need_fsck = false;
 	char *options = NULL;
-	int i;
+	int recovery = 0, i;

 try_onemore:
 	/* allocate memory for f2fs-specific super block info */
@@ -1056,7 +1083,7 @@ try_onemore:
 		goto free_sbi;
 	}

-	err = read_raw_super_block(sb, &raw_super, &raw_super_buf);
+	err = read_raw_super_block(sb, &raw_super, &raw_super_buf, &recovery);
 	if (err)
 		goto free_sbi;

@@ -1255,6 +1282,13 @@ try_onemore:
 			goto free_kobj;
 	}
 	kfree(options);
+
+	/* recover broken superblock */
+	if (recovery && !f2fs_readonly(sb) && !bdev_read_only(sb->s_bdev)) {
+		f2fs_msg(sb, KERN_INFO, "Superblock recovery");
+		f2fs_commit_super(sbi);
+	}
+
 	return 0;

 free_kobj:
-- 
1.6.0.2


------------------------------------------------------------------------------
One dashboard for servers and applications across Physical-Virtual-Cloud 
Widest out-of-the-box monitoring support with 50+ applications
Performance metrics, stats and reports that give you Actionable Insights
Deep dive visibility with transaction tracing using APM Insight.
http://ad.doubleclick.net/ddm/clk/290420510;117567292;y

Re: [PATCH RFC] f2fs: recovering broken superblock during mount

[Jaegeuk Kim]

Hi Hu,

Looks good to me, except one minor thing where 

+	int recovery = 0, i;
 -> AFAIK, kernel coding style recommends to split them.
 Let me change that to

 	int recovery = 0;
	int i;


Merged.

Thanks,

On Thu, May 21, 2015 at 02:42:53PM +0800, hujianyang wrote:
> On 2015/5/21 9:07, Jaegeuk Kim wrote:
> > Hi Hu,
> > 
> > On Wed, May 20, 2015 at 03:01:04PM +0800, hujianyang wrote:
> >> Hi Jaegeuk,
> >>
> >> There are two superblocks in f2fs. I wrapped one of them by 'dd' in
> >> a reliability test. After this, the polluted f2fs partition can be
> >> mounted with an error message:
> >>
> >> F2FS-fs (sdd3): Magic Mismatch, valid(0xf2f52010) - read(0x0)
> >> F2FS-fs (sdd3): Can't find valid F2FS filesystem in 1th superblock
> >>
> >> Seems the broken superblock can't be recovered by f2fs driver or
> >> fsck.f2fs. Tell me how to recover if I was wrong.:)
> >>
> >> So I'd like to provide a patch to recover broken f2fs superblock
> >> during mount. But I'm not pleased with my patch. I wish you and
> >> others could give me some suggestions.
> >>
> >> Do you think it's necessary to recover the superblock? and can
> >> we change 'f2fs_commit_super' a little to reuse this function
> >> during superblock recovery?
> > 
> > What is the real problem in the original code?
> > It's the message problem, isn't it?
> > Even though the first superblock is missing with that message, mount must be
> > done with the second superblock, right?
> > 
> > If so, it would be good to use f2fs_commit_super to recover that and leave some
> > messages.
> > 
> > Thanks,
> > 
> 
> Hi Jaegeuk,
> 
> Thanks for your suggestions. I've reworked the patch, how do you feel
> about the following one?
> 
> Current f2fs doesn't check the 2th superblock if the first one is
> valid, my patch changes to check both of superblocks during mount
> because I think either of them may corrupted and we should detect the
> corruption at once, then recover it.
> 
> Thanks,
> Hu
> 
> Signed-off-by: hujianyang <hujianyang@huawei.com>
> ---
>  fs/f2fs/super.c |   56 ++++++++++++++++++++++++++++++++++++++++++++----------
>  1 files changed, 45 insertions(+), 11 deletions(-)
> 
> diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
> index ae14fc4..8e5cd83 100644
> --- a/fs/f2fs/super.c
> +++ b/fs/f2fs/super.c
> @@ -972,29 +972,36 @@ static void init_sb_info(struct f2fs_sb_info *sbi)
>   */
>  static int read_raw_super_block(struct super_block *sb,
>  			struct f2fs_super_block **raw_super,
> -			struct buffer_head **raw_super_buf)
> +			struct buffer_head **raw_super_buf,
> +			int *recovery)
>  {
>  	int block = 0;
> +	struct buffer_head *buffer;
> +	struct f2fs_super_block *super;
> +	int err = 0;
> 
>  retry:
> -	*raw_super_buf = sb_bread(sb, block);
> -	if (!*raw_super_buf) {
> +	buffer = sb_bread(sb, block);
> +	if (!buffer) {
> +		*recovery = 1;
>  		f2fs_msg(sb, KERN_ERR, "Unable to read %dth superblock",
>  				block + 1);
>  		if (block == 0) {
>  			block++;
>  			goto retry;
>  		} else {
> -			return -EIO;
> +			err = -EIO;
> +			goto out;
>  		}
>  	}
> 
> -	*raw_super = (struct f2fs_super_block *)
> -		((char *)(*raw_super_buf)->b_data + F2FS_SUPER_OFFSET);
> +	super = (struct f2fs_super_block *)
> +		((char *)(buffer)->b_data + F2FS_SUPER_OFFSET);
> 
>  	/* sanity checking of raw super */
> -	if (sanity_check_raw_super(sb, *raw_super)) {
> -		brelse(*raw_super_buf);
> +	if (sanity_check_raw_super(sb, super)) {
> +		brelse(buffer);
> +		*recovery = 1;
>  		f2fs_msg(sb, KERN_ERR,
>  			"Can't find valid F2FS filesystem in %dth superblock",
>  								block + 1);
> @@ -1002,10 +1009,30 @@ retry:
>  			block++;
>  			goto retry;
>  		} else {
> -			return -EINVAL;
> +			err = -EINVAL;
> +			goto out;
>  		}
>  	}
> 
> +	if (!*raw_super) {
> +		*raw_super_buf = buffer;
> +		*raw_super = super;
> +	} else {
> +		/* already have a valid superblock */
> +		brelse(buffer);
> +	}
> +
> +	/* check the validity of the second superblock */
> +	if (block == 0) {
> +		block++;
> +		goto retry;
> +	}
> +
> +out:
> +	/* No valid superblock */
> +	if (!*raw_super)
> +		return err;
> +
>  	return 0;
>  }
> 
> @@ -1042,7 +1069,7 @@ static int f2fs_fill_super(struct super_block *sb, void *data, int silent)
>  	long err = -EINVAL;
>  	bool retry = true, need_fsck = false;
>  	char *options = NULL;
> -	int i;
> +	int recovery = 0, i;
> 
>  try_onemore:
>  	/* allocate memory for f2fs-specific super block info */
> @@ -1056,7 +1083,7 @@ try_onemore:
>  		goto free_sbi;
>  	}
> 
> -	err = read_raw_super_block(sb, &raw_super, &raw_super_buf);
> +	err = read_raw_super_block(sb, &raw_super, &raw_super_buf, &recovery);
>  	if (err)
>  		goto free_sbi;
> 
> @@ -1255,6 +1282,13 @@ try_onemore:
>  			goto free_kobj;
>  	}
>  	kfree(options);
> +
> +	/* recover broken superblock */
> +	if (recovery && !f2fs_readonly(sb) && !bdev_read_only(sb->s_bdev)) {
> +		f2fs_msg(sb, KERN_INFO, "Superblock recovery");
> +		f2fs_commit_super(sbi);
> +	}
> +
>  	return 0;
> 
>  free_kobj:
> -- 
> 1.6.0.2

------------------------------------------------------------------------------
One dashboard for servers and applications across Physical-Virtual-Cloud 
Widest out-of-the-box monitoring support with 50+ applications
Performance metrics, stats and reports that give you Actionable Insights
Deep dive visibility with transaction tracing using APM Insight.
http://ad.doubleclick.net/ddm/clk/290420510;117567292;y

Re: [PATCH RFC] f2fs: recovering broken superblock during mount

[hujianyang]

On 2015/5/22 6:44, Jaegeuk Kim wrote:
> Hi Hu,
> 
> Looks good to me, except one minor thing where 
> 
> +	int recovery = 0, i;
>  -> AFAIK, kernel coding style recommends to split them.
>  Let me change that to
> 
>  	int recovery = 0;
> 	int i;
> 
> 
> Merged.
> 
> Thanks,

Hi Jaegeuk,

Thank you!:)

Hu



------------------------------------------------------------------------------
One dashboard for servers and applications across Physical-Virtual-Cloud 
Widest out-of-the-box monitoring support with 50+ applications
Performance metrics, stats and reports that give you Actionable Insights
Deep dive visibility with transaction tracing using APM Insight.
http://ad.doubleclick.net/ddm/clk/290420510;117567292;y