Re: [f2fs-dev] (2) [PATCH] f2fs: avoid unused block when dio write in LFS mode

[Chao Yu <chao@kernel.org>]

On 2024/8/16 8:17, Daejun Park wrote:
> Hi Chao Yu,
>>   
>> --------- Original Message ---------
>> Sender : Chao Yu <chao@kernel.org>
>> Date : 2024-08-14 18:50 (GMT+9)
>> Title : Re: [PATCH] f2fs: avoid unused block when dio write in LFS mode
>>   
>> On 2024/8/1 15:47, Daejun Park wrote:
>>> This patch addresses the problem that when using LFS mode, unused blocks
>>> may occur in f2fs_map_blocks() during block allocation for dio writes.
>>>
>>> If a new section is allocated during block allocation, it will not be
>>> included in the map struct by map_is_mergeable() if the LBA of the
>>
>> I didn't get it, why below condition in map_is_mergeable() can not catch this
>> case? Can you please explain more?
>>
>>         if (map->m_pblk != NEW_ADDR && blkaddr == (map->m_pblk + ofs))
>>                 return true;
>>
> 
> Thank you for your review.
> map_is_mergeable() returns true when the last block in the section is merged.
> The problem is the next block allocation, which happens consecutively.
> Since it will be allocated a new section, its block will be the first
> block in the section.
> If the newly allocated section is not contiguous with the previous section,
> map_is_mergeable() will return false.
> So the block is allocated but unused.
> However, it is not freed, so the block is unusable.
> If my explanation was not clear enough, please feel free to ask questions.

It's clear to me now, thanks for your explanation.

> 
> Thanks
> Daejun
> 
>> Thanks,
>>
>>> allocated block is not contiguous. However, the block already allocated
>>> in this process will remain unused due to the LFS mode.
>>>
>>> This patch avoids the possibility of unused blocks by escaping
>>> f2fs_map_blocks() when allocating the last block in a section.
>>>
>>> Signed-off-by: Daejun Park <daejun7.park@samsung.com>
>>> ---
>>>    fs/f2fs/data.c 13 +++++++++++++
>>>    1 file changed, 13 insertions(+)
>>>
>>> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
>>> index b6dcb3bcaef7..b27a3f448f32 100644
>>> --- a/fs/f2fs/data.c
>>> +++ b/fs/f2fs/data.c
>>> @@ -1711,6 +1711,19 @@ int f2fs_map_blocks(struct inode *inode, struct f2fs_map_blocks *map, int flag)
>>>                     dn.ofs_in_node = end_offset;
>>>             }
>>>   
>>> +        if (flag == F2FS_GET_BLOCK_DIO && f2fs_lfs_mode(sbi)) {

The condition should check map->m_may_create as well, otherwise f2fs_map_blocks()
from read path will break here?

>>> +                int segno = GET_SEGNO(sbi, blkaddr);
>>> +                bool last_seg, last_blk;
>>> +
>>> +                last_seg = !((segno + 1) % SEGS_PER_SEC(sbi));

Should consider the case that segno #(SEGS_PER_SEC - 1) is not valid?
e.g. SEGS_PER_SEC is 4, CAP_SEGS_PER_SEC is 2?

>>> +                last_blk = (f2fs_usable_blks_in_seg(sbi, segno) - 1) ==
>>> +                                GET_BLKOFF_FROM_SEG0(sbi, blkaddr);

if (GET_SEGOFF_FROM_SEG0() % BLKS_PER_SEC() == CAP_BLKS_PER_SEC() - 1)
	goto sync_out;

Thanks,

>>> +
>>> +                /* LBA of the next block to be allocated may not be contiguous. */
>>> +                if (last_seg && last_blk)
>>> +                        goto sync_out;
>>> +        }
>>> +
>>>             if (pgofs >= end)
>>>                     goto sync_out;
>>>             else if (dn.ofs_in_node < end_offset)



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