Re: [PATCH] back-up raw_super in sbi

[Jaegeuk Kim <jaegeuk@kernel.org>]

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