Re: [f2fs-dev] [PATCH 2/2] f2fs: add a sysfs entry to request donate file-backed pages

[Jaegeuk Kim <jaegeuk@kernel.org>]

On 01/16, Chao Yu wrote:
> On 1/16/25 06:16, Jaegeuk Kim via Linux-f2fs-devel wrote:
> > 1. ioctl(fd1, F2FS_IOC_DONATE_RANGE, {0,3});
> > 2. ioctl(fd2, F2FS_IOC_DONATE_RANGE, {1,2});
> > 3. ioctl(fd3, F2FS_IOC_DONATE_RANGE, {3,1});
> > 4. echo 3 > /sys/fs/f2fs/blk/donate_caches
> > 
> > will reclaim 3 page cache ranges, registered by #1, #2, and #3.
> > 
> > Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
> > ---
> >   Documentation/ABI/testing/sysfs-fs-f2fs |  7 +++++++
> >   fs/f2fs/f2fs.h                          |  2 ++
> >   fs/f2fs/shrinker.c                      | 27 +++++++++++++++++++++++++
> >   fs/f2fs/sysfs.c                         |  8 ++++++++
> >   4 files changed, 44 insertions(+)
> > 
> > diff --git a/Documentation/ABI/testing/sysfs-fs-f2fs b/Documentation/ABI/testing/sysfs-fs-f2fs
> > index 3e1630c70d8a..6f9d8b8889fd 100644
> > --- a/Documentation/ABI/testing/sysfs-fs-f2fs
> > +++ b/Documentation/ABI/testing/sysfs-fs-f2fs
> > @@ -828,3 +828,10 @@ Date:		November 2024
> >   Contact:	"Chao Yu" <chao@kernel.org>
> >   Description:	It controls max read extent count for per-inode, the value of threshold
> >   		is 10240 by default.
> > +
> > +What:		/sys/fs/f2fs/<disk>/donate_caches
> > +Date:		December 2024
> > +Contact:	"Jaegeuk Kim" <jaegeuk@kernel.org>
> > +Description:	It reclaims the certian file-backed pages registered by
> > +		ioctl(F2FS_IOC_DONATE_RANGE).
> > +		For example, writing N tries to drop N address spaces in LRU.
> > diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> > index 951fbc3f94c7..399ddd10a94f 100644
> > --- a/fs/f2fs/f2fs.h
> > +++ b/fs/f2fs/f2fs.h
> > @@ -1637,6 +1637,7 @@ struct f2fs_sb_info {
> >   	/* control donate caches */
> >   	unsigned int donate_files;
> > +	unsigned int donate_caches;
> >   	/* basic filesystem units */
> >   	unsigned int log_sectors_per_block;	/* log2 sectors per block */
> > @@ -4259,6 +4260,7 @@ unsigned long f2fs_shrink_count(struct shrinker *shrink,
> >   			struct shrink_control *sc);
> >   unsigned long f2fs_shrink_scan(struct shrinker *shrink,
> >   			struct shrink_control *sc);
> > +void f2fs_donate_caches(struct f2fs_sb_info *sbi);
> >   void f2fs_join_shrinker(struct f2fs_sb_info *sbi);
> >   void f2fs_leave_shrinker(struct f2fs_sb_info *sbi);
> > diff --git a/fs/f2fs/shrinker.c b/fs/f2fs/shrinker.c
> > index 83d6fb97dcae..22f62813910b 100644
> > --- a/fs/f2fs/shrinker.c
> > +++ b/fs/f2fs/shrinker.c
> > @@ -130,6 +130,33 @@ unsigned long f2fs_shrink_scan(struct shrinker *shrink,
> >   	return freed;
> >   }
> > +void f2fs_donate_caches(struct f2fs_sb_info *sbi)
> > +{
> > +	struct inode *inode;
> > +	struct f2fs_inode_info *fi;
> > +	int nfiles = sbi->donate_caches;
> > +
> > +	while (nfiles--) {
> > +		spin_lock(&sbi->inode_lock[DONATE_INODE]);
> > +		if (list_empty(&sbi->inode_list[DONATE_INODE])) {
> > +			spin_unlock(&sbi->inode_lock[DONATE_INODE]);
> > +			break;
> > +		}
> > +		fi = list_first_entry(&sbi->inode_list[DONATE_INODE],
> > +					struct f2fs_inode_info, gdonate_list);
> > +		list_move_tail(&fi->gdonate_list, &sbi->inode_list[DONATE_INODE]);
> > +		inode = igrab(&fi->vfs_inode);
> > +		spin_unlock(&sbi->inode_lock[DONATE_INODE]);
> > +
> > +		if (!inode)
> > +			continue;
> > +
> > +		invalidate_inode_pages2_range(inode->i_mapping,
> > +					fi->donate_start, fi->donate_end);
> 
> fi->donate_start and fi->donate_end can be updated in ioctl concurrently and
> become inconsistent here, is it fine?

I think that needs to be managed by user, as there'll be no frequent updates to
the donate ranges.

> 
> Thanks,
> 
> > +		iput(inode);
> > +	}
> > +}
> > +
> >   void f2fs_join_shrinker(struct f2fs_sb_info *sbi)
> >   {
> >   	spin_lock(&f2fs_list_lock);
> > diff --git a/fs/f2fs/sysfs.c b/fs/f2fs/sysfs.c
> > index 6b99dc49f776..f81190fabdd3 100644
> > --- a/fs/f2fs/sysfs.c
> > +++ b/fs/f2fs/sysfs.c
> > @@ -811,6 +811,12 @@ static ssize_t __sbi_store(struct f2fs_attr *a,
> >   		return count;
> >   	}
> > +	if (!strcmp(a->attr.name, "donate_caches")) {
> > +		sbi->donate_caches = min(t, sbi->donate_files);
> > +		f2fs_donate_caches(sbi);
> > +		return count;
> > +	}
> > +
> >   	*ui = (unsigned int)t;
> >   	return count;
> > @@ -1030,6 +1036,7 @@ F2FS_SBI_GENERAL_RW_ATTR(max_victim_search);
> >   F2FS_SBI_GENERAL_RW_ATTR(migration_granularity);
> >   F2FS_SBI_GENERAL_RW_ATTR(migration_window_granularity);
> >   F2FS_SBI_GENERAL_RW_ATTR(dir_level);
> > +F2FS_SBI_GENERAL_RW_ATTR(donate_caches);
> >   #ifdef CONFIG_F2FS_IOSTAT
> >   F2FS_SBI_GENERAL_RW_ATTR(iostat_enable);
> >   F2FS_SBI_GENERAL_RW_ATTR(iostat_period_ms);
> > @@ -1178,6 +1185,7 @@ static struct attribute *f2fs_attrs[] = {
> >   	ATTR_LIST(migration_granularity),
> >   	ATTR_LIST(migration_window_granularity),
> >   	ATTR_LIST(dir_level),
> > +	ATTR_LIST(donate_caches),
> >   	ATTR_LIST(ram_thresh),
> >   	ATTR_LIST(ra_nid_pages),
> >   	ATTR_LIST(dirty_nats_ratio),