Re: [PATCH 2/2] f2fs: introduce CP_TRIMMED_FLAG to avoid unneeded discard

[Chao Yu]

Hi Jaegeuk,

On 2017/4/28 2:16, Jaegeuk Kim wrote:
> Hi Chao,
> 
> Could you add this state in print_cp_state() of f2fs-tools as well?

Oh, right, and we can set the flag after trimming in mkfs, also we need to
adjust this state into trace info.

Let me fix this. :)

Thanks,

> 
> Thanks,
> 
> On 04/27, Chao Yu wrote:
>> Introduce CP_TRIMMED_FLAG to indicate all invalid block were trimmed
>> before umount, so once we do mount with image which contain the flag,
>> we don't record invalid blocks as undiscard one, when fstrim is being
>> triggered, we can avoid issuing redundant discard commands.
>>
>> Signed-off-by: Chao Yu <yuchao0@xxxxxxxxxx>
>> ---
>>  fs/f2fs/checkpoint.c    |  3 +++
>>  fs/f2fs/f2fs.h          |  1 +
>>  fs/f2fs/segment.c       | 28 ++++++++++++++++++++--------
>>  fs/f2fs/super.c         |  7 +++++++
>>  include/linux/f2fs_fs.h |  1 +
>>  5 files changed, 32 insertions(+), 8 deletions(-)
>>
>> diff --git a/fs/f2fs/checkpoint.c b/fs/f2fs/checkpoint.c
>> index 27578903eeb6..ea9c317b5916 100644
>> --- a/fs/f2fs/checkpoint.c
>> +++ b/fs/f2fs/checkpoint.c
>> @@ -1059,6 +1059,9 @@ static void update_ckpt_flags(struct f2fs_sb_info *sbi, struct cp_control *cpc)
>>  			sbi->blocks_per_seg - NM_I(sbi)->nat_bits_blocks)
>>  		disable_nat_bits(sbi, false);
>>  
>> +	if (cpc->reason & CP_TRIMMED)
>> +		__set_ckpt_flags(ckpt, CP_TRIMMED_FLAG);
>> +
>>  	if (cpc->reason & CP_UMOUNT)
>>  		__set_ckpt_flags(ckpt, CP_UMOUNT_FLAG);
>>  	else
>> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
>> index 453eaa943a21..c6fce1631289 100644
>> --- a/fs/f2fs/f2fs.h
>> +++ b/fs/f2fs/f2fs.h
>> @@ -130,6 +130,7 @@ enum {
>>  #define	CP_SYNC		0x00000004
>>  #define	CP_RECOVERY	0x00000008
>>  #define	CP_DISCARD	0x00000010
>> +#define CP_TRIMMED	0x00000020
>>  
>>  #define DEF_BATCHED_TRIM_SECTIONS	2048
>>  #define BATCHED_TRIM_SEGMENTS(sbi)	\
>> diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
>> index a2cd10cdff60..e4dd1e4a42c9 100644
>> --- a/fs/f2fs/segment.c
>> +++ b/fs/f2fs/segment.c
>> @@ -2989,10 +2989,17 @@ static void build_sit_entries(struct f2fs_sb_info *sbi)
>>  
>>  			/* build discard map only one time */
>>  			if (f2fs_discard_en(sbi)) {
>> -				memcpy(se->discard_map, se->cur_valid_map,
>> -							SIT_VBLOCK_MAP_SIZE);
>> -				sbi->discard_blks += sbi->blocks_per_seg -
>> -							se->valid_blocks;
>> +				if (is_set_ckpt_flags(sbi, CP_TRIMMED_FLAG)) {
>> +					memset(se->discard_map, 0xff,
>> +						SIT_VBLOCK_MAP_SIZE);
>> +				} else {
>> +					memcpy(se->discard_map,
>> +						se->cur_valid_map,
>> +						SIT_VBLOCK_MAP_SIZE);
>> +					sbi->discard_blks +=
>> +						sbi->blocks_per_seg -
>> +						se->valid_blocks;
>> +				}
>>  			}
>>  
>>  			if (sbi->segs_per_sec > 1)
>> @@ -3016,10 +3023,15 @@ static void build_sit_entries(struct f2fs_sb_info *sbi)
>>  		seg_info_from_raw_sit(se, &sit);
>>  
>>  		if (f2fs_discard_en(sbi)) {
>> -			memcpy(se->discard_map, se->cur_valid_map,
>> -						SIT_VBLOCK_MAP_SIZE);
>> -			sbi->discard_blks += old_valid_blocks -
>> -						se->valid_blocks;
>> +			if (is_set_ckpt_flags(sbi, CP_TRIMMED_FLAG)) {
>> +				memset(se->discard_map, 0xff,
>> +							SIT_VBLOCK_MAP_SIZE);
>> +			} else {
>> +				memcpy(se->discard_map, se->cur_valid_map,
>> +							SIT_VBLOCK_MAP_SIZE);
>> +				sbi->discard_blks += old_valid_blocks -
>> +							se->valid_blocks;
>> +			}
>>  		}
>>  
>>  		if (sbi->segs_per_sec > 1)
>> diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
>> index 4cd3bee6775f..9a14b2590337 100644
>> --- a/fs/f2fs/super.c
>> +++ b/fs/f2fs/super.c
>> @@ -797,6 +797,13 @@ static void f2fs_put_super(struct super_block *sb)
>>  	/* be sure to wait for any on-going discard commands */
>>  	f2fs_wait_discard_bios(sbi);
>>  
>> +	if (!sbi->discard_blks) {
>> +		struct cp_control cpc = {
>> +			.reason = CP_UMOUNT | CP_TRIMMED,
>> +		};
>> +		write_checkpoint(sbi, &cpc);
>> +	}
>> +
>>  	/* write_checkpoint can update stat informaion */
>>  	f2fs_destroy_stats(sbi);
>>  
>> diff --git a/include/linux/f2fs_fs.h b/include/linux/f2fs_fs.h
>> index 093549e10ee2..b6feed6547ce 100644
>> --- a/include/linux/f2fs_fs.h
>> +++ b/include/linux/f2fs_fs.h
>> @@ -114,6 +114,7 @@ struct f2fs_super_block {
>>  /*
>>   * For checkpoint
>>   */
>> +#define CP_TRIMMED_FLAG		0x00000100
>>  #define CP_NAT_BITS_FLAG	0x00000080
>>  #define CP_CRC_RECOVERY_FLAG	0x00000040
>>  #define CP_FASTBOOT_FLAG	0x00000020
>> -- 
>> 2.12.2.510.ge1104a5ee539
> 
> .
>