All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jaegeuk Kim <jaegeuk@kernel.org>
To: Chao Yu <chao@kernel.org>
Cc: linux-kernel@vger.kernel.org, linux-f2fs-devel@lists.sourceforge.net
Subject: Re: [f2fs-dev] [PATCH 2/3 v2] f2fs: use BLKS_PER_SEG, BLKS_PER_SEC, and SEGS_PER_SEC
Date: Tue, 20 Feb 2024 12:33:06 -0800	[thread overview]
Message-ID: <ZdUMgrzpVHob6KvQ@google.com> (raw)
In-Reply-To: <bded9fde-9541-465d-86f8-292249226cc9@kernel.org>

Hi Chao,

I applied more tho, some are new and some are missing in your patch.
Please see v2.

Thanks,

On 02/20, Chao Yu wrote:
> On 2024/2/13 2:10, Jaegeuk Kim wrote:
> 
> How do you think of appending below diff which cleans up missing
> parts?
> 
> ---
>  fs/f2fs/f2fs.h    |  2 +-
>  fs/f2fs/file.c    |  4 ++--
>  fs/f2fs/segment.c |  4 ++--
>  fs/f2fs/segment.h | 22 +++++++++++-----------
>  fs/f2fs/super.c   |  2 +-
>  5 files changed, 17 insertions(+), 17 deletions(-)
> 
> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> index c4488e12c56e..fc9328655de8 100644
> --- a/fs/f2fs/f2fs.h
> +++ b/fs/f2fs/f2fs.h
> @@ -3491,7 +3491,7 @@ static inline __le32 *get_dnode_addr(struct inode *inode,
>  		sizeof((f2fs_inode)->field))			\
>  		<= (F2FS_OLD_ATTRIBUTE_SIZE + (extra_isize)))	\
> 
> -#define __is_large_section(sbi)		((sbi)->segs_per_sec > 1)
> +#define __is_large_section(sbi)		(SEGS_PER_SEC(sbi) > 1)
> 
>  #define __is_meta_io(fio) (PAGE_TYPE_OF_BIO((fio)->type) == META)
> 
> diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
> index 20a26bb5b889..ef43d33278ea 100644
> --- a/fs/f2fs/file.c
> +++ b/fs/f2fs/file.c
> @@ -2997,8 +2997,8 @@ static int f2fs_ioc_flush_device(struct file *filp, unsigned long arg)
> 
>  	if (!f2fs_is_multi_device(sbi) || sbi->s_ndevs - 1 <= range.dev_num ||
>  			__is_large_section(sbi)) {
> -		f2fs_warn(sbi, "Can't flush %u in %d for segs_per_sec %u != 1",
> -			  range.dev_num, sbi->s_ndevs, sbi->segs_per_sec);
> +		f2fs_warn(sbi, "Can't flush %u in %d for SEGS_PER_SEC %u != 1",
> +			  range.dev_num, sbi->s_ndevs, SEGS_PER_SEC(sbi));
>  		return -EINVAL;
>  	}
> 
> diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
> index 97ac733ceffe..b59e29608ae7 100644
> --- a/fs/f2fs/segment.c
> +++ b/fs/f2fs/segment.c
> @@ -2750,7 +2750,7 @@ static unsigned int __get_next_segno(struct f2fs_sb_info *sbi, int type)
>  	if (f2fs_need_rand_seg(sbi))
>  		return get_random_u32_below(MAIN_SECS(sbi) * SEGS_PER_SEC(sbi));
> 
> -	/* if segs_per_sec is large than 1, we need to keep original policy. */
> +	/* if SEGS_PER_SEC() is large than 1, we need to keep original policy. */
>  	if (__is_large_section(sbi))
>  		return curseg->segno;
> 
> @@ -3498,7 +3498,7 @@ int f2fs_allocate_data_block(struct f2fs_sb_info *sbi, struct page *page,
>  	 */
>  	if (segment_full) {
>  		if (type == CURSEG_COLD_DATA_PINNED &&
> -		    !((curseg->segno + 1) % sbi->segs_per_sec))
> +		    !((curseg->segno + 1) % SEGS_PER_SEC(sbi)))
>  			goto skip_new_segment;
> 
>  		if (from_gc) {
> diff --git a/fs/f2fs/segment.h b/fs/f2fs/segment.h
> index cb982af765c3..63f278210654 100644
> --- a/fs/f2fs/segment.h
> +++ b/fs/f2fs/segment.h
> @@ -48,21 +48,21 @@ static inline void sanity_check_seg_type(struct f2fs_sb_info *sbi,
> 
>  #define IS_CURSEC(sbi, secno)						\
>  	(((secno) == CURSEG_I(sbi, CURSEG_HOT_DATA)->segno /		\
> -	  (sbi)->segs_per_sec) ||	\
> +	  SEGS_PER_SEC(sbi)) ||	\
>  	 ((secno) == CURSEG_I(sbi, CURSEG_WARM_DATA)->segno /		\
> -	  (sbi)->segs_per_sec) ||	\
> +	  SEGS_PER_SEC(sbi)) ||	\
>  	 ((secno) == CURSEG_I(sbi, CURSEG_COLD_DATA)->segno /		\
> -	  (sbi)->segs_per_sec) ||	\
> +	  SEGS_PER_SEC(sbi)) ||	\
>  	 ((secno) == CURSEG_I(sbi, CURSEG_HOT_NODE)->segno /		\
> -	  (sbi)->segs_per_sec) ||	\
> +	  SEGS_PER_SEC(sbi)) ||	\
>  	 ((secno) == CURSEG_I(sbi, CURSEG_WARM_NODE)->segno /		\
> -	  (sbi)->segs_per_sec) ||	\
> +	  SEGS_PER_SEC(sbi)) ||	\
>  	 ((secno) == CURSEG_I(sbi, CURSEG_COLD_NODE)->segno /		\
> -	  (sbi)->segs_per_sec) ||	\
> +	  SEGS_PER_SEC(sbi)) ||	\
>  	 ((secno) == CURSEG_I(sbi, CURSEG_COLD_DATA_PINNED)->segno /	\
> -	  (sbi)->segs_per_sec) ||	\
> +	  SEGS_PER_SEC(sbi)) ||	\
>  	 ((secno) == CURSEG_I(sbi, CURSEG_ALL_DATA_ATGC)->segno /	\
> -	  (sbi)->segs_per_sec))
> +	  SEGS_PER_SEC(sbi)))
> 
>  #define MAIN_BLKADDR(sbi)						\
>  	(SM_I(sbi) ? SM_I(sbi)->main_blkaddr : 				\
> @@ -93,7 +93,7 @@ static inline void sanity_check_seg_type(struct f2fs_sb_info *sbi,
>  #define GET_SEGNO_FROM_SEG0(sbi, blk_addr)				\
>  	(GET_SEGOFF_FROM_SEG0(sbi, blk_addr) >> (sbi)->log_blocks_per_seg)
>  #define GET_BLKOFF_FROM_SEG0(sbi, blk_addr)				\
> -	(GET_SEGOFF_FROM_SEG0(sbi, blk_addr) & ((sbi)->blocks_per_seg - 1))
> +	(GET_SEGOFF_FROM_SEG0(sbi, blk_addr) & (BLKS_PER_SEG(sbi) - 1))
> 
>  #define GET_SEGNO(sbi, blk_addr)					\
>  	((!__is_valid_data_blkaddr(blk_addr) ||			\
> @@ -101,9 +101,9 @@ static inline void sanity_check_seg_type(struct f2fs_sb_info *sbi,
>  	NULL_SEGNO : GET_L2R_SEGNO(FREE_I(sbi),			\
>  		GET_SEGNO_FROM_SEG0(sbi, blk_addr)))
>  #define GET_SEC_FROM_SEG(sbi, segno)				\
> -	(((segno) == -1) ? -1 : (segno) / (sbi)->segs_per_sec)
> +	(((segno) == -1) ? -1 : (segno) / SEGS_PER_SEC(sbi))
>  #define GET_SEG_FROM_SEC(sbi, secno)				\
> -	((secno) * (sbi)->segs_per_sec)
> +	((secno) * SEGS_PER_SEC(sbi))
>  #define GET_ZONE_FROM_SEC(sbi, secno)				\
>  	(((secno) == -1) ? -1 : (secno) / (sbi)->secs_per_zone)
>  #define GET_ZONE_FROM_SEG(sbi, segno)				\
> diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
> index 2a8b6cfaf683..9d2c680a61f5 100644
> --- a/fs/f2fs/super.c
> +++ b/fs/f2fs/super.c
> @@ -4723,7 +4723,7 @@ static int f2fs_fill_super(struct super_block *sb, void *data, int silent)
>  			.reason = CP_DISCARD,
>  			.trim_start = 0,
>  			.trim_end = MAIN_SEGS(sbi) - 1,
> -			.trim_minlen = sbi->blocks_per_seg,
> +			.trim_minlen = BLKS_PER_SEG(sbi),

We don't have this.

>  		};
>  		f2fs_write_checkpoint(sbi, &cpc);
>  	}
> -- 
> 2.40.1


_______________________________________________
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: Jaegeuk Kim <jaegeuk@kernel.org>
To: Chao Yu <chao@kernel.org>
Cc: linux-kernel@vger.kernel.org, linux-f2fs-devel@lists.sourceforge.net
Subject: Re: [f2fs-dev] [PATCH 2/3 v2] f2fs: use BLKS_PER_SEG, BLKS_PER_SEC, and SEGS_PER_SEC
Date: Tue, 20 Feb 2024 12:33:06 -0800	[thread overview]
Message-ID: <ZdUMgrzpVHob6KvQ@google.com> (raw)
In-Reply-To: <bded9fde-9541-465d-86f8-292249226cc9@kernel.org>

Hi Chao,

I applied more tho, some are new and some are missing in your patch.
Please see v2.

Thanks,

On 02/20, Chao Yu wrote:
> On 2024/2/13 2:10, Jaegeuk Kim wrote:
> 
> How do you think of appending below diff which cleans up missing
> parts?
> 
> ---
>  fs/f2fs/f2fs.h    |  2 +-
>  fs/f2fs/file.c    |  4 ++--
>  fs/f2fs/segment.c |  4 ++--
>  fs/f2fs/segment.h | 22 +++++++++++-----------
>  fs/f2fs/super.c   |  2 +-
>  5 files changed, 17 insertions(+), 17 deletions(-)
> 
> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> index c4488e12c56e..fc9328655de8 100644
> --- a/fs/f2fs/f2fs.h
> +++ b/fs/f2fs/f2fs.h
> @@ -3491,7 +3491,7 @@ static inline __le32 *get_dnode_addr(struct inode *inode,
>  		sizeof((f2fs_inode)->field))			\
>  		<= (F2FS_OLD_ATTRIBUTE_SIZE + (extra_isize)))	\
> 
> -#define __is_large_section(sbi)		((sbi)->segs_per_sec > 1)
> +#define __is_large_section(sbi)		(SEGS_PER_SEC(sbi) > 1)
> 
>  #define __is_meta_io(fio) (PAGE_TYPE_OF_BIO((fio)->type) == META)
> 
> diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
> index 20a26bb5b889..ef43d33278ea 100644
> --- a/fs/f2fs/file.c
> +++ b/fs/f2fs/file.c
> @@ -2997,8 +2997,8 @@ static int f2fs_ioc_flush_device(struct file *filp, unsigned long arg)
> 
>  	if (!f2fs_is_multi_device(sbi) || sbi->s_ndevs - 1 <= range.dev_num ||
>  			__is_large_section(sbi)) {
> -		f2fs_warn(sbi, "Can't flush %u in %d for segs_per_sec %u != 1",
> -			  range.dev_num, sbi->s_ndevs, sbi->segs_per_sec);
> +		f2fs_warn(sbi, "Can't flush %u in %d for SEGS_PER_SEC %u != 1",
> +			  range.dev_num, sbi->s_ndevs, SEGS_PER_SEC(sbi));
>  		return -EINVAL;
>  	}
> 
> diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
> index 97ac733ceffe..b59e29608ae7 100644
> --- a/fs/f2fs/segment.c
> +++ b/fs/f2fs/segment.c
> @@ -2750,7 +2750,7 @@ static unsigned int __get_next_segno(struct f2fs_sb_info *sbi, int type)
>  	if (f2fs_need_rand_seg(sbi))
>  		return get_random_u32_below(MAIN_SECS(sbi) * SEGS_PER_SEC(sbi));
> 
> -	/* if segs_per_sec is large than 1, we need to keep original policy. */
> +	/* if SEGS_PER_SEC() is large than 1, we need to keep original policy. */
>  	if (__is_large_section(sbi))
>  		return curseg->segno;
> 
> @@ -3498,7 +3498,7 @@ int f2fs_allocate_data_block(struct f2fs_sb_info *sbi, struct page *page,
>  	 */
>  	if (segment_full) {
>  		if (type == CURSEG_COLD_DATA_PINNED &&
> -		    !((curseg->segno + 1) % sbi->segs_per_sec))
> +		    !((curseg->segno + 1) % SEGS_PER_SEC(sbi)))
>  			goto skip_new_segment;
> 
>  		if (from_gc) {
> diff --git a/fs/f2fs/segment.h b/fs/f2fs/segment.h
> index cb982af765c3..63f278210654 100644
> --- a/fs/f2fs/segment.h
> +++ b/fs/f2fs/segment.h
> @@ -48,21 +48,21 @@ static inline void sanity_check_seg_type(struct f2fs_sb_info *sbi,
> 
>  #define IS_CURSEC(sbi, secno)						\
>  	(((secno) == CURSEG_I(sbi, CURSEG_HOT_DATA)->segno /		\
> -	  (sbi)->segs_per_sec) ||	\
> +	  SEGS_PER_SEC(sbi)) ||	\
>  	 ((secno) == CURSEG_I(sbi, CURSEG_WARM_DATA)->segno /		\
> -	  (sbi)->segs_per_sec) ||	\
> +	  SEGS_PER_SEC(sbi)) ||	\
>  	 ((secno) == CURSEG_I(sbi, CURSEG_COLD_DATA)->segno /		\
> -	  (sbi)->segs_per_sec) ||	\
> +	  SEGS_PER_SEC(sbi)) ||	\
>  	 ((secno) == CURSEG_I(sbi, CURSEG_HOT_NODE)->segno /		\
> -	  (sbi)->segs_per_sec) ||	\
> +	  SEGS_PER_SEC(sbi)) ||	\
>  	 ((secno) == CURSEG_I(sbi, CURSEG_WARM_NODE)->segno /		\
> -	  (sbi)->segs_per_sec) ||	\
> +	  SEGS_PER_SEC(sbi)) ||	\
>  	 ((secno) == CURSEG_I(sbi, CURSEG_COLD_NODE)->segno /		\
> -	  (sbi)->segs_per_sec) ||	\
> +	  SEGS_PER_SEC(sbi)) ||	\
>  	 ((secno) == CURSEG_I(sbi, CURSEG_COLD_DATA_PINNED)->segno /	\
> -	  (sbi)->segs_per_sec) ||	\
> +	  SEGS_PER_SEC(sbi)) ||	\
>  	 ((secno) == CURSEG_I(sbi, CURSEG_ALL_DATA_ATGC)->segno /	\
> -	  (sbi)->segs_per_sec))
> +	  SEGS_PER_SEC(sbi)))
> 
>  #define MAIN_BLKADDR(sbi)						\
>  	(SM_I(sbi) ? SM_I(sbi)->main_blkaddr : 				\
> @@ -93,7 +93,7 @@ static inline void sanity_check_seg_type(struct f2fs_sb_info *sbi,
>  #define GET_SEGNO_FROM_SEG0(sbi, blk_addr)				\
>  	(GET_SEGOFF_FROM_SEG0(sbi, blk_addr) >> (sbi)->log_blocks_per_seg)
>  #define GET_BLKOFF_FROM_SEG0(sbi, blk_addr)				\
> -	(GET_SEGOFF_FROM_SEG0(sbi, blk_addr) & ((sbi)->blocks_per_seg - 1))
> +	(GET_SEGOFF_FROM_SEG0(sbi, blk_addr) & (BLKS_PER_SEG(sbi) - 1))
> 
>  #define GET_SEGNO(sbi, blk_addr)					\
>  	((!__is_valid_data_blkaddr(blk_addr) ||			\
> @@ -101,9 +101,9 @@ static inline void sanity_check_seg_type(struct f2fs_sb_info *sbi,
>  	NULL_SEGNO : GET_L2R_SEGNO(FREE_I(sbi),			\
>  		GET_SEGNO_FROM_SEG0(sbi, blk_addr)))
>  #define GET_SEC_FROM_SEG(sbi, segno)				\
> -	(((segno) == -1) ? -1 : (segno) / (sbi)->segs_per_sec)
> +	(((segno) == -1) ? -1 : (segno) / SEGS_PER_SEC(sbi))
>  #define GET_SEG_FROM_SEC(sbi, secno)				\
> -	((secno) * (sbi)->segs_per_sec)
> +	((secno) * SEGS_PER_SEC(sbi))
>  #define GET_ZONE_FROM_SEC(sbi, secno)				\
>  	(((secno) == -1) ? -1 : (secno) / (sbi)->secs_per_zone)
>  #define GET_ZONE_FROM_SEG(sbi, segno)				\
> diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
> index 2a8b6cfaf683..9d2c680a61f5 100644
> --- a/fs/f2fs/super.c
> +++ b/fs/f2fs/super.c
> @@ -4723,7 +4723,7 @@ static int f2fs_fill_super(struct super_block *sb, void *data, int silent)
>  			.reason = CP_DISCARD,
>  			.trim_start = 0,
>  			.trim_end = MAIN_SEGS(sbi) - 1,
> -			.trim_minlen = sbi->blocks_per_seg,
> +			.trim_minlen = BLKS_PER_SEG(sbi),

We don't have this.

>  		};
>  		f2fs_write_checkpoint(sbi, &cpc);
>  	}
> -- 
> 2.40.1

  reply	other threads:[~2024-02-20 20:33 UTC|newest]

Thread overview: 50+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-02-07  0:51 [f2fs-dev] [PATCH 1/3] f2fs: deprecate io_bits Jaegeuk Kim
2024-02-07  0:51 ` Jaegeuk Kim
2024-02-07  0:51 ` [f2fs-dev] [PATCH 2/3] f2fs: use BLKS_PER_SEG, BLKS_PER_SEC, and SEGS_PER_SEC Jaegeuk Kim
2024-02-07  0:51   ` Jaegeuk Kim
2024-02-07 23:51   ` [f2fs-dev] " Daeho Jeong
2024-02-07 23:51     ` Daeho Jeong
2024-02-12 18:10   ` [f2fs-dev] [PATCH 2/3 v2] " Jaegeuk Kim
2024-02-12 18:10     ` Jaegeuk Kim
2024-02-20  7:30     ` [f2fs-dev] " Chao Yu
2024-02-20  7:30       ` Chao Yu
2024-02-20 20:33       ` Jaegeuk Kim [this message]
2024-02-20 20:33         ` Jaegeuk Kim
2024-02-21  1:04         ` Chao Yu
2024-02-21  1:04           ` Chao Yu
2024-02-20 20:39     ` [f2fs-dev] [PATCH 2/3 v3] " Jaegeuk Kim
2024-02-20 20:39       ` Jaegeuk Kim
2024-02-21  1:30       ` [f2fs-dev] " Chao Yu
2024-02-21  1:30         ` Chao Yu
2024-02-21  1:54         ` Jaegeuk Kim
2024-02-21  1:54           ` Jaegeuk Kim
2024-02-21  2:17           ` Chao Yu
2024-02-21  2:17             ` Chao Yu
2024-02-07  0:51 ` [f2fs-dev] [PATCH 3/3] f2fs: kill zone-capacity support Jaegeuk Kim
2024-02-07  0:51   ` Jaegeuk Kim
2024-02-07  2:42   ` [f2fs-dev] " Jaegeuk Kim
2024-02-07  2:42     ` Jaegeuk Kim
2024-02-07 22:27     ` [f2fs-dev] " Daeho Jeong
2024-02-07 22:27       ` Daeho Jeong
2024-02-07 23:29   ` [f2fs-dev] [PATCH 3/3 v2] " Jaegeuk Kim
2024-02-07 23:29     ` Jaegeuk Kim
2024-02-07 23:51     ` [f2fs-dev] " Daeho Jeong
2024-02-07 23:51       ` Daeho Jeong
2024-02-20  9:48     ` Chao Yu
2024-02-20  9:48       ` Chao Yu
2024-02-20 19:01       ` Jaegeuk Kim
2024-02-20 19:01         ` Jaegeuk Kim
2024-02-21  8:14         ` Juhyung Park
2024-02-21  8:14           ` Juhyung Park
2024-02-21 17:27           ` Jaegeuk Kim
2024-02-21 17:27             ` Jaegeuk Kim
2024-02-22 14:33             ` Matias Bjørling
2024-02-22 14:33               ` Matias Bjørling
2024-02-23 17:38               ` Jaegeuk Kim
2024-02-23 17:38                 ` Jaegeuk Kim
2024-02-08  0:11 ` [f2fs-dev] [PATCH 1/3] f2fs: deprecate io_bits Daeho Jeong
2024-02-08  0:11   ` Daeho Jeong
2024-02-20  7:03 ` Chao Yu
2024-02-20  7:03   ` Chao Yu
2024-02-21 18:10 ` patchwork-bot+f2fs
2024-02-21 18:10   ` patchwork-bot+f2fs

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=ZdUMgrzpVHob6KvQ@google.com \
    --to=jaegeuk@kernel.org \
    --cc=chao@kernel.org \
    --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.