Re: [f2fs-dev] [PATCH 08/10] f2fs: relax async discard commands more

From: Jaegeuk Kim
Date: Thu Jan 05 2017 - 21:42:56 EST


Hi Chao,

On 01/06, Chao Yu wrote:
> On 2017/1/6 3:46, Jaegeuk Kim wrote:
> > On 01/05, Chao Yu wrote:
> >> On 2017/1/4 17:29, Chao Yu wrote:
> >>> 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 <jaegeuk@xxxxxxxxxx>
> >>>
> >>> Reviewed-by: Chao Yu <yuchao0@xxxxxxxxxx>
> >>>
> >>> One comment below,
> >>
> >> I still have a comment on this patch.
> >>
> >>>> -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 ?
>
> Still can not understand this judgment condition, we should wait completion of
> discard command only when segno is locate in range of [start_segno, end_segno]?
>
> But now, this condition can be true only when segno, start_segno, end_segno have
> equal value.
>
> Please correct me if I'm wrong.

Urg. I rewrote it to use block addresses.

How about this?