linux-kernel - Re: [PATCH] f2fs: don't break allocation when crossing contiguous sections

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] [day] [month] [year] [list]

Message-ID: <ae41b064-f45b-4410-9854-f4c07ea1d0bf@kernel.org>
Date: Wed, 30 Jul 2025 15:39:37 +0800
From: Chao Yu <chao@...nel.org>
To: daejun7.park@...sung.com, "jaegeuk@...nel.org" <jaegeuk@...nel.org>
Cc: chao@...nel.org,
 "linux-f2fs-devel@...ts.sourceforge.net"
 <linux-f2fs-devel@...ts.sourceforge.net>,
 "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] f2fs: don't break allocation when crossing contiguous
 sections

Hi Daejun

Thanks for the comments.

On 7/30/25 09:53, Daejun Park wrote:
> Hi Chao Yu,
> 
>> +        lfs_dio_write = (flag == F2FS_GET_BLOCK_DIO && f2fs_lfs_mode(sbi) &&
>> +                                map->m_may_create);
>> +
> 
> Consider moving the lfs_dio_write assignment to just after the if (!map->m_may_create …) check
> so it isn’t evaluated when creation isn’t allowed.

Correct.

It's too late to clean up the code as it is near to the merge window.

> 
>> @@ -4171,6 +4172,10 @@ static int f2fs_iomap_begin(struct inode *inode, loff_t offset, loff_t length,
>>          map.m_next_pgofs = &next_pgofs;
>>          map.m_seg_type = f2fs_rw_hint_to_seg_type(F2FS_I_SB(inode),
>>                                                  inode->i_write_hint);
>> +        if (flags & IOMAP_WRITE && iomap->private) {
> 
> Since iomap->private is only set on the LFS DIO path, you can drop the flags & IOMAP_WRITE and
> test the pointer directly.

Well, that's right, but I still want to check this condition explicitly
to avoid any missing case or further misunderstanding on the code. :)

Thanks,

> 
>> +                map.m_last_pblk = (unsigned long)iomap->private;
>> +                iomap->private = NULL;
>> +        }
>>
>>          /*
>>           * If the blocks being overwritten are already allocated,
>> @@ -4209,6 +4214,9 @@ static int f2fs_iomap_begin(struct inode *inode, loff_t offset, loff_t length,
>>                  iomap->flags = IOMAP_F_MERGED;
>>                  iomap->bdev = map.m_bdev;
>>                  iomap->addr = F2FS_BLK_TO_BYTES(map.m_pblk);
>> +
>> +                if (flags & IOMAP_WRITE && map.m_last_pblk)
>> +                        iomap->private = (void *)map.m_last_pblk;
> 
> Likewise, checking only map.m_last_pblk is sufficient for restoring.
> 
> Reviewed-by: Daejun Park <daejun7.park@...sung.com>
>