All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jaegeuk Kim via Linux-f2fs-devel <linux-f2fs-devel@lists.sourceforge.net>
To: Yi Sun <yi.sun@unisoc.com>
Cc: ke.wang@unisoc.com, linux-kernel@vger.kernel.org,
	linux-f2fs-devel@lists.sourceforge.net, sunyibuaa@gmail.com,
	hao_hao.wang@unisoc.com
Subject: Re: [f2fs-dev] [RFC PATCH 2/2] f2fs: introduce f2fs_invalidate_consecutive_blocks()
Date: Wed, 16 Oct 2024 16:04:26 +0000	[thread overview]
Message-ID: <Zw_kCu3J7f7iQrFF@google.com> (raw)
In-Reply-To: <20241016052758.3400359-3-yi.sun@unisoc.com>

2573 void f2fs_invalidate_consecutive_blocks(struct f2fs_sb_info *sbi, block_t addr, int cnt)
2574 {
2575         unsigned int segno = GET_SEGNO(sbi, addr);
2576         unsigned int segno2 = GET_SEGNO(sbi, addr + cnt - 1);
2577         struct sit_info *sit_i = SIT_I(sbi);
2578
2579         f2fs_bug_on(sbi, addr == NULL_ADDR || segno != segno2);

This hits a panic here while running fsstress.

On 10/16, Yi Sun wrote:
> When doing truncate, consecutive blocks in the same segment
> can be processed at the same time. So that the efficiency of
> doing truncate can be improved.
> 
> Add f2fs_invalidate_compress_pages_range() only for doing truncate.
> Add check_f2fs_invalidate_consecutive_blocks() only for doing
> truncate and to determine whether the blocks are continuous and
> belong to the same segment.
> 
> Signed-off-by: Yi Sun <yi.sun@unisoc.com>
> ---
>  fs/f2fs/compress.c | 14 ++++++++++++++
>  fs/f2fs/f2fs.h     |  5 +++++
>  fs/f2fs/file.c     | 34 +++++++++++++++++++++++++++++++++-
>  fs/f2fs/segment.c  | 25 +++++++++++++++++++++++++
>  4 files changed, 77 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/f2fs/compress.c b/fs/f2fs/compress.c
> index 7f26440e8595..70929a87e9bf 100644
> --- a/fs/f2fs/compress.c
> +++ b/fs/f2fs/compress.c
> @@ -2014,6 +2014,20 @@ void f2fs_invalidate_compress_pages(struct f2fs_sb_info *sbi, nid_t ino)
>  	} while (index < end);
>  }
>  
> +void f2fs_invalidate_compress_pages_range(struct f2fs_sb_info *sbi,
> +					block_t blkaddr, int cnt)
> +{
> +	if (!sbi->compress_inode)
> +		return;
> +
> +	if (cnt < 1) {
> +		f2fs_bug_on(sbi, 1);
> +		cnt = 1;
> +	}
> +
> +	invalidate_mapping_pages(COMPRESS_MAPPING(sbi), blkaddr, blkaddr + cnt - 1);
> +}
> +
>  int f2fs_init_compress_inode(struct f2fs_sb_info *sbi)
>  {
>  	struct inode *inode;
> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> index ce00cb546f4a..99767f35678f 100644
> --- a/fs/f2fs/f2fs.h
> +++ b/fs/f2fs/f2fs.h
> @@ -3716,6 +3716,7 @@ int f2fs_create_flush_cmd_control(struct f2fs_sb_info *sbi);
>  int f2fs_flush_device_cache(struct f2fs_sb_info *sbi);
>  void f2fs_destroy_flush_cmd_control(struct f2fs_sb_info *sbi, bool free);
>  void f2fs_invalidate_blocks(struct f2fs_sb_info *sbi, block_t addr);
> +void f2fs_invalidate_consecutive_blocks(struct f2fs_sb_info *sbi, block_t addr, int cnt);
>  bool f2fs_is_checkpointed_data(struct f2fs_sb_info *sbi, block_t blkaddr);
>  int f2fs_start_discard_thread(struct f2fs_sb_info *sbi);
>  void f2fs_drop_discard_cmd(struct f2fs_sb_info *sbi);
> @@ -4375,6 +4376,8 @@ void f2fs_cache_compressed_page(struct f2fs_sb_info *sbi, struct page *page,
>  bool f2fs_load_compressed_page(struct f2fs_sb_info *sbi, struct page *page,
>  								block_t blkaddr);
>  void f2fs_invalidate_compress_pages(struct f2fs_sb_info *sbi, nid_t ino);
> +void f2fs_invalidate_compress_pages_range(struct f2fs_sb_info *sbi,
> +					block_t blkaddr, int cnt);
>  #define inc_compr_inode_stat(inode)					\
>  	do {								\
>  		struct f2fs_sb_info *sbi = F2FS_I_SB(inode);		\
> @@ -4432,6 +4435,8 @@ static inline bool f2fs_load_compressed_page(struct f2fs_sb_info *sbi,
>  				struct page *page, block_t blkaddr) { return false; }
>  static inline void f2fs_invalidate_compress_pages(struct f2fs_sb_info *sbi,
>  							nid_t ino) { }
> +static inline void f2fs_invalidate_compress_pages_range(struct f2fs_sb_info *sbi,
> +						block_t blkaddr, int cnt) { }
>  #define inc_compr_inode_stat(inode)		do { } while (0)
>  static inline int f2fs_is_compressed_cluster(
>  				struct inode *inode,
> diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
> index 7057efa8ec17..634691e3b5f1 100644
> --- a/fs/f2fs/file.c
> +++ b/fs/f2fs/file.c
> @@ -612,6 +612,18 @@ static int f2fs_file_open(struct inode *inode, struct file *filp)
>  	return finish_preallocate_blocks(inode);
>  }
>  
> +static bool check_f2fs_invalidate_consecutive_blocks(struct f2fs_sb_info *sbi,
> +					block_t blkaddr1, block_t blkaddr2)
> +{
> +	if (blkaddr2 - blkaddr1 != 1)
> +		return false;
> +
> +	if (GET_SEGNO(sbi, blkaddr1) != GET_SEGNO(sbi, blkaddr2))
> +		return false;
> +
> +	return true;
> +}
> +
>  void f2fs_truncate_data_blocks_range(struct dnode_of_data *dn, int count)
>  {
>  	struct f2fs_sb_info *sbi = F2FS_I_SB(dn->inode);
> @@ -621,6 +633,9 @@ void f2fs_truncate_data_blocks_range(struct dnode_of_data *dn, int count)
>  	int cluster_index = 0, valid_blocks = 0;
>  	int cluster_size = F2FS_I(dn->inode)->i_cluster_size;
>  	bool released = !atomic_read(&F2FS_I(dn->inode)->i_compr_blocks);
> +	block_t con_start;
> +	bool run_invalid = true;
> +	int con_cnt = 1;
>  
>  	addr = get_dnode_addr(dn->inode, dn->node_page) + ofs;
>  
> @@ -652,7 +667,24 @@ void f2fs_truncate_data_blocks_range(struct dnode_of_data *dn, int count)
>  				valid_blocks++;
>  		}
>  
> -		f2fs_invalidate_blocks(sbi, blkaddr);
> +		if (run_invalid)
> +			con_start = blkaddr;
> +
> +		if (count > 1 &&
> +			check_f2fs_invalidate_consecutive_blocks(sbi, blkaddr,
> +				le32_to_cpu(*(addr + 1)))) {
> +			run_invalid = false;
> +
> +			if (con_cnt++ == 1)
> +				con_start = blkaddr;
> +		} else {
> +			run_invalid = true;
> +		}
> +
> +		if (run_invalid) {
> +			f2fs_invalidate_consecutive_blocks(sbi, con_start, con_cnt);
> +			con_cnt = 1;
> +		}
>  
>  		if (!released || blkaddr != COMPRESS_ADDR)
>  			nr_free++;
> diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
> index f118faf36d35..edb8a78985ba 100644
> --- a/fs/f2fs/segment.c
> +++ b/fs/f2fs/segment.c
> @@ -2570,6 +2570,31 @@ void f2fs_invalidate_blocks(struct f2fs_sb_info *sbi, block_t addr)
>  	up_write(&sit_i->sentry_lock);
>  }
>  
> +void f2fs_invalidate_consecutive_blocks(struct f2fs_sb_info *sbi, block_t addr, int cnt)
> +{
> +	unsigned int segno = GET_SEGNO(sbi, addr);
> +	unsigned int segno2 = GET_SEGNO(sbi, addr + cnt - 1);
> +	struct sit_info *sit_i = SIT_I(sbi);
> +
> +	f2fs_bug_on(sbi, addr == NULL_ADDR || segno != segno2);
> +	if (addr == NEW_ADDR || addr == COMPRESS_ADDR)
> +		return;
> +
> +	f2fs_truncate_meta_inode_pages(sbi, addr, cnt);
> +	f2fs_invalidate_compress_pages_range(sbi, addr, cnt);
> +
> +	/* add it into sit main buffer */
> +	down_write(&sit_i->sentry_lock);
> +
> +	update_segment_mtime(sbi, addr, 0);
> +	update_sit_entry(sbi, addr, -cnt);
> +
> +	/* add it into dirty seglist */
> +	locate_dirty_segment(sbi, segno);
> +
> +	up_write(&sit_i->sentry_lock);
> +}
> +
>  bool f2fs_is_checkpointed_data(struct f2fs_sb_info *sbi, block_t blkaddr)
>  {
>  	struct sit_info *sit_i = SIT_I(sbi);
> -- 
> 2.25.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: Yi Sun <yi.sun@unisoc.com>
Cc: chao@kernel.org, linux-f2fs-devel@lists.sourceforge.net,
	linux-kernel@vger.kernel.org, sunyibuaa@gmail.com,
	niuzhiguo84@gmail.com, hao_hao.wang@unisoc.com,
	ke.wang@unisoc.com
Subject: Re: [RFC PATCH 2/2] f2fs: introduce f2fs_invalidate_consecutive_blocks()
Date: Wed, 16 Oct 2024 16:04:26 +0000	[thread overview]
Message-ID: <Zw_kCu3J7f7iQrFF@google.com> (raw)
In-Reply-To: <20241016052758.3400359-3-yi.sun@unisoc.com>

2573 void f2fs_invalidate_consecutive_blocks(struct f2fs_sb_info *sbi, block_t addr, int cnt)
2574 {
2575         unsigned int segno = GET_SEGNO(sbi, addr);
2576         unsigned int segno2 = GET_SEGNO(sbi, addr + cnt - 1);
2577         struct sit_info *sit_i = SIT_I(sbi);
2578
2579         f2fs_bug_on(sbi, addr == NULL_ADDR || segno != segno2);

This hits a panic here while running fsstress.

On 10/16, Yi Sun wrote:
> When doing truncate, consecutive blocks in the same segment
> can be processed at the same time. So that the efficiency of
> doing truncate can be improved.
> 
> Add f2fs_invalidate_compress_pages_range() only for doing truncate.
> Add check_f2fs_invalidate_consecutive_blocks() only for doing
> truncate and to determine whether the blocks are continuous and
> belong to the same segment.
> 
> Signed-off-by: Yi Sun <yi.sun@unisoc.com>
> ---
>  fs/f2fs/compress.c | 14 ++++++++++++++
>  fs/f2fs/f2fs.h     |  5 +++++
>  fs/f2fs/file.c     | 34 +++++++++++++++++++++++++++++++++-
>  fs/f2fs/segment.c  | 25 +++++++++++++++++++++++++
>  4 files changed, 77 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/f2fs/compress.c b/fs/f2fs/compress.c
> index 7f26440e8595..70929a87e9bf 100644
> --- a/fs/f2fs/compress.c
> +++ b/fs/f2fs/compress.c
> @@ -2014,6 +2014,20 @@ void f2fs_invalidate_compress_pages(struct f2fs_sb_info *sbi, nid_t ino)
>  	} while (index < end);
>  }
>  
> +void f2fs_invalidate_compress_pages_range(struct f2fs_sb_info *sbi,
> +					block_t blkaddr, int cnt)
> +{
> +	if (!sbi->compress_inode)
> +		return;
> +
> +	if (cnt < 1) {
> +		f2fs_bug_on(sbi, 1);
> +		cnt = 1;
> +	}
> +
> +	invalidate_mapping_pages(COMPRESS_MAPPING(sbi), blkaddr, blkaddr + cnt - 1);
> +}
> +
>  int f2fs_init_compress_inode(struct f2fs_sb_info *sbi)
>  {
>  	struct inode *inode;
> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> index ce00cb546f4a..99767f35678f 100644
> --- a/fs/f2fs/f2fs.h
> +++ b/fs/f2fs/f2fs.h
> @@ -3716,6 +3716,7 @@ int f2fs_create_flush_cmd_control(struct f2fs_sb_info *sbi);
>  int f2fs_flush_device_cache(struct f2fs_sb_info *sbi);
>  void f2fs_destroy_flush_cmd_control(struct f2fs_sb_info *sbi, bool free);
>  void f2fs_invalidate_blocks(struct f2fs_sb_info *sbi, block_t addr);
> +void f2fs_invalidate_consecutive_blocks(struct f2fs_sb_info *sbi, block_t addr, int cnt);
>  bool f2fs_is_checkpointed_data(struct f2fs_sb_info *sbi, block_t blkaddr);
>  int f2fs_start_discard_thread(struct f2fs_sb_info *sbi);
>  void f2fs_drop_discard_cmd(struct f2fs_sb_info *sbi);
> @@ -4375,6 +4376,8 @@ void f2fs_cache_compressed_page(struct f2fs_sb_info *sbi, struct page *page,
>  bool f2fs_load_compressed_page(struct f2fs_sb_info *sbi, struct page *page,
>  								block_t blkaddr);
>  void f2fs_invalidate_compress_pages(struct f2fs_sb_info *sbi, nid_t ino);
> +void f2fs_invalidate_compress_pages_range(struct f2fs_sb_info *sbi,
> +					block_t blkaddr, int cnt);
>  #define inc_compr_inode_stat(inode)					\
>  	do {								\
>  		struct f2fs_sb_info *sbi = F2FS_I_SB(inode);		\
> @@ -4432,6 +4435,8 @@ static inline bool f2fs_load_compressed_page(struct f2fs_sb_info *sbi,
>  				struct page *page, block_t blkaddr) { return false; }
>  static inline void f2fs_invalidate_compress_pages(struct f2fs_sb_info *sbi,
>  							nid_t ino) { }
> +static inline void f2fs_invalidate_compress_pages_range(struct f2fs_sb_info *sbi,
> +						block_t blkaddr, int cnt) { }
>  #define inc_compr_inode_stat(inode)		do { } while (0)
>  static inline int f2fs_is_compressed_cluster(
>  				struct inode *inode,
> diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
> index 7057efa8ec17..634691e3b5f1 100644
> --- a/fs/f2fs/file.c
> +++ b/fs/f2fs/file.c
> @@ -612,6 +612,18 @@ static int f2fs_file_open(struct inode *inode, struct file *filp)
>  	return finish_preallocate_blocks(inode);
>  }
>  
> +static bool check_f2fs_invalidate_consecutive_blocks(struct f2fs_sb_info *sbi,
> +					block_t blkaddr1, block_t blkaddr2)
> +{
> +	if (blkaddr2 - blkaddr1 != 1)
> +		return false;
> +
> +	if (GET_SEGNO(sbi, blkaddr1) != GET_SEGNO(sbi, blkaddr2))
> +		return false;
> +
> +	return true;
> +}
> +
>  void f2fs_truncate_data_blocks_range(struct dnode_of_data *dn, int count)
>  {
>  	struct f2fs_sb_info *sbi = F2FS_I_SB(dn->inode);
> @@ -621,6 +633,9 @@ void f2fs_truncate_data_blocks_range(struct dnode_of_data *dn, int count)
>  	int cluster_index = 0, valid_blocks = 0;
>  	int cluster_size = F2FS_I(dn->inode)->i_cluster_size;
>  	bool released = !atomic_read(&F2FS_I(dn->inode)->i_compr_blocks);
> +	block_t con_start;
> +	bool run_invalid = true;
> +	int con_cnt = 1;
>  
>  	addr = get_dnode_addr(dn->inode, dn->node_page) + ofs;
>  
> @@ -652,7 +667,24 @@ void f2fs_truncate_data_blocks_range(struct dnode_of_data *dn, int count)
>  				valid_blocks++;
>  		}
>  
> -		f2fs_invalidate_blocks(sbi, blkaddr);
> +		if (run_invalid)
> +			con_start = blkaddr;
> +
> +		if (count > 1 &&
> +			check_f2fs_invalidate_consecutive_blocks(sbi, blkaddr,
> +				le32_to_cpu(*(addr + 1)))) {
> +			run_invalid = false;
> +
> +			if (con_cnt++ == 1)
> +				con_start = blkaddr;
> +		} else {
> +			run_invalid = true;
> +		}
> +
> +		if (run_invalid) {
> +			f2fs_invalidate_consecutive_blocks(sbi, con_start, con_cnt);
> +			con_cnt = 1;
> +		}
>  
>  		if (!released || blkaddr != COMPRESS_ADDR)
>  			nr_free++;
> diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
> index f118faf36d35..edb8a78985ba 100644
> --- a/fs/f2fs/segment.c
> +++ b/fs/f2fs/segment.c
> @@ -2570,6 +2570,31 @@ void f2fs_invalidate_blocks(struct f2fs_sb_info *sbi, block_t addr)
>  	up_write(&sit_i->sentry_lock);
>  }
>  
> +void f2fs_invalidate_consecutive_blocks(struct f2fs_sb_info *sbi, block_t addr, int cnt)
> +{
> +	unsigned int segno = GET_SEGNO(sbi, addr);
> +	unsigned int segno2 = GET_SEGNO(sbi, addr + cnt - 1);
> +	struct sit_info *sit_i = SIT_I(sbi);
> +
> +	f2fs_bug_on(sbi, addr == NULL_ADDR || segno != segno2);
> +	if (addr == NEW_ADDR || addr == COMPRESS_ADDR)
> +		return;
> +
> +	f2fs_truncate_meta_inode_pages(sbi, addr, cnt);
> +	f2fs_invalidate_compress_pages_range(sbi, addr, cnt);
> +
> +	/* add it into sit main buffer */
> +	down_write(&sit_i->sentry_lock);
> +
> +	update_segment_mtime(sbi, addr, 0);
> +	update_sit_entry(sbi, addr, -cnt);
> +
> +	/* add it into dirty seglist */
> +	locate_dirty_segment(sbi, segno);
> +
> +	up_write(&sit_i->sentry_lock);
> +}
> +
>  bool f2fs_is_checkpointed_data(struct f2fs_sb_info *sbi, block_t blkaddr)
>  {
>  	struct sit_info *sit_i = SIT_I(sbi);
> -- 
> 2.25.1

  reply	other threads:[~2024-10-16 16:04 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-10-16  5:27 [f2fs-dev] [RFC PATCH 0/2] Speed up f2fs truncate Yi Sun
2024-10-16  5:27 ` Yi Sun
2024-10-16  5:27 ` [f2fs-dev] [RFC PATCH 1/2] f2fs: introduce update_sit_entry_for_release() Yi Sun
2024-10-16  5:27   ` Yi Sun
2024-10-16  5:27 ` [f2fs-dev] [RFC PATCH 2/2] f2fs: introduce f2fs_invalidate_consecutive_blocks() Yi Sun
2024-10-16  5:27   ` Yi Sun
2024-10-16 16:04   ` Jaegeuk Kim via Linux-f2fs-devel [this message]
2024-10-16 16:04     ` Jaegeuk Kim
2024-10-17  1:40   ` [f2fs-dev] " Chao Yu via Linux-f2fs-devel
2024-10-17  1:40     ` Chao Yu
2024-10-24  9:54     ` [f2fs-dev] " yi sun
2024-10-24  9:54       ` yi sun
2024-10-24 10:26       ` [f2fs-dev] " Chao Yu via Linux-f2fs-devel
2024-10-24 10:26         ` Chao Yu
2024-10-29  3:02         ` [f2fs-dev] " yi sun
2024-10-29  3:02           ` yi sun
2024-10-28 17:40 ` [f2fs-dev] [RFC PATCH 0/2] Speed up f2fs truncate patchwork-bot+f2fs--- via Linux-f2fs-devel
2024-10-28 17:40   ` 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=Zw_kCu3J7f7iQrFF@google.com \
    --to=linux-f2fs-devel@lists.sourceforge.net \
    --cc=hao_hao.wang@unisoc.com \
    --cc=jaegeuk@kernel.org \
    --cc=ke.wang@unisoc.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=sunyibuaa@gmail.com \
    --cc=yi.sun@unisoc.com \
    /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.