All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] f2fs: fix deadloop in foreground GC
@ 2022-04-29 20:46 ` Chao Yu
  0 siblings, 0 replies; 6+ messages in thread
From: Chao Yu @ 2022-04-29 20:46 UTC (permalink / raw)
  To: jaegeuk
  Cc: linux-f2fs-devel, linux-kernel, Chao Yu, stable, Ming Yan, Chao Yu

As Yanming reported in bugzilla:

https://bugzilla.kernel.org/show_bug.cgi?id=215914

The root cause is: in a very small sized image, it's very easy to
exceed threshold of foreground GC, if we calculate free space and
dirty data based on section granularity, in corner case,
has_not_enough_free_secs() will always return true, result in
deadloop in f2fs_gc().

So this patch refactors has_not_enough_free_secs() as below to fix
this issue:
1. calculate needed space based on block granularity, and separate
all blocks to two parts, section part, and block part, comparing
section part to free section, and comparing block part to free space
in openned log.
2. account F2FS_DIRTY_NODES, F2FS_DIRTY_IMETA and F2FS_DIRTY_DENTS
as node block consumer;
3. account F2FS_DIRTY_DENTS as data block consumer;

Cc: stable@vger.kernel.org
Reported-by: Ming Yan <yanming@tju.edu.cn>
Signed-off-by: Chao Yu <chao.yu@oppo.com>
---
 fs/f2fs/segment.h | 30 +++++++++++++++++-------------
 1 file changed, 17 insertions(+), 13 deletions(-)

diff --git a/fs/f2fs/segment.h b/fs/f2fs/segment.h
index 8a591455d796..28f7aa9b40bf 100644
--- a/fs/f2fs/segment.h
+++ b/fs/f2fs/segment.h
@@ -575,11 +575,10 @@ static inline int reserved_sections(struct f2fs_sb_info *sbi)
 	return GET_SEC_FROM_SEG(sbi, reserved_segments(sbi));
 }
 
-static inline bool has_curseg_enough_space(struct f2fs_sb_info *sbi)
+static inline bool has_curseg_enough_space(struct f2fs_sb_info *sbi,
+			unsigned int node_blocks, unsigned int dent_blocks)
 {
-	unsigned int node_blocks = get_pages(sbi, F2FS_DIRTY_NODES) +
-					get_pages(sbi, F2FS_DIRTY_DENTS);
-	unsigned int dent_blocks = get_pages(sbi, F2FS_DIRTY_DENTS);
+
 	unsigned int segno, left_blocks;
 	int i;
 
@@ -605,19 +604,24 @@ static inline bool has_curseg_enough_space(struct f2fs_sb_info *sbi)
 static inline bool has_not_enough_free_secs(struct f2fs_sb_info *sbi,
 					int freed, int needed)
 {
-	int node_secs = get_blocktype_secs(sbi, F2FS_DIRTY_NODES);
-	int dent_secs = get_blocktype_secs(sbi, F2FS_DIRTY_DENTS);
-	int imeta_secs = get_blocktype_secs(sbi, F2FS_DIRTY_IMETA);
+	unsigned int total_node_blocks = get_pages(sbi, F2FS_DIRTY_NODES) +
+					get_pages(sbi, F2FS_DIRTY_DENTS) +
+					get_pages(sbi, F2FS_DIRTY_IMETA);
+	unsigned int total_dent_blocks = get_pages(sbi, F2FS_DIRTY_DENTS);
+	unsigned int node_secs = total_node_blocks / BLKS_PER_SEC(sbi);
+	unsigned int dent_secs = total_dent_blocks / BLKS_PER_SEC(sbi);
+	unsigned int node_blocks = total_node_blocks % BLKS_PER_SEC(sbi);
+	unsigned int dent_blocks = total_dent_blocks % BLKS_PER_SEC(sbi);
 
 	if (unlikely(is_sbi_flag_set(sbi, SBI_POR_DOING)))
 		return false;
 
-	if (free_sections(sbi) + freed == reserved_sections(sbi) + needed &&
-			has_curseg_enough_space(sbi))
-		return false;
-	return (free_sections(sbi) + freed) <=
-		(node_secs + 2 * dent_secs + imeta_secs +
-		reserved_sections(sbi) + needed);
+	if (free_sections(sbi) + freed <=
+			node_secs + dent_secs + reserved_sections(sbi) + needed)
+		return true;
+	if (!has_curseg_enough_space(sbi, node_blocks, dent_blocks))
+		return true;
+	return false;
 }
 
 static inline bool f2fs_is_checkpoint_ready(struct f2fs_sb_info *sbi)
-- 
2.32.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [f2fs-dev] [PATCH] f2fs: fix deadloop in foreground GC
@ 2022-04-29 20:46 ` Chao Yu
  0 siblings, 0 replies; 6+ messages in thread
From: Chao Yu @ 2022-04-29 20:46 UTC (permalink / raw)
  To: jaegeuk; +Cc: linux-kernel, stable, linux-f2fs-devel, Ming Yan

As Yanming reported in bugzilla:

https://bugzilla.kernel.org/show_bug.cgi?id=215914

The root cause is: in a very small sized image, it's very easy to
exceed threshold of foreground GC, if we calculate free space and
dirty data based on section granularity, in corner case,
has_not_enough_free_secs() will always return true, result in
deadloop in f2fs_gc().

So this patch refactors has_not_enough_free_secs() as below to fix
this issue:
1. calculate needed space based on block granularity, and separate
all blocks to two parts, section part, and block part, comparing
section part to free section, and comparing block part to free space
in openned log.
2. account F2FS_DIRTY_NODES, F2FS_DIRTY_IMETA and F2FS_DIRTY_DENTS
as node block consumer;
3. account F2FS_DIRTY_DENTS as data block consumer;

Cc: stable@vger.kernel.org
Reported-by: Ming Yan <yanming@tju.edu.cn>
Signed-off-by: Chao Yu <chao.yu@oppo.com>
---
 fs/f2fs/segment.h | 30 +++++++++++++++++-------------
 1 file changed, 17 insertions(+), 13 deletions(-)

diff --git a/fs/f2fs/segment.h b/fs/f2fs/segment.h
index 8a591455d796..28f7aa9b40bf 100644
--- a/fs/f2fs/segment.h
+++ b/fs/f2fs/segment.h
@@ -575,11 +575,10 @@ static inline int reserved_sections(struct f2fs_sb_info *sbi)
 	return GET_SEC_FROM_SEG(sbi, reserved_segments(sbi));
 }
 
-static inline bool has_curseg_enough_space(struct f2fs_sb_info *sbi)
+static inline bool has_curseg_enough_space(struct f2fs_sb_info *sbi,
+			unsigned int node_blocks, unsigned int dent_blocks)
 {
-	unsigned int node_blocks = get_pages(sbi, F2FS_DIRTY_NODES) +
-					get_pages(sbi, F2FS_DIRTY_DENTS);
-	unsigned int dent_blocks = get_pages(sbi, F2FS_DIRTY_DENTS);
+
 	unsigned int segno, left_blocks;
 	int i;
 
@@ -605,19 +604,24 @@ static inline bool has_curseg_enough_space(struct f2fs_sb_info *sbi)
 static inline bool has_not_enough_free_secs(struct f2fs_sb_info *sbi,
 					int freed, int needed)
 {
-	int node_secs = get_blocktype_secs(sbi, F2FS_DIRTY_NODES);
-	int dent_secs = get_blocktype_secs(sbi, F2FS_DIRTY_DENTS);
-	int imeta_secs = get_blocktype_secs(sbi, F2FS_DIRTY_IMETA);
+	unsigned int total_node_blocks = get_pages(sbi, F2FS_DIRTY_NODES) +
+					get_pages(sbi, F2FS_DIRTY_DENTS) +
+					get_pages(sbi, F2FS_DIRTY_IMETA);
+	unsigned int total_dent_blocks = get_pages(sbi, F2FS_DIRTY_DENTS);
+	unsigned int node_secs = total_node_blocks / BLKS_PER_SEC(sbi);
+	unsigned int dent_secs = total_dent_blocks / BLKS_PER_SEC(sbi);
+	unsigned int node_blocks = total_node_blocks % BLKS_PER_SEC(sbi);
+	unsigned int dent_blocks = total_dent_blocks % BLKS_PER_SEC(sbi);
 
 	if (unlikely(is_sbi_flag_set(sbi, SBI_POR_DOING)))
 		return false;
 
-	if (free_sections(sbi) + freed == reserved_sections(sbi) + needed &&
-			has_curseg_enough_space(sbi))
-		return false;
-	return (free_sections(sbi) + freed) <=
-		(node_secs + 2 * dent_secs + imeta_secs +
-		reserved_sections(sbi) + needed);
+	if (free_sections(sbi) + freed <=
+			node_secs + dent_secs + reserved_sections(sbi) + needed)
+		return true;
+	if (!has_curseg_enough_space(sbi, node_blocks, dent_blocks))
+		return true;
+	return false;
 }
 
 static inline bool f2fs_is_checkpoint_ready(struct f2fs_sb_info *sbi)
-- 
2.32.0



_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] f2fs: fix deadloop in foreground GC
  2022-04-29 20:46 ` [f2fs-dev] " Chao Yu
@ 2022-05-03 22:25   ` Jaegeuk Kim
  -1 siblings, 0 replies; 6+ messages in thread
From: Jaegeuk Kim @ 2022-05-03 22:25 UTC (permalink / raw)
  To: Chao Yu; +Cc: linux-f2fs-devel, linux-kernel, stable, Ming Yan, Chao Yu

On 04/30, Chao Yu wrote:
> As Yanming reported in bugzilla:
> 
> https://bugzilla.kernel.org/show_bug.cgi?id=215914
> 
> The root cause is: in a very small sized image, it's very easy to
> exceed threshold of foreground GC, if we calculate free space and
> dirty data based on section granularity, in corner case,
> has_not_enough_free_secs() will always return true, result in
> deadloop in f2fs_gc().

Performance regression was reported. Can we check this for very small sized
image only?

> 
> So this patch refactors has_not_enough_free_secs() as below to fix
> this issue:
> 1. calculate needed space based on block granularity, and separate
> all blocks to two parts, section part, and block part, comparing
> section part to free section, and comparing block part to free space
> in openned log.
> 2. account F2FS_DIRTY_NODES, F2FS_DIRTY_IMETA and F2FS_DIRTY_DENTS
> as node block consumer;
> 3. account F2FS_DIRTY_DENTS as data block consumer;
> 
> Cc: stable@vger.kernel.org
> Reported-by: Ming Yan <yanming@tju.edu.cn>
> Signed-off-by: Chao Yu <chao.yu@oppo.com>
> ---
>  fs/f2fs/segment.h | 30 +++++++++++++++++-------------
>  1 file changed, 17 insertions(+), 13 deletions(-)
> 
> diff --git a/fs/f2fs/segment.h b/fs/f2fs/segment.h
> index 8a591455d796..28f7aa9b40bf 100644
> --- a/fs/f2fs/segment.h
> +++ b/fs/f2fs/segment.h
> @@ -575,11 +575,10 @@ static inline int reserved_sections(struct f2fs_sb_info *sbi)
>  	return GET_SEC_FROM_SEG(sbi, reserved_segments(sbi));
>  }
>  
> -static inline bool has_curseg_enough_space(struct f2fs_sb_info *sbi)
> +static inline bool has_curseg_enough_space(struct f2fs_sb_info *sbi,
> +			unsigned int node_blocks, unsigned int dent_blocks)
>  {
> -	unsigned int node_blocks = get_pages(sbi, F2FS_DIRTY_NODES) +
> -					get_pages(sbi, F2FS_DIRTY_DENTS);
> -	unsigned int dent_blocks = get_pages(sbi, F2FS_DIRTY_DENTS);
> +
>  	unsigned int segno, left_blocks;
>  	int i;
>  
> @@ -605,19 +604,24 @@ static inline bool has_curseg_enough_space(struct f2fs_sb_info *sbi)
>  static inline bool has_not_enough_free_secs(struct f2fs_sb_info *sbi,
>  					int freed, int needed)
>  {
> -	int node_secs = get_blocktype_secs(sbi, F2FS_DIRTY_NODES);
> -	int dent_secs = get_blocktype_secs(sbi, F2FS_DIRTY_DENTS);
> -	int imeta_secs = get_blocktype_secs(sbi, F2FS_DIRTY_IMETA);
> +	unsigned int total_node_blocks = get_pages(sbi, F2FS_DIRTY_NODES) +
> +					get_pages(sbi, F2FS_DIRTY_DENTS) +
> +					get_pages(sbi, F2FS_DIRTY_IMETA);
> +	unsigned int total_dent_blocks = get_pages(sbi, F2FS_DIRTY_DENTS);
> +	unsigned int node_secs = total_node_blocks / BLKS_PER_SEC(sbi);
> +	unsigned int dent_secs = total_dent_blocks / BLKS_PER_SEC(sbi);
> +	unsigned int node_blocks = total_node_blocks % BLKS_PER_SEC(sbi);
> +	unsigned int dent_blocks = total_dent_blocks % BLKS_PER_SEC(sbi);
>  
>  	if (unlikely(is_sbi_flag_set(sbi, SBI_POR_DOING)))
>  		return false;
>  
> -	if (free_sections(sbi) + freed == reserved_sections(sbi) + needed &&
> -			has_curseg_enough_space(sbi))
> -		return false;
> -	return (free_sections(sbi) + freed) <=
> -		(node_secs + 2 * dent_secs + imeta_secs +
> -		reserved_sections(sbi) + needed);
> +	if (free_sections(sbi) + freed <=
> +			node_secs + dent_secs + reserved_sections(sbi) + needed)
> +		return true;
> +	if (!has_curseg_enough_space(sbi, node_blocks, dent_blocks))
> +		return true;
> +	return false;
>  }
>  
>  static inline bool f2fs_is_checkpoint_ready(struct f2fs_sb_info *sbi)
> -- 
> 2.32.0

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [f2fs-dev] [PATCH] f2fs: fix deadloop in foreground GC
@ 2022-05-03 22:25   ` Jaegeuk Kim
  0 siblings, 0 replies; 6+ messages in thread
From: Jaegeuk Kim @ 2022-05-03 22:25 UTC (permalink / raw)
  To: Chao Yu; +Cc: Ming Yan, linux-kernel, stable, linux-f2fs-devel

On 04/30, Chao Yu wrote:
> As Yanming reported in bugzilla:
> 
> https://bugzilla.kernel.org/show_bug.cgi?id=215914
> 
> The root cause is: in a very small sized image, it's very easy to
> exceed threshold of foreground GC, if we calculate free space and
> dirty data based on section granularity, in corner case,
> has_not_enough_free_secs() will always return true, result in
> deadloop in f2fs_gc().

Performance regression was reported. Can we check this for very small sized
image only?

> 
> So this patch refactors has_not_enough_free_secs() as below to fix
> this issue:
> 1. calculate needed space based on block granularity, and separate
> all blocks to two parts, section part, and block part, comparing
> section part to free section, and comparing block part to free space
> in openned log.
> 2. account F2FS_DIRTY_NODES, F2FS_DIRTY_IMETA and F2FS_DIRTY_DENTS
> as node block consumer;
> 3. account F2FS_DIRTY_DENTS as data block consumer;
> 
> Cc: stable@vger.kernel.org
> Reported-by: Ming Yan <yanming@tju.edu.cn>
> Signed-off-by: Chao Yu <chao.yu@oppo.com>
> ---
>  fs/f2fs/segment.h | 30 +++++++++++++++++-------------
>  1 file changed, 17 insertions(+), 13 deletions(-)
> 
> diff --git a/fs/f2fs/segment.h b/fs/f2fs/segment.h
> index 8a591455d796..28f7aa9b40bf 100644
> --- a/fs/f2fs/segment.h
> +++ b/fs/f2fs/segment.h
> @@ -575,11 +575,10 @@ static inline int reserved_sections(struct f2fs_sb_info *sbi)
>  	return GET_SEC_FROM_SEG(sbi, reserved_segments(sbi));
>  }
>  
> -static inline bool has_curseg_enough_space(struct f2fs_sb_info *sbi)
> +static inline bool has_curseg_enough_space(struct f2fs_sb_info *sbi,
> +			unsigned int node_blocks, unsigned int dent_blocks)
>  {
> -	unsigned int node_blocks = get_pages(sbi, F2FS_DIRTY_NODES) +
> -					get_pages(sbi, F2FS_DIRTY_DENTS);
> -	unsigned int dent_blocks = get_pages(sbi, F2FS_DIRTY_DENTS);
> +
>  	unsigned int segno, left_blocks;
>  	int i;
>  
> @@ -605,19 +604,24 @@ static inline bool has_curseg_enough_space(struct f2fs_sb_info *sbi)
>  static inline bool has_not_enough_free_secs(struct f2fs_sb_info *sbi,
>  					int freed, int needed)
>  {
> -	int node_secs = get_blocktype_secs(sbi, F2FS_DIRTY_NODES);
> -	int dent_secs = get_blocktype_secs(sbi, F2FS_DIRTY_DENTS);
> -	int imeta_secs = get_blocktype_secs(sbi, F2FS_DIRTY_IMETA);
> +	unsigned int total_node_blocks = get_pages(sbi, F2FS_DIRTY_NODES) +
> +					get_pages(sbi, F2FS_DIRTY_DENTS) +
> +					get_pages(sbi, F2FS_DIRTY_IMETA);
> +	unsigned int total_dent_blocks = get_pages(sbi, F2FS_DIRTY_DENTS);
> +	unsigned int node_secs = total_node_blocks / BLKS_PER_SEC(sbi);
> +	unsigned int dent_secs = total_dent_blocks / BLKS_PER_SEC(sbi);
> +	unsigned int node_blocks = total_node_blocks % BLKS_PER_SEC(sbi);
> +	unsigned int dent_blocks = total_dent_blocks % BLKS_PER_SEC(sbi);
>  
>  	if (unlikely(is_sbi_flag_set(sbi, SBI_POR_DOING)))
>  		return false;
>  
> -	if (free_sections(sbi) + freed == reserved_sections(sbi) + needed &&
> -			has_curseg_enough_space(sbi))
> -		return false;
> -	return (free_sections(sbi) + freed) <=
> -		(node_secs + 2 * dent_secs + imeta_secs +
> -		reserved_sections(sbi) + needed);
> +	if (free_sections(sbi) + freed <=
> +			node_secs + dent_secs + reserved_sections(sbi) + needed)
> +		return true;
> +	if (!has_curseg_enough_space(sbi, node_blocks, dent_blocks))
> +		return true;
> +	return false;
>  }
>  
>  static inline bool f2fs_is_checkpoint_ready(struct f2fs_sb_info *sbi)
> -- 
> 2.32.0


_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] f2fs: fix deadloop in foreground GC
  2022-05-03 22:25   ` [f2fs-dev] " Jaegeuk Kim
@ 2022-05-04  0:39     ` Chao Yu
  -1 siblings, 0 replies; 6+ messages in thread
From: Chao Yu @ 2022-05-04  0:39 UTC (permalink / raw)
  To: Jaegeuk Kim; +Cc: linux-f2fs-devel, linux-kernel, stable, Ming Yan, Chao Yu

On 2022/5/4 6:25, Jaegeuk Kim wrote:
> On 04/30, Chao Yu wrote:
>> As Yanming reported in bugzilla:
>>
>> https://bugzilla.kernel.org/show_bug.cgi?id=215914
>>
>> The root cause is: in a very small sized image, it's very easy to
>> exceed threshold of foreground GC, if we calculate free space and
>> dirty data based on section granularity, in corner case,
>> has_not_enough_free_secs() will always return true, result in
>> deadloop in f2fs_gc().
> 
> Performance regression was reported. Can we check this for very small sized
> image only?

I noticed that, I've fixed the issue in v2, could you please take a look?

Thanks,

> 
>>
>> So this patch refactors has_not_enough_free_secs() as below to fix
>> this issue:
>> 1. calculate needed space based on block granularity, and separate
>> all blocks to two parts, section part, and block part, comparing
>> section part to free section, and comparing block part to free space
>> in openned log.
>> 2. account F2FS_DIRTY_NODES, F2FS_DIRTY_IMETA and F2FS_DIRTY_DENTS
>> as node block consumer;
>> 3. account F2FS_DIRTY_DENTS as data block consumer;
>>
>> Cc: stable@vger.kernel.org
>> Reported-by: Ming Yan <yanming@tju.edu.cn>
>> Signed-off-by: Chao Yu <chao.yu@oppo.com>
>> ---
>>   fs/f2fs/segment.h | 30 +++++++++++++++++-------------
>>   1 file changed, 17 insertions(+), 13 deletions(-)
>>
>> diff --git a/fs/f2fs/segment.h b/fs/f2fs/segment.h
>> index 8a591455d796..28f7aa9b40bf 100644
>> --- a/fs/f2fs/segment.h
>> +++ b/fs/f2fs/segment.h
>> @@ -575,11 +575,10 @@ static inline int reserved_sections(struct f2fs_sb_info *sbi)
>>   	return GET_SEC_FROM_SEG(sbi, reserved_segments(sbi));
>>   }
>>   
>> -static inline bool has_curseg_enough_space(struct f2fs_sb_info *sbi)
>> +static inline bool has_curseg_enough_space(struct f2fs_sb_info *sbi,
>> +			unsigned int node_blocks, unsigned int dent_blocks)
>>   {
>> -	unsigned int node_blocks = get_pages(sbi, F2FS_DIRTY_NODES) +
>> -					get_pages(sbi, F2FS_DIRTY_DENTS);
>> -	unsigned int dent_blocks = get_pages(sbi, F2FS_DIRTY_DENTS);
>> +
>>   	unsigned int segno, left_blocks;
>>   	int i;
>>   
>> @@ -605,19 +604,24 @@ static inline bool has_curseg_enough_space(struct f2fs_sb_info *sbi)
>>   static inline bool has_not_enough_free_secs(struct f2fs_sb_info *sbi,
>>   					int freed, int needed)
>>   {
>> -	int node_secs = get_blocktype_secs(sbi, F2FS_DIRTY_NODES);
>> -	int dent_secs = get_blocktype_secs(sbi, F2FS_DIRTY_DENTS);
>> -	int imeta_secs = get_blocktype_secs(sbi, F2FS_DIRTY_IMETA);
>> +	unsigned int total_node_blocks = get_pages(sbi, F2FS_DIRTY_NODES) +
>> +					get_pages(sbi, F2FS_DIRTY_DENTS) +
>> +					get_pages(sbi, F2FS_DIRTY_IMETA);
>> +	unsigned int total_dent_blocks = get_pages(sbi, F2FS_DIRTY_DENTS);
>> +	unsigned int node_secs = total_node_blocks / BLKS_PER_SEC(sbi);
>> +	unsigned int dent_secs = total_dent_blocks / BLKS_PER_SEC(sbi);
>> +	unsigned int node_blocks = total_node_blocks % BLKS_PER_SEC(sbi);
>> +	unsigned int dent_blocks = total_dent_blocks % BLKS_PER_SEC(sbi);
>>   
>>   	if (unlikely(is_sbi_flag_set(sbi, SBI_POR_DOING)))
>>   		return false;
>>   
>> -	if (free_sections(sbi) + freed == reserved_sections(sbi) + needed &&
>> -			has_curseg_enough_space(sbi))
>> -		return false;
>> -	return (free_sections(sbi) + freed) <=
>> -		(node_secs + 2 * dent_secs + imeta_secs +
>> -		reserved_sections(sbi) + needed);
>> +	if (free_sections(sbi) + freed <=
>> +			node_secs + dent_secs + reserved_sections(sbi) + needed)
>> +		return true;
>> +	if (!has_curseg_enough_space(sbi, node_blocks, dent_blocks))
>> +		return true;
>> +	return false;
>>   }
>>   
>>   static inline bool f2fs_is_checkpoint_ready(struct f2fs_sb_info *sbi)
>> -- 
>> 2.32.0

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [f2fs-dev] [PATCH] f2fs: fix deadloop in foreground GC
@ 2022-05-04  0:39     ` Chao Yu
  0 siblings, 0 replies; 6+ messages in thread
From: Chao Yu @ 2022-05-04  0:39 UTC (permalink / raw)
  To: Jaegeuk Kim; +Cc: Ming Yan, linux-kernel, stable, linux-f2fs-devel

On 2022/5/4 6:25, Jaegeuk Kim wrote:
> On 04/30, Chao Yu wrote:
>> As Yanming reported in bugzilla:
>>
>> https://bugzilla.kernel.org/show_bug.cgi?id=215914
>>
>> The root cause is: in a very small sized image, it's very easy to
>> exceed threshold of foreground GC, if we calculate free space and
>> dirty data based on section granularity, in corner case,
>> has_not_enough_free_secs() will always return true, result in
>> deadloop in f2fs_gc().
> 
> Performance regression was reported. Can we check this for very small sized
> image only?

I noticed that, I've fixed the issue in v2, could you please take a look?

Thanks,

> 
>>
>> So this patch refactors has_not_enough_free_secs() as below to fix
>> this issue:
>> 1. calculate needed space based on block granularity, and separate
>> all blocks to two parts, section part, and block part, comparing
>> section part to free section, and comparing block part to free space
>> in openned log.
>> 2. account F2FS_DIRTY_NODES, F2FS_DIRTY_IMETA and F2FS_DIRTY_DENTS
>> as node block consumer;
>> 3. account F2FS_DIRTY_DENTS as data block consumer;
>>
>> Cc: stable@vger.kernel.org
>> Reported-by: Ming Yan <yanming@tju.edu.cn>
>> Signed-off-by: Chao Yu <chao.yu@oppo.com>
>> ---
>>   fs/f2fs/segment.h | 30 +++++++++++++++++-------------
>>   1 file changed, 17 insertions(+), 13 deletions(-)
>>
>> diff --git a/fs/f2fs/segment.h b/fs/f2fs/segment.h
>> index 8a591455d796..28f7aa9b40bf 100644
>> --- a/fs/f2fs/segment.h
>> +++ b/fs/f2fs/segment.h
>> @@ -575,11 +575,10 @@ static inline int reserved_sections(struct f2fs_sb_info *sbi)
>>   	return GET_SEC_FROM_SEG(sbi, reserved_segments(sbi));
>>   }
>>   
>> -static inline bool has_curseg_enough_space(struct f2fs_sb_info *sbi)
>> +static inline bool has_curseg_enough_space(struct f2fs_sb_info *sbi,
>> +			unsigned int node_blocks, unsigned int dent_blocks)
>>   {
>> -	unsigned int node_blocks = get_pages(sbi, F2FS_DIRTY_NODES) +
>> -					get_pages(sbi, F2FS_DIRTY_DENTS);
>> -	unsigned int dent_blocks = get_pages(sbi, F2FS_DIRTY_DENTS);
>> +
>>   	unsigned int segno, left_blocks;
>>   	int i;
>>   
>> @@ -605,19 +604,24 @@ static inline bool has_curseg_enough_space(struct f2fs_sb_info *sbi)
>>   static inline bool has_not_enough_free_secs(struct f2fs_sb_info *sbi,
>>   					int freed, int needed)
>>   {
>> -	int node_secs = get_blocktype_secs(sbi, F2FS_DIRTY_NODES);
>> -	int dent_secs = get_blocktype_secs(sbi, F2FS_DIRTY_DENTS);
>> -	int imeta_secs = get_blocktype_secs(sbi, F2FS_DIRTY_IMETA);
>> +	unsigned int total_node_blocks = get_pages(sbi, F2FS_DIRTY_NODES) +
>> +					get_pages(sbi, F2FS_DIRTY_DENTS) +
>> +					get_pages(sbi, F2FS_DIRTY_IMETA);
>> +	unsigned int total_dent_blocks = get_pages(sbi, F2FS_DIRTY_DENTS);
>> +	unsigned int node_secs = total_node_blocks / BLKS_PER_SEC(sbi);
>> +	unsigned int dent_secs = total_dent_blocks / BLKS_PER_SEC(sbi);
>> +	unsigned int node_blocks = total_node_blocks % BLKS_PER_SEC(sbi);
>> +	unsigned int dent_blocks = total_dent_blocks % BLKS_PER_SEC(sbi);
>>   
>>   	if (unlikely(is_sbi_flag_set(sbi, SBI_POR_DOING)))
>>   		return false;
>>   
>> -	if (free_sections(sbi) + freed == reserved_sections(sbi) + needed &&
>> -			has_curseg_enough_space(sbi))
>> -		return false;
>> -	return (free_sections(sbi) + freed) <=
>> -		(node_secs + 2 * dent_secs + imeta_secs +
>> -		reserved_sections(sbi) + needed);
>> +	if (free_sections(sbi) + freed <=
>> +			node_secs + dent_secs + reserved_sections(sbi) + needed)
>> +		return true;
>> +	if (!has_curseg_enough_space(sbi, node_blocks, dent_blocks))
>> +		return true;
>> +	return false;
>>   }
>>   
>>   static inline bool f2fs_is_checkpoint_ready(struct f2fs_sb_info *sbi)
>> -- 
>> 2.32.0


_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2022-05-04  0:39 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-04-29 20:46 [PATCH] f2fs: fix deadloop in foreground GC Chao Yu
2022-04-29 20:46 ` [f2fs-dev] " Chao Yu
2022-05-03 22:25 ` Jaegeuk Kim
2022-05-03 22:25   ` [f2fs-dev] " Jaegeuk Kim
2022-05-04  0:39   ` Chao Yu
2022-05-04  0:39     ` [f2fs-dev] " Chao Yu

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.