All of lore.kernel.org
 help / color / mirror / Atom feed
From: Chao Yu <chao@kernel.org>
To: Daeho Jeong <daeho43@gmail.com>,
	linux-kernel@vger.kernel.org,
	linux-f2fs-devel@lists.sourceforge.net, kernel-team@android.com
Cc: Jaegeuk Kim <jaegeuk@kernel.org>, Daeho Jeong <daehojeong@google.com>
Subject: Re: [f2fs-dev] [PATCH v2] f2fs: add sysfs node to control ra_pages for fadvise seq file
Date: Tue, 3 Aug 2021 10:42:08 +0800	[thread overview]
Message-ID: <448ec877-7486-c02b-e6ba-3c32aafff9bf@kernel.org> (raw)
In-Reply-To: <20210802175703.895964-1-daeho43@gmail.com>

On 2021/8/3 1:57, Daeho Jeong wrote:
> From: Daeho Jeong <daehojeong@google.com>
> 
> fadvise() allows the user to expand the readahead window to double with
> POSIX_FADV_SEQUENTIAL, now. But, in some use cases, it is not that
> sufficient and we need to meet the need in a restricted way. We can
> control the multiplier value of bdi device readahead between 2 (default)
> and 256 for POSIX_FADV_SEQUENTIAL advise option.
> 
> Signed-off-by: Daeho Jeong <daehojeong@google.com>
> Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
> 
> ---
> v2: fix minor style issues
> ---
>   Documentation/ABI/testing/sysfs-fs-f2fs |  6 +++++
>   fs/f2fs/f2fs.h                          |  2 ++
>   fs/f2fs/file.c                          | 30 +++++++++++++++++++++++++
>   fs/f2fs/super.c                         |  1 +
>   fs/f2fs/sysfs.c                         | 13 +++++++++++
>   5 files changed, 52 insertions(+)
> 
> diff --git a/Documentation/ABI/testing/sysfs-fs-f2fs b/Documentation/ABI/testing/sysfs-fs-f2fs
> index 845c4be535b0..73211f77d11e 100644
> --- a/Documentation/ABI/testing/sysfs-fs-f2fs
> +++ b/Documentation/ABI/testing/sysfs-fs-f2fs
> @@ -507,3 +507,9 @@ Date:		July 2021
>   Contact:	"Daeho Jeong" <daehojeong@google.com>
>   Description:	You can control for which gc mode the "gc_reclaimed_segments" node shows.
>   		Refer to the description of the modes in "gc_reclaimed_segments".
> +
> +What:		/sys/fs/f2fs/<disk>/seq_file_ra_mul
> +Date:		July 2021
> +Contact:	"Daeho Jeong" <daehojeong@google.com>
> +Description:	You can	control the multiplier value of	bdi device readahead window size
> +		between 2 (default) and 256 for POSIX_FADV_SEQUENTIAL advise option.
> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> index 8459b6d5a2f8..5d16486feb8f 100644
> --- a/fs/f2fs/f2fs.h
> +++ b/fs/f2fs/f2fs.h
> @@ -1738,6 +1738,8 @@ struct f2fs_sb_info {
>   	unsigned int gc_segment_mode;		/* GC state for reclaimed segments */
>   	unsigned int gc_reclaimed_segs[MAX_GC_MODE];	/* Reclaimed segs for each mode */
>   
> +	unsigned long seq_file_ra_mul;		/* multiplier for ra_pages of seq. files in fadvise */
> +
>   #ifdef CONFIG_F2FS_FS_COMPRESSION
>   	struct kmem_cache *page_array_slab;	/* page array entry */
>   	unsigned int page_array_slab_size;	/* default page array slab size */
> diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
> index b1cb5b50faac..1a631e6d3e9b 100644
> --- a/fs/f2fs/file.c
> +++ b/fs/f2fs/file.c
> @@ -23,6 +23,7 @@
>   #include <linux/nls.h>
>   #include <linux/sched/signal.h>
>   #include <linux/fileattr.h>
> +#include <linux/fadvise.h>
>   
>   #include "f2fs.h"
>   #include "node.h"
> @@ -4332,6 +4333,34 @@ static ssize_t f2fs_file_write_iter(struct kiocb *iocb, struct iov_iter *from)
>   	return ret;
>   }
>   
> +static int f2fs_file_fadvise(struct file *filp, loff_t offset, loff_t len,
> +		int advice)
> +{
> +	struct inode *inode;
> +	struct address_space *mapping;
> +	struct backing_dev_info *bdi;
> +
> +	if (advice == POSIX_FADV_SEQUENTIAL) {
> +		inode = file_inode(filp);
> +		if (S_ISFIFO(inode->i_mode))
> +			return -ESPIPE;
> +
> +		mapping = filp->f_mapping;
> +		if (!mapping || len < 0)
> +			return -EINVAL;
> +
> +		bdi = inode_to_bdi(mapping->host);
> +		filp->f_ra.ra_pages = bdi->ra_pages *
> +			F2FS_I_SB(inode)->seq_file_ra_mul;
> +		spin_lock(&filp->f_lock);
> +		filp->f_mode &= ~FMODE_RANDOM;
> +		spin_unlock(&filp->f_lock);
> +		return 0;
> +	}
> +
> +	return generic_fadvise(filp, offset, len, advice);
> +}
> +
>   #ifdef CONFIG_COMPAT
>   struct compat_f2fs_gc_range {
>   	u32 sync;
> @@ -4460,4 +4489,5 @@ const struct file_operations f2fs_file_operations = {
>   #endif
>   	.splice_read	= generic_file_splice_read,
>   	.splice_write	= iter_file_splice_write,
> +	.fadvise	= f2fs_file_fadvise,
>   };
> diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
> index 72eb9d70969f..41765e90caa2 100644
> --- a/fs/f2fs/super.c
> +++ b/fs/f2fs/super.c
> @@ -3422,6 +3422,7 @@ static void init_sb_info(struct f2fs_sb_info *sbi)
>   	sbi->next_victim_seg[FG_GC] = NULL_SEGNO;
>   	sbi->max_victim_search = DEF_MAX_VICTIM_SEARCH;
>   	sbi->migration_granularity = sbi->segs_per_sec;
> +	sbi->seq_file_ra_mul = 2;

sbi->seq_file_ra_mul = MIN_RA_MUL;

Otherwise, it looks good to me.

Reviewed-by: Chao Yu <chao@kernel.org>

Thanks,


>   
>   	sbi->dir_level = DEF_DIR_LEVEL;
>   	sbi->interval_time[CP_TIME] = DEF_CP_INTERVAL;
> diff --git a/fs/f2fs/sysfs.c b/fs/f2fs/sysfs.c
> index b1725620c07d..44b086e5b607 100644
> --- a/fs/f2fs/sysfs.c
> +++ b/fs/f2fs/sysfs.c
> @@ -50,6 +50,9 @@ struct f2fs_attr {
>   	int id;
>   };
>   
> +#define MIN_RA_MUL	2
> +#define MAX_RA_MUL	256
> +
>   static ssize_t f2fs_sbi_show(struct f2fs_attr *a,
>   			     struct f2fs_sb_info *sbi, char *buf);
>   
> @@ -538,6 +541,14 @@ static ssize_t __sbi_store(struct f2fs_attr *a,
>   		return count;
>   	}
>   
> +	if (!strcmp(a->attr.name, "seq_file_ra_mul")) {
> +		if (t >= MIN_RA_MUL && t <= MAX_RA_MUL)
> +			sbi->seq_file_ra_mul = t;
> +		else
> +			return -EINVAL;
> +		return count;
> +	}
> +
>   	*ui = (unsigned int)t;
>   
>   	return count;
> @@ -763,6 +774,7 @@ F2FS_RW_ATTR(ATGC_INFO, atgc_management, atgc_candidate_count, max_candidate_cou
>   F2FS_RW_ATTR(ATGC_INFO, atgc_management, atgc_age_weight, age_weight);
>   F2FS_RW_ATTR(ATGC_INFO, atgc_management, atgc_age_threshold, age_threshold);
>   
> +F2FS_RW_ATTR(F2FS_SBI, f2fs_sb_info, seq_file_ra_mul, seq_file_ra_mul);
>   F2FS_RW_ATTR(F2FS_SBI, f2fs_sb_info, gc_segment_mode, gc_segment_mode);
>   F2FS_RW_ATTR(F2FS_SBI, f2fs_sb_info, gc_reclaimed_segments, gc_reclaimed_segs);
>   
> @@ -838,6 +850,7 @@ static struct attribute *f2fs_attrs[] = {
>   	ATTR_LIST(atgc_candidate_count),
>   	ATTR_LIST(atgc_age_weight),
>   	ATTR_LIST(atgc_age_threshold),
> +	ATTR_LIST(seq_file_ra_mul),
>   	ATTR_LIST(gc_segment_mode),
>   	ATTR_LIST(gc_reclaimed_segments),
>   	NULL,
> 


_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

WARNING: multiple messages have this Message-ID (diff)
From: Chao Yu <chao@kernel.org>
To: Daeho Jeong <daeho43@gmail.com>,
	linux-kernel@vger.kernel.org,
	linux-f2fs-devel@lists.sourceforge.net, kernel-team@android.com
Cc: Jaegeuk Kim <jaegeuk@kernel.org>, Daeho Jeong <daehojeong@google.com>
Subject: Re: [f2fs-dev] [PATCH v2] f2fs: add sysfs node to control ra_pages for fadvise seq file
Date: Tue, 3 Aug 2021 10:42:08 +0800	[thread overview]
Message-ID: <448ec877-7486-c02b-e6ba-3c32aafff9bf@kernel.org> (raw)
In-Reply-To: <20210802175703.895964-1-daeho43@gmail.com>

On 2021/8/3 1:57, Daeho Jeong wrote:
> From: Daeho Jeong <daehojeong@google.com>
> 
> fadvise() allows the user to expand the readahead window to double with
> POSIX_FADV_SEQUENTIAL, now. But, in some use cases, it is not that
> sufficient and we need to meet the need in a restricted way. We can
> control the multiplier value of bdi device readahead between 2 (default)
> and 256 for POSIX_FADV_SEQUENTIAL advise option.
> 
> Signed-off-by: Daeho Jeong <daehojeong@google.com>
> Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
> 
> ---
> v2: fix minor style issues
> ---
>   Documentation/ABI/testing/sysfs-fs-f2fs |  6 +++++
>   fs/f2fs/f2fs.h                          |  2 ++
>   fs/f2fs/file.c                          | 30 +++++++++++++++++++++++++
>   fs/f2fs/super.c                         |  1 +
>   fs/f2fs/sysfs.c                         | 13 +++++++++++
>   5 files changed, 52 insertions(+)
> 
> diff --git a/Documentation/ABI/testing/sysfs-fs-f2fs b/Documentation/ABI/testing/sysfs-fs-f2fs
> index 845c4be535b0..73211f77d11e 100644
> --- a/Documentation/ABI/testing/sysfs-fs-f2fs
> +++ b/Documentation/ABI/testing/sysfs-fs-f2fs
> @@ -507,3 +507,9 @@ Date:		July 2021
>   Contact:	"Daeho Jeong" <daehojeong@google.com>
>   Description:	You can control for which gc mode the "gc_reclaimed_segments" node shows.
>   		Refer to the description of the modes in "gc_reclaimed_segments".
> +
> +What:		/sys/fs/f2fs/<disk>/seq_file_ra_mul
> +Date:		July 2021
> +Contact:	"Daeho Jeong" <daehojeong@google.com>
> +Description:	You can	control the multiplier value of	bdi device readahead window size
> +		between 2 (default) and 256 for POSIX_FADV_SEQUENTIAL advise option.
> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> index 8459b6d5a2f8..5d16486feb8f 100644
> --- a/fs/f2fs/f2fs.h
> +++ b/fs/f2fs/f2fs.h
> @@ -1738,6 +1738,8 @@ struct f2fs_sb_info {
>   	unsigned int gc_segment_mode;		/* GC state for reclaimed segments */
>   	unsigned int gc_reclaimed_segs[MAX_GC_MODE];	/* Reclaimed segs for each mode */
>   
> +	unsigned long seq_file_ra_mul;		/* multiplier for ra_pages of seq. files in fadvise */
> +
>   #ifdef CONFIG_F2FS_FS_COMPRESSION
>   	struct kmem_cache *page_array_slab;	/* page array entry */
>   	unsigned int page_array_slab_size;	/* default page array slab size */
> diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
> index b1cb5b50faac..1a631e6d3e9b 100644
> --- a/fs/f2fs/file.c
> +++ b/fs/f2fs/file.c
> @@ -23,6 +23,7 @@
>   #include <linux/nls.h>
>   #include <linux/sched/signal.h>
>   #include <linux/fileattr.h>
> +#include <linux/fadvise.h>
>   
>   #include "f2fs.h"
>   #include "node.h"
> @@ -4332,6 +4333,34 @@ static ssize_t f2fs_file_write_iter(struct kiocb *iocb, struct iov_iter *from)
>   	return ret;
>   }
>   
> +static int f2fs_file_fadvise(struct file *filp, loff_t offset, loff_t len,
> +		int advice)
> +{
> +	struct inode *inode;
> +	struct address_space *mapping;
> +	struct backing_dev_info *bdi;
> +
> +	if (advice == POSIX_FADV_SEQUENTIAL) {
> +		inode = file_inode(filp);
> +		if (S_ISFIFO(inode->i_mode))
> +			return -ESPIPE;
> +
> +		mapping = filp->f_mapping;
> +		if (!mapping || len < 0)
> +			return -EINVAL;
> +
> +		bdi = inode_to_bdi(mapping->host);
> +		filp->f_ra.ra_pages = bdi->ra_pages *
> +			F2FS_I_SB(inode)->seq_file_ra_mul;
> +		spin_lock(&filp->f_lock);
> +		filp->f_mode &= ~FMODE_RANDOM;
> +		spin_unlock(&filp->f_lock);
> +		return 0;
> +	}
> +
> +	return generic_fadvise(filp, offset, len, advice);
> +}
> +
>   #ifdef CONFIG_COMPAT
>   struct compat_f2fs_gc_range {
>   	u32 sync;
> @@ -4460,4 +4489,5 @@ const struct file_operations f2fs_file_operations = {
>   #endif
>   	.splice_read	= generic_file_splice_read,
>   	.splice_write	= iter_file_splice_write,
> +	.fadvise	= f2fs_file_fadvise,
>   };
> diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
> index 72eb9d70969f..41765e90caa2 100644
> --- a/fs/f2fs/super.c
> +++ b/fs/f2fs/super.c
> @@ -3422,6 +3422,7 @@ static void init_sb_info(struct f2fs_sb_info *sbi)
>   	sbi->next_victim_seg[FG_GC] = NULL_SEGNO;
>   	sbi->max_victim_search = DEF_MAX_VICTIM_SEARCH;
>   	sbi->migration_granularity = sbi->segs_per_sec;
> +	sbi->seq_file_ra_mul = 2;

sbi->seq_file_ra_mul = MIN_RA_MUL;

Otherwise, it looks good to me.

Reviewed-by: Chao Yu <chao@kernel.org>

Thanks,


>   
>   	sbi->dir_level = DEF_DIR_LEVEL;
>   	sbi->interval_time[CP_TIME] = DEF_CP_INTERVAL;
> diff --git a/fs/f2fs/sysfs.c b/fs/f2fs/sysfs.c
> index b1725620c07d..44b086e5b607 100644
> --- a/fs/f2fs/sysfs.c
> +++ b/fs/f2fs/sysfs.c
> @@ -50,6 +50,9 @@ struct f2fs_attr {
>   	int id;
>   };
>   
> +#define MIN_RA_MUL	2
> +#define MAX_RA_MUL	256
> +
>   static ssize_t f2fs_sbi_show(struct f2fs_attr *a,
>   			     struct f2fs_sb_info *sbi, char *buf);
>   
> @@ -538,6 +541,14 @@ static ssize_t __sbi_store(struct f2fs_attr *a,
>   		return count;
>   	}
>   
> +	if (!strcmp(a->attr.name, "seq_file_ra_mul")) {
> +		if (t >= MIN_RA_MUL && t <= MAX_RA_MUL)
> +			sbi->seq_file_ra_mul = t;
> +		else
> +			return -EINVAL;
> +		return count;
> +	}
> +
>   	*ui = (unsigned int)t;
>   
>   	return count;
> @@ -763,6 +774,7 @@ F2FS_RW_ATTR(ATGC_INFO, atgc_management, atgc_candidate_count, max_candidate_cou
>   F2FS_RW_ATTR(ATGC_INFO, atgc_management, atgc_age_weight, age_weight);
>   F2FS_RW_ATTR(ATGC_INFO, atgc_management, atgc_age_threshold, age_threshold);
>   
> +F2FS_RW_ATTR(F2FS_SBI, f2fs_sb_info, seq_file_ra_mul, seq_file_ra_mul);
>   F2FS_RW_ATTR(F2FS_SBI, f2fs_sb_info, gc_segment_mode, gc_segment_mode);
>   F2FS_RW_ATTR(F2FS_SBI, f2fs_sb_info, gc_reclaimed_segments, gc_reclaimed_segs);
>   
> @@ -838,6 +850,7 @@ static struct attribute *f2fs_attrs[] = {
>   	ATTR_LIST(atgc_candidate_count),
>   	ATTR_LIST(atgc_age_weight),
>   	ATTR_LIST(atgc_age_threshold),
> +	ATTR_LIST(seq_file_ra_mul),
>   	ATTR_LIST(gc_segment_mode),
>   	ATTR_LIST(gc_reclaimed_segments),
>   	NULL,
> 

  reply	other threads:[~2021-08-03  2:42 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-08-02 17:57 [f2fs-dev] [PATCH v2] f2fs: add sysfs node to control ra_pages for fadvise seq file Daeho Jeong
2021-08-02 17:57 ` Daeho Jeong
2021-08-03  2:42 ` Chao Yu [this message]
2021-08-03  2:42   ` [f2fs-dev] " Chao Yu

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=448ec877-7486-c02b-e6ba-3c32aafff9bf@kernel.org \
    --to=chao@kernel.org \
    --cc=daeho43@gmail.com \
    --cc=daehojeong@google.com \
    --cc=jaegeuk@kernel.org \
    --cc=kernel-team@android.com \
    --cc=linux-f2fs-devel@lists.sourceforge.net \
    --cc=linux-kernel@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.