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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <5b892d6a-620d-47f4-b870-214dcf4bc480@kernel.org>
Date: Fri, 16 Aug 2024 09:56:25 +0800
From: Chao Yu <chao@...nel.org>
To: daejun7.park@...sung.com, "jaegeuk@...nel.org" <jaegeuk@...nel.org>,
 "linux-f2fs-devel@...ts.sourceforge.net"
 <linux-f2fs-devel@...ts.sourceforge.net>,
 "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Cc: Seokhwan Kim <sukka.kim@...sung.com>,
 Dongjin Kim <dongjin_.kim@...sung.com>,
 Yonggil Song <yonggil.song@...sung.com>,
 Jaeyoon Choi <j_yoon.choi@...sung.com>, Jeuk Kim <jeuk20.kim@...sung.com>,
 Nayeon Kim <nayeoni.kim@...sung.com>, Siwoo Jung <siu.jung@...sung.com>
Subject: 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 <chao@...nel.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@...sung.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)