[PATCH 1/2] f2fs: stop issue discard if something wrong with f2fs

[PATCH 1/2] f2fs: stop issue discard if something wrong with f2fs

[Yunlei He]

This patch stop discard thread to issue discard io if
something wrong with f2fs, which is similar to fstrim.

Signed-off-by: Yunlei He <heyunlei@huawei.com>
---
 fs/f2fs/segment.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
index 5854cc4..f75f503 100644
--- a/fs/f2fs/segment.c
+++ b/fs/f2fs/segment.c
@@ -1410,6 +1410,11 @@ static int issue_discard_thread(void *data)
 			continue;
 		if (kthread_should_stop())
 			return 0;
+		if (is_sbi_flag_set(sbi, SBI_NEED_FSCK)) {
+			f2fs_msg(sbi->sb, KERN_WARNING,
+				"Found FS corruption, run fsck to fix.");
+			continue;
+		}
 
 		if (dcc->discard_wake)
 			dcc->discard_wake = 0;
-- 
1.9.1


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot

[PATCH 2/2] f2fs: let discard thread wait a little longer if dev is busy

[Yunlei He]

This patch modify discard thread wait policy as below:
	issued       io_interrupted     wait time(ms)
1.        8                 0               50
2.      (0,8)               1               50
3.        0                 1              500 (dev is busy)
4.        0                 0            60000 (no candidates)

Signed-off-by: Yunlei He <heyunlei@huawei.com>
---
 fs/f2fs/f2fs.h    | 2 ++
 fs/f2fs/segment.c | 6 +++++-
 2 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index 1df7f10..0a6453b 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -184,6 +184,7 @@ enum {
 #define MAX_DISCARD_BLOCKS(sbi)		BLKS_PER_SEC(sbi)
 #define DEF_MAX_DISCARD_REQUEST		8	/* issue 8 discards per round */
 #define DEF_MIN_DISCARD_ISSUE_TIME	50	/* 50 ms, if exists */
+#define DEF_MID_DISCARD_ISSUE_TIME	500	/* 500 ms, if device busy */
 #define DEF_MAX_DISCARD_ISSUE_TIME	60000	/* 60 s, if no candidates */
 #define DEF_CP_INTERVAL			60	/* 60 secs */
 #define DEF_IDLE_INTERVAL		5	/* 5 secs */
@@ -285,6 +286,7 @@ enum {
 struct discard_policy {
 	int type;			/* type of discard */
 	unsigned int min_interval;	/* used for candidates exist */
+	unsigned int mid_interval;	/* used for device busy */
 	unsigned int max_interval;	/* used for candidates not exist */
 	unsigned int max_requests;	/* # of discards issued per round */
 	unsigned int io_aware_gran;	/* minimum granularity discard not be aware of I/O */
diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
index f75f503..c7a8161e 100644
--- a/fs/f2fs/segment.c
+++ b/fs/f2fs/segment.c
@@ -1425,9 +1425,11 @@ static int issue_discard_thread(void *data)
 		sb_start_intwrite(sbi->sb);
 
 		issued = __issue_discard_cmd(sbi, &dpolicy);
-		if (issued) {
+		if (issued > 0) {
 			__wait_all_discard_cmd(sbi, &dpolicy);
 			wait_ms = dpolicy.min_interval;
+		} else if (issued == -1){
+			wait_ms = dpolicy.mid_interval;
 		} else {
 			wait_ms = dpolicy.max_interval;
 		}
@@ -1726,10 +1728,12 @@ void init_discard_policy(struct discard_policy *dpolicy,
 
 	if (discard_type == DPOLICY_BG) {
 		dpolicy->min_interval = DEF_MIN_DISCARD_ISSUE_TIME;
+		dpolicy->mid_interval = DEF_MID_DISCARD_ISSUE_TIME;
 		dpolicy->max_interval = DEF_MAX_DISCARD_ISSUE_TIME;
 		dpolicy->io_aware = true;
 	} else if (discard_type == DPOLICY_FORCE) {
 		dpolicy->min_interval = DEF_MIN_DISCARD_ISSUE_TIME;
+		dpolicy->mid_interval = DEF_MID_DISCARD_ISSUE_TIME;
 		dpolicy->max_interval = DEF_MAX_DISCARD_ISSUE_TIME;
 		dpolicy->io_aware = false;
 	} else if (discard_type == DPOLICY_FSTRIM) {
-- 
1.9.1


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot

Re: [PATCH 2/2] f2fs: let discard thread wait a little longer if dev is busy

[Chao Yu]

On 2018/4/8 15:11, Yunlei He wrote:
> This patch modify discard thread wait policy as below:
> 	issued       io_interrupted     wait time(ms)
> 1.        8                 0               50
> 2.      (0,8)               1               50
> 3.        0                 1              500 (dev is busy)
> 4.        0                 0            60000 (no candidates)
> 
> Signed-off-by: Yunlei He <heyunlei@huawei.com>

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

Thanks,

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot

Re: [PATCH 1/2] f2fs: stop issue discard if something wrong with f2fs

[Chao Yu]

On 2018/4/8 15:11, Yunlei He wrote:
> This patch stop discard thread to issue discard io if
> something wrong with f2fs, which is similar to fstrim.
> 
> Signed-off-by: Yunlei He <heyunlei@huawei.com>
> ---
>  fs/f2fs/segment.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
> index 5854cc4..f75f503 100644
> --- a/fs/f2fs/segment.c
> +++ b/fs/f2fs/segment.c
> @@ -1410,6 +1410,11 @@ static int issue_discard_thread(void *data)
>  			continue;
>  		if (kthread_should_stop())
>  			return 0;
> +		if (is_sbi_flag_set(sbi, SBI_NEED_FSCK)) {
> +			f2fs_msg(sbi->sb, KERN_WARNING,
> +				"Found FS corruption, run fsck to fix.");

__get_nat_bitmaps can set this flag during mounting? so its meaning is not only
indicate the image is corrupted but also mean that we need run fsck to recover
status like nat bitmaps?

Thanks,

> +			continue;
> +		}
>  
>  		if (dcc->discard_wake)
>  			dcc->discard_wake = 0;
> 


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot

Re: [PATCH 1/2] f2fs: stop issue discard if something wrong with f2fs

[heyunlei]

>-----Original Message-----
>From: Yuchao (T)
>Sent: Sunday, April 08, 2018 4:26 PM
>To: heyunlei; jaegeuk@kernel.org; linux-f2fs-devel@lists.sourceforge.net
>Cc: Wangbintian
>Subject: Re: [f2fs-dev][PATCH 1/2] f2fs: stop issue discard if something wrong with f2fs
>
>On 2018/4/8 15:11, Yunlei He wrote:
>> This patch stop discard thread to issue discard io if
>> something wrong with f2fs, which is similar to fstrim.
>>
>> Signed-off-by: Yunlei He <heyunlei@huawei.com>
>> ---
>>  fs/f2fs/segment.c | 5 +++++
>>  1 file changed, 5 insertions(+)
>>
>> diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
>> index 5854cc4..f75f503 100644
>> --- a/fs/f2fs/segment.c
>> +++ b/fs/f2fs/segment.c
>> @@ -1410,6 +1410,11 @@ static int issue_discard_thread(void *data)
>>  			continue;
>>  		if (kthread_should_stop())
>>  			return 0;
>> +		if (is_sbi_flag_set(sbi, SBI_NEED_FSCK)) {
>> +			f2fs_msg(sbi->sb, KERN_WARNING,
>> +				"Found FS corruption, run fsck to fix.");
>
>__get_nat_bitmaps can set this flag during mounting? so its meaning is not only
>indicate the image is corrupted but also mean that we need run fsck to recover
>status like nat bitmaps?

Yes，nat bitmaps will set SBI_NEED_FSCK in two cases:

i.   no place to write nat bits  (fsck also failed to recover)
ii.   mount process with failed cp version verify   (sudden power off or some hw reasons)

Could we do recover nat bitmaps status not by fsck but kernel self during mounting?

Other places set SBI_NEED_FSCK flag now all indicate the image has something wrong.

BTW, should we move the judgment to function __issue_discard_cmd in order to cover umount process?

Thanks.

>
>Thanks,
>
>> +			continue;
>> +		}
>>
>>  		if (dcc->discard_wake)
>>  			dcc->discard_wake = 0;
>>

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

Re: [PATCH 1/2] f2fs: stop issue discard if something wrong with f2fs

[Chao Yu]

On 2018/4/8 17:19, heyunlei wrote:
> 
> 
>> -----Original Message-----
>> From: Yuchao (T)
>> Sent: Sunday, April 08, 2018 4:26 PM
>> To: heyunlei; jaegeuk@kernel.org; linux-f2fs-devel@lists.sourceforge.net
>> Cc: Wangbintian
>> Subject: Re: [f2fs-dev][PATCH 1/2] f2fs: stop issue discard if something wrong with f2fs
>>
>> On 2018/4/8 15:11, Yunlei He wrote:
>>> This patch stop discard thread to issue discard io if
>>> something wrong with f2fs, which is similar to fstrim.
>>>
>>> Signed-off-by: Yunlei He <heyunlei@huawei.com>
>>> ---
>>>  fs/f2fs/segment.c | 5 +++++
>>>  1 file changed, 5 insertions(+)
>>>
>>> diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
>>> index 5854cc4..f75f503 100644
>>> --- a/fs/f2fs/segment.c
>>> +++ b/fs/f2fs/segment.c
>>> @@ -1410,6 +1410,11 @@ static int issue_discard_thread(void *data)
>>>  			continue;
>>>  		if (kthread_should_stop())
>>>  			return 0;
>>> +		if (is_sbi_flag_set(sbi, SBI_NEED_FSCK)) {
>>> +			f2fs_msg(sbi->sb, KERN_WARNING,
>>> +				"Found FS corruption, run fsck to fix.");
>>
>> __get_nat_bitmaps can set this flag during mounting? so its meaning is not only
>> indicate the image is corrupted but also mean that we need run fsck to recover
>> status like nat bitmaps?
> 
> Yes，nat bitmaps will set SBI_NEED_FSCK in two cases:
> 
> i.   no place to write nat bits  (fsck also failed to recover)
> ii.   mount process with failed cp version verify   (sudden power off or some hw reasons)
> 
> Could we do recover nat bitmaps status not by fsck but kernel self during mounting?

We could, but during rebuilding nat bitmap, we need to travel all nat blocks, if
system owns huge number of nat entry, we will suffer long latency in mount/umount.

Or we can split that flag to two to indicate different meaning?

> 
> Other places set SBI_NEED_FSCK flag now all indicate the image has something wrong.
> 
> BTW, should we move the judgment to function __issue_discard_cmd in order to cover umount process?

We can add it in both places.

Thanks,

> 
> Thanks.
> 
>>
>> Thanks,
>>
>>> +			continue;
>>> +		}
>>>
>>>  		if (dcc->discard_wake)
>>>  			dcc->discard_wake = 0;
>>>
> 


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel