Hi Chao Yu,
>
>--------- Original Message ---------
>Sender : Chao Yu <c...@kernel.org>
>Date : 2024-08-16 10:56 (GMT+9)
>Title : Re: (2) [PATCH] f2fs: avoid unused block when dio write in LFS mode
>
>On 2024/8/16 8:17, Daejun Park wrote:
>> Hi Chao Yu,
>>>
>>> --------- Original Message ---------
>>> Sender : Chao Yu <c...@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.p...@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?
I will add checking map->m_may_create.
>
>>>> + 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;
I will use the suggested checking code.
Thanks,
Daejun
>
>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
