[PATCH] back-up raw_super in sbi

[PATCH] back-up raw_super in sbi

[Yunlei He]

write_checkpoint() tries to get cp_blkaddr from superblock buffer,
if the buffer happen to be destroied by something else, it may
bring in unpredictable effect on f2fs.

this patch fix it by back-up a raw_super copy.

Signed-off-by: Yunlei He <heyunlei@huawei.com>
---
 fs/f2fs/checkpoint.c |  3 ++-
 fs/f2fs/f2fs.h       | 10 ++++++++++
 fs/f2fs/node.c       |  2 +-
 fs/f2fs/super.c      | 12 ++++++++----
 4 files changed, 21 insertions(+), 6 deletions(-)

diff --git a/fs/f2fs/checkpoint.c b/fs/f2fs/checkpoint.c
index f661d80..1e342fd 100644
--- a/fs/f2fs/checkpoint.c
+++ b/fs/f2fs/checkpoint.c
@@ -276,13 +276,14 @@ long sync_meta_pages(struct f2fs_sb_info *sbi, enum page_type type,
 						long nr_to_write)
 {
 	struct address_space *mapping = META_MAPPING(sbi);
-	pgoff_t index = 0, end = LONG_MAX, prev = LONG_MAX;
+	pgoff_t index, end = LONG_MAX, prev = LONG_MAX;
 	struct pagevec pvec;
 	long nwritten = 0;
 	struct writeback_control wbc = {
 		.for_reclaim = 0,
 	};
 
+	index = __start_cp_addr(sbi);
 	pagevec_init(&pvec, 0);
 
 	while (index <= end) {
diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index 0052ae8..6afb56c 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -1167,6 +1167,16 @@ static inline block_t __start_sum_addr(struct f2fs_sb_info *sbi)
 	return le32_to_cpu(F2FS_CKPT(sbi)->cp_pack_start_sum);
 }
 
+static inline block_t __start_main_addr(struct f2fs_sb_info *sbi)
+{
+	return le32_to_cpu(F2FS_RAW_SUPER(sbi)->main_blkaddr);
+}
+
+static inline block_t __start_sum_addr(struct f2fs_sb_info *sbi)
+{
+	return le32_to_cpu(F2FS_CKPT(sbi)->cp_pack_start_sum);
+}
+
 static inline bool inc_valid_node_count(struct f2fs_sb_info *sbi,
 						struct inode *inode)
 {
diff --git a/fs/f2fs/node.c b/fs/f2fs/node.c
index 7bcbc6e..7d2b0bb 100644
--- a/fs/f2fs/node.c
+++ b/fs/f2fs/node.c
@@ -1161,7 +1161,7 @@ int sync_node_pages(struct f2fs_sb_info *sbi, nid_t ino,
 	pagevec_init(&pvec, 0);
 
 next_step:
-	index = 0;
+	index = __start_main_addr(sbi);
 	end = LONG_MAX;
 
 	while (index <= end) {
diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
index bd7e9c6..d394711 100644
--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -1040,6 +1040,9 @@ static int read_raw_super_block(struct super_block *sb,
 	struct f2fs_super_block *super;
 	int err = 0;
 
+	super = kzalloc(sizeof(struct f2fs_super_block), GFP_KERNEL);
+	if (!super)
+		return -ENOMEM;
 retry:
 	buffer = sb_bread(sb, block);
 	if (!buffer) {
@@ -1055,9 +1058,8 @@ retry:
 		}
 	}
 
-	super = (struct f2fs_super_block *)
-		((char *)(buffer)->b_data + F2FS_SUPER_OFFSET);
-
+	memcpy(super, buffer->b_data + F2FS_SUPER_OFFSET,
+					sizeof(struct f2fs_super_block));
 	/* sanity checking of raw super */
 	if (sanity_check_raw_super(sb, super)) {
 		brelse(buffer);
@@ -1090,8 +1092,10 @@ retry:
 
 out:
 	/* No valid superblock */
-	if (!*raw_super)
+	if (!*raw_super) {
+		kfree(super);
 		return err;
+	}
 
 	return 0;
 }
-- 
1.9.1


------------------------------------------------------------------------------
Go from Idea to Many App Stores Faster with Intel(R) XDK
Give your users amazing mobile app experiences with Intel(R) XDK.
Use one codebase in this all-in-one HTML5 development environment.
Design, debug & build mobile apps & 2D/3D high-impact games for multiple OSs.
http://pubads.g.doubleclick.net/gampad/clk?id=254741911&iu=/4140

Re: [PATCH] back-up raw_super in sbi

[Jaegeuk Kim]

Hi Yunlei,

On Tue, Dec 08, 2015 at 09:17:13PM +0800, Yunlei He wrote:
> write_checkpoint() tries to get cp_blkaddr from superblock buffer,
> if the buffer happen to be destroied by something else, it may
> bring in unpredictable effect on f2fs.
> 
> this patch fix it by back-up a raw_super copy.
> 
> Signed-off-by: Yunlei He <heyunlei@huawei.com>
> ---
>  fs/f2fs/checkpoint.c |  3 ++-
>  fs/f2fs/f2fs.h       | 10 ++++++++++
>  fs/f2fs/node.c       |  2 +-
>  fs/f2fs/super.c      | 12 ++++++++----
>  4 files changed, 21 insertions(+), 6 deletions(-)
> 
> diff --git a/fs/f2fs/checkpoint.c b/fs/f2fs/checkpoint.c
> index f661d80..1e342fd 100644
> --- a/fs/f2fs/checkpoint.c
> +++ b/fs/f2fs/checkpoint.c
> @@ -276,13 +276,14 @@ long sync_meta_pages(struct f2fs_sb_info *sbi, enum page_type type,
>  						long nr_to_write)
>  {
>  	struct address_space *mapping = META_MAPPING(sbi);
> -	pgoff_t index = 0, end = LONG_MAX, prev = LONG_MAX;
> +	pgoff_t index, end = LONG_MAX, prev = LONG_MAX;
>  	struct pagevec pvec;
>  	long nwritten = 0;
>  	struct writeback_control wbc = {
>  		.for_reclaim = 0,
>  	};
>  
> +	index = __start_cp_addr(sbi);

No need to do this.

>  	pagevec_init(&pvec, 0);
>  
>  	while (index <= end) {
> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> index 0052ae8..6afb56c 100644
> --- a/fs/f2fs/f2fs.h
> +++ b/fs/f2fs/f2fs.h
> @@ -1167,6 +1167,16 @@ static inline block_t __start_sum_addr(struct f2fs_sb_info *sbi)
>  	return le32_to_cpu(F2FS_CKPT(sbi)->cp_pack_start_sum);
>  }
>  
> +static inline block_t __start_main_addr(struct f2fs_sb_info *sbi)
> +{
> +	return le32_to_cpu(F2FS_RAW_SUPER(sbi)->main_blkaddr);
> +}
> +
> +static inline block_t __start_sum_addr(struct f2fs_sb_info *sbi)
> +{
> +	return le32_to_cpu(F2FS_CKPT(sbi)->cp_pack_start_sum);
> +}
> +

Ditto.

>  static inline bool inc_valid_node_count(struct f2fs_sb_info *sbi,
>  						struct inode *inode)
>  {
> diff --git a/fs/f2fs/node.c b/fs/f2fs/node.c
> index 7bcbc6e..7d2b0bb 100644
> --- a/fs/f2fs/node.c
> +++ b/fs/f2fs/node.c
> @@ -1161,7 +1161,7 @@ int sync_node_pages(struct f2fs_sb_info *sbi, nid_t ino,
>  	pagevec_init(&pvec, 0);
>  
>  next_step:
> -	index = 0;
> +	index = __start_main_addr(sbi);

No, its index is for nid.

>  	end = LONG_MAX;
>  
>  	while (index <= end) {
> diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
> index bd7e9c6..d394711 100644
> --- a/fs/f2fs/super.c
> +++ b/fs/f2fs/super.c
> @@ -1040,6 +1040,9 @@ static int read_raw_super_block(struct super_block *sb,
>  	struct f2fs_super_block *super;
>  	int err = 0;
>  
> +	super = kzalloc(sizeof(struct f2fs_super_block), GFP_KERNEL);
> +	if (!super)
> +		return -ENOMEM;
>  retry:
>  	buffer = sb_bread(sb, block);
>  	if (!buffer) {
> @@ -1055,9 +1058,8 @@ retry:
>  		}
>  	}
>  
> -	super = (struct f2fs_super_block *)
> -		((char *)(buffer)->b_data + F2FS_SUPER_OFFSET);
> -
> +	memcpy(super, buffer->b_data + F2FS_SUPER_OFFSET,
> +					sizeof(struct f2fs_super_block));
>  	/* sanity checking of raw super */
>  	if (sanity_check_raw_super(sb, super)) {
>  		brelse(buffer);
> @@ -1090,8 +1092,10 @@ retry:
>  
>  out:
>  	/* No valid superblock */
> -	if (!*raw_super)
> +	if (!*raw_super) {
> +		kfree(super);
>  		return err;
> +	}

Need to consider f2fs_commit_super and kfree() in put_super.

Could you check out this patch?

---
 fs/f2fs/super.c | 18 ++++++++++++++----
 1 file changed, 14 insertions(+), 4 deletions(-)

diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
index dbf16ad..8541c93 100644
--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -567,6 +567,7 @@ static void f2fs_put_super(struct super_block *sb)
 
 	sb->s_fs_info = NULL;
 	brelse(sbi->raw_super_buf);
+	kfree(sbi->raw_super);
 	kfree(sbi);
 }
 
@@ -1040,6 +1041,9 @@ static int read_raw_super_block(struct super_block *sb,
 	struct f2fs_super_block *super;
 	int err = 0;
 
+	super = kzalloc(sizeof(struct f2fs_super_block), GFP_KERNEL);
+	if (!super)
+		return -ENOMEM;
 retry:
 	buffer = sb_bread(sb, block);
 	if (!buffer) {
@@ -1055,8 +1059,7 @@ retry:
 		}
 	}
 
-	super = (struct f2fs_super_block *)
-		((char *)(buffer)->b_data + F2FS_SUPER_OFFSET);
+	memcpy(super, buffer->b_data + F2FS_SUPER_OFFSET, sizeof(*super));
 
 	/* sanity checking of raw super */
 	if (sanity_check_raw_super(sb, super)) {
@@ -1090,14 +1093,17 @@ retry:
 
 out:
 	/* No valid superblock */
-	if (!*raw_super)
+	if (!*raw_super) {
+		kfree(super);
 		return err;
+	}
 
 	return 0;
 }
 
 int f2fs_commit_super(struct f2fs_sb_info *sbi, bool recover)
 {
+	struct f2fs_super_block *super = F2FS_RAW_SUPER(sbi);
 	struct buffer_head *sbh = sbi->raw_super_buf;
 	struct buffer_head *bh;
 	int err;
@@ -1108,7 +1114,7 @@ int f2fs_commit_super(struct f2fs_sb_info *sbi, bool recover)
 		return -EIO;
 
 	lock_buffer(bh);
-	memcpy(bh->b_data, sbh->b_data, sbh->b_size);
+	memcpy(bh->b_data + F2FS_SUPER_OFFSET, super, sizeof(*super));
 	WARN_ON(sbh->b_size != F2FS_BLKSIZE);
 	set_buffer_uptodate(bh);
 	set_buffer_dirty(bh);
@@ -1124,6 +1130,10 @@ int f2fs_commit_super(struct f2fs_sb_info *sbi, bool recover)
 
 	/* write current valid superblock */
 	lock_buffer(sbh);
+	if (memcmp(sbh->b_data + F2FS_SUPER_OFFSET, super, sizeof(*super))) {
+		f2fs_msg(sbi->sb, KERN_INFO, "Write modified valid superblock");
+		memcpy(sbh->b_data + F2FS_SUPER_OFFSET, super, sizeof(*super));
+	}
 	set_buffer_dirty(sbh);
 	unlock_buffer(sbh);
 
-- 
2.4.9 (Apple Git-60)


------------------------------------------------------------------------------
Go from Idea to Many App Stores Faster with Intel(R) XDK
Give your users amazing mobile app experiences with Intel(R) XDK.
Use one codebase in this all-in-one HTML5 development environment.
Design, debug & build mobile apps & 2D/3D high-impact games for multiple OSs.
http://pubads.g.doubleclick.net/gampad/clk?id=254741911&iu=/4140

Re: [PATCH] back-up raw_super in sbi

[He YunLei]

On 2015/12/9 2:19, Jaegeuk Kim wrote:
> Hi Yunlei,
>
> On Tue, Dec 08, 2015 at 09:17:13PM +0800, Yunlei He wrote:
>> write_checkpoint() tries to get cp_blkaddr from superblock buffer,
>> if the buffer happen to be destroied by something else, it may
>> bring in unpredictable effect on f2fs.
>>
>> this patch fix it by back-up a raw_super copy.
>>
>> Signed-off-by: Yunlei He <heyunlei@huawei.com>
>> ---
>>   fs/f2fs/checkpoint.c |  3 ++-
>>   fs/f2fs/f2fs.h       | 10 ++++++++++
>>   fs/f2fs/node.c       |  2 +-
>>   fs/f2fs/super.c      | 12 ++++++++----
>>   4 files changed, 21 insertions(+), 6 deletions(-)
>>
>> diff --git a/fs/f2fs/checkpoint.c b/fs/f2fs/checkpoint.c
>> index f661d80..1e342fd 100644
>> --- a/fs/f2fs/checkpoint.c
>> +++ b/fs/f2fs/checkpoint.c
>> @@ -276,13 +276,14 @@ long sync_meta_pages(struct f2fs_sb_info *sbi, enum page_type type,
>>   						long nr_to_write)
>>   {
>>   	struct address_space *mapping = META_MAPPING(sbi);
>> -	pgoff_t index = 0, end = LONG_MAX, prev = LONG_MAX;
>> +	pgoff_t index, end = LONG_MAX, prev = LONG_MAX;
>>   	struct pagevec pvec;
>>   	long nwritten = 0;
>>   	struct writeback_control wbc = {
>>   		.for_reclaim = 0,
>>   	};
>>
>> +	index = __start_cp_addr(sbi);
>
> No need to do this.
>
>>   	pagevec_init(&pvec, 0);
>>
>>   	while (index <= end) {
>> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
>> index 0052ae8..6afb56c 100644
>> --- a/fs/f2fs/f2fs.h
>> +++ b/fs/f2fs/f2fs.h
>> @@ -1167,6 +1167,16 @@ static inline block_t __start_sum_addr(struct f2fs_sb_info *sbi)
>>   	return le32_to_cpu(F2FS_CKPT(sbi)->cp_pack_start_sum);
>>   }
>>
>> +static inline block_t __start_main_addr(struct f2fs_sb_info *sbi)
>> +{
>> +	return le32_to_cpu(F2FS_RAW_SUPER(sbi)->main_blkaddr);
>> +}
>> +
>> +static inline block_t __start_sum_addr(struct f2fs_sb_info *sbi)
>> +{
>> +	return le32_to_cpu(F2FS_CKPT(sbi)->cp_pack_start_sum);
>> +}
>> +
>
> Ditto.
>
>>   static inline bool inc_valid_node_count(struct f2fs_sb_info *sbi,
>>   						struct inode *inode)
>>   {
>> diff --git a/fs/f2fs/node.c b/fs/f2fs/node.c
>> index 7bcbc6e..7d2b0bb 100644
>> --- a/fs/f2fs/node.c
>> +++ b/fs/f2fs/node.c
>> @@ -1161,7 +1161,7 @@ int sync_node_pages(struct f2fs_sb_info *sbi, nid_t ino,
>>   	pagevec_init(&pvec, 0);
>>
>>   next_step:
>> -	index = 0;
>> +	index = __start_main_addr(sbi);
>
> No, its index is for nid.
>
>>   	end = LONG_MAX;
>>
>>   	while (index <= end) {
>> diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
>> index bd7e9c6..d394711 100644
>> --- a/fs/f2fs/super.c
>> +++ b/fs/f2fs/super.c
>> @@ -1040,6 +1040,9 @@ static int read_raw_super_block(struct super_block *sb,
>>   	struct f2fs_super_block *super;
>>   	int err = 0;
>>
>> +	super = kzalloc(sizeof(struct f2fs_super_block), GFP_KERNEL);
>> +	if (!super)
>> +		return -ENOMEM;
>>   retry:
>>   	buffer = sb_bread(sb, block);
>>   	if (!buffer) {
>> @@ -1055,9 +1058,8 @@ retry:
>>   		}
>>   	}
>>
>> -	super = (struct f2fs_super_block *)
>> -		((char *)(buffer)->b_data + F2FS_SUPER_OFFSET);
>> -
>> +	memcpy(super, buffer->b_data + F2FS_SUPER_OFFSET,
>> +					sizeof(struct f2fs_super_block));
>>   	/* sanity checking of raw super */
>>   	if (sanity_check_raw_super(sb, super)) {
>>   		brelse(buffer);
>> @@ -1090,8 +1092,10 @@ retry:
>>
>>   out:
>>   	/* No valid superblock */
>> -	if (!*raw_super)
>> +	if (!*raw_super) {
>> +		kfree(super);
>>   		return err;
>> +	}
>
> Need to consider f2fs_commit_super and kfree() in put_super.

We also need to consider f2fs_fill_super error processing procedure:

@@ -1388,6 +1388,7 @@ free_options:
         kfree(options);
  free_sb_buf:
         brelse(raw_super_buf);
+       kfree(raw_super);
	
>
> Could you check out this patch?
>
> ---
>   fs/f2fs/super.c | 18 ++++++++++++++----
>   1 file changed, 14 insertions(+), 4 deletions(-)
>
> diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
> index dbf16ad..8541c93 100644
> --- a/fs/f2fs/super.c
> +++ b/fs/f2fs/super.c
> @@ -567,6 +567,7 @@ static void f2fs_put_super(struct super_block *sb)
>
>   	sb->s_fs_info = NULL;
>   	brelse(sbi->raw_super_buf);
> +	kfree(sbi->raw_super);
>   	kfree(sbi);
>   }
>
> @@ -1040,6 +1041,9 @@ static int read_raw_super_block(struct super_block *sb,
>   	struct f2fs_super_block *super;
>   	int err = 0;
>
> +	super = kzalloc(sizeof(struct f2fs_super_block), GFP_KERNEL);
> +	if (!super)
> +		return -ENOMEM;
>   retry:
>   	buffer = sb_bread(sb, block);
>   	if (!buffer) {
> @@ -1055,8 +1059,7 @@ retry:
>   		}
>   	}
>
> -	super = (struct f2fs_super_block *)
> -		((char *)(buffer)->b_data + F2FS_SUPER_OFFSET);
> +	memcpy(super, buffer->b_data + F2FS_SUPER_OFFSET, sizeof(*super));
>
>   	/* sanity checking of raw super */
>   	if (sanity_check_raw_super(sb, super)) {
> @@ -1090,14 +1093,17 @@ retry:
>
>   out:
>   	/* No valid superblock */
> -	if (!*raw_super)
> +	if (!*raw_super) {
> +		kfree(super);
>   		return err;
> +	}
>
>   	return 0;
>   }
>
>   int f2fs_commit_super(struct f2fs_sb_info *sbi, bool recover)
>   {
> +	struct f2fs_super_block *super = F2FS_RAW_SUPER(sbi);
>   	struct buffer_head *sbh = sbi->raw_super_buf;
>   	struct buffer_head *bh;
>   	int err;
> @@ -1108,7 +1114,7 @@ int f2fs_commit_super(struct f2fs_sb_info *sbi, bool recover)
>   		return -EIO;
>
>   	lock_buffer(bh);
> -	memcpy(bh->b_data, sbh->b_data, sbh->b_size);
> +	memcpy(bh->b_data + F2FS_SUPER_OFFSET, super, sizeof(*super));
>   	WARN_ON(sbh->b_size != F2FS_BLKSIZE);
>   	set_buffer_uptodate(bh);
>   	set_buffer_dirty(bh);
> @@ -1124,6 +1130,10 @@ int f2fs_commit_super(struct f2fs_sb_info *sbi, bool recover)
>
>   	/* write current valid superblock */
>   	lock_buffer(sbh);
> +	if (memcmp(sbh->b_data + F2FS_SUPER_OFFSET, super, sizeof(*super))) {
> +		f2fs_msg(sbi->sb, KERN_INFO, "Write modified valid superblock");
> +		memcpy(sbh->b_data + F2FS_SUPER_OFFSET, super, sizeof(*super));
> +	}
>   	set_buffer_dirty(sbh);
>   	unlock_buffer(sbh);
>
>
It looks good to me, but can we releasing grabbed block buffer 'sbi->raw_super_buf'
as Chao Yu said.

Thanks.
Yunlei


------------------------------------------------------------------------------

Re: [PATCH] back-up raw_super in sbi

[Jaegeuk Kim]

Hi,

On Mon, Dec 14, 2015 at 05:56:49PM +0800, He YunLei wrote:
> On 2015/12/9 2:19, Jaegeuk Kim wrote:
> >Hi Yunlei,
> >
> >On Tue, Dec 08, 2015 at 09:17:13PM +0800, Yunlei He wrote:
> >>write_checkpoint() tries to get cp_blkaddr from superblock buffer,
> >>if the buffer happen to be destroied by something else, it may
> >>bring in unpredictable effect on f2fs.
> >>
> >>this patch fix it by back-up a raw_super copy.
> >>
> >>Signed-off-by: Yunlei He <heyunlei@huawei.com>
> >>---
> >>  fs/f2fs/checkpoint.c |  3 ++-
> >>  fs/f2fs/f2fs.h       | 10 ++++++++++
> >>  fs/f2fs/node.c       |  2 +-
> >>  fs/f2fs/super.c      | 12 ++++++++----
> >>  4 files changed, 21 insertions(+), 6 deletions(-)
> >>
> >>diff --git a/fs/f2fs/checkpoint.c b/fs/f2fs/checkpoint.c
> >>index f661d80..1e342fd 100644
> >>--- a/fs/f2fs/checkpoint.c
> >>+++ b/fs/f2fs/checkpoint.c
> >>@@ -276,13 +276,14 @@ long sync_meta_pages(struct f2fs_sb_info *sbi, enum page_type type,
> >>  						long nr_to_write)
> >>  {
> >>  	struct address_space *mapping = META_MAPPING(sbi);
> >>-	pgoff_t index = 0, end = LONG_MAX, prev = LONG_MAX;
> >>+	pgoff_t index, end = LONG_MAX, prev = LONG_MAX;
> >>  	struct pagevec pvec;
> >>  	long nwritten = 0;
> >>  	struct writeback_control wbc = {
> >>  		.for_reclaim = 0,
> >>  	};
> >>
> >>+	index = __start_cp_addr(sbi);
> >
> >No need to do this.
> >
> >>  	pagevec_init(&pvec, 0);
> >>
> >>  	while (index <= end) {
> >>diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> >>index 0052ae8..6afb56c 100644
> >>--- a/fs/f2fs/f2fs.h
> >>+++ b/fs/f2fs/f2fs.h
> >>@@ -1167,6 +1167,16 @@ static inline block_t __start_sum_addr(struct f2fs_sb_info *sbi)
> >>  	return le32_to_cpu(F2FS_CKPT(sbi)->cp_pack_start_sum);
> >>  }
> >>
> >>+static inline block_t __start_main_addr(struct f2fs_sb_info *sbi)
> >>+{
> >>+	return le32_to_cpu(F2FS_RAW_SUPER(sbi)->main_blkaddr);
> >>+}
> >>+
> >>+static inline block_t __start_sum_addr(struct f2fs_sb_info *sbi)
> >>+{
> >>+	return le32_to_cpu(F2FS_CKPT(sbi)->cp_pack_start_sum);
> >>+}
> >>+
> >
> >Ditto.
> >
> >>  static inline bool inc_valid_node_count(struct f2fs_sb_info *sbi,
> >>  						struct inode *inode)
> >>  {
> >>diff --git a/fs/f2fs/node.c b/fs/f2fs/node.c
> >>index 7bcbc6e..7d2b0bb 100644
> >>--- a/fs/f2fs/node.c
> >>+++ b/fs/f2fs/node.c
> >>@@ -1161,7 +1161,7 @@ int sync_node_pages(struct f2fs_sb_info *sbi, nid_t ino,
> >>  	pagevec_init(&pvec, 0);
> >>
> >>  next_step:
> >>-	index = 0;
> >>+	index = __start_main_addr(sbi);
> >
> >No, its index is for nid.
> >
> >>  	end = LONG_MAX;
> >>
> >>  	while (index <= end) {
> >>diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
> >>index bd7e9c6..d394711 100644
> >>--- a/fs/f2fs/super.c
> >>+++ b/fs/f2fs/super.c
> >>@@ -1040,6 +1040,9 @@ static int read_raw_super_block(struct super_block *sb,
> >>  	struct f2fs_super_block *super;
> >>  	int err = 0;
> >>
> >>+	super = kzalloc(sizeof(struct f2fs_super_block), GFP_KERNEL);
> >>+	if (!super)
> >>+		return -ENOMEM;
> >>  retry:
> >>  	buffer = sb_bread(sb, block);
> >>  	if (!buffer) {
> >>@@ -1055,9 +1058,8 @@ retry:
> >>  		}
> >>  	}
> >>
> >>-	super = (struct f2fs_super_block *)
> >>-		((char *)(buffer)->b_data + F2FS_SUPER_OFFSET);
> >>-
> >>+	memcpy(super, buffer->b_data + F2FS_SUPER_OFFSET,
> >>+					sizeof(struct f2fs_super_block));
> >>  	/* sanity checking of raw super */
> >>  	if (sanity_check_raw_super(sb, super)) {
> >>  		brelse(buffer);
> >>@@ -1090,8 +1092,10 @@ retry:
> >>
> >>  out:
> >>  	/* No valid superblock */
> >>-	if (!*raw_super)
> >>+	if (!*raw_super) {
> >>+		kfree(super);
> >>  		return err;
> >>+	}
> >
> >Need to consider f2fs_commit_super and kfree() in put_super.
> 
> We also need to consider f2fs_fill_super error processing procedure:
> 
> @@ -1388,6 +1388,7 @@ free_options:
>         kfree(options);
>  free_sb_buf:
>         brelse(raw_super_buf);
> +       kfree(raw_super);

Agreed.

> 	
> >
> >Could you check out this patch?
> >
> >---
> >  fs/f2fs/super.c | 18 ++++++++++++++----
> >  1 file changed, 14 insertions(+), 4 deletions(-)
> >
> >diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
> >index dbf16ad..8541c93 100644
> >--- a/fs/f2fs/super.c
> >+++ b/fs/f2fs/super.c
> >@@ -567,6 +567,7 @@ static void f2fs_put_super(struct super_block *sb)
> >
> >  	sb->s_fs_info = NULL;
> >  	brelse(sbi->raw_super_buf);
> >+	kfree(sbi->raw_super);
> >  	kfree(sbi);
> >  }
> >
> >@@ -1040,6 +1041,9 @@ static int read_raw_super_block(struct super_block *sb,
> >  	struct f2fs_super_block *super;
> >  	int err = 0;
> >
> >+	super = kzalloc(sizeof(struct f2fs_super_block), GFP_KERNEL);
> >+	if (!super)
> >+		return -ENOMEM;
> >  retry:
> >  	buffer = sb_bread(sb, block);
> >  	if (!buffer) {
> >@@ -1055,8 +1059,7 @@ retry:
> >  		}
> >  	}
> >
> >-	super = (struct f2fs_super_block *)
> >-		((char *)(buffer)->b_data + F2FS_SUPER_OFFSET);
> >+	memcpy(super, buffer->b_data + F2FS_SUPER_OFFSET, sizeof(*super));
> >
> >  	/* sanity checking of raw super */
> >  	if (sanity_check_raw_super(sb, super)) {
> >@@ -1090,14 +1093,17 @@ retry:
> >
> >  out:
> >  	/* No valid superblock */
> >-	if (!*raw_super)
> >+	if (!*raw_super) {
> >+		kfree(super);
> >  		return err;
> >+	}
> >
> >  	return 0;
> >  }
> >
> >  int f2fs_commit_super(struct f2fs_sb_info *sbi, bool recover)
> >  {
> >+	struct f2fs_super_block *super = F2FS_RAW_SUPER(sbi);
> >  	struct buffer_head *sbh = sbi->raw_super_buf;
> >  	struct buffer_head *bh;
> >  	int err;
> >@@ -1108,7 +1114,7 @@ int f2fs_commit_super(struct f2fs_sb_info *sbi, bool recover)
> >  		return -EIO;
> >
> >  	lock_buffer(bh);
> >-	memcpy(bh->b_data, sbh->b_data, sbh->b_size);
> >+	memcpy(bh->b_data + F2FS_SUPER_OFFSET, super, sizeof(*super));
> >  	WARN_ON(sbh->b_size != F2FS_BLKSIZE);
> >  	set_buffer_uptodate(bh);
> >  	set_buffer_dirty(bh);
> >@@ -1124,6 +1130,10 @@ int f2fs_commit_super(struct f2fs_sb_info *sbi, bool recover)
> >
> >  	/* write current valid superblock */
> >  	lock_buffer(sbh);
> >+	if (memcmp(sbh->b_data + F2FS_SUPER_OFFSET, super, sizeof(*super))) {
> >+		f2fs_msg(sbi->sb, KERN_INFO, "Write modified valid superblock");
> >+		memcpy(sbh->b_data + F2FS_SUPER_OFFSET, super, sizeof(*super));
> >+	}
> >  	set_buffer_dirty(sbh);
> >  	unlock_buffer(sbh);
> >
> >
> It looks good to me, but can we releasing grabbed block buffer 'sbi->raw_super_buf'
> as Chao Yu said.

If then, don't we need to recover the polluted superblock?

> 
> Thanks.
> Yunlei

------------------------------------------------------------------------------

Re: [PATCH] back-up raw_super in sbi

[Chao Yu]

Hi all,

> > It looks good to me, but can we releasing grabbed block buffer 'sbi->raw_super_buf'
> > as Chao Yu said.
> 
> If then, don't we need to recover the polluted superblock?

We need, if we change to not grab super block buffer header, instead,
we have to record current super block no that raw_super referred to,
then in f2fs_commit_super we can try to grab block buffer of super block
with stored no.

Could you check the following patches?

Thanks,


------------------------------------------------------------------------------

Re: [PATCH] back-up raw_super in sbi

[Chao Yu]

Hi,

> -----Original Message-----
> From: Jaegeuk Kim [mailto:jaegeuk@kernel.org]
> Sent: Wednesday, December 09, 2015 2:19 AM
> To: Yunlei He
> Cc: linux-f2fs-devel@lists.sourceforge.net; chao2.yu@samsung.com; hebiao6@huawei.com;
> stuart.li@huawei.com
> Subject: Re: [f2fs-dev] [PATCH] back-up raw_super in sbi
> 
> Hi Yunlei,
> 
> On Tue, Dec 08, 2015 at 09:17:13PM +0800, Yunlei He wrote:
> > write_checkpoint() tries to get cp_blkaddr from superblock buffer,
> > if the buffer happen to be destroied by something else, it may

Yunlei,

You mean hacking in memory? could you share more about process of destroying?

> > bring in unpredictable effect on f2fs.
> >
> > this patch fix it by back-up a raw_super copy.
> >
> > Signed-off-by: Yunlei He <heyunlei@huawei.com>
> > ---
> >  fs/f2fs/checkpoint.c |  3 ++-
> >  fs/f2fs/f2fs.h       | 10 ++++++++++
> >  fs/f2fs/node.c       |  2 +-
> >  fs/f2fs/super.c      | 12 ++++++++----
> >  4 files changed, 21 insertions(+), 6 deletions(-)
> >
> > diff --git a/fs/f2fs/checkpoint.c b/fs/f2fs/checkpoint.c
> > index f661d80..1e342fd 100644
> > --- a/fs/f2fs/checkpoint.c
> > +++ b/fs/f2fs/checkpoint.c
> > @@ -276,13 +276,14 @@ long sync_meta_pages(struct f2fs_sb_info *sbi, enum page_type type,
> >  						long nr_to_write)
> >  {
> >  	struct address_space *mapping = META_MAPPING(sbi);
> > -	pgoff_t index = 0, end = LONG_MAX, prev = LONG_MAX;
> > +	pgoff_t index, end = LONG_MAX, prev = LONG_MAX;
> >  	struct pagevec pvec;
> >  	long nwritten = 0;
> >  	struct writeback_control wbc = {
> >  		.for_reclaim = 0,
> >  	};
> >
> > +	index = __start_cp_addr(sbi);
> 
> No need to do this.
> 
> >  	pagevec_init(&pvec, 0);
> >
> >  	while (index <= end) {
> > diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> > index 0052ae8..6afb56c 100644
> > --- a/fs/f2fs/f2fs.h
> > +++ b/fs/f2fs/f2fs.h
> > @@ -1167,6 +1167,16 @@ static inline block_t __start_sum_addr(struct f2fs_sb_info *sbi)
> >  	return le32_to_cpu(F2FS_CKPT(sbi)->cp_pack_start_sum);
> >  }
> >
> > +static inline block_t __start_main_addr(struct f2fs_sb_info *sbi)
> > +{
> > +	return le32_to_cpu(F2FS_RAW_SUPER(sbi)->main_blkaddr);
> > +}
> > +
> > +static inline block_t __start_sum_addr(struct f2fs_sb_info *sbi)
> > +{
> > +	return le32_to_cpu(F2FS_CKPT(sbi)->cp_pack_start_sum);
> > +}
> > +
> 
> Ditto.
> 
> >  static inline bool inc_valid_node_count(struct f2fs_sb_info *sbi,
> >  						struct inode *inode)
> >  {
> > diff --git a/fs/f2fs/node.c b/fs/f2fs/node.c
> > index 7bcbc6e..7d2b0bb 100644
> > --- a/fs/f2fs/node.c
> > +++ b/fs/f2fs/node.c
> > @@ -1161,7 +1161,7 @@ int sync_node_pages(struct f2fs_sb_info *sbi, nid_t ino,
> >  	pagevec_init(&pvec, 0);
> >
> >  next_step:
> > -	index = 0;
> > +	index = __start_main_addr(sbi);
> 
> No, its index is for nid.
> 
> >  	end = LONG_MAX;
> >
> >  	while (index <= end) {
> > diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
> > index bd7e9c6..d394711 100644
> > --- a/fs/f2fs/super.c
> > +++ b/fs/f2fs/super.c
> > @@ -1040,6 +1040,9 @@ static int read_raw_super_block(struct super_block *sb,
> >  	struct f2fs_super_block *super;
> >  	int err = 0;
> >
> > +	super = kzalloc(sizeof(struct f2fs_super_block), GFP_KERNEL);
> > +	if (!super)
> > +		return -ENOMEM;
> >  retry:
> >  	buffer = sb_bread(sb, block);
> >  	if (!buffer) {
> > @@ -1055,9 +1058,8 @@ retry:
> >  		}
> >  	}
> >
> > -	super = (struct f2fs_super_block *)
> > -		((char *)(buffer)->b_data + F2FS_SUPER_OFFSET);
> > -
> > +	memcpy(super, buffer->b_data + F2FS_SUPER_OFFSET,
> > +					sizeof(struct f2fs_super_block));
> >  	/* sanity checking of raw super */
> >  	if (sanity_check_raw_super(sb, super)) {
> >  		brelse(buffer);
> > @@ -1090,8 +1092,10 @@ retry:
> >
> >  out:
> >  	/* No valid superblock */
> > -	if (!*raw_super)
> > +	if (!*raw_super) {
> > +		kfree(super);
> >  		return err;
> > +	}
> 
> Need to consider f2fs_commit_super and kfree() in put_super.

How about releasing grabbed block buffer 'sbi->raw_super_buf' since we
already had one copy of it?

Thanks,

> 
> Could you check out this patch?
> 
> ---
>  fs/f2fs/super.c | 18 ++++++++++++++----
>  1 file changed, 14 insertions(+), 4 deletions(-)
> 
> diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
> index dbf16ad..8541c93 100644
> --- a/fs/f2fs/super.c
> +++ b/fs/f2fs/super.c
> @@ -567,6 +567,7 @@ static void f2fs_put_super(struct super_block *sb)
> 
>  	sb->s_fs_info = NULL;
>  	brelse(sbi->raw_super_buf);
> +	kfree(sbi->raw_super);
>  	kfree(sbi);
>  }
> 
> @@ -1040,6 +1041,9 @@ static int read_raw_super_block(struct super_block *sb,
>  	struct f2fs_super_block *super;
>  	int err = 0;
> 
> +	super = kzalloc(sizeof(struct f2fs_super_block), GFP_KERNEL);
> +	if (!super)
> +		return -ENOMEM;
>  retry:
>  	buffer = sb_bread(sb, block);
>  	if (!buffer) {
> @@ -1055,8 +1059,7 @@ retry:
>  		}
>  	}
> 
> -	super = (struct f2fs_super_block *)
> -		((char *)(buffer)->b_data + F2FS_SUPER_OFFSET);
> +	memcpy(super, buffer->b_data + F2FS_SUPER_OFFSET, sizeof(*super));
> 
>  	/* sanity checking of raw super */
>  	if (sanity_check_raw_super(sb, super)) {
> @@ -1090,14 +1093,17 @@ retry:
> 
>  out:
>  	/* No valid superblock */
> -	if (!*raw_super)
> +	if (!*raw_super) {
> +		kfree(super);
>  		return err;
> +	}
> 
>  	return 0;
>  }
> 
>  int f2fs_commit_super(struct f2fs_sb_info *sbi, bool recover)
>  {
> +	struct f2fs_super_block *super = F2FS_RAW_SUPER(sbi);
>  	struct buffer_head *sbh = sbi->raw_super_buf;
>  	struct buffer_head *bh;
>  	int err;
> @@ -1108,7 +1114,7 @@ int f2fs_commit_super(struct f2fs_sb_info *sbi, bool recover)
>  		return -EIO;
> 
>  	lock_buffer(bh);
> -	memcpy(bh->b_data, sbh->b_data, sbh->b_size);
> +	memcpy(bh->b_data + F2FS_SUPER_OFFSET, super, sizeof(*super));
>  	WARN_ON(sbh->b_size != F2FS_BLKSIZE);
>  	set_buffer_uptodate(bh);
>  	set_buffer_dirty(bh);
> @@ -1124,6 +1130,10 @@ int f2fs_commit_super(struct f2fs_sb_info *sbi, bool recover)
> 
>  	/* write current valid superblock */
>  	lock_buffer(sbh);
> +	if (memcmp(sbh->b_data + F2FS_SUPER_OFFSET, super, sizeof(*super))) {
> +		f2fs_msg(sbi->sb, KERN_INFO, "Write modified valid superblock");
> +		memcpy(sbh->b_data + F2FS_SUPER_OFFSET, super, sizeof(*super));
> +	}
>  	set_buffer_dirty(sbh);
>  	unlock_buffer(sbh);
> 
> --
> 2.4.9 (Apple Git-60)



------------------------------------------------------------------------------

Re: [PATCH] back-up raw_super in sbi

[He YunLei]

On 2015/12/9 14:11, Chao Yu wrote:
> Hi,
>
>> -----Original Message-----
>> From: Jaegeuk Kim [mailto:jaegeuk@kernel.org]
>> Sent: Wednesday, December 09, 2015 2:19 AM
>> To: Yunlei He
>> Cc: linux-f2fs-devel@lists.sourceforge.net; chao2.yu@samsung.com; hebiao6@huawei.com;
>> stuart.li@huawei.com
>> Subject: Re: [f2fs-dev] [PATCH] back-up raw_super in sbi
>>
>> Hi Yunlei,
>>
>> On Tue, Dec 08, 2015 at 09:17:13PM +0800, Yunlei He wrote:
>>> write_checkpoint() tries to get cp_blkaddr from superblock buffer,
>>> if the buffer happen to be destroied by something else, it may
>
> Yunlei,
>
> You mean hacking in memory? could you share more about process of destroying?

I do some test on kernel version 3.10 like this:

with mounted f2fs partition, use dd to erase the first sb

dd if=/dev/zero of=/dev/sdx bs=4k count=1

then sync, and the system will panic.

the log added in function do_checkpoint show:

heyunlei:start_blk = 0

So maybe dd dirty the super block buffer stored in sbi.

But, in latest dev branch, it has no problem, I don't know why.


>
>>> bring in unpredictable effect on f2fs.
>>>
>>> this patch fix it by back-up a raw_super copy.
>>>
>>> Signed-off-by: Yunlei He <heyunlei@huawei.com>
>>> ---
>>>   fs/f2fs/checkpoint.c |  3 ++-
>>>   fs/f2fs/f2fs.h       | 10 ++++++++++
>>>   fs/f2fs/node.c       |  2 +-
>>>   fs/f2fs/super.c      | 12 ++++++++----
>>>   4 files changed, 21 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/fs/f2fs/checkpoint.c b/fs/f2fs/checkpoint.c
>>> index f661d80..1e342fd 100644
>>> --- a/fs/f2fs/checkpoint.c
>>> +++ b/fs/f2fs/checkpoint.c
>>> @@ -276,13 +276,14 @@ long sync_meta_pages(struct f2fs_sb_info *sbi, enum page_type type,
>>>   						long nr_to_write)
>>>   {
>>>   	struct address_space *mapping = META_MAPPING(sbi);
>>> -	pgoff_t index = 0, end = LONG_MAX, prev = LONG_MAX;
>>> +	pgoff_t index, end = LONG_MAX, prev = LONG_MAX;
>>>   	struct pagevec pvec;
>>>   	long nwritten = 0;
>>>   	struct writeback_control wbc = {
>>>   		.for_reclaim = 0,
>>>   	};
>>>
>>> +	index = __start_cp_addr(sbi);
>>
>> No need to do this.
>>
>>>   	pagevec_init(&pvec, 0);
>>>
>>>   	while (index <= end) {
>>> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
>>> index 0052ae8..6afb56c 100644
>>> --- a/fs/f2fs/f2fs.h
>>> +++ b/fs/f2fs/f2fs.h
>>> @@ -1167,6 +1167,16 @@ static inline block_t __start_sum_addr(struct f2fs_sb_info *sbi)
>>>   	return le32_to_cpu(F2FS_CKPT(sbi)->cp_pack_start_sum);
>>>   }
>>>
>>> +static inline block_t __start_main_addr(struct f2fs_sb_info *sbi)
>>> +{
>>> +	return le32_to_cpu(F2FS_RAW_SUPER(sbi)->main_blkaddr);
>>> +}
>>> +
>>> +static inline block_t __start_sum_addr(struct f2fs_sb_info *sbi)
>>> +{
>>> +	return le32_to_cpu(F2FS_CKPT(sbi)->cp_pack_start_sum);
>>> +}
>>> +
>>
>> Ditto.
>>
>>>   static inline bool inc_valid_node_count(struct f2fs_sb_info *sbi,
>>>   						struct inode *inode)
>>>   {
>>> diff --git a/fs/f2fs/node.c b/fs/f2fs/node.c
>>> index 7bcbc6e..7d2b0bb 100644
>>> --- a/fs/f2fs/node.c
>>> +++ b/fs/f2fs/node.c
>>> @@ -1161,7 +1161,7 @@ int sync_node_pages(struct f2fs_sb_info *sbi, nid_t ino,
>>>   	pagevec_init(&pvec, 0);
>>>
>>>   next_step:
>>> -	index = 0;
>>> +	index = __start_main_addr(sbi);
>>
>> No, its index is for nid.
>>
>>>   	end = LONG_MAX;
>>>
>>>   	while (index <= end) {
>>> diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
>>> index bd7e9c6..d394711 100644
>>> --- a/fs/f2fs/super.c
>>> +++ b/fs/f2fs/super.c
>>> @@ -1040,6 +1040,9 @@ static int read_raw_super_block(struct super_block *sb,
>>>   	struct f2fs_super_block *super;
>>>   	int err = 0;
>>>
>>> +	super = kzalloc(sizeof(struct f2fs_super_block), GFP_KERNEL);
>>> +	if (!super)
>>> +		return -ENOMEM;
>>>   retry:
>>>   	buffer = sb_bread(sb, block);
>>>   	if (!buffer) {
>>> @@ -1055,9 +1058,8 @@ retry:
>>>   		}
>>>   	}
>>>
>>> -	super = (struct f2fs_super_block *)
>>> -		((char *)(buffer)->b_data + F2FS_SUPER_OFFSET);
>>> -
>>> +	memcpy(super, buffer->b_data + F2FS_SUPER_OFFSET,
>>> +					sizeof(struct f2fs_super_block));
>>>   	/* sanity checking of raw super */
>>>   	if (sanity_check_raw_super(sb, super)) {
>>>   		brelse(buffer);
>>> @@ -1090,8 +1092,10 @@ retry:
>>>
>>>   out:
>>>   	/* No valid superblock */
>>> -	if (!*raw_super)
>>> +	if (!*raw_super) {
>>> +		kfree(super);
>>>   		return err;
>>> +	}
>>
>> Need to consider f2fs_commit_super and kfree() in put_super.
>
> How about releasing grabbed block buffer 'sbi->raw_super_buf' since we
> already had one copy of it?
>
> Thanks,
>
>>
>> Could you check out this patch?
>>
>> ---
>>   fs/f2fs/super.c | 18 ++++++++++++++----
>>   1 file changed, 14 insertions(+), 4 deletions(-)
>>
>> diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
>> index dbf16ad..8541c93 100644
>> --- a/fs/f2fs/super.c
>> +++ b/fs/f2fs/super.c
>> @@ -567,6 +567,7 @@ static void f2fs_put_super(struct super_block *sb)
>>
>>   	sb->s_fs_info = NULL;
>>   	brelse(sbi->raw_super_buf);
>> +	kfree(sbi->raw_super);
>>   	kfree(sbi);
>>   }
>>
>> @@ -1040,6 +1041,9 @@ static int read_raw_super_block(struct super_block *sb,
>>   	struct f2fs_super_block *super;
>>   	int err = 0;
>>
>> +	super = kzalloc(sizeof(struct f2fs_super_block), GFP_KERNEL);
>> +	if (!super)
>> +		return -ENOMEM;
>>   retry:
>>   	buffer = sb_bread(sb, block);
>>   	if (!buffer) {
>> @@ -1055,8 +1059,7 @@ retry:
>>   		}
>>   	}
>>
>> -	super = (struct f2fs_super_block *)
>> -		((char *)(buffer)->b_data + F2FS_SUPER_OFFSET);
>> +	memcpy(super, buffer->b_data + F2FS_SUPER_OFFSET, sizeof(*super));
>>
>>   	/* sanity checking of raw super */
>>   	if (sanity_check_raw_super(sb, super)) {
>> @@ -1090,14 +1093,17 @@ retry:
>>
>>   out:
>>   	/* No valid superblock */
>> -	if (!*raw_super)
>> +	if (!*raw_super) {
>> +		kfree(super);
>>   		return err;
>> +	}
>>
>>   	return 0;
>>   }
>>
>>   int f2fs_commit_super(struct f2fs_sb_info *sbi, bool recover)
>>   {
>> +	struct f2fs_super_block *super = F2FS_RAW_SUPER(sbi);
>>   	struct buffer_head *sbh = sbi->raw_super_buf;
>>   	struct buffer_head *bh;
>>   	int err;
>> @@ -1108,7 +1114,7 @@ int f2fs_commit_super(struct f2fs_sb_info *sbi, bool recover)
>>   		return -EIO;
>>
>>   	lock_buffer(bh);
>> -	memcpy(bh->b_data, sbh->b_data, sbh->b_size);
>> +	memcpy(bh->b_data + F2FS_SUPER_OFFSET, super, sizeof(*super));
>>   	WARN_ON(sbh->b_size != F2FS_BLKSIZE);
>>   	set_buffer_uptodate(bh);
>>   	set_buffer_dirty(bh);
>> @@ -1124,6 +1130,10 @@ int f2fs_commit_super(struct f2fs_sb_info *sbi, bool recover)
>>
>>   	/* write current valid superblock */
>>   	lock_buffer(sbh);
>> +	if (memcmp(sbh->b_data + F2FS_SUPER_OFFSET, super, sizeof(*super))) {
>> +		f2fs_msg(sbi->sb, KERN_INFO, "Write modified valid superblock");
>> +		memcpy(sbh->b_data + F2FS_SUPER_OFFSET, super, sizeof(*super));
>> +	}
>>   	set_buffer_dirty(sbh);
>>   	unlock_buffer(sbh);
>>
>> --
>> 2.4.9 (Apple Git-60)
>
>
>
> .
>


------------------------------------------------------------------------------

Re: [PATCH] back-up raw_super in sbi

[Chao Yu]

Hi all,

> -----Original Message-----
> From: He YunLei [mailto:heyunlei@huawei.com]
> Sent: Wednesday, December 09, 2015 3:15 PM
> To: Chao Yu
> Cc: 'Jaegeuk Kim'; linux-f2fs-devel@lists.sourceforge.net; hebiao6@huawei.com;
> stuart.li@huawei.com
> Subject: Re: [f2fs-dev] [PATCH] back-up raw_super in sbi
> 
> On 2015/12/9 14:11, Chao Yu wrote:
> > Hi,
> >
> >> -----Original Message-----
> >> From: Jaegeuk Kim [mailto:jaegeuk@kernel.org]
> >> Sent: Wednesday, December 09, 2015 2:19 AM
> >> To: Yunlei He
> >> Cc: linux-f2fs-devel@lists.sourceforge.net; chao2.yu@samsung.com; hebiao6@huawei.com;
> >> stuart.li@huawei.com
> >> Subject: Re: [f2fs-dev] [PATCH] back-up raw_super in sbi
> >>
> >> Hi Yunlei,
> >>
> >> On Tue, Dec 08, 2015 at 09:17:13PM +0800, Yunlei He wrote:
> >>> write_checkpoint() tries to get cp_blkaddr from superblock buffer,
> >>> if the buffer happen to be destroied by something else, it may
> >
> > Yunlei,
> >
> > You mean hacking in memory? could you share more about process of destroying?
> 
> I do some test on kernel version 3.10 like this:
> 
> with mounted f2fs partition, use dd to erase the first sb
> 
> dd if=/dev/zero of=/dev/sdx bs=4k count=1
> 
> then sync, and the system will panic.
> 
> the log added in function do_checkpoint show:
> 
> heyunlei:start_blk = 0
> 
> So maybe dd dirty the super block buffer stored in sbi.

Ah, I can understand your concerns now, thanks for your explanation! :)

> 
> But, in latest dev branch, it has no problem, I don't know why.

I do the test with latest dev branch, it shows warning dmesg info as
F2FS_CHECK_FS is off. I think it has problem actually.

> >> Need to consider f2fs_commit_super and kfree() in put_super.
> >
> > How about releasing grabbed block buffer 'sbi->raw_super_buf' since we
> > already had one copy of it?

Hi Jaegeuk, Yunlei,

Any thoughts?


------------------------------------------------------------------------------