Re: [f2fs-dev] [PATCH] f2fs: add a fast path in finish_preallocate_blocks()

[Chao Yu <chao@kernel.org>]

On 3/25/25 14:31, Zhiguo Niu wrote:
> Chao Yu via Linux-f2fs-devel <linux-f2fs-devel@lists.sourceforge.net>
> 于2025年3月24日周一 19:36写道：
>>
>> This patch uses i_sem to protect access/update on f2fs_inode_info.flag
>> in finish_preallocate_blocks(), it avoids grabbing inode_lock() in
>> each open().
>>
>> Signed-off-by: Chao Yu <chao@kernel.org>
>> ---
>>  fs/f2fs/file.c | 40 +++++++++++++++++++++++-----------------
>>  1 file changed, 23 insertions(+), 17 deletions(-)
>>
>> diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
>> index abbcbb5865a3..bb6ba3269de0 100644
>> --- a/fs/f2fs/file.c
>> +++ b/fs/f2fs/file.c
>> @@ -554,19 +554,24 @@ static int f2fs_file_mmap(struct file *file, struct vm_area_struct *vma)
>>
>>  static int finish_preallocate_blocks(struct inode *inode)
>>  {
>> -       int ret;
>> +       int ret = 0;
>> +       bool opened;
>>
>> -       inode_lock(inode);
>> -       if (is_inode_flag_set(inode, FI_OPENED_FILE)) {
>> -               inode_unlock(inode);
>> +       f2fs_down_read(&F2FS_I(inode)->i_sem);
>> +       opened = is_inode_flag_set(inode, FI_OPENED_FILE);
>> +       f2fs_up_read(&F2FS_I(inode)->i_sem);
>> +       if (opened)
>>                 return 0;
>> -       }
>>
>> -       if (!file_should_truncate(inode)) {
>> -               set_inode_flag(inode, FI_OPENED_FILE);
>> -               inode_unlock(inode);
>> -               return 0;
>> -       }
>> +       inode_lock(inode);
>> +       f2fs_down_read(&F2FS_I(inode)->i_sem);
>> +       opened = is_inode_flag_set(inode, FI_OPENED_FILE);
>> +       f2fs_up_read(&F2FS_I(inode)->i_sem);
> Hi Chao,
> F2FS_I(inode)->i_sem is not needed for this judgment FI_OPENED_FILE area?
> because inode_lock has been hold and this is a write lock, if process
> A get inode_lock,
> other proccesses will be blocked inode_lock until inode_unlock with
> FI_OPENED_FILE  been set?
> how do you think?

Zhiguo,

Agreed, let me update it. Thank you.

Thanks,

> thanks！
>> +       if (opened)
>> +               goto out_unlock;
>> +
>> +       if (!file_should_truncate(inode))
>> +               goto out_update;
>>
>>         f2fs_down_write(&F2FS_I(inode)->i_gc_rwsem[WRITE]);
>>         filemap_invalidate_lock(inode->i_mapping);
>> @@ -576,16 +581,17 @@ static int finish_preallocate_blocks(struct inode *inode)
>>
>>         filemap_invalidate_unlock(inode->i_mapping);
>>         f2fs_up_write(&F2FS_I(inode)->i_gc_rwsem[WRITE]);
>> -
>> -       if (!ret)
>> -               set_inode_flag(inode, FI_OPENED_FILE);
>> -
>> -       inode_unlock(inode);
>>         if (ret)
>> -               return ret;
>> +               goto out_unlock;
>>
>>         file_dont_truncate(inode);
>> -       return 0;
>> +out_update:
>> +       f2fs_down_write(&F2FS_I(inode)->i_sem);
>> +       set_inode_flag(inode, FI_OPENED_FILE);
>> +       f2fs_up_write(&F2FS_I(inode)->i_sem);
>> +out_unlock:
>> +       inode_unlock(inode);
>> +       return ret;
>>  }
>>
>>  static int f2fs_file_open(struct inode *inode, struct file *filp)
>> --
>> 2.48.1
>>
>>
>>
>> _______________________________________________
>> Linux-f2fs-devel mailing list
>> Linux-f2fs-devel@lists.sourceforge.net
>> https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel