On 2016/12/31 2:51, Jaegeuk Kim wrote:
> This patch relaxes async discard commands to avoid waiting its end_io during
> checkpoint.
> Instead of waiting them during checkpoint, it will be done when actually 
> reusing
> them.
> 
> Test on initial partition of nvme drive.
> 
>  # time fstrim /mnt/test
> 
> Before : 6.158s
> After : 4.822s
> 
> Signed-off-by: Jaegeuk Kim <jaeg...@kernel.org>

Reviewed-by: Chao Yu <yuch...@huawei.com>

One comment below,

> ---
>  fs/f2fs/checkpoint.c |  3 ---
>  fs/f2fs/f2fs.h       |  4 +++-
>  fs/f2fs/segment.c    | 25 +++++++++++++++++++++----
>  fs/f2fs/super.c      |  3 +++
>  4 files changed, 27 insertions(+), 8 deletions(-)
> 
> diff --git a/fs/f2fs/checkpoint.c b/fs/f2fs/checkpoint.c
> index 34bfe2b494ae..1a9ba69a22ba 100644
> --- a/fs/f2fs/checkpoint.c
> +++ b/fs/f2fs/checkpoint.c
> @@ -1254,7 +1254,6 @@ int write_checkpoint(struct f2fs_sb_info *sbi, struct 
> cp_control *cpc)
>               f2fs_bug_on(sbi, prefree_segments(sbi));
>               flush_sit_entries(sbi, cpc);
>               clear_prefree_segments(sbi, cpc);
> -             f2fs_wait_all_discard_bio(sbi);
>               unblock_operations(sbi);
>               goto out;
>       }
> @@ -1278,8 +1277,6 @@ int write_checkpoint(struct f2fs_sb_info *sbi, struct 
> cp_control *cpc)
>       else
>               clear_prefree_segments(sbi, cpc);
>  
> -     f2fs_wait_all_discard_bio(sbi);
> -
>       unblock_operations(sbi);
>       stat_inc_cp_count(sbi->stat_info);
>  
> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> index bdcfe2a9b532..f2f40fce9d31 100644
> --- a/fs/f2fs/f2fs.h
> +++ b/fs/f2fs/f2fs.h
> @@ -183,6 +183,8 @@ struct discard_entry {
>  
>  struct bio_entry {
>       struct list_head list;
> +     unsigned int start_segno;
> +     unsigned int end_segno;
>       struct bio *bio;
>       struct completion event;
>       int error;
> @@ -2111,7 +2113,7 @@ void destroy_flush_cmd_control(struct f2fs_sb_info *, 
> bool);
>  void invalidate_blocks(struct f2fs_sb_info *, block_t);
>  bool is_checkpointed_data(struct f2fs_sb_info *, block_t);
>  void refresh_sit_entry(struct f2fs_sb_info *, block_t, block_t);
> -void f2fs_wait_all_discard_bio(struct f2fs_sb_info *);
> +void f2fs_wait_discard_bio(struct f2fs_sb_info *, unsigned int);
>  void clear_prefree_segments(struct f2fs_sb_info *, struct cp_control *);
>  void release_discard_addrs(struct f2fs_sb_info *);
>  int npages_for_summary_flush(struct f2fs_sb_info *, bool);
> diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
> index 6d197f0c8151..9a38424c3c1f 100644
> --- a/fs/f2fs/segment.c
> +++ b/fs/f2fs/segment.c
> @@ -624,20 +624,24 @@ static void locate_dirty_segment(struct f2fs_sb_info 
> *sbi, unsigned int segno)
>  }
>  
>  static struct bio_entry *__add_bio_entry(struct f2fs_sb_info *sbi,
> -                                                     struct bio *bio)
> +                     struct bio *bio, unsigned int start_segno,
> +                                     unsigned int end_segno)
>  {
>       struct list_head *wait_list = &(SM_I(sbi)->wait_list);
>       struct bio_entry *be = f2fs_kmem_cache_alloc(bio_entry_slab, GFP_NOFS);
>  
>       INIT_LIST_HEAD(&be->list);
>       be->bio = bio;
> +     be->start_segno = start_segno;
> +     be->end_segno = end_segno;
>       init_completion(&be->event);
>       list_add_tail(&be->list, wait_list);
>  
>       return be;
>  }
>  
> -void f2fs_wait_all_discard_bio(struct f2fs_sb_info *sbi)
> +/* This should be covered by global mutex, &sit_i->sentry_lock */
> +void f2fs_wait_discard_bio(struct f2fs_sb_info *sbi, unsigned int segno)
>  {
>       struct list_head *wait_list = &(SM_I(sbi)->wait_list);
>       struct bio_entry *be, *tmp;
> @@ -646,7 +650,15 @@ void f2fs_wait_all_discard_bio(struct f2fs_sb_info *sbi)
>               struct bio *bio = be->bio;
>               int err;
>  
> -             wait_for_completion_io(&be->event);
> +             if (!completion_done(&be->event)) {
> +                     if ((be->start_segno >= segno &&
> +                                     be->end_segno <= segno) ||

segno >= be->start_segno && segno < be->end_segno ?

Thanks,

> +                                     segno == NULL_SEGNO)
> +                             wait_for_completion_io(&be->event);
> +                     else
> +                             continue;
> +             }
> +
>               err = be->error;
>               if (err == -EOPNOTSUPP)
>                       err = 0;
> @@ -674,6 +686,8 @@ static int __f2fs_issue_discard_async(struct f2fs_sb_info 
> *sbi,
>               struct block_device *bdev, block_t blkstart, block_t blklen)
>  {
>       struct bio *bio = NULL;
> +     unsigned int start_segno = GET_SEGNO(sbi, blkstart);
> +     unsigned int end_segno = GET_SEGNO(sbi, blkstart + blklen);
>       int err;
>  
>       trace_f2fs_issue_discard(sbi->sb, blkstart, blklen);
> @@ -688,7 +702,8 @@ static int __f2fs_issue_discard_async(struct f2fs_sb_info 
> *sbi,
>                               SECTOR_FROM_BLOCK(blklen),
>                               GFP_NOFS, 0, &bio);
>       if (!err && bio) {
> -             struct bio_entry *be = __add_bio_entry(sbi, bio);
> +             struct bio_entry *be = __add_bio_entry(sbi, bio,
> +                                     start_segno, end_segno);
>  
>               bio->bi_private = be;
>               bio->bi_end_io = f2fs_submit_bio_wait_endio;
> @@ -1574,6 +1589,8 @@ void allocate_data_block(struct f2fs_sb_info *sbi, 
> struct page *page,
>  
>       *new_blkaddr = NEXT_FREE_BLKADDR(sbi, curseg);
>  
> +     f2fs_wait_discard_bio(sbi, GET_SEGNO(sbi, *new_blkaddr));
> +
>       /*
>        * __add_sum_entry should be resided under the curseg_mutex
>        * because, this function updates a summary entry in the
> diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
> index f3697f97e527..16e2bc5209bb 100644
> --- a/fs/f2fs/super.c
> +++ b/fs/f2fs/super.c
> @@ -770,6 +770,9 @@ static void f2fs_put_super(struct super_block *sb)
>               write_checkpoint(sbi, &cpc);
>       }
>  
> +     /* be sure to wait for any on-going discard commands */
> +     f2fs_wait_discard_bio(sbi, NULL_SEGNO);
> +
>       /* write_checkpoint can update stat informaion */
>       f2fs_destroy_stats(sbi);
>  
> 

Reply via email to