During a checkpoint, the current active segment X may not be handled
properly. This occurs when segment X has 0 valid blocks and a non-zero
number of discard blocks, for the following reasons:

locate_dirty_segment() does not mark any active segment as a prefree
segment. As a result, segment X is not included in dirty_segmap[PRE], and
f2fs_clear_prefree_segments() skips it when handling prefree segments.

add_discard_addrs() skips any segment with 0 valid blocks, so segment X is
also skipped.

Consequently, no `struct discard_cmd` is actually created for segment X.
However, the ckpt_valid_map and cur_valid_map of segment X are synced by
seg_info_to_raw_sit() during the current checkpoint process. As a result,
it cannot find the missing discard bits even in subsequent checkpoints.
Consequently, the value of sbi->discard_blks remains non-zero. Thus, when
f2fs is umounted, CP_TRIMMED_FLAG will not be set due to the non-zero
sbi->discard_blks.

Relevant code process:

f2fs_write_checkpoint()
    f2fs_flush_sit_entries()
         list_for_each_entry_safe(ses, tmp, head, set_list) {
             for_each_set_bit_from(segno, bitmap, end) {
                 ...
                 add_discard_addrs(sbi, cpc, false); // skip segment X due to its 0 valid blocks
                 ...
                 seg_info_to_raw_sit(); // sync ckpt_valid_map with cur_valid_map for segment X
                 ...
             }
         }
    f2fs_clear_prefree_segments(); // segment X is not included in dirty_segmap[PRE] and is skipped

Since add_discard_addrs() can handle active segments with non-zero valid
blocks, it is reasonable to fix this issue by allowing it to also handle
active segments with 0 valid blocks. 

Fixes: b29555505d81 ("f2fs: add key functions for small discards")
Signed-off-by: Chunhai Guo <guochunhai@vivo.com>
---
v1: https://lore.kernel.org/linux-f2fs-devel/20241203065108.2763436-1-guochunhai@vivo.com/
v1->v2:
 - Modify the commit message to make it easier to understand.
 - Add fixes to the commit.
---
 fs/f2fs/segment.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
index 86e547f008f9..13ee73a3c481 100644
--- a/fs/f2fs/segment.c
+++ b/fs/f2fs/segment.c
@@ -2090,7 +2090,9 @@ static bool add_discard_addrs(struct f2fs_sb_info *sbi, struct cp_control *cpc,
 		return false;
 
 	if (!force) {
-		if (!f2fs_realtime_discard_enable(sbi) || !se->valid_blocks ||
+		if (!f2fs_realtime_discard_enable(sbi) ||
+			(!se->valid_blocks &&
+				!IS_CURSEG(sbi, cpc->trim_start)) ||
 			SM_I(sbi)->dcc_info->nr_discards >=
 				SM_I(sbi)->dcc_info->max_discards)
 			return false;
-- 
2.34.1

On 1/9/25 20:27, Chunhai Guo wrote:
> During a checkpoint, the current active segment X may not be handled
> properly. This occurs when segment X has 0 valid blocks and a non-zero

How does this happen? Allocator selects a dirty segment w/ SSR? and the
left valid data blocks were deleted later before following checkpoint?

If so, pending discard count in that segment should be in range of (0, 512)?

Thanks,

> number of discard blocks, for the following reasons:
> 
> locate_dirty_segment() does not mark any active segment as a prefree
> segment. As a result, segment X is not included in dirty_segmap[PRE], and
> f2fs_clear_prefree_segments() skips it when handling prefree segments.
> 
> add_discard_addrs() skips any segment with 0 valid blocks, so segment X is
> also skipped.
> 
> Consequently, no `struct discard_cmd` is actually created for segment X.
> However, the ckpt_valid_map and cur_valid_map of segment X are synced by
> seg_info_to_raw_sit() during the current checkpoint process. As a result,
> it cannot find the missing discard bits even in subsequent checkpoints.
> Consequently, the value of sbi->discard_blks remains non-zero. Thus, when
> f2fs is umounted, CP_TRIMMED_FLAG will not be set due to the non-zero
> sbi->discard_blks.
> 
> Relevant code process:
> 
> f2fs_write_checkpoint()
>      f2fs_flush_sit_entries()
>           list_for_each_entry_safe(ses, tmp, head, set_list) {
>               for_each_set_bit_from(segno, bitmap, end) {
>                   ...
>                   add_discard_addrs(sbi, cpc, false); // skip segment X due to its 0 valid blocks
>                   ...
>                   seg_info_to_raw_sit(); // sync ckpt_valid_map with cur_valid_map for segment X
>                   ...
>               }
>           }
>      f2fs_clear_prefree_segments(); // segment X is not included in dirty_segmap[PRE] and is skipped
> 
> Since add_discard_addrs() can handle active segments with non-zero valid
> blocks, it is reasonable to fix this issue by allowing it to also handle
> active segments with 0 valid blocks.
> 
> Fixes: b29555505d81 ("f2fs: add key functions for small discards")
> Signed-off-by: Chunhai Guo <guochunhai@vivo.com>
> ---
> v1: https://lore.kernel.org/linux-f2fs-devel/20241203065108.2763436-1-guochunhai@vivo.com/
> v1->v2:
>   - Modify the commit message to make it easier to understand.
>   - Add fixes to the commit.
> ---
>   fs/f2fs/segment.c | 4 +++-
>   1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
> index 86e547f008f9..13ee73a3c481 100644
> --- a/fs/f2fs/segment.c
> +++ b/fs/f2fs/segment.c
> @@ -2090,7 +2090,9 @@ static bool add_discard_addrs(struct f2fs_sb_info *sbi, struct cp_control *cpc,
>   		return false;
>   
>   	if (!force) {
> -		if (!f2fs_realtime_discard_enable(sbi) || !se->valid_blocks ||
> +		if (!f2fs_realtime_discard_enable(sbi) ||
> +			(!se->valid_blocks &&
> +				!IS_CURSEG(sbi, cpc->trim_start)) ||
>   			SM_I(sbi)->dcc_info->nr_discards >=
>   				SM_I(sbi)->dcc_info->max_discards)
>   			return false;

在 1/20/2025 8:25 PM, Chao Yu 写道:
> On 1/9/25 20:27, Chunhai Guo wrote:
>> During a checkpoint, the current active segment X may not be handled
>> properly. This occurs when segment X has 0 valid blocks and a non-zero
> How does this happen? Allocator selects a dirty segment w/ SSR? and the
> left valid data blocks were deleted later before following checkpoint?
>
> If so, pending discard count in that segment should be in range of (0, 512)?


This issue is found with LFS rather than SSR. Here's what happens: some
data blocks are allocated for a file in the current active segment, and
then the file is deleted, resulting in all valid data blocks in the
current active segment being deleted before the following checkpoint.
This issue is easy to reproduce with the following operations:


# mkfs.f2fs -f /dev/nvme2n1
# mount -t f2fs /dev/nvme2n1 /vtmp/mnt/f2fs
# dd if=/dev/nvme0n1 of=/vtmp/mnt/f2fs/1.bin bs=4k count=256
# sync
# rm /vtmp/mnt/f2fs/1.bin
# umount /vtmp/mnt/f2fs
# dump.f2fs /dev/nvme2n1 | grep "checkpoint state"
Info: checkpoint state = 45 :  crc compacted_summary unmount ----
'trimmed' flag is missing

The pending discard count in that segment indeed falls within the range
of (0, 512).

Thanks,
> Thanks,
>
>> number of discard blocks, for the following reasons:
>>
>> locate_dirty_segment() does not mark any active segment as a prefree
>> segment. As a result, segment X is not included in dirty_segmap[PRE], and
>> f2fs_clear_prefree_segments() skips it when handling prefree segments.
>>
>> add_discard_addrs() skips any segment with 0 valid blocks, so segment X is
>> also skipped.
>>
>> Consequently, no `struct discard_cmd` is actually created for segment X.
>> However, the ckpt_valid_map and cur_valid_map of segment X are synced by
>> seg_info_to_raw_sit() during the current checkpoint process. As a result,
>> it cannot find the missing discard bits even in subsequent checkpoints.
>> Consequently, the value of sbi->discard_blks remains non-zero. Thus, when
>> f2fs is umounted, CP_TRIMMED_FLAG will not be set due to the non-zero
>> sbi->discard_blks.
>>
>> Relevant code process:
>>
>> f2fs_write_checkpoint()
>>       f2fs_flush_sit_entries()
>>            list_for_each_entry_safe(ses, tmp, head, set_list) {
>>                for_each_set_bit_from(segno, bitmap, end) {
>>                    ...
>>                    add_discard_addrs(sbi, cpc, false); // skip segment X due to its 0 valid blocks
>>                    ...
>>                    seg_info_to_raw_sit(); // sync ckpt_valid_map with cur_valid_map for segment X
>>                    ...
>>                }
>>            }
>>       f2fs_clear_prefree_segments(); // segment X is not included in dirty_segmap[PRE] and is skipped
>>
>> Since add_discard_addrs() can handle active segments with non-zero valid
>> blocks, it is reasonable to fix this issue by allowing it to also handle
>> active segments with 0 valid blocks.
>>
>> Fixes: b29555505d81 ("f2fs: add key functions for small discards")
>> Signed-off-by: Chunhai Guo <guochunhai@vivo.com>
>> ---
>> v1: https://lore.kernel.org/linux-f2fs-devel/20241203065108.2763436-1-guochunhai@vivo.com/
>> v1->v2:
>>    - Modify the commit message to make it easier to understand.
>>    - Add fixes to the commit.
>> ---
>>    fs/f2fs/segment.c | 4 +++-
>>    1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
>> index 86e547f008f9..13ee73a3c481 100644
>> --- a/fs/f2fs/segment.c
>> +++ b/fs/f2fs/segment.c
>> @@ -2090,7 +2090,9 @@ static bool add_discard_addrs(struct f2fs_sb_info *sbi, struct cp_control *cpc,
>>              return false;
>>
>>      if (!force) {
>> -            if (!f2fs_realtime_discard_enable(sbi) || !se->valid_blocks ||
>> +            if (!f2fs_realtime_discard_enable(sbi) ||
>> +                    (!se->valid_blocks &&
>> +                            !IS_CURSEG(sbi, cpc->trim_start)) ||
>>                      SM_I(sbi)->dcc_info->nr_discards >=
>>                              SM_I(sbi)->dcc_info->max_discards)
>>                      return false;

On 3/13/25 10:25, Chunhai Guo wrote:
> 在 1/20/2025 8:25 PM, Chao Yu 写道:
>> On 1/9/25 20:27, Chunhai Guo wrote:
>>> During a checkpoint, the current active segment X may not be handled
>>> properly. This occurs when segment X has 0 valid blocks and a non-zero
>> How does this happen? Allocator selects a dirty segment w/ SSR? and the
>> left valid data blocks were deleted later before following checkpoint?
>>
>> If so, pending discard count in that segment should be in range of (0, 512)?
> 
> 
> This issue is found with LFS rather than SSR. Here's what happens: some
> data blocks are allocated for a file in the current active segment, and
> then the file is deleted, resulting in all valid data blocks in the
> current active segment being deleted before the following checkpoint.
> This issue is easy to reproduce with the following operations:
> 
> 
> # mkfs.f2fs -f /dev/nvme2n1
> # mount -t f2fs /dev/nvme2n1 /vtmp/mnt/f2fs
> # dd if=/dev/nvme0n1 of=/vtmp/mnt/f2fs/1.bin bs=4k count=256
> # sync
> # rm /vtmp/mnt/f2fs/1.bin
> # umount /vtmp/mnt/f2fs
> # dump.f2fs /dev/nvme2n1 | grep "checkpoint state"
> Info: checkpoint state = 45 :  crc compacted_summary unmount ----
> 'trimmed' flag is missing
> 
> The pending discard count in that segment indeed falls within the range
> of (0, 512).

Please add this testcase into commit message, otherwise it looks
good to me, feel free to add:

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

Thanks,

> 
> Thanks,
>> Thanks,
>>
>>> number of discard blocks, for the following reasons:
>>>
>>> locate_dirty_segment() does not mark any active segment as a prefree
>>> segment. As a result, segment X is not included in dirty_segmap[PRE], and
>>> f2fs_clear_prefree_segments() skips it when handling prefree segments.
>>>
>>> add_discard_addrs() skips any segment with 0 valid blocks, so segment X is
>>> also skipped.
>>>
>>> Consequently, no `struct discard_cmd` is actually created for segment X.
>>> However, the ckpt_valid_map and cur_valid_map of segment X are synced by
>>> seg_info_to_raw_sit() during the current checkpoint process. As a result,
>>> it cannot find the missing discard bits even in subsequent checkpoints.
>>> Consequently, the value of sbi->discard_blks remains non-zero. Thus, when
>>> f2fs is umounted, CP_TRIMMED_FLAG will not be set due to the non-zero
>>> sbi->discard_blks.
>>>
>>> Relevant code process:
>>>
>>> f2fs_write_checkpoint()
>>>       f2fs_flush_sit_entries()
>>>            list_for_each_entry_safe(ses, tmp, head, set_list) {
>>>                for_each_set_bit_from(segno, bitmap, end) {
>>>                    ...
>>>                    add_discard_addrs(sbi, cpc, false); // skip segment X due to its 0 valid blocks
>>>                    ...
>>>                    seg_info_to_raw_sit(); // sync ckpt_valid_map with cur_valid_map for segment X
>>>                    ...
>>>                }
>>>            }
>>>       f2fs_clear_prefree_segments(); // segment X is not included in dirty_segmap[PRE] and is skipped
>>>
>>> Since add_discard_addrs() can handle active segments with non-zero valid
>>> blocks, it is reasonable to fix this issue by allowing it to also handle
>>> active segments with 0 valid blocks.
>>>
>>> Fixes: b29555505d81 ("f2fs: add key functions for small discards")
>>> Signed-off-by: Chunhai Guo <guochunhai@vivo.com>
>>> ---
>>> v1: https://lore.kernel.org/linux-f2fs-devel/20241203065108.2763436-1-guochunhai@vivo.com/
>>> v1->v2:
>>>    - Modify the commit message to make it easier to understand.
>>>    - Add fixes to the commit.
>>> ---
>>>    fs/f2fs/segment.c | 4 +++-
>>>    1 file changed, 3 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
>>> index 86e547f008f9..13ee73a3c481 100644
>>> --- a/fs/f2fs/segment.c
>>> +++ b/fs/f2fs/segment.c
>>> @@ -2090,7 +2090,9 @@ static bool add_discard_addrs(struct f2fs_sb_info *sbi, struct cp_control *cpc,
>>>              return false;
>>>
>>>      if (!force) {
>>> -            if (!f2fs_realtime_discard_enable(sbi) || !se->valid_blocks ||
>>> +            if (!f2fs_realtime_discard_enable(sbi) ||
>>> +                    (!se->valid_blocks &&
>>> +                            !IS_CURSEG(sbi, cpc->trim_start)) ||
>>>                      SM_I(sbi)->dcc_info->nr_discards >=
>>>                              SM_I(sbi)->dcc_info->max_discards)
>>>                      return false;
> 
> 

在 3/17/2025 3:12 PM, Chao Yu 写道:
> On 3/13/25 10:25, Chunhai Guo wrote:
>> 在 1/20/2025 8:25 PM, Chao Yu 写道:
>>> On 1/9/25 20:27, Chunhai Guo wrote:
>>>> During a checkpoint, the current active segment X may not be handled
>>>> properly. This occurs when segment X has 0 valid blocks and a non-zero
>>> How does this happen? Allocator selects a dirty segment w/ SSR? and the
>>> left valid data blocks were deleted later before following checkpoint?
>>>
>>> If so, pending discard count in that segment should be in range of (0, 512)?
>>
>> This issue is found with LFS rather than SSR. Here's what happens: some
>> data blocks are allocated for a file in the current active segment, and
>> then the file is deleted, resulting in all valid data blocks in the
>> current active segment being deleted before the following checkpoint.
>> This issue is easy to reproduce with the following operations:
>>
>>
>> # mkfs.f2fs -f /dev/nvme2n1
>> # mount -t f2fs /dev/nvme2n1 /vtmp/mnt/f2fs
>> # dd if=/dev/nvme0n1 of=/vtmp/mnt/f2fs/1.bin bs=4k count=256
>> # sync
>> # rm /vtmp/mnt/f2fs/1.bin
>> # umount /vtmp/mnt/f2fs
>> # dump.f2fs /dev/nvme2n1 | grep "checkpoint state"
>> Info: checkpoint state = 45 :  crc compacted_summary unmount ----
>> 'trimmed' flag is missing
>>
>> The pending discard count in that segment indeed falls within the range
>> of (0, 512).
> Please add this testcase into commit message, otherwise it looks
> good to me, feel free to add:


OK. I will send the v3 patch later.

Thanks,


>
> Reviewed-by: Chao Yu <chao@kernel.org>
>
> Thanks,
>
>> Thanks,
>>> Thanks,
>>>
>>>> number of discard blocks, for the following reasons:
>>>>
>>>> locate_dirty_segment() does not mark any active segment as a prefree
>>>> segment. As a result, segment X is not included in dirty_segmap[PRE], and
>>>> f2fs_clear_prefree_segments() skips it when handling prefree segments.
>>>>
>>>> add_discard_addrs() skips any segment with 0 valid blocks, so segment X is
>>>> also skipped.
>>>>
>>>> Consequently, no `struct discard_cmd` is actually created for segment X.
>>>> However, the ckpt_valid_map and cur_valid_map of segment X are synced by
>>>> seg_info_to_raw_sit() during the current checkpoint process. As a result,
>>>> it cannot find the missing discard bits even in subsequent checkpoints.
>>>> Consequently, the value of sbi->discard_blks remains non-zero. Thus, when
>>>> f2fs is umounted, CP_TRIMMED_FLAG will not be set due to the non-zero
>>>> sbi->discard_blks.
>>>>
>>>> Relevant code process:
>>>>
>>>> f2fs_write_checkpoint()
>>>>        f2fs_flush_sit_entries()
>>>>             list_for_each_entry_safe(ses, tmp, head, set_list) {
>>>>                 for_each_set_bit_from(segno, bitmap, end) {
>>>>                     ...
>>>>                     add_discard_addrs(sbi, cpc, false); // skip segment X due to its 0 valid blocks
>>>>                     ...
>>>>                     seg_info_to_raw_sit(); // sync ckpt_valid_map with cur_valid_map for segment X
>>>>                     ...
>>>>                 }
>>>>             }
>>>>        f2fs_clear_prefree_segments(); // segment X is not included in dirty_segmap[PRE] and is skipped
>>>>
>>>> Since add_discard_addrs() can handle active segments with non-zero valid
>>>> blocks, it is reasonable to fix this issue by allowing it to also handle
>>>> active segments with 0 valid blocks.
>>>>
>>>> Fixes: b29555505d81 ("f2fs: add key functions for small discards")
>>>> Signed-off-by: Chunhai Guo <guochunhai@vivo.com>
>>>> ---
>>>> v1: https://lore.kernel.org/linux-f2fs-devel/20241203065108.2763436-1-guochunhai@vivo.com/
>>>> v1->v2:
>>>>     - Modify the commit message to make it easier to understand.
>>>>     - Add fixes to the commit.
>>>> ---
>>>>     fs/f2fs/segment.c | 4 +++-
>>>>     1 file changed, 3 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
>>>> index 86e547f008f9..13ee73a3c481 100644
>>>> --- a/fs/f2fs/segment.c
>>>> +++ b/fs/f2fs/segment.c
>>>> @@ -2090,7 +2090,9 @@ static bool add_discard_addrs(struct f2fs_sb_info *sbi, struct cp_control *cpc,
>>>>               return false;
>>>>
>>>>       if (!force) {
>>>> -            if (!f2fs_realtime_discard_enable(sbi) || !se->valid_blocks ||
>>>> +            if (!f2fs_realtime_discard_enable(sbi) ||
>>>> +                    (!se->valid_blocks &&
>>>> +                            !IS_CURSEG(sbi, cpc->trim_start)) ||
>>>>                       SM_I(sbi)->dcc_info->nr_discards >=
>>>>                               SM_I(sbi)->dcc_info->max_discards)
>>>>                       return false;
>>

On 3/13/25 10:25, Chunhai Guo wrote:
> 在 1/20/2025 8:25 PM, Chao Yu 写道:
>> On 1/9/25 20:27, Chunhai Guo wrote:
>>> During a checkpoint, the current active segment X may not be handled
>>> properly. This occurs when segment X has 0 valid blocks and a non-zero
>> How does this happen? Allocator selects a dirty segment w/ SSR? and the
>> left valid data blocks were deleted later before following checkpoint?
>>
>> If so, pending discard count in that segment should be in range of (0, 512)?
> 
> 
> This issue is found with LFS rather than SSR. Here's what happens: some
> data blocks are allocated for a file in the current active segment, and
> then the file is deleted, resulting in all valid data blocks in the
> current active segment being deleted before the following checkpoint.
> This issue is easy to reproduce with the following operations:
> 
> 
> # mkfs.f2fs -f /dev/nvme2n1
> # mount -t f2fs /dev/nvme2n1 /vtmp/mnt/f2fs
> # dd if=/dev/nvme0n1 of=/vtmp/mnt/f2fs/1.bin bs=4k count=256
> # sync
> # rm /vtmp/mnt/f2fs/1.bin
> # umount /vtmp/mnt/f2fs
> # dump.f2fs /dev/nvme2n1 | grep "checkpoint state"
> Info: checkpoint state = 45 :  crc compacted_summary unmount ----
> 'trimmed' flag is missing

Chunhai,

Thank you for providing testcase, are you interest in upstream this
as a f2fs testcase to xfstests?

Thanks,

> 
> The pending discard count in that segment indeed falls within the range
> of (0, 512).
> 
> Thanks,
>> Thanks,
>>
>>> number of discard blocks, for the following reasons:
>>>
>>> locate_dirty_segment() does not mark any active segment as a prefree
>>> segment. As a result, segment X is not included in dirty_segmap[PRE], and
>>> f2fs_clear_prefree_segments() skips it when handling prefree segments.
>>>
>>> add_discard_addrs() skips any segment with 0 valid blocks, so segment X is
>>> also skipped.
>>>
>>> Consequently, no `struct discard_cmd` is actually created for segment X.
>>> However, the ckpt_valid_map and cur_valid_map of segment X are synced by
>>> seg_info_to_raw_sit() during the current checkpoint process. As a result,
>>> it cannot find the missing discard bits even in subsequent checkpoints.
>>> Consequently, the value of sbi->discard_blks remains non-zero. Thus, when
>>> f2fs is umounted, CP_TRIMMED_FLAG will not be set due to the non-zero
>>> sbi->discard_blks.
>>>
>>> Relevant code process:
>>>
>>> f2fs_write_checkpoint()
>>>       f2fs_flush_sit_entries()
>>>            list_for_each_entry_safe(ses, tmp, head, set_list) {
>>>                for_each_set_bit_from(segno, bitmap, end) {
>>>                    ...
>>>                    add_discard_addrs(sbi, cpc, false); // skip segment X due to its 0 valid blocks
>>>                    ...
>>>                    seg_info_to_raw_sit(); // sync ckpt_valid_map with cur_valid_map for segment X
>>>                    ...
>>>                }
>>>            }
>>>       f2fs_clear_prefree_segments(); // segment X is not included in dirty_segmap[PRE] and is skipped
>>>
>>> Since add_discard_addrs() can handle active segments with non-zero valid
>>> blocks, it is reasonable to fix this issue by allowing it to also handle
>>> active segments with 0 valid blocks.
>>>
>>> Fixes: b29555505d81 ("f2fs: add key functions for small discards")
>>> Signed-off-by: Chunhai Guo <guochunhai@vivo.com>
>>> ---
>>> v1: https://lore.kernel.org/linux-f2fs-devel/20241203065108.2763436-1-guochunhai@vivo.com/
>>> v1->v2:
>>>    - Modify the commit message to make it easier to understand.
>>>    - Add fixes to the commit.
>>> ---
>>>    fs/f2fs/segment.c | 4 +++-
>>>    1 file changed, 3 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
>>> index 86e547f008f9..13ee73a3c481 100644
>>> --- a/fs/f2fs/segment.c
>>> +++ b/fs/f2fs/segment.c
>>> @@ -2090,7 +2090,9 @@ static bool add_discard_addrs(struct f2fs_sb_info *sbi, struct cp_control *cpc,
>>>              return false;
>>>
>>>      if (!force) {
>>> -            if (!f2fs_realtime_discard_enable(sbi) || !se->valid_blocks ||
>>> +            if (!f2fs_realtime_discard_enable(sbi) ||
>>> +                    (!se->valid_blocks &&
>>> +                            !IS_CURSEG(sbi, cpc->trim_start)) ||
>>>                      SM_I(sbi)->dcc_info->nr_discards >=
>>>                              SM_I(sbi)->dcc_info->max_discards)
>>>                      return false;
> 
> 

在 3/17/2025 3:11 PM, Chao Yu 写道:
> On 3/13/25 10:25, Chunhai Guo wrote:
>> 在 1/20/2025 8:25 PM, Chao Yu 写道:
>>> On 1/9/25 20:27, Chunhai Guo wrote:
>>>> During a checkpoint, the current active segment X may not be handled
>>>> properly. This occurs when segment X has 0 valid blocks and a non-zero
>>> How does this happen? Allocator selects a dirty segment w/ SSR? and the
>>> left valid data blocks were deleted later before following checkpoint?
>>>
>>> If so, pending discard count in that segment should be in range of (0, 512)?
>>
>> This issue is found with LFS rather than SSR. Here's what happens: some
>> data blocks are allocated for a file in the current active segment, and
>> then the file is deleted, resulting in all valid data blocks in the
>> current active segment being deleted before the following checkpoint.
>> This issue is easy to reproduce with the following operations:
>>
>>
>> # mkfs.f2fs -f /dev/nvme2n1
>> # mount -t f2fs /dev/nvme2n1 /vtmp/mnt/f2fs
>> # dd if=/dev/nvme0n1 of=/vtmp/mnt/f2fs/1.bin bs=4k count=256
>> # sync
>> # rm /vtmp/mnt/f2fs/1.bin
>> # umount /vtmp/mnt/f2fs
>> # dump.f2fs /dev/nvme2n1 | grep "checkpoint state"
>> Info: checkpoint state = 45 :  crc compacted_summary unmount ----
>> 'trimmed' flag is missing
> Chunhai,
>
> Thank you for providing testcase, are you interest in upstream this
> as a f2fs testcase to xfstests?


Yes, but I may not be able to start this work for a few days since I
will be busy in the next few days.

Thanks,


>
> Thanks,
>
>> The pending discard count in that segment indeed falls within the range
>> of (0, 512).
>>
>> Thanks,
>>> Thanks,
>>>
>>>> number of discard blocks, for the following reasons:
>>>>
>>>> locate_dirty_segment() does not mark any active segment as a prefree
>>>> segment. As a result, segment X is not included in dirty_segmap[PRE], and
>>>> f2fs_clear_prefree_segments() skips it when handling prefree segments.
>>>>
>>>> add_discard_addrs() skips any segment with 0 valid blocks, so segment X is
>>>> also skipped.
>>>>
>>>> Consequently, no `struct discard_cmd` is actually created for segment X.
>>>> However, the ckpt_valid_map and cur_valid_map of segment X are synced by
>>>> seg_info_to_raw_sit() during the current checkpoint process. As a result,
>>>> it cannot find the missing discard bits even in subsequent checkpoints.
>>>> Consequently, the value of sbi->discard_blks remains non-zero. Thus, when
>>>> f2fs is umounted, CP_TRIMMED_FLAG will not be set due to the non-zero
>>>> sbi->discard_blks.
>>>>
>>>> Relevant code process:
>>>>
>>>> f2fs_write_checkpoint()
>>>>        f2fs_flush_sit_entries()
>>>>             list_for_each_entry_safe(ses, tmp, head, set_list) {
>>>>                 for_each_set_bit_from(segno, bitmap, end) {
>>>>                     ...
>>>>                     add_discard_addrs(sbi, cpc, false); // skip segment X due to its 0 valid blocks
>>>>                     ...
>>>>                     seg_info_to_raw_sit(); // sync ckpt_valid_map with cur_valid_map for segment X
>>>>                     ...
>>>>                 }
>>>>             }
>>>>        f2fs_clear_prefree_segments(); // segment X is not included in dirty_segmap[PRE] and is skipped
>>>>
>>>> Since add_discard_addrs() can handle active segments with non-zero valid
>>>> blocks, it is reasonable to fix this issue by allowing it to also handle
>>>> active segments with 0 valid blocks.
>>>>
>>>> Fixes: b29555505d81 ("f2fs: add key functions for small discards")
>>>> Signed-off-by: Chunhai Guo <guochunhai@vivo.com>
>>>> ---
>>>> v1: https://lore.kernel.org/linux-f2fs-devel/20241203065108.2763436-1-guochunhai@vivo.com/
>>>> v1->v2:
>>>>     - Modify the commit message to make it easier to understand.
>>>>     - Add fixes to the commit.
>>>> ---
>>>>     fs/f2fs/segment.c | 4 +++-
>>>>     1 file changed, 3 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
>>>> index 86e547f008f9..13ee73a3c481 100644
>>>> --- a/fs/f2fs/segment.c
>>>> +++ b/fs/f2fs/segment.c
>>>> @@ -2090,7 +2090,9 @@ static bool add_discard_addrs(struct f2fs_sb_info *sbi, struct cp_control *cpc,
>>>>               return false;
>>>>
>>>>       if (!force) {
>>>> -            if (!f2fs_realtime_discard_enable(sbi) || !se->valid_blocks ||
>>>> +            if (!f2fs_realtime_discard_enable(sbi) ||
>>>> +                    (!se->valid_blocks &&
>>>> +                            !IS_CURSEG(sbi, cpc->trim_start)) ||
>>>>                       SM_I(sbi)->dcc_info->nr_discards >=
>>>>                               SM_I(sbi)->dcc_info->max_discards)
>>>>                       return false;
>>