Messages in this thread Patch in this message | | | Date | Mon, 11 Apr 2022 13:20:30 -0700 | From | Jaegeuk Kim <> | Subject | Re: [PATCH v3] f2fs: give priority to select unpinned section for foreground GC |
| |
On 04/06, Chao Yu wrote: > Previously, during foreground GC, if victims contain data of pinned file, > it will fail migration of the data, and meanwhile i_gc_failures of that > pinned file may increase, and when it exceeds threshold, GC will unpin > the file, result in breaking pinfile's semantics. > > In order to mitigate such condition, let's record and skip section which > has pinned file's data and give priority to select unpinned one. > > Signed-off-by: Chao Yu <chao.yu@oppo.com> > --- > v3: > - check pin status before pinning section in pin_section(). > fs/f2fs/gc.c | 56 ++++++++++++++++++++++++++++++++++++++++++++--- > fs/f2fs/segment.c | 7 ++++++ > fs/f2fs/segment.h | 2 ++ > 3 files changed, 62 insertions(+), 3 deletions(-) > > diff --git a/fs/f2fs/gc.c b/fs/f2fs/gc.c > index 6a7e4148ff9d..df23824ae3c2 100644 > --- a/fs/f2fs/gc.c > +++ b/fs/f2fs/gc.c > @@ -646,6 +646,37 @@ static void release_victim_entry(struct f2fs_sb_info *sbi) > f2fs_bug_on(sbi, !list_empty(&am->victim_list)); > } > > +static void pin_section(struct f2fs_sb_info *sbi, unsigned int segno)
Need f2fs_...?
> +{ > + struct dirty_seglist_info *dirty_i = DIRTY_I(sbi); > + unsigned int secno = GET_SEC_FROM_SEG(sbi, segno); > + > + if (test_bit(secno, dirty_i->pinned_secmap)) > + return; > + set_bit(secno, dirty_i->pinned_secmap); > + dirty_i->pinned_secmap_cnt++; > +} > + > +static bool pinned_section_exists(struct dirty_seglist_info *dirty_i) > +{ > + return dirty_i->pinned_secmap_cnt; > +} > + > +static bool section_is_pinned(struct dirty_seglist_info *dirty_i, > + unsigned int secno) > +{ > + return pinned_section_exists(dirty_i) && > + test_bit(secno, dirty_i->pinned_secmap); > +} > + > +static void unpin_all_sections(struct f2fs_sb_info *sbi) > +{ > + unsigned int bitmap_size = f2fs_bitmap_size(MAIN_SECS(sbi)); > + > + memset(DIRTY_I(sbi)->pinned_secmap, 0, bitmap_size); > + DIRTY_I(sbi)->pinned_secmap_cnt = 0; > +} > + > /* > * This function is called from two paths. > * One is garbage collection and the other is SSR segment selection. > @@ -787,6 +818,9 @@ static int get_victim_by_default(struct f2fs_sb_info *sbi, > if (gc_type == BG_GC && test_bit(secno, dirty_i->victim_secmap)) > goto next; > > + if (gc_type == FG_GC && section_is_pinned(dirty_i, secno)) > + goto next; > + > if (is_atgc) { > add_victim_entry(sbi, &p, segno); > goto next; > @@ -1202,8 +1236,10 @@ static int move_data_block(struct inode *inode, block_t bidx, > } > > if (f2fs_is_pinned_file(inode)) { > - if (gc_type == FG_GC) > + if (gc_type == FG_GC) { > f2fs_pin_file_control(inode, true); > + pin_section(F2FS_I_SB(inode), segno);
Do we need to check unpinning the inode? if (!f2fs_pin_file_control()) f2fs_set_pin_section();
> + } > err = -EAGAIN; > goto out; > } > @@ -1352,8 +1388,10 @@ static int move_data_page(struct inode *inode, block_t bidx, int gc_type, > goto out; > } > if (f2fs_is_pinned_file(inode)) { > - if (gc_type == FG_GC) > + if (gc_type == FG_GC) { > f2fs_pin_file_control(inode, true); > + pin_section(F2FS_I_SB(inode), segno); > + } > err = -EAGAIN; > goto out; > } > @@ -1485,6 +1523,7 @@ static int gc_data_segment(struct f2fs_sb_info *sbi, struct f2fs_summary *sum, > gc_type == FG_GC) { > f2fs_pin_file_control(inode, true); > iput(inode); > + pin_section(sbi, segno);
We don't have this code.
> return submitted; > } > > @@ -1766,9 +1805,17 @@ int f2fs_gc(struct f2fs_sb_info *sbi, bool sync, > ret = -EINVAL; > goto stop; > } > +retry: > ret = __get_victim(sbi, &segno, gc_type); > - if (ret) > + if (ret) { > + /* allow to search victim from sections has pinned data */ > + if (ret == -ENODATA && gc_type == FG_GC && > + pinned_section_exists(DIRTY_I(sbi))) { > + unpin_all_sections(sbi); > + goto retry; > + } > goto stop; > + } > > seg_freed = do_garbage_collect(sbi, segno, &gc_list, gc_type, force); > if (gc_type == FG_GC && > @@ -1811,6 +1858,9 @@ int f2fs_gc(struct f2fs_sb_info *sbi, bool sync, > SIT_I(sbi)->last_victim[ALLOC_NEXT] = 0; > SIT_I(sbi)->last_victim[FLUSH_DEVICE] = init_segno; > > + if (gc_type == FG_GC && pinned_section_exists(DIRTY_I(sbi))) > + unpin_all_sections(sbi); > + > trace_f2fs_gc_end(sbi->sb, ret, total_freed, sec_freed, > get_pages(sbi, F2FS_DIRTY_NODES), > get_pages(sbi, F2FS_DIRTY_DENTS), > diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c > index 012524db7437..1c20d7c9eca3 100644 > --- a/fs/f2fs/segment.c > +++ b/fs/f2fs/segment.c > @@ -4736,6 +4736,12 @@ static int init_victim_secmap(struct f2fs_sb_info *sbi) > dirty_i->victim_secmap = f2fs_kvzalloc(sbi, bitmap_size, GFP_KERNEL); > if (!dirty_i->victim_secmap) > return -ENOMEM; > + > + dirty_i->pinned_secmap = f2fs_kvzalloc(sbi, bitmap_size, GFP_KERNEL); > + if (!dirty_i->pinned_secmap) > + return -ENOMEM; > + > + dirty_i->pinned_secmap_cnt = 0; > return 0; > } > > @@ -5324,6 +5330,7 @@ static void destroy_victim_secmap(struct f2fs_sb_info *sbi) > { > struct dirty_seglist_info *dirty_i = DIRTY_I(sbi); > > + kvfree(dirty_i->pinned_secmap); > kvfree(dirty_i->victim_secmap); > } > > diff --git a/fs/f2fs/segment.h b/fs/f2fs/segment.h > index 5c94caf0c0a1..fd6f246e649c 100644 > --- a/fs/f2fs/segment.h > +++ b/fs/f2fs/segment.h > @@ -294,6 +294,8 @@ struct dirty_seglist_info { > struct mutex seglist_lock; /* lock for segment bitmaps */ > int nr_dirty[NR_DIRTY_TYPE]; /* # of dirty segments */ > unsigned long *victim_secmap; /* background GC victims */ > + unsigned long *pinned_secmap; /* pinned victims from foreground GC */ > + unsigned int pinned_secmap_cnt; /* count of victims which has pinned data */ > }; > > /* victim selection function for cleaning and SSR */ > -- > 2.32.0
| |