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 <[email protected]>
> > > Signed-off-by: Chao Yu <[email protected]>
> > > ---
> > >   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
[email protected]
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

Reply via email to