linux-f2fs-devel.lists.sourceforge.net archive mirror
 help / color / mirror / Atom feed
* [f2fs-dev] [PATCH] f2fs: allocate consective blkaddrs for defragment
@ 2022-08-12  3:56 Weichao Guo via Linux-f2fs-devel
  2022-08-12 17:38 ` Jaegeuk Kim
  0 siblings, 1 reply; 6+ messages in thread
From: Weichao Guo via Linux-f2fs-devel @ 2022-08-12  3:56 UTC (permalink / raw)
  To: jaegeuk, chao; +Cc: zhangshiming, linux-f2fs-devel

When we try to defrag a file, its data blocks may mess with others if there
are lots of concurrent writes. This causes the file is still fragmented
after defrag. So It's better to isolate defrag writes from others.

Signed-off-by: Weichao Guo <guoweichao@oppo.com>
Signed-off-by: Chao Yu <chao@kernel.org>
---
 fs/f2fs/debug.c   | 4 ++++
 fs/f2fs/f2fs.h    | 2 ++
 fs/f2fs/file.c    | 5 +++++
 fs/f2fs/segment.c | 7 +++++++
 fs/f2fs/segment.h | 5 ++++-
 5 files changed, 22 insertions(+), 1 deletion(-)

diff --git a/fs/f2fs/debug.c b/fs/f2fs/debug.c
index c014715..d85dc17 100644
--- a/fs/f2fs/debug.c
+++ b/fs/f2fs/debug.c
@@ -442,6 +442,10 @@ static int stat_show(struct seq_file *s, void *v)
 			   si->curseg[CURSEG_ALL_DATA_ATGC],
 			   si->cursec[CURSEG_ALL_DATA_ATGC],
 			   si->curzone[CURSEG_ALL_DATA_ATGC]);
+		seq_printf(s, "  - DEFRAG   data: %8d %8d %8d\n",
+			   si->curseg[CURSEG_COLD_DATA_DEFRAG],
+			   si->cursec[CURSEG_COLD_DATA_DEFRAG],
+			   si->curzone[CURSEG_COLD_DATA_DEFRAG]);
 		seq_printf(s, "\n  - Valid: %d\n  - Dirty: %d\n",
 			   si->main_area_segs - si->dirty_count -
 			   si->prefree_count - si->free_segs,
diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index 3c7cdb7..5f9a185 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -760,6 +760,7 @@ enum {
 	FI_COMPRESS_RELEASED,	/* compressed blocks were released */
 	FI_ALIGNED_WRITE,	/* enable aligned write */
 	FI_COW_FILE,		/* indicate COW file */
+	FI_DEFRAG_WRITE,	/* indicate defragment writes need consective blkaddrs*/
 	FI_MAX,			/* max flag, never be used */
 };
 
@@ -1017,6 +1018,7 @@ enum {
 	CURSEG_COLD_DATA_PINNED = NR_PERSISTENT_LOG,
 				/* pinned file that needs consecutive block address */
 	CURSEG_ALL_DATA_ATGC,	/* SSR alloctor in hot/warm/cold data area */
+	CURSEG_COLD_DATA_DEFRAG,/* used for defragment which needs consective blkaddrs */
 	NO_CHECK_TYPE,		/* number of persistent & inmem log */
 };
 
diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
index ce4905a0..f104e2e 100644
--- a/fs/f2fs/file.c
+++ b/fs/f2fs/file.c
@@ -2626,7 +2626,12 @@ static int f2fs_defragment_range(struct f2fs_sb_info *sbi,
 
 		clear_inode_flag(inode, FI_SKIP_WRITES);
 
+		set_inode_flag(inode, FI_DEFRAG_WRITE);
+		f2fs_lock_op(sbi);
+		f2fs_allocate_new_section(sbi, CURSEG_COLD_DATA_DEFRAG, false);
+		f2fs_unlock_op(sbi);
 		err = filemap_fdatawrite(inode->i_mapping);
+		clear_inode_flag(inode, FI_DEFRAG_WRITE);
 		if (err)
 			goto out;
 	}
diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
index 0de21f8..17e7d14 100644
--- a/fs/f2fs/segment.c
+++ b/fs/f2fs/segment.c
@@ -2749,6 +2749,7 @@ static void __f2fs_save_inmem_curseg(struct f2fs_sb_info *sbi, int type)
 void f2fs_save_inmem_curseg(struct f2fs_sb_info *sbi)
 {
 	__f2fs_save_inmem_curseg(sbi, CURSEG_COLD_DATA_PINNED);
+	__f2fs_save_inmem_curseg(sbi, CURSEG_COLD_DATA_DEFRAG);
 
 	if (sbi->am.atgc_enabled)
 		__f2fs_save_inmem_curseg(sbi, CURSEG_ALL_DATA_ATGC);
@@ -2774,6 +2775,7 @@ static void __f2fs_restore_inmem_curseg(struct f2fs_sb_info *sbi, int type)
 void f2fs_restore_inmem_curseg(struct f2fs_sb_info *sbi)
 {
 	__f2fs_restore_inmem_curseg(sbi, CURSEG_COLD_DATA_PINNED);
+	__f2fs_restore_inmem_curseg(sbi, CURSEG_COLD_DATA_DEFRAG);
 
 	if (sbi->am.atgc_enabled)
 		__f2fs_restore_inmem_curseg(sbi, CURSEG_ALL_DATA_ATGC);
@@ -3161,6 +3163,9 @@ static int __get_segment_type_6(struct f2fs_io_info *fio)
 		if (is_inode_flag_set(inode, FI_ALIGNED_WRITE))
 			return CURSEG_COLD_DATA_PINNED;
 
+		if (is_inode_flag_set(inode, FI_DEFRAG_WRITE))
+			return CURSEG_COLD_DATA_DEFRAG;
+
 		if (page_private_gcing(fio->page)) {
 			if (fio->sbi->am.atgc_enabled &&
 				(fio->io_type == FS_DATA_IO) &&
@@ -4332,6 +4337,8 @@ static int build_curseg(struct f2fs_sb_info *sbi)
 			array[i].seg_type = CURSEG_COLD_DATA;
 		else if (i == CURSEG_ALL_DATA_ATGC)
 			array[i].seg_type = CURSEG_COLD_DATA;
+		else if (i == CURSEG_COLD_DATA_DEFRAG)
+			array[i].seg_type = CURSEG_COLD_DATA;
 		array[i].segno = NULL_SEGNO;
 		array[i].next_blkoff = 0;
 		array[i].inited = false;
diff --git a/fs/f2fs/segment.h b/fs/f2fs/segment.h
index d1d6376..75e2aa8 100644
--- a/fs/f2fs/segment.h
+++ b/fs/f2fs/segment.h
@@ -44,7 +44,8 @@ static inline void sanity_check_seg_type(struct f2fs_sb_info *sbi,
 	 ((seg) == CURSEG_I(sbi, CURSEG_WARM_NODE)->segno) ||	\
 	 ((seg) == CURSEG_I(sbi, CURSEG_COLD_NODE)->segno) ||	\
 	 ((seg) == CURSEG_I(sbi, CURSEG_COLD_DATA_PINNED)->segno) ||	\
-	 ((seg) == CURSEG_I(sbi, CURSEG_ALL_DATA_ATGC)->segno))
+	 ((seg) == CURSEG_I(sbi, CURSEG_ALL_DATA_ATGC)->segno) ||	\
+	 ((seg) == CURSEG_I(sbi, CURSEG_COLD_DATA_DEFRAG)->segno))
 
 #define IS_CURSEC(sbi, secno)						\
 	(((secno) == CURSEG_I(sbi, CURSEG_HOT_DATA)->segno /		\
@@ -62,6 +63,8 @@ static inline void sanity_check_seg_type(struct f2fs_sb_info *sbi,
 	 ((secno) == CURSEG_I(sbi, CURSEG_COLD_DATA_PINNED)->segno /	\
 	  (sbi)->segs_per_sec) ||	\
 	 ((secno) == CURSEG_I(sbi, CURSEG_ALL_DATA_ATGC)->segno /	\
+	  (sbi)->segs_per_sec) ||	\
+	 ((secno) == CURSEG_I(sbi, CURSEG_COLD_DATA_DEFRAG)->segno /	\
 	  (sbi)->segs_per_sec))
 
 #define MAIN_BLKADDR(sbi)						\
-- 
2.7.4



_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [f2fs-dev] [PATCH] f2fs: allocate consective blkaddrs for defragment
  2022-08-12  3:56 [f2fs-dev] [PATCH] f2fs: allocate consective blkaddrs for defragment Weichao Guo via Linux-f2fs-devel
@ 2022-08-12 17:38 ` Jaegeuk Kim
  2022-08-15  3:30   ` Chao Yu
       [not found]   ` <DM5PR17MB095307935ABD5D5A6EEDD050C6699@DM5PR17MB0953.namprd17.prod.outlook.com>
  0 siblings, 2 replies; 6+ messages in thread
From: Jaegeuk Kim @ 2022-08-12 17:38 UTC (permalink / raw)
  To: Weichao Guo; +Cc: zhangshiming, linux-f2fs-devel

Hi Weichao,

On 08/12, Weichao Guo wrote:
> When we try to defrag a file, its data blocks may mess with others if there
> are lots of concurrent writes. This causes the file is still fragmented
> after defrag. So It's better to isolate defrag writes from others.

I really don't want to add more log like this. What about using
wb_sync_req[DATA] to prevent async writes at least?

> 
> Signed-off-by: Weichao Guo <guoweichao@oppo.com>
> Signed-off-by: Chao Yu <chao@kernel.org>
> ---
>  fs/f2fs/debug.c   | 4 ++++
>  fs/f2fs/f2fs.h    | 2 ++
>  fs/f2fs/file.c    | 5 +++++
>  fs/f2fs/segment.c | 7 +++++++
>  fs/f2fs/segment.h | 5 ++++-
>  5 files changed, 22 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/f2fs/debug.c b/fs/f2fs/debug.c
> index c014715..d85dc17 100644
> --- a/fs/f2fs/debug.c
> +++ b/fs/f2fs/debug.c
> @@ -442,6 +442,10 @@ static int stat_show(struct seq_file *s, void *v)
>  			   si->curseg[CURSEG_ALL_DATA_ATGC],
>  			   si->cursec[CURSEG_ALL_DATA_ATGC],
>  			   si->curzone[CURSEG_ALL_DATA_ATGC]);
> +		seq_printf(s, "  - DEFRAG   data: %8d %8d %8d\n",
> +			   si->curseg[CURSEG_COLD_DATA_DEFRAG],
> +			   si->cursec[CURSEG_COLD_DATA_DEFRAG],
> +			   si->curzone[CURSEG_COLD_DATA_DEFRAG]);
>  		seq_printf(s, "\n  - Valid: %d\n  - Dirty: %d\n",
>  			   si->main_area_segs - si->dirty_count -
>  			   si->prefree_count - si->free_segs,
> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> index 3c7cdb7..5f9a185 100644
> --- a/fs/f2fs/f2fs.h
> +++ b/fs/f2fs/f2fs.h
> @@ -760,6 +760,7 @@ enum {
>  	FI_COMPRESS_RELEASED,	/* compressed blocks were released */
>  	FI_ALIGNED_WRITE,	/* enable aligned write */
>  	FI_COW_FILE,		/* indicate COW file */
> +	FI_DEFRAG_WRITE,	/* indicate defragment writes need consective blkaddrs*/
>  	FI_MAX,			/* max flag, never be used */
>  };
>  
> @@ -1017,6 +1018,7 @@ enum {
>  	CURSEG_COLD_DATA_PINNED = NR_PERSISTENT_LOG,
>  				/* pinned file that needs consecutive block address */
>  	CURSEG_ALL_DATA_ATGC,	/* SSR alloctor in hot/warm/cold data area */
> +	CURSEG_COLD_DATA_DEFRAG,/* used for defragment which needs consective blkaddrs */
>  	NO_CHECK_TYPE,		/* number of persistent & inmem log */
>  };
>  
> diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
> index ce4905a0..f104e2e 100644
> --- a/fs/f2fs/file.c
> +++ b/fs/f2fs/file.c
> @@ -2626,7 +2626,12 @@ static int f2fs_defragment_range(struct f2fs_sb_info *sbi,
>  
>  		clear_inode_flag(inode, FI_SKIP_WRITES);
>  
> +		set_inode_flag(inode, FI_DEFRAG_WRITE);
> +		f2fs_lock_op(sbi);
> +		f2fs_allocate_new_section(sbi, CURSEG_COLD_DATA_DEFRAG, false);
> +		f2fs_unlock_op(sbi);
>  		err = filemap_fdatawrite(inode->i_mapping);
> +		clear_inode_flag(inode, FI_DEFRAG_WRITE);
>  		if (err)
>  			goto out;
>  	}
> diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
> index 0de21f8..17e7d14 100644
> --- a/fs/f2fs/segment.c
> +++ b/fs/f2fs/segment.c
> @@ -2749,6 +2749,7 @@ static void __f2fs_save_inmem_curseg(struct f2fs_sb_info *sbi, int type)
>  void f2fs_save_inmem_curseg(struct f2fs_sb_info *sbi)
>  {
>  	__f2fs_save_inmem_curseg(sbi, CURSEG_COLD_DATA_PINNED);
> +	__f2fs_save_inmem_curseg(sbi, CURSEG_COLD_DATA_DEFRAG);
>  
>  	if (sbi->am.atgc_enabled)
>  		__f2fs_save_inmem_curseg(sbi, CURSEG_ALL_DATA_ATGC);
> @@ -2774,6 +2775,7 @@ static void __f2fs_restore_inmem_curseg(struct f2fs_sb_info *sbi, int type)
>  void f2fs_restore_inmem_curseg(struct f2fs_sb_info *sbi)
>  {
>  	__f2fs_restore_inmem_curseg(sbi, CURSEG_COLD_DATA_PINNED);
> +	__f2fs_restore_inmem_curseg(sbi, CURSEG_COLD_DATA_DEFRAG);
>  
>  	if (sbi->am.atgc_enabled)
>  		__f2fs_restore_inmem_curseg(sbi, CURSEG_ALL_DATA_ATGC);
> @@ -3161,6 +3163,9 @@ static int __get_segment_type_6(struct f2fs_io_info *fio)
>  		if (is_inode_flag_set(inode, FI_ALIGNED_WRITE))
>  			return CURSEG_COLD_DATA_PINNED;
>  
> +		if (is_inode_flag_set(inode, FI_DEFRAG_WRITE))
> +			return CURSEG_COLD_DATA_DEFRAG;
> +
>  		if (page_private_gcing(fio->page)) {
>  			if (fio->sbi->am.atgc_enabled &&
>  				(fio->io_type == FS_DATA_IO) &&
> @@ -4332,6 +4337,8 @@ static int build_curseg(struct f2fs_sb_info *sbi)
>  			array[i].seg_type = CURSEG_COLD_DATA;
>  		else if (i == CURSEG_ALL_DATA_ATGC)
>  			array[i].seg_type = CURSEG_COLD_DATA;
> +		else if (i == CURSEG_COLD_DATA_DEFRAG)
> +			array[i].seg_type = CURSEG_COLD_DATA;
>  		array[i].segno = NULL_SEGNO;
>  		array[i].next_blkoff = 0;
>  		array[i].inited = false;
> diff --git a/fs/f2fs/segment.h b/fs/f2fs/segment.h
> index d1d6376..75e2aa8 100644
> --- a/fs/f2fs/segment.h
> +++ b/fs/f2fs/segment.h
> @@ -44,7 +44,8 @@ static inline void sanity_check_seg_type(struct f2fs_sb_info *sbi,
>  	 ((seg) == CURSEG_I(sbi, CURSEG_WARM_NODE)->segno) ||	\
>  	 ((seg) == CURSEG_I(sbi, CURSEG_COLD_NODE)->segno) ||	\
>  	 ((seg) == CURSEG_I(sbi, CURSEG_COLD_DATA_PINNED)->segno) ||	\
> -	 ((seg) == CURSEG_I(sbi, CURSEG_ALL_DATA_ATGC)->segno))
> +	 ((seg) == CURSEG_I(sbi, CURSEG_ALL_DATA_ATGC)->segno) ||	\
> +	 ((seg) == CURSEG_I(sbi, CURSEG_COLD_DATA_DEFRAG)->segno))
>  
>  #define IS_CURSEC(sbi, secno)						\
>  	(((secno) == CURSEG_I(sbi, CURSEG_HOT_DATA)->segno /		\
> @@ -62,6 +63,8 @@ static inline void sanity_check_seg_type(struct f2fs_sb_info *sbi,
>  	 ((secno) == CURSEG_I(sbi, CURSEG_COLD_DATA_PINNED)->segno /	\
>  	  (sbi)->segs_per_sec) ||	\
>  	 ((secno) == CURSEG_I(sbi, CURSEG_ALL_DATA_ATGC)->segno /	\
> +	  (sbi)->segs_per_sec) ||	\
> +	 ((secno) == CURSEG_I(sbi, CURSEG_COLD_DATA_DEFRAG)->segno /	\
>  	  (sbi)->segs_per_sec))
>  
>  #define MAIN_BLKADDR(sbi)						\
> -- 
> 2.7.4


_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [f2fs-dev] [PATCH] f2fs: allocate consective blkaddrs for defragment
  2022-08-12 17:38 ` Jaegeuk Kim
@ 2022-08-15  3:30   ` Chao Yu
  2022-08-19 23:42     ` Jaegeuk Kim
       [not found]   ` <DM5PR17MB095307935ABD5D5A6EEDD050C6699@DM5PR17MB0953.namprd17.prod.outlook.com>
  1 sibling, 1 reply; 6+ messages in thread
From: Chao Yu @ 2022-08-15  3:30 UTC (permalink / raw)
  To: Jaegeuk Kim, Weichao Guo; +Cc: zhangshiming, linux-f2fs-devel

Hi Jaegeuk,

On 2022/8/13 1:38, Jaegeuk Kim wrote:
> Hi Weichao,
> 
> On 08/12, Weichao Guo wrote:
>> When we try to defrag a file, its data blocks may mess with others if there
>> are lots of concurrent writes. This causes the file is still fragmented
>> after defrag. So It's better to isolate defrag writes from others.
> 
> I really don't want to add more log like this. What about using

Any concern about adding more in-mem logs?

> wb_sync_req[DATA] to prevent async writes at least?

__should_serialize_io() says for synchronous write or small writes, it won't
use writepages lock to serialize IO, so it may cause fragmentation if such
writes were mixed w/ writes from defragment ioctl.

Thanks,

> 
>>
>> Signed-off-by: Weichao Guo <guoweichao@oppo.com>
>> Signed-off-by: Chao Yu <chao@kernel.org>
>> ---
>>   fs/f2fs/debug.c   | 4 ++++
>>   fs/f2fs/f2fs.h    | 2 ++
>>   fs/f2fs/file.c    | 5 +++++
>>   fs/f2fs/segment.c | 7 +++++++
>>   fs/f2fs/segment.h | 5 ++++-
>>   5 files changed, 22 insertions(+), 1 deletion(-)
>>
>> diff --git a/fs/f2fs/debug.c b/fs/f2fs/debug.c
>> index c014715..d85dc17 100644
>> --- a/fs/f2fs/debug.c
>> +++ b/fs/f2fs/debug.c
>> @@ -442,6 +442,10 @@ static int stat_show(struct seq_file *s, void *v)
>>   			   si->curseg[CURSEG_ALL_DATA_ATGC],
>>   			   si->cursec[CURSEG_ALL_DATA_ATGC],
>>   			   si->curzone[CURSEG_ALL_DATA_ATGC]);
>> +		seq_printf(s, "  - DEFRAG   data: %8d %8d %8d\n",
>> +			   si->curseg[CURSEG_COLD_DATA_DEFRAG],
>> +			   si->cursec[CURSEG_COLD_DATA_DEFRAG],
>> +			   si->curzone[CURSEG_COLD_DATA_DEFRAG]);
>>   		seq_printf(s, "\n  - Valid: %d\n  - Dirty: %d\n",
>>   			   si->main_area_segs - si->dirty_count -
>>   			   si->prefree_count - si->free_segs,
>> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
>> index 3c7cdb7..5f9a185 100644
>> --- a/fs/f2fs/f2fs.h
>> +++ b/fs/f2fs/f2fs.h
>> @@ -760,6 +760,7 @@ enum {
>>   	FI_COMPRESS_RELEASED,	/* compressed blocks were released */
>>   	FI_ALIGNED_WRITE,	/* enable aligned write */
>>   	FI_COW_FILE,		/* indicate COW file */
>> +	FI_DEFRAG_WRITE,	/* indicate defragment writes need consective blkaddrs*/
>>   	FI_MAX,			/* max flag, never be used */
>>   };
>>   
>> @@ -1017,6 +1018,7 @@ enum {
>>   	CURSEG_COLD_DATA_PINNED = NR_PERSISTENT_LOG,
>>   				/* pinned file that needs consecutive block address */
>>   	CURSEG_ALL_DATA_ATGC,	/* SSR alloctor in hot/warm/cold data area */
>> +	CURSEG_COLD_DATA_DEFRAG,/* used for defragment which needs consective blkaddrs */
>>   	NO_CHECK_TYPE,		/* number of persistent & inmem log */
>>   };
>>   
>> diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
>> index ce4905a0..f104e2e 100644
>> --- a/fs/f2fs/file.c
>> +++ b/fs/f2fs/file.c
>> @@ -2626,7 +2626,12 @@ static int f2fs_defragment_range(struct f2fs_sb_info *sbi,
>>   
>>   		clear_inode_flag(inode, FI_SKIP_WRITES);
>>   
>> +		set_inode_flag(inode, FI_DEFRAG_WRITE);
>> +		f2fs_lock_op(sbi);
>> +		f2fs_allocate_new_section(sbi, CURSEG_COLD_DATA_DEFRAG, false);
>> +		f2fs_unlock_op(sbi);
>>   		err = filemap_fdatawrite(inode->i_mapping);
>> +		clear_inode_flag(inode, FI_DEFRAG_WRITE);
>>   		if (err)
>>   			goto out;
>>   	}
>> diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
>> index 0de21f8..17e7d14 100644
>> --- a/fs/f2fs/segment.c
>> +++ b/fs/f2fs/segment.c
>> @@ -2749,6 +2749,7 @@ static void __f2fs_save_inmem_curseg(struct f2fs_sb_info *sbi, int type)
>>   void f2fs_save_inmem_curseg(struct f2fs_sb_info *sbi)
>>   {
>>   	__f2fs_save_inmem_curseg(sbi, CURSEG_COLD_DATA_PINNED);
>> +	__f2fs_save_inmem_curseg(sbi, CURSEG_COLD_DATA_DEFRAG);
>>   
>>   	if (sbi->am.atgc_enabled)
>>   		__f2fs_save_inmem_curseg(sbi, CURSEG_ALL_DATA_ATGC);
>> @@ -2774,6 +2775,7 @@ static void __f2fs_restore_inmem_curseg(struct f2fs_sb_info *sbi, int type)
>>   void f2fs_restore_inmem_curseg(struct f2fs_sb_info *sbi)
>>   {
>>   	__f2fs_restore_inmem_curseg(sbi, CURSEG_COLD_DATA_PINNED);
>> +	__f2fs_restore_inmem_curseg(sbi, CURSEG_COLD_DATA_DEFRAG);
>>   
>>   	if (sbi->am.atgc_enabled)
>>   		__f2fs_restore_inmem_curseg(sbi, CURSEG_ALL_DATA_ATGC);
>> @@ -3161,6 +3163,9 @@ static int __get_segment_type_6(struct f2fs_io_info *fio)
>>   		if (is_inode_flag_set(inode, FI_ALIGNED_WRITE))
>>   			return CURSEG_COLD_DATA_PINNED;
>>   
>> +		if (is_inode_flag_set(inode, FI_DEFRAG_WRITE))
>> +			return CURSEG_COLD_DATA_DEFRAG;
>> +
>>   		if (page_private_gcing(fio->page)) {
>>   			if (fio->sbi->am.atgc_enabled &&
>>   				(fio->io_type == FS_DATA_IO) &&
>> @@ -4332,6 +4337,8 @@ static int build_curseg(struct f2fs_sb_info *sbi)
>>   			array[i].seg_type = CURSEG_COLD_DATA;
>>   		else if (i == CURSEG_ALL_DATA_ATGC)
>>   			array[i].seg_type = CURSEG_COLD_DATA;
>> +		else if (i == CURSEG_COLD_DATA_DEFRAG)
>> +			array[i].seg_type = CURSEG_COLD_DATA;
>>   		array[i].segno = NULL_SEGNO;
>>   		array[i].next_blkoff = 0;
>>   		array[i].inited = false;
>> diff --git a/fs/f2fs/segment.h b/fs/f2fs/segment.h
>> index d1d6376..75e2aa8 100644
>> --- a/fs/f2fs/segment.h
>> +++ b/fs/f2fs/segment.h
>> @@ -44,7 +44,8 @@ static inline void sanity_check_seg_type(struct f2fs_sb_info *sbi,
>>   	 ((seg) == CURSEG_I(sbi, CURSEG_WARM_NODE)->segno) ||	\
>>   	 ((seg) == CURSEG_I(sbi, CURSEG_COLD_NODE)->segno) ||	\
>>   	 ((seg) == CURSEG_I(sbi, CURSEG_COLD_DATA_PINNED)->segno) ||	\
>> -	 ((seg) == CURSEG_I(sbi, CURSEG_ALL_DATA_ATGC)->segno))
>> +	 ((seg) == CURSEG_I(sbi, CURSEG_ALL_DATA_ATGC)->segno) ||	\
>> +	 ((seg) == CURSEG_I(sbi, CURSEG_COLD_DATA_DEFRAG)->segno))
>>   
>>   #define IS_CURSEC(sbi, secno)						\
>>   	(((secno) == CURSEG_I(sbi, CURSEG_HOT_DATA)->segno /		\
>> @@ -62,6 +63,8 @@ static inline void sanity_check_seg_type(struct f2fs_sb_info *sbi,
>>   	 ((secno) == CURSEG_I(sbi, CURSEG_COLD_DATA_PINNED)->segno /	\
>>   	  (sbi)->segs_per_sec) ||	\
>>   	 ((secno) == CURSEG_I(sbi, CURSEG_ALL_DATA_ATGC)->segno /	\
>> +	  (sbi)->segs_per_sec) ||	\
>> +	 ((secno) == CURSEG_I(sbi, CURSEG_COLD_DATA_DEFRAG)->segno /	\
>>   	  (sbi)->segs_per_sec))
>>   
>>   #define MAIN_BLKADDR(sbi)						\
>> -- 
>> 2.7.4


_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [f2fs-dev] [PATCH] f2fs: allocate consective blkaddrs for defragment
  2022-08-15  3:30   ` Chao Yu
@ 2022-08-19 23:42     ` Jaegeuk Kim
  2022-08-20  2:30       ` Chao Yu
  0 siblings, 1 reply; 6+ messages in thread
From: Jaegeuk Kim @ 2022-08-19 23:42 UTC (permalink / raw)
  To: Chao Yu; +Cc: zhangshiming, linux-f2fs-devel

On 08/15, Chao Yu wrote:
> Hi Jaegeuk,
> 
> On 2022/8/13 1:38, Jaegeuk Kim wrote:
> > Hi Weichao,
> > 
> > On 08/12, Weichao Guo wrote:
> > > When we try to defrag a file, its data blocks may mess with others if there
> > > are lots of concurrent writes. This causes the file is still fragmented
> > > after defrag. So It's better to isolate defrag writes from others.
> > 
> > I really don't want to add more log like this. What about using
> 
> Any concern about adding more in-mem logs?

It gives more complexity and abuses free sections.

> 
> > wb_sync_req[DATA] to prevent async writes at least?
> 
> __should_serialize_io() says for synchronous write or small writes, it won't
> use writepages lock to serialize IO, so it may cause fragmentation if such
> writes were mixed w/ writes from defragment ioctl.

I think that, if you want to make a perfect layout, it'd be better to freeze
f2fs. Can we measure the realistic overhead on our current best-effort approach?

> 
> Thanks,
> 
> > 
> > > 
> > > Signed-off-by: Weichao Guo <guoweichao@oppo.com>
> > > Signed-off-by: Chao Yu <chao@kernel.org>
> > > ---
> > >   fs/f2fs/debug.c   | 4 ++++
> > >   fs/f2fs/f2fs.h    | 2 ++
> > >   fs/f2fs/file.c    | 5 +++++
> > >   fs/f2fs/segment.c | 7 +++++++
> > >   fs/f2fs/segment.h | 5 ++++-
> > >   5 files changed, 22 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/fs/f2fs/debug.c b/fs/f2fs/debug.c
> > > index c014715..d85dc17 100644
> > > --- a/fs/f2fs/debug.c
> > > +++ b/fs/f2fs/debug.c
> > > @@ -442,6 +442,10 @@ static int stat_show(struct seq_file *s, void *v)
> > >   			   si->curseg[CURSEG_ALL_DATA_ATGC],
> > >   			   si->cursec[CURSEG_ALL_DATA_ATGC],
> > >   			   si->curzone[CURSEG_ALL_DATA_ATGC]);
> > > +		seq_printf(s, "  - DEFRAG   data: %8d %8d %8d\n",
> > > +			   si->curseg[CURSEG_COLD_DATA_DEFRAG],
> > > +			   si->cursec[CURSEG_COLD_DATA_DEFRAG],
> > > +			   si->curzone[CURSEG_COLD_DATA_DEFRAG]);
> > >   		seq_printf(s, "\n  - Valid: %d\n  - Dirty: %d\n",
> > >   			   si->main_area_segs - si->dirty_count -
> > >   			   si->prefree_count - si->free_segs,
> > > diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> > > index 3c7cdb7..5f9a185 100644
> > > --- a/fs/f2fs/f2fs.h
> > > +++ b/fs/f2fs/f2fs.h
> > > @@ -760,6 +760,7 @@ enum {
> > >   	FI_COMPRESS_RELEASED,	/* compressed blocks were released */
> > >   	FI_ALIGNED_WRITE,	/* enable aligned write */
> > >   	FI_COW_FILE,		/* indicate COW file */
> > > +	FI_DEFRAG_WRITE,	/* indicate defragment writes need consective blkaddrs*/
> > >   	FI_MAX,			/* max flag, never be used */
> > >   };
> > > @@ -1017,6 +1018,7 @@ enum {
> > >   	CURSEG_COLD_DATA_PINNED = NR_PERSISTENT_LOG,
> > >   				/* pinned file that needs consecutive block address */
> > >   	CURSEG_ALL_DATA_ATGC,	/* SSR alloctor in hot/warm/cold data area */
> > > +	CURSEG_COLD_DATA_DEFRAG,/* used for defragment which needs consective blkaddrs */
> > >   	NO_CHECK_TYPE,		/* number of persistent & inmem log */
> > >   };
> > > diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
> > > index ce4905a0..f104e2e 100644
> > > --- a/fs/f2fs/file.c
> > > +++ b/fs/f2fs/file.c
> > > @@ -2626,7 +2626,12 @@ static int f2fs_defragment_range(struct f2fs_sb_info *sbi,
> > >   		clear_inode_flag(inode, FI_SKIP_WRITES);
> > > +		set_inode_flag(inode, FI_DEFRAG_WRITE);
> > > +		f2fs_lock_op(sbi);
> > > +		f2fs_allocate_new_section(sbi, CURSEG_COLD_DATA_DEFRAG, false);
> > > +		f2fs_unlock_op(sbi);
> > >   		err = filemap_fdatawrite(inode->i_mapping);
> > > +		clear_inode_flag(inode, FI_DEFRAG_WRITE);
> > >   		if (err)
> > >   			goto out;
> > >   	}
> > > diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
> > > index 0de21f8..17e7d14 100644
> > > --- a/fs/f2fs/segment.c
> > > +++ b/fs/f2fs/segment.c
> > > @@ -2749,6 +2749,7 @@ static void __f2fs_save_inmem_curseg(struct f2fs_sb_info *sbi, int type)
> > >   void f2fs_save_inmem_curseg(struct f2fs_sb_info *sbi)
> > >   {
> > >   	__f2fs_save_inmem_curseg(sbi, CURSEG_COLD_DATA_PINNED);
> > > +	__f2fs_save_inmem_curseg(sbi, CURSEG_COLD_DATA_DEFRAG);
> > >   	if (sbi->am.atgc_enabled)
> > >   		__f2fs_save_inmem_curseg(sbi, CURSEG_ALL_DATA_ATGC);
> > > @@ -2774,6 +2775,7 @@ static void __f2fs_restore_inmem_curseg(struct f2fs_sb_info *sbi, int type)
> > >   void f2fs_restore_inmem_curseg(struct f2fs_sb_info *sbi)
> > >   {
> > >   	__f2fs_restore_inmem_curseg(sbi, CURSEG_COLD_DATA_PINNED);
> > > +	__f2fs_restore_inmem_curseg(sbi, CURSEG_COLD_DATA_DEFRAG);
> > >   	if (sbi->am.atgc_enabled)
> > >   		__f2fs_restore_inmem_curseg(sbi, CURSEG_ALL_DATA_ATGC);
> > > @@ -3161,6 +3163,9 @@ static int __get_segment_type_6(struct f2fs_io_info *fio)
> > >   		if (is_inode_flag_set(inode, FI_ALIGNED_WRITE))
> > >   			return CURSEG_COLD_DATA_PINNED;
> > > +		if (is_inode_flag_set(inode, FI_DEFRAG_WRITE))
> > > +			return CURSEG_COLD_DATA_DEFRAG;
> > > +
> > >   		if (page_private_gcing(fio->page)) {
> > >   			if (fio->sbi->am.atgc_enabled &&
> > >   				(fio->io_type == FS_DATA_IO) &&
> > > @@ -4332,6 +4337,8 @@ static int build_curseg(struct f2fs_sb_info *sbi)
> > >   			array[i].seg_type = CURSEG_COLD_DATA;
> > >   		else if (i == CURSEG_ALL_DATA_ATGC)
> > >   			array[i].seg_type = CURSEG_COLD_DATA;
> > > +		else if (i == CURSEG_COLD_DATA_DEFRAG)
> > > +			array[i].seg_type = CURSEG_COLD_DATA;
> > >   		array[i].segno = NULL_SEGNO;
> > >   		array[i].next_blkoff = 0;
> > >   		array[i].inited = false;
> > > diff --git a/fs/f2fs/segment.h b/fs/f2fs/segment.h
> > > index d1d6376..75e2aa8 100644
> > > --- a/fs/f2fs/segment.h
> > > +++ b/fs/f2fs/segment.h
> > > @@ -44,7 +44,8 @@ static inline void sanity_check_seg_type(struct f2fs_sb_info *sbi,
> > >   	 ((seg) == CURSEG_I(sbi, CURSEG_WARM_NODE)->segno) ||	\
> > >   	 ((seg) == CURSEG_I(sbi, CURSEG_COLD_NODE)->segno) ||	\
> > >   	 ((seg) == CURSEG_I(sbi, CURSEG_COLD_DATA_PINNED)->segno) ||	\
> > > -	 ((seg) == CURSEG_I(sbi, CURSEG_ALL_DATA_ATGC)->segno))
> > > +	 ((seg) == CURSEG_I(sbi, CURSEG_ALL_DATA_ATGC)->segno) ||	\
> > > +	 ((seg) == CURSEG_I(sbi, CURSEG_COLD_DATA_DEFRAG)->segno))
> > >   #define IS_CURSEC(sbi, secno)						\
> > >   	(((secno) == CURSEG_I(sbi, CURSEG_HOT_DATA)->segno /		\
> > > @@ -62,6 +63,8 @@ static inline void sanity_check_seg_type(struct f2fs_sb_info *sbi,
> > >   	 ((secno) == CURSEG_I(sbi, CURSEG_COLD_DATA_PINNED)->segno /	\
> > >   	  (sbi)->segs_per_sec) ||	\
> > >   	 ((secno) == CURSEG_I(sbi, CURSEG_ALL_DATA_ATGC)->segno /	\
> > > +	  (sbi)->segs_per_sec) ||	\
> > > +	 ((secno) == CURSEG_I(sbi, CURSEG_COLD_DATA_DEFRAG)->segno /	\
> > >   	  (sbi)->segs_per_sec))
> > >   #define MAIN_BLKADDR(sbi)						\
> > > -- 
> > > 2.7.4


_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [f2fs-dev] [PATCH] f2fs: allocate consective blkaddrs for defragment
       [not found]   ` <DM5PR17MB095307935ABD5D5A6EEDD050C6699@DM5PR17MB0953.namprd17.prod.outlook.com>
@ 2022-08-19 23:45     ` Jaegeuk Kim
  0 siblings, 0 replies; 6+ messages in thread
From: Jaegeuk Kim @ 2022-08-19 23:45 UTC (permalink / raw)
  To: guo weichao; +Cc: zhangshiming, linux-f2fs-devel

On 08/14, guo weichao wrote:
> Hi Jaegeuk,
> 
> IMO, defragment is important for many user scenarios. If it's still fragment after defrag, users may trigger more defrag ioctls, which hurts performance & device lifetime. So how about add an idle check & retry one more time besides adding "wb_sync_req[DATA]" count, draft liked the following:
> 
> --- a/fs/f2fs/file.c
> +++ b/fs/f2fs/file.c
> +#define DEFRAG_RETRY_COUNT 2
> +#define DEFRAG_WAIT_SEC 10
> 
> @@ -2508,7 +2512,7 @@ static int f2fs_defragment_range(struct f2fs_sb_info *sbi,
>                                                 range->start + range->len - 1);
>         if (err)
>                 goto out;
> +frag_check:
>         /*
>          * lookup mapping info in extent cache, skip defragmenting if physical
>          * block addresses are continuous.
> @@ -2611,9 +2615,16 @@ static int f2fs_defragment_range(struct f2fs_sb_info *sbi,
> 
>                 clear_inode_flag(inode, FI_SKIP_WRITES);
> 
> +               wait_secs = 0;
> +               while (!is_idle(sbi, REQ_TIME) && wait_secs++ < DEFRAG_WAIT_SEC)
> +                       msleep(1000);
> +               atomic_inc(&sbi->wb_sync_req[DATA]);
>                 err = filemap_fdatawrite(inode->i_mapping);
> +               atomic_dec(&sbi->wb_sync_req[DATA]);
>                 if (err)
>                         goto out;
> +               else if (retries++ < DEFRAG_RETRY_COUNT)
> +                       goto frag_check;

Doesn't it need to run some GCs to get a consecutive free sections?
What is the goal here? How can we quantify the fragmented level?

> ________________________________
> From: Jaegeuk Kim <jaegeuk@kernel.org>
> To: Weichao Guo <guoweichao@oppo.com>
> CC: zhangshiming@oppo.com <zhangshiming@oppo.com>; linux-f2fs-devel@lists.sourceforge.net <linux-f2fs-devel@lists.sourceforge.net>
> Title: Re: [f2fs-dev] [PATCH] f2fs: allocate consective blkaddrs for defragment
> 
> Hi Weichao,
> 
> On 08/12, Weichao Guo wrote:
> > When we try to defrag a file, its data blocks may mess with others if there
> > are lots of concurrent writes. This causes the file is still fragmented
> > after defrag. So It's better to isolate defrag writes from others.
> 
> I really don't want to add more log like this. What about using
> wb_sync_req[DATA] to prevent async writes at least?
> 
> >
> > Signed-off-by: Weichao Guo <guoweichao@oppo.com>
> > Signed-off-by: Chao Yu <chao@kernel.org>
> > ---
> >  fs/f2fs/debug.c   | 4 ++++
> >  fs/f2fs/f2fs.h    | 2 ++
> >  fs/f2fs/file.c    | 5 +++++
> >  fs/f2fs/segment.c | 7 +++++++
> >  fs/f2fs/segment.h | 5 ++++-
> >  5 files changed, 22 insertions(+), 1 deletion(-)
> >
> > diff --git a/fs/f2fs/debug.c b/fs/f2fs/debug.c
> > index c014715..d85dc17 100644
> > --- a/fs/f2fs/debug.c
> > +++ b/fs/f2fs/debug.c
> > @@ -442,6 +442,10 @@ static int stat_show(struct seq_file *s, void *v)
> >                           si->curseg[CURSEG_ALL_DATA_ATGC],
> >                           si->cursec[CURSEG_ALL_DATA_ATGC],
> >                           si->curzone[CURSEG_ALL_DATA_ATGC]);
> > +             seq_printf(s, "  - DEFRAG   data: %8d %8d %8d\n",
> > +                        si->curseg[CURSEG_COLD_DATA_DEFRAG],
> > +                        si->cursec[CURSEG_COLD_DATA_DEFRAG],
> > +                        si->curzone[CURSEG_COLD_DATA_DEFRAG]);
> >                seq_printf(s, "\n  - Valid: %d\n  - Dirty: %d\n",
> >                           si->main_area_segs - si->dirty_count -
> >                           si->prefree_count - si->free_segs,
> > diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> > index 3c7cdb7..5f9a185 100644
> > --- a/fs/f2fs/f2fs.h
> > +++ b/fs/f2fs/f2fs.h
> > @@ -760,6 +760,7 @@ enum {
> >        FI_COMPRESS_RELEASED,   /* compressed blocks were released */
> >        FI_ALIGNED_WRITE,       /* enable aligned write */
> >        FI_COW_FILE,            /* indicate COW file */
> > +     FI_DEFRAG_WRITE,        /* indicate defragment writes need consective blkaddrs*/
> >        FI_MAX,                 /* max flag, never be used */
> >  };
> >
> > @@ -1017,6 +1018,7 @@ enum {
> >        CURSEG_COLD_DATA_PINNED = NR_PERSISTENT_LOG,
> >                                /* pinned file that needs consecutive block address */
> >        CURSEG_ALL_DATA_ATGC,   /* SSR alloctor in hot/warm/cold data area */
> > +     CURSEG_COLD_DATA_DEFRAG,/* used for defragment which needs consective blkaddrs */
> >        NO_CHECK_TYPE,          /* number of persistent & inmem log */
> >  };
> >
> > diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
> > index ce4905a0..f104e2e 100644
> > --- a/fs/f2fs/file.c
> > +++ b/fs/f2fs/file.c
> > @@ -2626,7 +2626,12 @@ static int f2fs_defragment_range(struct f2fs_sb_info *sbi,
> >
> >                clear_inode_flag(inode, FI_SKIP_WRITES);
> >
> > +             set_inode_flag(inode, FI_DEFRAG_WRITE);
> > +             f2fs_lock_op(sbi);
> > +             f2fs_allocate_new_section(sbi, CURSEG_COLD_DATA_DEFRAG, false);
> > +             f2fs_unlock_op(sbi);
> >                err = filemap_fdatawrite(inode->i_mapping);
> > +             clear_inode_flag(inode, FI_DEFRAG_WRITE);
> >                if (err)
> >                        goto out;
> >        }
> > diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
> > index 0de21f8..17e7d14 100644
> > --- a/fs/f2fs/segment.c
> > +++ b/fs/f2fs/segment.c
> > @@ -2749,6 +2749,7 @@ static void __f2fs_save_inmem_curseg(struct f2fs_sb_info *sbi, int type)
> >  void f2fs_save_inmem_curseg(struct f2fs_sb_info *sbi)
> >  {
> >        __f2fs_save_inmem_curseg(sbi, CURSEG_COLD_DATA_PINNED);
> > +     __f2fs_save_inmem_curseg(sbi, CURSEG_COLD_DATA_DEFRAG);
> >
> >        if (sbi->am.atgc_enabled)
> >                __f2fs_save_inmem_curseg(sbi, CURSEG_ALL_DATA_ATGC);
> > @@ -2774,6 +2775,7 @@ static void __f2fs_restore_inmem_curseg(struct f2fs_sb_info *sbi, int type)
> >  void f2fs_restore_inmem_curseg(struct f2fs_sb_info *sbi)
> >  {
> >        __f2fs_restore_inmem_curseg(sbi, CURSEG_COLD_DATA_PINNED);
> > +     __f2fs_restore_inmem_curseg(sbi, CURSEG_COLD_DATA_DEFRAG);
> >
> >        if (sbi->am.atgc_enabled)
> >                __f2fs_restore_inmem_curseg(sbi, CURSEG_ALL_DATA_ATGC);
> > @@ -3161,6 +3163,9 @@ static int __get_segment_type_6(struct f2fs_io_info *fio)
> >                if (is_inode_flag_set(inode, FI_ALIGNED_WRITE))
> >                        return CURSEG_COLD_DATA_PINNED;
> >
> > +             if (is_inode_flag_set(inode, FI_DEFRAG_WRITE))
> > +                     return CURSEG_COLD_DATA_DEFRAG;
> > +
> >                if (page_private_gcing(fio->page)) {
> >                        if (fio->sbi->am.atgc_enabled &&
> >                                (fio->io_type == FS_DATA_IO) &&
> > @@ -4332,6 +4337,8 @@ static int build_curseg(struct f2fs_sb_info *sbi)
> >                        array[i].seg_type = CURSEG_COLD_DATA;
> >                else if (i == CURSEG_ALL_DATA_ATGC)
> >                        array[i].seg_type = CURSEG_COLD_DATA;
> > +             else if (i == CURSEG_COLD_DATA_DEFRAG)
> > +                     array[i].seg_type = CURSEG_COLD_DATA;
> >                array[i].segno = NULL_SEGNO;
> >                array[i].next_blkoff = 0;
> >                array[i].inited = false;
> > diff --git a/fs/f2fs/segment.h b/fs/f2fs/segment.h
> > index d1d6376..75e2aa8 100644
> > --- a/fs/f2fs/segment.h
> > +++ b/fs/f2fs/segment.h
> > @@ -44,7 +44,8 @@ static inline void sanity_check_seg_type(struct f2fs_sb_info *sbi,
> >         ((seg) == CURSEG_I(sbi, CURSEG_WARM_NODE)->segno) ||    \
> >         ((seg) == CURSEG_I(sbi, CURSEG_COLD_NODE)->segno) ||    \
> >         ((seg) == CURSEG_I(sbi, CURSEG_COLD_DATA_PINNED)->segno) ||     \
> > -      ((seg) == CURSEG_I(sbi, CURSEG_ALL_DATA_ATGC)->segno))
> > +      ((seg) == CURSEG_I(sbi, CURSEG_ALL_DATA_ATGC)->segno) ||        \
> > +      ((seg) == CURSEG_I(sbi, CURSEG_COLD_DATA_DEFRAG)->segno))
> >
> >  #define IS_CURSEC(sbi, secno)                                                \
> >        (((secno) == CURSEG_I(sbi, CURSEG_HOT_DATA)->segno /            \
> > @@ -62,6 +63,8 @@ static inline void sanity_check_seg_type(struct f2fs_sb_info *sbi,
> >         ((secno) == CURSEG_I(sbi, CURSEG_COLD_DATA_PINNED)->segno /     \
> >          (sbi)->segs_per_sec) ||        \
> >         ((secno) == CURSEG_I(sbi, CURSEG_ALL_DATA_ATGC)->segno /        \
> > +       (sbi)->segs_per_sec) ||        \
> > +      ((secno) == CURSEG_I(sbi, CURSEG_COLD_DATA_DEFRAG)->segno /     \
> >          (sbi)->segs_per_sec))
> >
> >  #define MAIN_BLKADDR(sbi)                                            \
> > --
> > 2.7.4
> 
> 
> _______________________________________________
> Linux-f2fs-devel mailing list
> Linux-f2fs-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel


_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [f2fs-dev] [PATCH] f2fs: allocate consective blkaddrs for defragment
  2022-08-19 23:42     ` Jaegeuk Kim
@ 2022-08-20  2:30       ` Chao Yu
  0 siblings, 0 replies; 6+ messages in thread
From: Chao Yu @ 2022-08-20  2:30 UTC (permalink / raw)
  To: Jaegeuk Kim; +Cc: zhangshiming, linux-f2fs-devel

On 2022/8/20 7:42, Jaegeuk Kim wrote:
> On 08/15, Chao Yu wrote:
>> Hi Jaegeuk,
>>
>> On 2022/8/13 1:38, Jaegeuk Kim wrote:
>>> Hi Weichao,
>>>
>>> On 08/12, Weichao Guo wrote:
>>>> When we try to defrag a file, its data blocks may mess with others if there
>>>> are lots of concurrent writes. This causes the file is still fragmented
>>>> after defrag. So It's better to isolate defrag writes from others.
>>>
>>> I really don't want to add more log like this. What about using
>>
>> Any concern about adding more in-mem logs?
> 
> It gives more complexity and abuses free sections.

Well, what about just using in-mem logs allocation for those segment size aligned
defragment? and so newly allocated free section can be used entirely, thoughts?

If you don't want to add more logs, how about reusing CURSEG_COLD_DATA_PINNED here?

> 
>>
>>> wb_sync_req[DATA] to prevent async writes at least?
>>
>> __should_serialize_io() says for synchronous write or small writes, it won't
>> use writepages lock to serialize IO, so it may cause fragmentation if such
>> writes were mixed w/ writes from defragment ioctl.
> 
> I think that, if you want to make a perfect layout, it'd be better to freeze
> f2fs. Can we measure the realistic overhead on our current best-effort approach?

Sure, let's evaluate wb_sync_req scheme as well.

Thanks,

> 
>>
>> Thanks,
>>
>>>
>>>>
>>>> Signed-off-by: Weichao Guo <guoweichao@oppo.com>
>>>> Signed-off-by: Chao Yu <chao@kernel.org>
>>>> ---
>>>>    fs/f2fs/debug.c   | 4 ++++
>>>>    fs/f2fs/f2fs.h    | 2 ++
>>>>    fs/f2fs/file.c    | 5 +++++
>>>>    fs/f2fs/segment.c | 7 +++++++
>>>>    fs/f2fs/segment.h | 5 ++++-
>>>>    5 files changed, 22 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/fs/f2fs/debug.c b/fs/f2fs/debug.c
>>>> index c014715..d85dc17 100644
>>>> --- a/fs/f2fs/debug.c
>>>> +++ b/fs/f2fs/debug.c
>>>> @@ -442,6 +442,10 @@ static int stat_show(struct seq_file *s, void *v)
>>>>    			   si->curseg[CURSEG_ALL_DATA_ATGC],
>>>>    			   si->cursec[CURSEG_ALL_DATA_ATGC],
>>>>    			   si->curzone[CURSEG_ALL_DATA_ATGC]);
>>>> +		seq_printf(s, "  - DEFRAG   data: %8d %8d %8d\n",
>>>> +			   si->curseg[CURSEG_COLD_DATA_DEFRAG],
>>>> +			   si->cursec[CURSEG_COLD_DATA_DEFRAG],
>>>> +			   si->curzone[CURSEG_COLD_DATA_DEFRAG]);
>>>>    		seq_printf(s, "\n  - Valid: %d\n  - Dirty: %d\n",
>>>>    			   si->main_area_segs - si->dirty_count -
>>>>    			   si->prefree_count - si->free_segs,
>>>> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
>>>> index 3c7cdb7..5f9a185 100644
>>>> --- a/fs/f2fs/f2fs.h
>>>> +++ b/fs/f2fs/f2fs.h
>>>> @@ -760,6 +760,7 @@ enum {
>>>>    	FI_COMPRESS_RELEASED,	/* compressed blocks were released */
>>>>    	FI_ALIGNED_WRITE,	/* enable aligned write */
>>>>    	FI_COW_FILE,		/* indicate COW file */
>>>> +	FI_DEFRAG_WRITE,	/* indicate defragment writes need consective blkaddrs*/
>>>>    	FI_MAX,			/* max flag, never be used */
>>>>    };
>>>> @@ -1017,6 +1018,7 @@ enum {
>>>>    	CURSEG_COLD_DATA_PINNED = NR_PERSISTENT_LOG,
>>>>    				/* pinned file that needs consecutive block address */
>>>>    	CURSEG_ALL_DATA_ATGC,	/* SSR alloctor in hot/warm/cold data area */
>>>> +	CURSEG_COLD_DATA_DEFRAG,/* used for defragment which needs consective blkaddrs */
>>>>    	NO_CHECK_TYPE,		/* number of persistent & inmem log */
>>>>    };
>>>> diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
>>>> index ce4905a0..f104e2e 100644
>>>> --- a/fs/f2fs/file.c
>>>> +++ b/fs/f2fs/file.c
>>>> @@ -2626,7 +2626,12 @@ static int f2fs_defragment_range(struct f2fs_sb_info *sbi,
>>>>    		clear_inode_flag(inode, FI_SKIP_WRITES);
>>>> +		set_inode_flag(inode, FI_DEFRAG_WRITE);
>>>> +		f2fs_lock_op(sbi);
>>>> +		f2fs_allocate_new_section(sbi, CURSEG_COLD_DATA_DEFRAG, false);
>>>> +		f2fs_unlock_op(sbi);
>>>>    		err = filemap_fdatawrite(inode->i_mapping);
>>>> +		clear_inode_flag(inode, FI_DEFRAG_WRITE);
>>>>    		if (err)
>>>>    			goto out;
>>>>    	}
>>>> diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
>>>> index 0de21f8..17e7d14 100644
>>>> --- a/fs/f2fs/segment.c
>>>> +++ b/fs/f2fs/segment.c
>>>> @@ -2749,6 +2749,7 @@ static void __f2fs_save_inmem_curseg(struct f2fs_sb_info *sbi, int type)
>>>>    void f2fs_save_inmem_curseg(struct f2fs_sb_info *sbi)
>>>>    {
>>>>    	__f2fs_save_inmem_curseg(sbi, CURSEG_COLD_DATA_PINNED);
>>>> +	__f2fs_save_inmem_curseg(sbi, CURSEG_COLD_DATA_DEFRAG);
>>>>    	if (sbi->am.atgc_enabled)
>>>>    		__f2fs_save_inmem_curseg(sbi, CURSEG_ALL_DATA_ATGC);
>>>> @@ -2774,6 +2775,7 @@ static void __f2fs_restore_inmem_curseg(struct f2fs_sb_info *sbi, int type)
>>>>    void f2fs_restore_inmem_curseg(struct f2fs_sb_info *sbi)
>>>>    {
>>>>    	__f2fs_restore_inmem_curseg(sbi, CURSEG_COLD_DATA_PINNED);
>>>> +	__f2fs_restore_inmem_curseg(sbi, CURSEG_COLD_DATA_DEFRAG);
>>>>    	if (sbi->am.atgc_enabled)
>>>>    		__f2fs_restore_inmem_curseg(sbi, CURSEG_ALL_DATA_ATGC);
>>>> @@ -3161,6 +3163,9 @@ static int __get_segment_type_6(struct f2fs_io_info *fio)
>>>>    		if (is_inode_flag_set(inode, FI_ALIGNED_WRITE))
>>>>    			return CURSEG_COLD_DATA_PINNED;
>>>> +		if (is_inode_flag_set(inode, FI_DEFRAG_WRITE))
>>>> +			return CURSEG_COLD_DATA_DEFRAG;
>>>> +
>>>>    		if (page_private_gcing(fio->page)) {
>>>>    			if (fio->sbi->am.atgc_enabled &&
>>>>    				(fio->io_type == FS_DATA_IO) &&
>>>> @@ -4332,6 +4337,8 @@ static int build_curseg(struct f2fs_sb_info *sbi)
>>>>    			array[i].seg_type = CURSEG_COLD_DATA;
>>>>    		else if (i == CURSEG_ALL_DATA_ATGC)
>>>>    			array[i].seg_type = CURSEG_COLD_DATA;
>>>> +		else if (i == CURSEG_COLD_DATA_DEFRAG)
>>>> +			array[i].seg_type = CURSEG_COLD_DATA;
>>>>    		array[i].segno = NULL_SEGNO;
>>>>    		array[i].next_blkoff = 0;
>>>>    		array[i].inited = false;
>>>> diff --git a/fs/f2fs/segment.h b/fs/f2fs/segment.h
>>>> index d1d6376..75e2aa8 100644
>>>> --- a/fs/f2fs/segment.h
>>>> +++ b/fs/f2fs/segment.h
>>>> @@ -44,7 +44,8 @@ static inline void sanity_check_seg_type(struct f2fs_sb_info *sbi,
>>>>    	 ((seg) == CURSEG_I(sbi, CURSEG_WARM_NODE)->segno) ||	\
>>>>    	 ((seg) == CURSEG_I(sbi, CURSEG_COLD_NODE)->segno) ||	\
>>>>    	 ((seg) == CURSEG_I(sbi, CURSEG_COLD_DATA_PINNED)->segno) ||	\
>>>> -	 ((seg) == CURSEG_I(sbi, CURSEG_ALL_DATA_ATGC)->segno))
>>>> +	 ((seg) == CURSEG_I(sbi, CURSEG_ALL_DATA_ATGC)->segno) ||	\
>>>> +	 ((seg) == CURSEG_I(sbi, CURSEG_COLD_DATA_DEFRAG)->segno))
>>>>    #define IS_CURSEC(sbi, secno)						\
>>>>    	(((secno) == CURSEG_I(sbi, CURSEG_HOT_DATA)->segno /		\
>>>> @@ -62,6 +63,8 @@ static inline void sanity_check_seg_type(struct f2fs_sb_info *sbi,
>>>>    	 ((secno) == CURSEG_I(sbi, CURSEG_COLD_DATA_PINNED)->segno /	\
>>>>    	  (sbi)->segs_per_sec) ||	\
>>>>    	 ((secno) == CURSEG_I(sbi, CURSEG_ALL_DATA_ATGC)->segno /	\
>>>> +	  (sbi)->segs_per_sec) ||	\
>>>> +	 ((secno) == CURSEG_I(sbi, CURSEG_COLD_DATA_DEFRAG)->segno /	\
>>>>    	  (sbi)->segs_per_sec))
>>>>    #define MAIN_BLKADDR(sbi)						\
>>>> -- 
>>>> 2.7.4


_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2022-08-20  2:31 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-08-12  3:56 [f2fs-dev] [PATCH] f2fs: allocate consective blkaddrs for defragment Weichao Guo via Linux-f2fs-devel
2022-08-12 17:38 ` Jaegeuk Kim
2022-08-15  3:30   ` Chao Yu
2022-08-19 23:42     ` Jaegeuk Kim
2022-08-20  2:30       ` Chao Yu
     [not found]   ` <DM5PR17MB095307935ABD5D5A6EEDD050C6699@DM5PR17MB0953.namprd17.prod.outlook.com>
2022-08-19 23:45     ` Jaegeuk Kim

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).