[f2fs-dev] [PATCH] f2fs: skip further SIT updates for already invalidated blocks

[f2fs-dev] [PATCH] f2fs: skip further SIT updates for already invalidated blocks

[Seongjae Jeong]

When update_sit_entry_for_release() encounters a block whose bit is
already cleared in cur_valid_map, it reports the inconsistency and
corrects se->valid_blocks and del. However, it still proceeds with
the discard bitmap and checkpoint accounting updates.

This can clear the discard bit unnecessarily and incorrectly decrement
the segment and section ckpt_valid_blocks counters when the block is
not marked in ckpt_valid_map.

Continue to the next block after correcting se->valid_blocks and del,
so that an already invalidated block does not trigger further discard
bitmap or checkpoint counter updates.

Fixes: 81ffbd224e5f ("f2fs: update_sit_entry_for_release() supports consecutive blocks.")
Signed-off-by: Seongjae Jeong <jsjlee1020@gmail.com>
---
 fs/f2fs/segment.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
index 51858f64e43e..fc2f0fd8e318 100644
--- a/fs/f2fs/segment.c
+++ b/fs/f2fs/segment.c
@@ -2560,7 +2560,10 @@ static int update_sit_entry_for_release(struct f2fs_sb_info *sbi, struct seg_ent
 			f2fs_bug_on(sbi, 1);
 			se->valid_blocks++;
 			del += 1;
-		} else if (unlikely(is_sbi_flag_set(sbi, SBI_CP_DISABLED))) {
+			continue;
+		}
+
+		if (unlikely(is_sbi_flag_set(sbi, SBI_CP_DISABLED))) {
 			/*
 			 * If checkpoints are off, we must not reuse data that
 			 * was used in the previous checkpoint. If it was used
-- 
2.53.0



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

Re: [f2fs-dev] [PATCH] f2fs: skip further SIT updates for already invalidated blocks

[Chao Yu via Linux-f2fs-devel]

On 10/4/26 14:10, Seongjae Jeong wrote:
> When update_sit_entry_for_release() encounters a block whose bit is
> already cleared in cur_valid_map, it reports the inconsistency and
> corrects se->valid_blocks and del. However, it still proceeds with
> the discard bitmap and checkpoint accounting updates.
> 
> This can clear the discard bit unnecessarily and incorrectly decrement
> the segment and section ckpt_valid_blocks counters when the block is
> not marked in ckpt_valid_map.
> 
> Continue to the next block after correcting se->valid_blocks and del,
> so that an already invalidated block does not trigger further discard
> bitmap or checkpoint counter updates.
> 

Cc stable.

> Fixes: 81ffbd224e5f ("f2fs: update_sit_entry_for_release() supports consecutive blocks.")

This commit didn't change previous logic? IIUC.

Otherwise it looks good to me.

Thanks,

> Signed-off-by: Seongjae Jeong <jsjlee1020@gmail.com>
> ---
>   fs/f2fs/segment.c | 5 ++++-
>   1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
> index 51858f64e43e..fc2f0fd8e318 100644
> --- a/fs/f2fs/segment.c
> +++ b/fs/f2fs/segment.c
> @@ -2560,7 +2560,10 @@ static int update_sit_entry_for_release(struct f2fs_sb_info *sbi, struct seg_ent
>   			f2fs_bug_on(sbi, 1);
>   			se->valid_blocks++;
>   			del += 1;
> -		} else if (unlikely(is_sbi_flag_set(sbi, SBI_CP_DISABLED))) {
> +			continue;
> +		}
> +
> +		if (unlikely(is_sbi_flag_set(sbi, SBI_CP_DISABLED))) {
>   			/*
>   			 * If checkpoints are off, we must not reuse data that
>   			 * was used in the previous checkpoint. If it was used



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

Re: [f2fs-dev] [PATCH] f2fs: skip further SIT updates for already invalidated blocks

[Seongjae Jeong]

Hi Chao,

> Cc stable.
>
> > Fixes: 81ffbd224e5f ("f2fs: update_sit_entry_for_release() supports consecutive blocks.")
>
> This commit didn't change previous logic? IIUC.

The discard bitmap handling already existed before this commit. It was
introduced
by a66cdd985532 ("f2fs: introduce discard_map for f2fs_trim_fs").

However, 81ffbd224e5f did change the checkpoint accounting.
Previously, del was set to 0 when the cur_valid_map bit was already cleared,
so se->ckpt_valid_blocks += del left the counter unchanged.
That commit replaced this logic with se->ckpt_valid_blocks -= 1,
regardless of the del value.
Thus, the counter is incorrectly decremented.

Would it make sense to add a second Fixes tag for a66cdd985532 to cover
the discard bitmap handling?


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