Re: [PATCH v2] f2fs: give another chance to issue discard with current granularity

[Chao Yu]

Hi Jaegeuk,

On 2018/7/7 6:49, Jaegeuk Kim wrote:
> On 07/05, Chao Yu wrote:
>> If discard IOs are blocked by user IO, do not skip to select and issue
>> discard with lower granularity, retry with current granularity.
> 
> We need to stop as soon as possible since user activity comes. Later, discard
> thread will try it again in another idle time. What's your concern?

Currently, our implementation is that we will try to issue
DEF_MAX_DISCARD_REQUEST discards as many as possible in batch, for example:

If there is 4 discard entry in rb-tree, we will try to issue them by below order
in batch:

No.		size		is_idle		op
#1 discard	2MB		false		skip
#2 discard	2MB		false		skip
#3 discard	256KB		true		issue
#4 discard	16KB		true		issue

So if is_idle is false temporarily, we can still have chance to issue #3 & #4
discard in this round once is_idle is flipped?

But the problem is we skip issue discard with big granularity, so I add this
patch to do retry with current granularity.

Do you mean that we need to stop issuing discard immediately once we detect IO
is busy?

Thanks,

> 
>>
>> Signed-off-by: Chao Yu <yuchao0@xxxxxxxxxx>
>> ---
>> v2:
>> - fix deadloop.
>>  fs/f2fs/segment.c | 16 +++++++++++++---
>>  1 file changed, 13 insertions(+), 3 deletions(-)
>>
>> diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
>> index 1f9ae8270f86..6e2b2e717a40 100644
>> --- a/fs/f2fs/segment.c
>> +++ b/fs/f2fs/segment.c
>> @@ -1191,7 +1191,7 @@ static int __issue_discard_cmd(struct f2fs_sb_info *sbi,
>>  	struct discard_cmd *dc, *tmp;
>>  	struct blk_plug plug;
>>  	int i, iter = 0, issued = 0;
>> -	bool io_interrupted = false;
>> +	bool io_interrupted = false, end_up = false;
>>  
>>  	for (i = MAX_PLIST_NUM - 1; i >= 0; i--) {
>>  		if (i + 1 < dpolicy->granularity)
>> @@ -1199,6 +1199,7 @@ static int __issue_discard_cmd(struct f2fs_sb_info *sbi,
>>  		pend_list = &dcc->pend_list[i];
>>  
>>  		mutex_lock(&dcc->cmd_lock);
>> +retry:
>>  		if (list_empty(pend_list))
>>  			goto next;
>>  		if (unlikely(dcc->rbtree_check))
>> @@ -1217,14 +1218,23 @@ static int __issue_discard_cmd(struct f2fs_sb_info *sbi,
>>  			__submit_discard_cmd(sbi, dpolicy, dc);
>>  			issued++;
>>  skip:
>> -			if (++iter >= dpolicy->max_requests)
>> +			if (++iter >= dpolicy->max_requests) {
>> +				end_up = true;
>>  				break;
>> +			}
>>  		}
>>  		blk_finish_plug(&plug);
>> +
>> +		/*
>> +		 * if discard IO was interrupted by user IOs, give another
>> +		 * chance to issue discard with current granularity.
>> +		 */
>> +		if (io_interrupted && !end_up)
>> +			goto retry;
>>  next:
>>  		mutex_unlock(&dcc->cmd_lock);
>>  
>> -		if (iter >= dpolicy->max_requests)
>> +		if (end_up)
>>  			break;
>>  	}
>>  
>> -- 
>> 2.18.0.rc1