f2fs: init discard policy after thread wakeup

Message ID 20221118034600.59489-1-frank.li@vivo.com
State New
Headers
Series f2fs: init discard policy after thread wakeup |

Commit Message

李扬韬 Nov. 18, 2022, 3:46 a.m. UTC
  Under the current logic, after the discard thread wakes up, it will not
run according to the expected policy, but will use the expected policy
before sleep. Move the strategy selection to after the thread wakes up,
so that the running state of the thread meets expectations.

Signed-off-by: Yangtao Li <frank.li@vivo.com>
---
 fs/f2fs/segment.c | 20 +++++++++-----------
 1 file changed, 9 insertions(+), 11 deletions(-)
  

Comments

Chao Yu Nov. 23, 2022, 3:36 p.m. UTC | #1
On 2022/11/18 11:46, Yangtao Li wrote:
> Under the current logic, after the discard thread wakes up, it will not
> run according to the expected policy, but will use the expected policy
> before sleep. Move the strategy selection to after the thread wakes up,
> so that the running state of the thread meets expectations.
> 
> Signed-off-by: Yangtao Li <frank.li@vivo.com>
> ---
>   fs/f2fs/segment.c | 20 +++++++++-----------
>   1 file changed, 9 insertions(+), 11 deletions(-)
> 
> diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
> index 8b0b76550578..609e90aa80c2 100644
> --- a/fs/f2fs/segment.c
> +++ b/fs/f2fs/segment.c
> @@ -1685,6 +1685,11 @@ static int issue_discard_thread(void *data)
>   	set_freezable();
>   
>   	do {

		if (!atomic_read(&dcc->discard_cmd_cnt))
		       wait_ms = dpolicy.max_interval;

Thanks,

> +		wait_event_interruptible_timeout(*q,
> +				kthread_should_stop() || freezing(current) ||
> +				dcc->discard_wake,
> +				msecs_to_jiffies(wait_ms));
> +
>   		if (sbi->gc_mode == GC_URGENT_HIGH ||
>   			!f2fs_available_free_memory(sbi, DISCARD_CACHE))
>   			__init_discard_policy(sbi, &dpolicy, DPOLICY_FORCE, 1);
> @@ -1692,14 +1697,6 @@ static int issue_discard_thread(void *data)
>   			__init_discard_policy(sbi, &dpolicy, DPOLICY_BG,
>   						dcc->discard_granularity);
>   
> -		if (!atomic_read(&dcc->discard_cmd_cnt))
> -		       wait_ms = dpolicy.max_interval;
> -
> -		wait_event_interruptible_timeout(*q,
> -				kthread_should_stop() || freezing(current) ||
> -				dcc->discard_wake,
> -				msecs_to_jiffies(wait_ms));
> -
>   		if (dcc->discard_wake)
>   			dcc->discard_wake = 0;
>   
> @@ -1713,12 +1710,11 @@ static int issue_discard_thread(void *data)
>   			continue;
>   		if (kthread_should_stop())
>   			return 0;
> -		if (is_sbi_flag_set(sbi, SBI_NEED_FSCK)) {
> +		if (is_sbi_flag_set(sbi, SBI_NEED_FSCK) ||
> +			!atomic_read(&dcc->discard_cmd_cnt)) {
>   			wait_ms = dpolicy.max_interval;
>   			continue;
>   		}
> -		if (!atomic_read(&dcc->discard_cmd_cnt))
> -			continue;
>   
>   		sb_start_intwrite(sbi->sb);
>   
> @@ -1733,6 +1729,8 @@ static int issue_discard_thread(void *data)
>   		} else {
>   			wait_ms = dpolicy.max_interval;
>   		}
> +		if (!atomic_read(&dcc->discard_cmd_cnt))
> +			wait_ms = dpolicy.max_interval;
>   
>   		sb_end_intwrite(sbi->sb);
>
  
李扬韬 Nov. 23, 2022, 4:06 p.m. UTC | #2
HI Chao,
>>		set_freezable();
>>   
>>		do {
>
>		if (!atomic_read(&dcc->discard_cmd_cnt))
>		       wait_ms = dpolicy.max_interval;

dpolicy has not been initialized for the first time, and the value in the stack is unstable.

Thx,
Yangtao
  
Chao Yu Nov. 24, 2022, 2:20 p.m. UTC | #3
On 2022/11/18 11:46, Yangtao Li wrote:
> Under the current logic, after the discard thread wakes up, it will not
> run according to the expected policy, but will use the expected policy
> before sleep. Move the strategy selection to after the thread wakes up,
> so that the running state of the thread meets expectations.
> 
> Signed-off-by: Yangtao Li <frank.li@vivo.com>

Reviewed-by: Chao Yu <chao@kernel.org>

Thanks,
  

Patch

diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
index 8b0b76550578..609e90aa80c2 100644
--- a/fs/f2fs/segment.c
+++ b/fs/f2fs/segment.c
@@ -1685,6 +1685,11 @@  static int issue_discard_thread(void *data)
 	set_freezable();
 
 	do {
+		wait_event_interruptible_timeout(*q,
+				kthread_should_stop() || freezing(current) ||
+				dcc->discard_wake,
+				msecs_to_jiffies(wait_ms));
+
 		if (sbi->gc_mode == GC_URGENT_HIGH ||
 			!f2fs_available_free_memory(sbi, DISCARD_CACHE))
 			__init_discard_policy(sbi, &dpolicy, DPOLICY_FORCE, 1);
@@ -1692,14 +1697,6 @@  static int issue_discard_thread(void *data)
 			__init_discard_policy(sbi, &dpolicy, DPOLICY_BG,
 						dcc->discard_granularity);
 
-		if (!atomic_read(&dcc->discard_cmd_cnt))
-		       wait_ms = dpolicy.max_interval;
-
-		wait_event_interruptible_timeout(*q,
-				kthread_should_stop() || freezing(current) ||
-				dcc->discard_wake,
-				msecs_to_jiffies(wait_ms));
-
 		if (dcc->discard_wake)
 			dcc->discard_wake = 0;
 
@@ -1713,12 +1710,11 @@  static int issue_discard_thread(void *data)
 			continue;
 		if (kthread_should_stop())
 			return 0;
-		if (is_sbi_flag_set(sbi, SBI_NEED_FSCK)) {
+		if (is_sbi_flag_set(sbi, SBI_NEED_FSCK) ||
+			!atomic_read(&dcc->discard_cmd_cnt)) {
 			wait_ms = dpolicy.max_interval;
 			continue;
 		}
-		if (!atomic_read(&dcc->discard_cmd_cnt))
-			continue;
 
 		sb_start_intwrite(sbi->sb);
 
@@ -1733,6 +1729,8 @@  static int issue_discard_thread(void *data)
 		} else {
 			wait_ms = dpolicy.max_interval;
 		}
+		if (!atomic_read(&dcc->discard_cmd_cnt))
+			wait_ms = dpolicy.max_interval;
 
 		sb_end_intwrite(sbi->sb);