On Mon, Jun 16, 2025 at 5:50 AM Chao Yu <c...@kernel.org> wrote:
>
> On 6/15/25 22:42, Daniel Lee wrote:
> > Apply the `ioprio_hint` to set `F2FS_IOPRIO_WRITE` priority
> > on files identified as "hot" at creation and on files that are
> > pinned via ioctl.
> >
> > Signed-off-by: Daniel Lee <chul...@google.com>
> > ---
> > fs/f2fs/f2fs.h | 19 +++++++++++++++++++
> > fs/f2fs/file.c | 3 +++
> > fs/f2fs/namei.c | 11 +++++++----
> > 3 files changed, 29 insertions(+), 4 deletions(-)
> >
> > diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> > index 3e02687c1b58..0c4f52892ff7 100644
> > --- a/fs/f2fs/f2fs.h
> > +++ b/fs/f2fs/f2fs.h
> > @@ -3440,6 +3440,25 @@ static inline void set_file(struct inode *inode, int
> > type)
> > f2fs_mark_inode_dirty_sync(inode, true);
> > }
> >
> > +static inline int get_ioprio(struct inode *inode)
> > +{
> > + return F2FS_I(inode)->ioprio_hint;
> > +}
> > +
> > +static inline void set_ioprio(struct inode *inode, int level)
> > +{
> > + if (get_ioprio(inode) == level)
> > + return;
> > + F2FS_I(inode)->ioprio_hint = level;
> > +}
> > +
> > +static inline void clear_ioprio(struct inode *inode)
> > +{
> > + if (get_ioprio(inode) == 0)
> > + return;
> > + F2FS_I(inode)->ioprio_hint = 0;
> > +}
> > +
> > static inline void clear_file(struct inode *inode, int type)
> > {
> > if (!is_file(inode, type))
> > diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
> > index 3eb40d7bf602..a18fb7f3d019 100644
> > --- a/fs/f2fs/file.c
> > +++ b/fs/f2fs/file.c
> > @@ -3496,6 +3496,7 @@ static int f2fs_ioc_set_pin_file(struct file *filp,
> > unsigned long arg)
> >
> > if (!pin) {
> > clear_inode_flag(inode, FI_PIN_FILE);
> > + clear_ioprio(inode);
>
> I guess there are more places clearing FI_PIN_FILE? we need to cover
> them all?
Yes, you're right. FI_PIN_FILE is toggled in several places. However,
this change is intended to set the HOT and IOPRIO on the files that
users explicitly pin through IOCTL. The other kernel internal
mechanisms (e.g., swap or gc_failures) remain the same. Are there any
potential issues that I should consider?
>
> > f2fs_i_gc_failures_write(inode, 0);
> > goto done;
> > } else if (f2fs_is_pinned_file(inode)) {
> > @@ -3529,6 +3530,8 @@ static int f2fs_ioc_set_pin_file(struct file *filp,
> > unsigned long arg)
> > }
> >
> > set_inode_flag(inode, FI_PIN_FILE);
> > + file_set_hot(inode);
>
> Unnecessary file_set_hot() invoking? Or am I missing anything?
>
> Thanks,
Setting HOT and IOPRIO by default is also intentional. We set both
flags by default because the main use case for pinned files involves
frequently updated or short-lived data that needs fast write speeds.
>
> > + set_ioprio(inode, F2FS_IOPRIO_WRITE);
> > ret = F2FS_I(inode)->i_gc_failures;
> > done:
> > f2fs_update_time(sbi, REQ_TIME);
> > diff --git a/fs/f2fs/namei.c b/fs/f2fs/namei.c
> > index 07e333ee21b7..0f96a0b86c40 100644
> > --- a/fs/f2fs/namei.c
> > +++ b/fs/f2fs/namei.c
> > @@ -191,9 +191,10 @@ static void set_compress_new_inode(struct f2fs_sb_info
> > *sbi, struct inode *dir,
> > }
> >
> > /*
> > - * Set file's temperature for hot/cold data separation
> > + * Set file's temperature (for hot/cold data separation) and
> > + * I/O priority, based on filename extension
> > */
> > -static void set_file_temperature(struct f2fs_sb_info *sbi, struct inode
> > *inode,
> > +static void set_file_temp_prio(struct f2fs_sb_info *sbi, struct inode
> > *inode,
> > const unsigned char *name)
> > {
> > __u8 (*extlist)[F2FS_EXTENSION_LEN] = sbi->raw_super->extension_list;
> > @@ -212,8 +213,10 @@ static void set_file_temperature(struct f2fs_sb_info
> > *sbi, struct inode *inode,
> >
> > if (i < cold_count)
> > file_set_cold(inode);
> > - else
> > + else {
> > file_set_hot(inode);
> > + set_ioprio(inode, F2FS_IOPRIO_WRITE);
> > + }
> > }
> >
> > static struct inode *f2fs_new_inode(struct mnt_idmap *idmap,
> > @@ -317,7 +320,7 @@ static struct inode *f2fs_new_inode(struct mnt_idmap
> > *idmap,
> > set_inode_flag(inode, FI_INLINE_DATA);
> >
> > if (name && !test_opt(sbi, DISABLE_EXT_IDENTIFY))
> > - set_file_temperature(sbi, inode, name);
> > + set_file_temp_prio(sbi, inode, name);
> >
> > stat_inc_inline_xattr(inode);
> > stat_inc_inline_inode(inode);
>
_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
