linux-f2fs-devel.lists.sourceforge.net archive mirror
 help / color / mirror / Atom feed
From: Junling Zheng <zhengjunling@huawei.com>
To: Chao Yu <chao@kernel.org>, jaegeuk@kernel.org, yuchao0@huawei.com
Cc: miaoxie@huawei.com, linux-f2fs-devel@lists.sourceforge.net
Subject: Re: [RFC PATCH 3/4] f2fs-tools: unify the writeback of superblock
Date: Tue, 14 Aug 2018 10:08:36 +0800	[thread overview]
Message-ID: <36bd4ddb-cd08-921c-01eb-e06e1578b58c@huawei.com> (raw)
In-Reply-To: <59b7e960-7d32-d233-0c2e-7628a1e50bf6@kernel.org>

On 2018/8/13 22:25, Chao Yu wrote:
> On 2018/8/13 21:32, Junling Zheng wrote:
>> Introduce __write_superblock() to support updating specified one
>> superblock or both, thus we can wrapper it in update_superblock() and
>> f2fs_write_super_block to unify all places where sb needs to be updated.
>>
>> Signed-off-by: Junling Zheng <zhengjunling@huawei.com>
>> ---
>>  fsck/fsck.h        |  2 +-
>>  fsck/mount.c       | 74 +++++++++++-----------------------------------
>>  fsck/resize.c      | 20 ++-----------
>>  include/f2fs_fs.h  | 31 +++++++++++++++++++
>>  mkfs/f2fs_format.c | 19 +-----------
>>  5 files changed, 52 insertions(+), 94 deletions(-)
>>
>> diff --git a/fsck/fsck.h b/fsck/fsck.h
>> index 6042e68..e3490e6 100644
>> --- a/fsck/fsck.h
>> +++ b/fsck/fsck.h
>> @@ -178,7 +178,7 @@ extern void move_curseg_info(struct f2fs_sb_info *, u64, int);
>>  extern void write_curseg_info(struct f2fs_sb_info *);
>>  extern int find_next_free_block(struct f2fs_sb_info *, u64 *, int, int);
>>  extern void write_checkpoint(struct f2fs_sb_info *);
>> -extern void write_superblock(struct f2fs_super_block *);
>> +extern void update_superblock(struct f2fs_super_block *, int);
>>  extern void update_data_blkaddr(struct f2fs_sb_info *, nid_t, u16, block_t);
>>  extern void update_nat_blkaddr(struct f2fs_sb_info *, nid_t, nid_t, block_t);
>>  
>> diff --git a/fsck/mount.c b/fsck/mount.c
>> index 58ef3e6..e7ceb8d 100644
>> --- a/fsck/mount.c
>> +++ b/fsck/mount.c
>> @@ -476,7 +476,7 @@ void print_sb_state(struct f2fs_super_block *sb)
>>  }
>>  
>>  static inline int sanity_check_area_boundary(struct f2fs_super_block *sb,
>> -							u64 offset)
>> +							enum SB_ADDR sb_addr)
>>  {
>>  	u32 segment0_blkaddr = get_sb(segment0_blkaddr);
>>  	u32 cp_blkaddr = get_sb(cp_blkaddr);
>> @@ -542,14 +542,11 @@ static inline int sanity_check_area_boundary(struct f2fs_super_block *sb,
>>  			segment_count_main << log_blocks_per_seg);
>>  		return -1;
>>  	} else if (main_end_blkaddr < seg_end_blkaddr) {
>> -		int err;
>> -
>>  		set_sb(segment_count, (main_end_blkaddr -
>>  				segment0_blkaddr) >> log_blocks_per_seg);
>>  
>> -		err = dev_write(sb, offset, sizeof(struct f2fs_super_block));
>> -		MSG(0, "Info: Fix alignment: %s, start(%u) end(%u) block(%u)\n",
>> -			err ? "failed": "done",
>> +		update_superblock(sb, 1 << sb_addr);
>> +		MSG(0, "Info: Fix alignment: start(%u) end(%u) block(%u)\n",
>>  			main_blkaddr,
>>  			segment0_blkaddr +
>>  				(segment_count << log_blocks_per_seg),
>> @@ -558,7 +555,7 @@ static inline int sanity_check_area_boundary(struct f2fs_super_block *sb,
>>  	return 0;
>>  }
>>  
>> -int sanity_check_raw_super(struct f2fs_super_block *sb, u64 offset)
>> +int sanity_check_raw_super(struct f2fs_super_block *sb, enum SB_ADDR sb_addr)
>>  {
>>  	unsigned int blocksize;
>>  
>> @@ -600,30 +597,24 @@ int sanity_check_raw_super(struct f2fs_super_block *sb, u64 offset)
>>  	if (get_sb(segment_count) > F2FS_MAX_SEGMENT)
>>  		return -1;
>>  
>> -	if (sanity_check_area_boundary(sb, offset))
>> +	if (sanity_check_area_boundary(sb, sb_addr))
>>  		return -1;
>>  	return 0;
>>  }
>>  
>> -int validate_super_block(struct f2fs_sb_info *sbi, int block)
>> +int validate_super_block(struct f2fs_sb_info *sbi, enum SB_ADDR sb_addr)
>>  {
>> -	u64 offset;
>>  	char buf[F2FS_BLKSIZE];
>>  
>>  	sbi->raw_super = malloc(sizeof(struct f2fs_super_block));
>>  
>> -	if (block == 0)
>> -		offset = F2FS_SUPER_OFFSET;
>> -	else
>> -		offset = F2FS_BLKSIZE + F2FS_SUPER_OFFSET;
>> -
>> -	if (dev_read_block(buf, block))
>> +	if (dev_read_block(buf, sb_addr))
>>  		return -1;
>>  
>>  	memcpy(sbi->raw_super, buf + F2FS_SUPER_OFFSET,
>>  					sizeof(struct f2fs_super_block));
>>  
>> -	if (!sanity_check_raw_super(sbi->raw_super, offset)) {
>> +	if (!sanity_check_raw_super(sbi->raw_super, sb_addr)) {
>>  		/* get kernel version */
>>  		if (c.kd >= 0) {
>>  			dev_read_version(c.version, 0, VERSION_LEN);
>> @@ -642,13 +633,9 @@ int validate_super_block(struct f2fs_sb_info *sbi, int block)
>>  		MSG(0, "Info: FSCK version\n  from \"%s\"\n    to \"%s\"\n",
>>  					c.sb_version, c.version);
>>  		if (memcmp(c.sb_version, c.version, VERSION_LEN)) {
>> -			int ret;
>> -
>>  			memcpy(sbi->raw_super->version,
>>  						c.version, VERSION_LEN);
>> -			ret = dev_write(sbi->raw_super, offset,
>> -					sizeof(struct f2fs_super_block));
>> -			ASSERT(ret >= 0);
>> +			update_superblock(sbi->raw_super, 1 << sb_addr);
>>  
>>  			c.auto_fix = 0;
>>  			c.fix_on = 1;
>> @@ -659,7 +646,7 @@ int validate_super_block(struct f2fs_sb_info *sbi, int block)
>>  
>>  	free(sbi->raw_super);
>>  	sbi->raw_super = NULL;
>> -	MSG(0, "\tCan't find a valid F2FS superblock at 0x%x\n", block);
>> +	MSG(0, "\tCan't find a valid F2FS superblock at 0x%x\n", sb_addr);
>>  
>>  	return -EINVAL;
>>  }
>> @@ -2230,21 +2217,10 @@ void write_checkpoint(struct f2fs_sb_info *sbi)
>>  	ASSERT(ret >= 0);
>>  }
>>  
>> -void write_superblock(struct f2fs_super_block *new_sb)
>> +void update_superblock(struct f2fs_super_block *new_sb, int sb_mask)
>>  {
>> -	int index, ret;
>> -	u_int8_t *buf;
>> -
>> -	buf = calloc(BLOCK_SZ, 1);
>> -	ASSERT(buf);
>> -
>> -	memcpy(buf + F2FS_SUPER_OFFSET, new_sb, sizeof(*new_sb));
>> -	for (index = 0; index < 2; index++) {
>> -		ret = dev_write_block(buf, index);
>> -		ASSERT(ret >= 0);
>> -	}
>> -	free(buf);
>> -	DBG(0, "Info: Done to rebuild superblock\n");
>> +	ASSERT(__write_superblock(new_sb, sb_mask));
>> +	DBG(0, "Info: Done to update superblock\n");
>>  }
>>  
>>  void build_nat_area_bitmap(struct f2fs_sb_info *sbi)
>> @@ -2385,9 +2361,7 @@ void build_nat_area_bitmap(struct f2fs_sb_info *sbi)
>>  
>>  static int check_sector_size(struct f2fs_super_block *sb)
>>  {
>> -	int index;
>>  	u_int32_t log_sectorsize, log_sectors_per_block;
>> -	u_int8_t *zero_buff;
>>  
>>  	log_sectorsize = log_base_2(c.sector_size);
>>  	log_sectors_per_block = log_base_2(c.sectors_per_blk);
>> @@ -2396,24 +2370,10 @@ static int check_sector_size(struct f2fs_super_block *sb)
>>  			log_sectors_per_block == get_sb(log_sectors_per_block))
>>  		return 0;
>>  
>> -	zero_buff = calloc(F2FS_BLKSIZE, 1);
>> -	ASSERT(zero_buff);
>> -
>>  	set_sb(log_sectorsize, log_sectorsize);
>>  	set_sb(log_sectors_per_block, log_sectors_per_block);
>>  
>> -	memcpy(zero_buff + F2FS_SUPER_OFFSET, sb, sizeof(*sb));
>> -	DBG(1, "\tWriting super block, at offset 0x%08x\n", 0);
>> -	for (index = 0; index < 2; index++) {
>> -		if (dev_write(zero_buff, index * F2FS_BLKSIZE, F2FS_BLKSIZE)) {
>> -			MSG(1, "\tError: Failed while writing supe_blk "
>> -				"on disk!!! index : %d\n", index);
>> -			free(zero_buff);
>> -			return -1;
>> -		}
>> -	}
>> -
>> -	free(zero_buff);
>> +	update_superblock(sb, SB_ALL);
>>  	return 0;
>>  }
>>  
>> @@ -2434,7 +2394,7 @@ static void tune_sb_features(struct f2fs_sb_info *sbi)
>>  	if (!sb_changed)
>>  		return;
>>  
>> -	write_superblock(sb);
>> +	update_superblock(sb, SB_ALL);
>>  }
>>  
>>  int f2fs_do_mount(struct f2fs_sb_info *sbi)
>> @@ -2444,9 +2404,9 @@ int f2fs_do_mount(struct f2fs_sb_info *sbi)
>>  	int ret;
>>  
>>  	sbi->active_logs = NR_CURSEG_TYPE;
>> -	ret = validate_super_block(sbi, 0);
>> +	ret = validate_super_block(sbi, SB_ADDR_0);
>>  	if (ret) {
>> -		ret = validate_super_block(sbi, 1);
>> +		ret = validate_super_block(sbi, SB_ADDR_1);
>>  		if (ret)
>>  			return -1;
>>  	}
>> diff --git a/fsck/resize.c b/fsck/resize.c
>> index e9612b3..5161a1f 100644
>> --- a/fsck/resize.c
>> +++ b/fsck/resize.c
>> @@ -577,22 +577,6 @@ static void rebuild_checkpoint(struct f2fs_sb_info *sbi,
>>  	DBG(0, "Info: Done to rebuild checkpoint blocks\n");
>>  }
>>  
>> -static void rebuild_superblock(struct f2fs_super_block *new_sb)
>> -{
>> -	int index, ret;
>> -	u_int8_t *buf;
>> -
>> -	buf = calloc(BLOCK_SZ, 1);
>> -
>> -	memcpy(buf + F2FS_SUPER_OFFSET, new_sb, sizeof(*new_sb));
>> -	for (index = 0; index < 2; index++) {
>> -		ret = dev_write_block(buf, index);
>> -		ASSERT(ret >= 0);
>> -	}
>> -	free(buf);
>> -	DBG(0, "Info: Done to rebuild superblock\n");
>> -}
>> -
>>  static int f2fs_resize_grow(struct f2fs_sb_info *sbi)
>>  {
>>  	struct f2fs_super_block *sb = F2FS_RAW_SUPER(sbi);
>> @@ -644,7 +628,7 @@ static int f2fs_resize_grow(struct f2fs_sb_info *sbi)
>>  	migrate_nat(sbi, new_sb);
>>  	migrate_sit(sbi, new_sb, offset_seg);
>>  	rebuild_checkpoint(sbi, new_sb, offset_seg);
>> -	write_superblock(new_sb);
>> +	update_superblock(new_sb, SB_ALL);
>>  	return 0;
>>  }
>>  
>> @@ -695,7 +679,7 @@ static int f2fs_resize_shrink(struct f2fs_sb_info *sbi)
>>  		return -ENOSPC;
>>  	}
>>  
>> -	rebuild_superblock(new_sb);
>> +	update_superblock(new_sb, SB_ALL);
>>  	rebuild_checkpoint(sbi, new_sb, 0);
>>  	/*if (!c.safe_resize) {
>>  		migrate_sit(sbi, new_sb, offset_seg);
>> diff --git a/include/f2fs_fs.h b/include/f2fs_fs.h
>> index e279b9f..b0d9b9b 100644
>> --- a/include/f2fs_fs.h
>> +++ b/include/f2fs_fs.h
>> @@ -1410,4 +1410,35 @@ static inline int parse_root_owner(char *ids,
>>  	return 0;
>>  }
>>  
>> +enum SB_ADDR {
>> +	SB_ADDR_0,
>> +	SB_ADDR_1,
>> +	SB_ADDR_MAX,
>> +};
>> +
>> +#define SB_FIRST	(1 << SB_ADDR_0)
>> +#define SB_SECOND	(1 << SB_ADDR_1)
>> +#define SB_ALL		(SB_FIRST | SB_SECOND)
>> +
>> +static inline int __write_superblock(struct f2fs_super_block *sb, int sb_mask)
>> +{
>> +	int index, ret;
>> +	u_int8_t *buf;
>> +
>> +	buf = calloc(F2FS_BLKSIZE, 1);
>> +	ASSERT(buf);
>> +
>> +	memcpy(buf + F2FS_SUPER_OFFSET, sb, sizeof(*sb));
>> +	for (index = 0; index < SB_ADDR_MAX; index++) {
>> +		if ((1 << index) & sb_mask) {
>> +			ret = dev_write_block(buf, index);
>> +			if (ret)
> 
> Leaving some messages here and call free(buf) to avoid memory leak
> 

:( Sorry, I'll fix it in the next version.

> Thanks,
> 
>> +				return ret;
>> +		}
>> +	}
>> +
>> +	free(buf);
>> +	return 0;
>> +}
>> +
>>  #endif	/*__F2FS_FS_H */
>> diff --git a/mkfs/f2fs_format.c b/mkfs/f2fs_format.c
>> index 621126c..7e3846e 100644
>> --- a/mkfs/f2fs_format.c
>> +++ b/mkfs/f2fs_format.c
>> @@ -979,24 +979,7 @@ free_cp:
>>  
>>  static int f2fs_write_super_block(void)
>>  {
>> -	int index;
>> -	u_int8_t *zero_buff;
>> -
>> -	zero_buff = calloc(F2FS_BLKSIZE, 1);
>> -
>> -	memcpy(zero_buff + F2FS_SUPER_OFFSET, sb, sizeof(*sb));
>> -	DBG(1, "\tWriting super block, at offset 0x%08x\n", 0);
>> -	for (index = 0; index < 2; index++) {
>> -		if (dev_write_block(zero_buff, index)) {
>> -			MSG(1, "\tError: While while writing supe_blk "
>> -					"on disk!!! index : %d\n", index);
>> -			free(zero_buff);
>> -			return -1;
>> -		}
>> -	}
>> -
>> -	free(zero_buff);
>> -	return 0;
>> +	return __write_superblock(sb, SB_ALL);
>>  }
>>  
>>  #ifndef WITH_ANDROID
>>
> 
> .
> 



------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot

  reply	other threads:[~2018-08-14  2:08 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-08-13 13:32 [RFC PATCH RESEND 1/4] f2fs: support superblock checksum Junling Zheng
2018-08-13 13:32 ` [RFC PATCH RESEND 2/4] f2fs-tools: rename CHECKSUM_OFFSET to CP_CHKSUM_OFFSET Junling Zheng
2018-08-13 13:32 ` [RFC PATCH 3/4] f2fs-tools: unify the writeback of superblock Junling Zheng
2018-08-13 14:25   ` Chao Yu
2018-08-14  2:08     ` Junling Zheng [this message]
2018-08-13 13:32 ` [RFC PATCH 4/4] f2fs-tools: introduce sb checksum Junling Zheng
2018-08-13 14:33   ` Chao Yu
2018-08-14  2:03     ` Junling Zheng
2018-08-14  5:54       ` Chao Yu

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=36bd4ddb-cd08-921c-01eb-e06e1578b58c@huawei.com \
    --to=zhengjunling@huawei.com \
    --cc=chao@kernel.org \
    --cc=jaegeuk@kernel.org \
    --cc=linux-f2fs-devel@lists.sourceforge.net \
    --cc=miaoxie@huawei.com \
    --cc=yuchao0@huawei.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).