linux-kernel - Re: [f2fs-dev] [PATCH] f2fs: add a fast path in finish_preallocate

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: <CAHJ8P3L0vJKdLQMSGGQJuDD3_++8BhV1b4cvL4PCFu0WY+EWwg@mail.gmail.com>
Date: Tue, 25 Mar 2025 14:31:43 +0800
From: Zhiguo Niu <niuzhiguo84@...il.com>
To: Chao Yu <chao@...nel.org>
Cc: jaegeuk@...nel.org, linux-kernel@...r.kernel.org, 
	linux-f2fs-devel@...ts.sourceforge.net
Subject: Re: [f2fs-dev] [PATCH] f2fs: add a fast path in finish_preallocate_blocks()

Chao Yu via Linux-f2fs-devel <linux-f2fs-devel@...ts.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@...nel.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?
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@...ts.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel