Hi Jaegeuk,
On 2017/1/6 10:42, Jaegeuk Kim wrote:
> 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?
Looks good to me. Nice work!
Thanks,
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
