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.p...@samsung.com>
>
_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
