[f2fs-dev] [PATCH V8] f2fs: unify the error handling of f2fs_is_valid_blkaddr

2024-03-07 Thread Zhiguo Niu 
There are some cases of f2fs_is_valid_blkaddr not handled as
ERROR_INVALID_BLKADDR,so unify the error handling about all of
f2fs_is_valid_blkaddr.
Do f2fs_handle_error in __f2fs_is_valid_blkaddr for cleanup.

Signed-off-by: Zhiguo Niu 
Signed-off-by: Chao Yu 
---
v8: do reboot/monkey stability test for this version and result pass.
do f2fs_handle_error in __is_bitmap_valid  for resolving the
ugly code problem mentioned by Jaegeuk, besides handle the situation
of “type == DATA_GENERIC_ENHANCE_UPDATE” reasonably.
---
---
 fs/f2fs/checkpoint.c   | 41 ++---
 fs/f2fs/data.c | 22 +++---
 fs/f2fs/extent_cache.c |  5 +
 fs/f2fs/file.c | 16 +++-
 fs/f2fs/gc.c   |  2 --
 fs/f2fs/recovery.c |  4 
 fs/f2fs/segment.c  |  2 --
 7 files changed, 29 insertions(+), 63 deletions(-)

diff --git a/fs/f2fs/checkpoint.c b/fs/f2fs/checkpoint.c
index a09a960..b8a7e83 100644
--- a/fs/f2fs/checkpoint.c
+++ b/fs/f2fs/checkpoint.c
@@ -154,19 +154,19 @@ static bool __is_bitmap_valid(struct f2fs_sb_info *sbi, 
block_t blkaddr,
if (unlikely(f2fs_cp_error(sbi)))
return exist;
 
-   if (exist && type == DATA_GENERIC_ENHANCE_UPDATE) {
-   f2fs_err(sbi, "Inconsistent error blkaddr:%u, sit bitmap:%d",
-blkaddr, exist);
-   set_sbi_flag(sbi, SBI_NEED_FSCK);
-   return exist;
-   }
-
-   if (!exist && type == DATA_GENERIC_ENHANCE) {
-   f2fs_err(sbi, "Inconsistent error blkaddr:%u, sit bitmap:%d",
-blkaddr, exist);
-   set_sbi_flag(sbi, SBI_NEED_FSCK);
-   dump_stack();
-   }
+   if ((exist && type == DATA_GENERIC_ENHANCE_UPDATE) ||
+   (!exist && type == DATA_GENERIC_ENHANCE))
+   goto out_err;
+   if (!exist && type != DATA_GENERIC_ENHANCE_UPDATE)
+   goto out_handle;
+   return exist;
+out_err:
+   f2fs_err(sbi, "Inconsistent error blkaddr:%u, sit bitmap:%d",
+blkaddr, exist);
+   set_sbi_flag(sbi, SBI_NEED_FSCK);
+   dump_stack();
+out_handle:
+   f2fs_handle_error(sbi, ERROR_INVALID_BLKADDR);
return exist;
 }
 
@@ -178,22 +178,22 @@ static bool __f2fs_is_valid_blkaddr(struct f2fs_sb_info 
*sbi,
break;
case META_SIT:
if (unlikely(blkaddr >= SIT_BLK_CNT(sbi)))
-   return false;
+   goto err;
break;
case META_SSA:
if (unlikely(blkaddr >= MAIN_BLKADDR(sbi) ||
blkaddr < SM_I(sbi)->ssa_blkaddr))
-   return false;
+   goto err;
break;
case META_CP:
if (unlikely(blkaddr >= SIT_I(sbi)->sit_base_addr ||
blkaddr < __start_cp_addr(sbi)))
-   return false;
+   goto err;
break;
case META_POR:
if (unlikely(blkaddr >= MAX_BLKADDR(sbi) ||
blkaddr < MAIN_BLKADDR(sbi)))
-   return false;
+   goto err;
break;
case DATA_GENERIC:
case DATA_GENERIC_ENHANCE:
@@ -210,7 +210,7 @@ static bool __f2fs_is_valid_blkaddr(struct f2fs_sb_info 
*sbi,
  blkaddr);
set_sbi_flag(sbi, SBI_NEED_FSCK);
dump_stack();
-   return false;
+   goto err;
} else {
return __is_bitmap_valid(sbi, blkaddr, type);
}
@@ -218,13 +218,16 @@ static bool __f2fs_is_valid_blkaddr(struct f2fs_sb_info 
*sbi,
case META_GENERIC:
if (unlikely(blkaddr < SEG0_BLKADDR(sbi) ||
blkaddr >= MAIN_BLKADDR(sbi)))
-   return false;
+   goto err;
break;
default:
BUG();
}
 
return true;
+err:
+   f2fs_handle_error(sbi, ERROR_INVALID_BLKADDR);
+   return false;
 }
 
 bool f2fs_is_valid_blkaddr(struct f2fs_sb_info *sbi,
diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
index 2fbbf8f..f8ae684 100644
--- a/fs/f2fs/data.c
+++ b/fs/f2fs/data.c
@@ -690,10 +690,8 @@ int f2fs_submit_page_bio(struct f2fs_io_info *fio)
 
if (!f2fs_is_valid_blkaddr(fio->sbi, fio->new_blkaddr,
fio->is_por ? META_POR : (__is_meta_io(fio) ?
-   META_GENERIC : DATA_GENERIC_ENHANCE))) {
-   f2fs_handle_error(fio->sbi, ERROR_INVALID_BLKADDR);
+   META_GENERIC : DATA_GENERIC_ENHANCE)))
return -EFSCORRUPTED;
-   }
 
trace_f2fs_submit_page_bio(page, fio);
 
@@ -890,10 +888,8 @@ int f2fs_merge_page_bio(struct f2fs_io_info *fio)

Re: [f2fs-dev] [PATCH 1/5] f2fs: fix to wait page writeback before update

2024-03-07 Thread Chao Yu 

On 2020/6/20 6:47, Jaegeuk Kim wrote:

diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
index 3268f8dd59bb..1862073b96d2 100644
--- a/fs/f2fs/file.c
+++ b/fs/f2fs/file.c
@@ -1250,6 +1250,7 @@ static int __clone_blkaddrs(struct inode *src_inode, 
struct inode *dst_inode,
f2fs_put_page(psrc, 1);
return PTR_ERR(pdst);
}
+   f2fs_wait_on_page_writeback(pdst, DATA, true, true);


Do you mean pdst can be under writeback?


Jaegeuk,

Look back this again, is it possible that pdst is writebacking
in below race condition? or am I missing something?

Thread AGC Thread
- f2fs_move_file_range
 - filemap_write_and_wait_range(dst)
- gc_data_segment
 - f2fs_down_write(dst)
 - move_data_page
  - set_page_writeback(dst_page)
  - f2fs_submit_page_write
 - f2fs_up_write(dst)
 - f2fs_down_write(dst)
 - __exchange_data_block
  - __clone_blkaddrs
   - f2fs_get_new_data_page
   - memcpy_page

Thanks,


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

[f2fs-dev] [PATCH] f2fs: zone: fix to remove pow2 check condition for zoned block device

2024-03-07 Thread Chao Yu 
Commit 2e2c6e9b72ce ("f2fs: remove power-of-two limitation of zoned
device") missed to remove pow2 check condition in init_blkz_info(),
fix it.

Fixes: 2e2c6e9b72ce ("f2fs: remove power-of-two limitation of zoned device")
Signed-off-by: Feng Song 
Signed-off-by: Yongpeng Yang 
Signed-off-by: Chao Yu 
---
 fs/f2fs/super.c | 5 -
 1 file changed, 5 deletions(-)

diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
index 0676c2dcbbf7..fb4c1ed72710 100644
--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -3874,11 +3874,6 @@ static int init_blkz_info(struct f2fs_sb_info *sbi, int 
devi)
return 0;
 
zone_sectors = bdev_zone_sectors(bdev);
-   if (!is_power_of_2(zone_sectors)) {
-   f2fs_err(sbi, "F2FS does not support non power of 2 zone 
sizes\n");
-   return -EINVAL;
-   }
-
if (sbi->blocks_per_blkz && sbi->blocks_per_blkz !=
SECTOR_TO_BLOCK(zone_sectors))
return -EINVAL;
-- 
2.40.1



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

[f2fs-dev] [PATCH v2] f2fs: fix to truncate meta inode pages forcely

2024-03-07 Thread Chao Yu 
Below race case can cause data corruption:

Thread AGC thread
- gc_data_segment
 - ra_data_block
  - locked meta_inode page
- f2fs_inplace_write_data
 - invalidate_mapping_pages
 : fail to invalidate meta_inode page
   due to lock failure or dirty|writeback
   status
 - f2fs_submit_page_bio
 : write last dirty data to old blkaddr
 - move_data_block
  - load old data from meta_inode page
  - f2fs_submit_page_write
  : write old data to new blkaddr

Because invalidate_mapping_pages() will skip invalidating page which
has unclear status including locked, dirty, writeback and so on, so
we need to use truncate_inode_pages_range() instead of
invalidate_mapping_pages() to make sure meta_inode page will be dropped.

Fixes: 6aa58d8ad20a ("f2fs: readahead encrypted block during GC")
Fixes: e3b49ea36802 ("f2fs: invalidate META_MAPPING before IPU/DIO write")
Signed-off-by: Chao Yu 
---
v2:
- fix race condition description.
 fs/f2fs/checkpoint.c|  5 +++--
 fs/f2fs/f2fs.h  | 28 +++-
 fs/f2fs/segment.c   |  5 ++---
 include/linux/f2fs_fs.h |  1 +
 4 files changed, 33 insertions(+), 6 deletions(-)

diff --git a/fs/f2fs/checkpoint.c b/fs/f2fs/checkpoint.c
index a09a9609e228..55b7d2cf030f 100644
--- a/fs/f2fs/checkpoint.c
+++ b/fs/f2fs/checkpoint.c
@@ -1598,8 +1598,9 @@ static int do_checkpoint(struct f2fs_sb_info *sbi, struct 
cp_control *cpc)
 */
if (f2fs_sb_has_encrypt(sbi) || f2fs_sb_has_verity(sbi) ||
f2fs_sb_has_compression(sbi))
-   invalidate_mapping_pages(META_MAPPING(sbi),
-   MAIN_BLKADDR(sbi), MAX_BLKADDR(sbi) - 1);
+   f2fs_bug_on(sbi,
+   invalidate_inode_pages2_range(META_MAPPING(sbi),
+   MAIN_BLKADDR(sbi), MAX_BLKADDR(sbi) - 1));
 
f2fs_release_ino_entry(sbi, false);
 
diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index 4836e7cb0efe..9814e5981a6a 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -4655,10 +4655,36 @@ static inline bool f2fs_is_readonly(struct f2fs_sb_info 
*sbi)
return f2fs_sb_has_readonly(sbi) || f2fs_readonly(sbi->sb);
 }
 
+static inline void f2fs_truncate_meta_inode_pages(struct f2fs_sb_info *sbi,
+   block_t blkaddr, unsigned int cnt)
+{
+   bool need_submit = false;
+   int i = 0;
+
+   do {
+   struct page *page;
+
+   page = find_get_page(META_MAPPING(sbi), blkaddr + i);
+   if (page) {
+   if (PageWriteback(page))
+   need_submit = true;
+   f2fs_put_page(page, 0);
+   }
+   } while (++i < cnt && !need_submit);
+
+   if (need_submit)
+   f2fs_submit_merged_write_cond(sbi, sbi->meta_inode,
+   NULL, 0, DATA);
+
+   truncate_inode_pages_range(META_MAPPING(sbi),
+   F2FS_BLK_TO_BYTES((loff_t)blkaddr),
+   F2FS_BLK_END_BYTES((loff_t)(blkaddr + cnt - 1)));
+}
+
 static inline void f2fs_invalidate_internal_cache(struct f2fs_sb_info *sbi,
block_t blkaddr)
 {
-   invalidate_mapping_pages(META_MAPPING(sbi), blkaddr, blkaddr);
+   f2fs_truncate_meta_inode_pages(sbi, blkaddr, 1);
f2fs_invalidate_compress_page(sbi, blkaddr);
 }
 
diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
index 4ff3b2d14ddf..20af48d7f784 100644
--- a/fs/f2fs/segment.c
+++ b/fs/f2fs/segment.c
@@ -3741,8 +3741,7 @@ int f2fs_inplace_write_data(struct f2fs_io_info *fio)
}
 
if (fio->post_read)
-   invalidate_mapping_pages(META_MAPPING(sbi),
-   fio->new_blkaddr, fio->new_blkaddr);
+   f2fs_truncate_meta_inode_pages(sbi, fio->new_blkaddr, 1);
 
stat_inc_inplace_blocks(fio->sbi);
 
@@ -3932,7 +3931,7 @@ void f2fs_wait_on_block_writeback_range(struct inode 
*inode, block_t blkaddr,
for (i = 0; i < len; i++)
f2fs_wait_on_block_writeback(inode, blkaddr + i);
 
-   invalidate_mapping_pages(META_MAPPING(sbi), blkaddr, blkaddr + len - 1);
+   f2fs_truncate_meta_inode_pages(sbi, blkaddr, len);
 }
 
 static int read_compacted_summaries(struct f2fs_sb_info *sbi)
diff --git a/include/linux/f2fs_fs.h b/include/linux/f2fs_fs.h
index 755e9a41b196..a357287eac1e 100644
--- a/include/linux/f2fs_fs.h
+++ b/include/linux/f2fs_fs.h
@@ -27,6 +27,7 @@
 
 #define F2FS_BYTES_TO_BLK(bytes)   ((bytes) >> F2FS_BLKSIZE_BITS)
 #define F2FS_BLK_TO_BYTES(blk) ((blk) << F2FS_BLKSIZE_BITS)

Re: [f2fs-dev] [PATCH] f2fs: fix to truncate meta inode pages forcely

2024-03-07 Thread Chao Yu 

On 2024/3/8 2:37, Jaegeuk Kim wrote:

On 03/07, Chao Yu wrote:

Below race case can cause data corruption:

Thread AGC thread
- f2fs_inplace_write_data
- gc_data_segment
 - ra_data_block
  - locked meta_inode page
  - invalidate_mapping_pages
  : fail to invalidate meta_inode page
due to lock failure or dirty|writeback
status


Wasn't the original data page locked in both cases?


Oh, the race case needs to fixed as below:

Thread AGC thread
- gc_data_segment
 - ra_data_block
  - locked meta_inode page
- f2fs_inplace_write_data
 - invalidate_mapping_pages
 : fail to invalidate meta_inode page
   due to lock failure or dirty|writeback
   status
 - f2fs_submit_page_bio
 : write last dirty data to old blkaddr
 - move_data_block
  - load old data from meta_inode page
  - f2fs_submit_page_write
  : write old data to new blkaddr

There is a hole in between ra_data_block() and move_data_block(),
in where the data page is unlocked.

Thanks,




  - f2fs_submit_page_bio
  : write last dirty data to old blkaddr
 - move_data_block
  - load old data from meta_inode page
  - f2fs_submit_page_write
  : write old data to new blkaddr

Because invalidate_mapping_pages() will skip invalidating page when the
page has unclear status including locked, dirty, writeback and so on, so
we need to use truncate_inode_pages_range() instead of
invalidate_mapping_pages() to make sure meta_inode page will be dropped.

Fixes: 6aa58d8ad20a ("f2fs: readahead encrypted block during GC")
Fixes: e3b49ea36802 ("f2fs: invalidate META_MAPPING before IPU/DIO write")
Signed-off-by: Chao Yu 
---
  fs/f2fs/checkpoint.c|  5 +++--
  fs/f2fs/f2fs.h  | 28 +++-
  fs/f2fs/segment.c   |  5 ++---
  include/linux/f2fs_fs.h |  1 +
  4 files changed, 33 insertions(+), 6 deletions(-)

diff --git a/fs/f2fs/checkpoint.c b/fs/f2fs/checkpoint.c
index a09a9609e228..55b7d2cf030f 100644
--- a/fs/f2fs/checkpoint.c
+++ b/fs/f2fs/checkpoint.c
@@ -1598,8 +1598,9 @@ static int do_checkpoint(struct f2fs_sb_info *sbi, struct 
cp_control *cpc)
 */
if (f2fs_sb_has_encrypt(sbi) || f2fs_sb_has_verity(sbi) ||
f2fs_sb_has_compression(sbi))
-   invalidate_mapping_pages(META_MAPPING(sbi),
-   MAIN_BLKADDR(sbi), MAX_BLKADDR(sbi) - 1);
+   f2fs_bug_on(sbi,
+   invalidate_inode_pages2_range(META_MAPPING(sbi),
+   MAIN_BLKADDR(sbi), MAX_BLKADDR(sbi) - 1));
  
  	f2fs_release_ino_entry(sbi, false);
  
diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h

index 4836e7cb0efe..9814e5981a6a 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -4655,10 +4655,36 @@ static inline bool f2fs_is_readonly(struct f2fs_sb_info 
*sbi)
return f2fs_sb_has_readonly(sbi) || f2fs_readonly(sbi->sb);
  }
  
+static inline void f2fs_truncate_meta_inode_pages(struct f2fs_sb_info *sbi,

+   block_t blkaddr, unsigned int cnt)
+{
+   bool need_submit = false;
+   int i = 0;
+
+   do {
+   struct page *page;
+
+   page = find_get_page(META_MAPPING(sbi), blkaddr + i);
+   if (page) {
+   if (PageWriteback(page))
+   need_submit = true;
+   f2fs_put_page(page, 0);
+   }
+   } while (++i < cnt && !need_submit);
+
+   if (need_submit)
+   f2fs_submit_merged_write_cond(sbi, sbi->meta_inode,
+   NULL, 0, DATA);
+
+   truncate_inode_pages_range(META_MAPPING(sbi),
+   F2FS_BLK_TO_BYTES((loff_t)blkaddr),
+   F2FS_BLK_END_BYTES((loff_t)(blkaddr + cnt - 1)));
+}
+
  static inline void f2fs_invalidate_internal_cache(struct f2fs_sb_info *sbi,
block_t blkaddr)
  {
-   invalidate_mapping_pages(META_MAPPING(sbi), blkaddr, blkaddr);
+   f2fs_truncate_meta_inode_pages(sbi, blkaddr, 1);
f2fs_invalidate_compress_page(sbi, blkaddr);
  }
  
diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c

index 4ff3b2d14ddf..20af48d7f784 100644
--- a/fs/f2fs/segment.c
+++ b/fs/f2fs/segment.c
@@ -3741,8 +3741,7 @@ int f2fs_inplace_write_data(struct f2fs_io_info *fio)
}
  
  	if (fio->post_read)

-

Re: [f2fs-dev] [PATCH] f2fs: fix to truncate meta inode pages forcely

2024-03-07 Thread Jaegeuk Kim 
On 03/07, Chao Yu wrote:
> Below race case can cause data corruption:
> 
> Thread A  GC thread
> - f2fs_inplace_write_data
>   - gc_data_segment
>- ra_data_block
> - locked meta_inode page
>  - invalidate_mapping_pages
>  : fail to invalidate meta_inode page
>due to lock failure or dirty|writeback
>status

Wasn't the original data page locked in both cases?

>  - f2fs_submit_page_bio
>  : write last dirty data to old blkaddr
>- move_data_block
> - load old data from meta_inode page
> - f2fs_submit_page_write
> : write old data to new blkaddr
> 
> Because invalidate_mapping_pages() will skip invalidating page when the
> page has unclear status including locked, dirty, writeback and so on, so
> we need to use truncate_inode_pages_range() instead of
> invalidate_mapping_pages() to make sure meta_inode page will be dropped.
> 
> Fixes: 6aa58d8ad20a ("f2fs: readahead encrypted block during GC")
> Fixes: e3b49ea36802 ("f2fs: invalidate META_MAPPING before IPU/DIO write")
> Signed-off-by: Chao Yu 
> ---
>  fs/f2fs/checkpoint.c|  5 +++--
>  fs/f2fs/f2fs.h  | 28 +++-
>  fs/f2fs/segment.c   |  5 ++---
>  include/linux/f2fs_fs.h |  1 +
>  4 files changed, 33 insertions(+), 6 deletions(-)
> 
> diff --git a/fs/f2fs/checkpoint.c b/fs/f2fs/checkpoint.c
> index a09a9609e228..55b7d2cf030f 100644
> --- a/fs/f2fs/checkpoint.c
> +++ b/fs/f2fs/checkpoint.c
> @@ -1598,8 +1598,9 @@ static int do_checkpoint(struct f2fs_sb_info *sbi, 
> struct cp_control *cpc)
>*/
>   if (f2fs_sb_has_encrypt(sbi) || f2fs_sb_has_verity(sbi) ||
>   f2fs_sb_has_compression(sbi))
> - invalidate_mapping_pages(META_MAPPING(sbi),
> - MAIN_BLKADDR(sbi), MAX_BLKADDR(sbi) - 1);
> + f2fs_bug_on(sbi,
> + invalidate_inode_pages2_range(META_MAPPING(sbi),
> + MAIN_BLKADDR(sbi), MAX_BLKADDR(sbi) - 1));
>  
>   f2fs_release_ino_entry(sbi, false);
>  
> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> index 4836e7cb0efe..9814e5981a6a 100644
> --- a/fs/f2fs/f2fs.h
> +++ b/fs/f2fs/f2fs.h
> @@ -4655,10 +4655,36 @@ static inline bool f2fs_is_readonly(struct 
> f2fs_sb_info *sbi)
>   return f2fs_sb_has_readonly(sbi) || f2fs_readonly(sbi->sb);
>  }
>  
> +static inline void f2fs_truncate_meta_inode_pages(struct f2fs_sb_info *sbi,
> + block_t blkaddr, unsigned int cnt)
> +{
> + bool need_submit = false;
> + int i = 0;
> +
> + do {
> + struct page *page;
> +
> + page = find_get_page(META_MAPPING(sbi), blkaddr + i);
> + if (page) {
> + if (PageWriteback(page))
> + need_submit = true;
> + f2fs_put_page(page, 0);
> + }
> + } while (++i < cnt && !need_submit);
> +
> + if (need_submit)
> + f2fs_submit_merged_write_cond(sbi, sbi->meta_inode,
> + NULL, 0, DATA);
> +
> + truncate_inode_pages_range(META_MAPPING(sbi),
> + F2FS_BLK_TO_BYTES((loff_t)blkaddr),
> + F2FS_BLK_END_BYTES((loff_t)(blkaddr + cnt - 1)));
> +}
> +
>  static inline void f2fs_invalidate_internal_cache(struct f2fs_sb_info *sbi,
>   block_t blkaddr)
>  {
> - invalidate_mapping_pages(META_MAPPING(sbi), blkaddr, blkaddr);
> + f2fs_truncate_meta_inode_pages(sbi, blkaddr, 1);
>   f2fs_invalidate_compress_page(sbi, blkaddr);
>  }
>  
> diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
> index 4ff3b2d14ddf..20af48d7f784 100644
> --- a/fs/f2fs/segment.c
> +++ b/fs/f2fs/segment.c
> @@ -3741,8 +3741,7 @@ int f2fs_inplace_write_data(struct f2fs_io_info *fio)
>   }
>  
>   if (fio->post_read)
> - invalidate_mapping_pages(META_MAPPING(sbi),
> - fio->new_blkaddr, fio->new_blkaddr);
> + f2fs_truncate_meta_inode_pages(sbi, fio->new_blkaddr, 1);
>  
>   stat_inc_inplace_blocks(fio->sbi);
>  
> @@ -3932,7 +3931,7 @@ void f2fs_wait_on_block_writeback_range(struct inode 
> *inode, block_t blkaddr,
>   for (i = 0; i < len; i++)
>   f2fs_wait_on_block_writeback(inode, blkaddr + i);
>  
> - invalidate_mapping_pages(META_MAPPING(sbi), blkaddr, blkaddr + len - 1);
> + f2fs_truncate_meta_inode_pages(sbi, blkaddr, len);
>  }
>  
>  static int read_compacted_summaries(struct f2fs_sb_info *sbi)
> diff --git a/include/linux/f2fs_fs.h b/include/linux/f2fs_fs.h
> index 755e9a41b196..a357287eac1e 100644
> --- a/include/linux/f2fs_fs.h
> +++

[f2fs-dev] [PATCH] f2fs: fix to truncate meta inode pages forcely

2024-03-07 Thread Chao Yu 
Below race case can cause data corruption:

Thread AGC thread
- f2fs_inplace_write_data
- gc_data_segment
 - ra_data_block
  - locked meta_inode page
 - invalidate_mapping_pages
 : fail to invalidate meta_inode page
   due to lock failure or dirty|writeback
   status
 - f2fs_submit_page_bio
 : write last dirty data to old blkaddr
 - move_data_block
  - load old data from meta_inode page
  - f2fs_submit_page_write
  : write old data to new blkaddr

Because invalidate_mapping_pages() will skip invalidating page when the
page has unclear status including locked, dirty, writeback and so on, so
we need to use truncate_inode_pages_range() instead of
invalidate_mapping_pages() to make sure meta_inode page will be dropped.

Fixes: 6aa58d8ad20a ("f2fs: readahead encrypted block during GC")
Fixes: e3b49ea36802 ("f2fs: invalidate META_MAPPING before IPU/DIO write")
Signed-off-by: Chao Yu 
---
 fs/f2fs/checkpoint.c|  5 +++--
 fs/f2fs/f2fs.h  | 28 +++-
 fs/f2fs/segment.c   |  5 ++---
 include/linux/f2fs_fs.h |  1 +
 4 files changed, 33 insertions(+), 6 deletions(-)

diff --git a/fs/f2fs/checkpoint.c b/fs/f2fs/checkpoint.c
index a09a9609e228..55b7d2cf030f 100644
--- a/fs/f2fs/checkpoint.c
+++ b/fs/f2fs/checkpoint.c
@@ -1598,8 +1598,9 @@ static int do_checkpoint(struct f2fs_sb_info *sbi, struct 
cp_control *cpc)
 */
if (f2fs_sb_has_encrypt(sbi) || f2fs_sb_has_verity(sbi) ||
f2fs_sb_has_compression(sbi))
-   invalidate_mapping_pages(META_MAPPING(sbi),
-   MAIN_BLKADDR(sbi), MAX_BLKADDR(sbi) - 1);
+   f2fs_bug_on(sbi,
+   invalidate_inode_pages2_range(META_MAPPING(sbi),
+   MAIN_BLKADDR(sbi), MAX_BLKADDR(sbi) - 1));
 
f2fs_release_ino_entry(sbi, false);
 
diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index 4836e7cb0efe..9814e5981a6a 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -4655,10 +4655,36 @@ static inline bool f2fs_is_readonly(struct f2fs_sb_info 
*sbi)
return f2fs_sb_has_readonly(sbi) || f2fs_readonly(sbi->sb);
 }
 
+static inline void f2fs_truncate_meta_inode_pages(struct f2fs_sb_info *sbi,
+   block_t blkaddr, unsigned int cnt)
+{
+   bool need_submit = false;
+   int i = 0;
+
+   do {
+   struct page *page;
+
+   page = find_get_page(META_MAPPING(sbi), blkaddr + i);
+   if (page) {
+   if (PageWriteback(page))
+   need_submit = true;
+   f2fs_put_page(page, 0);
+   }
+   } while (++i < cnt && !need_submit);
+
+   if (need_submit)
+   f2fs_submit_merged_write_cond(sbi, sbi->meta_inode,
+   NULL, 0, DATA);
+
+   truncate_inode_pages_range(META_MAPPING(sbi),
+   F2FS_BLK_TO_BYTES((loff_t)blkaddr),
+   F2FS_BLK_END_BYTES((loff_t)(blkaddr + cnt - 1)));
+}
+
 static inline void f2fs_invalidate_internal_cache(struct f2fs_sb_info *sbi,
block_t blkaddr)
 {
-   invalidate_mapping_pages(META_MAPPING(sbi), blkaddr, blkaddr);
+   f2fs_truncate_meta_inode_pages(sbi, blkaddr, 1);
f2fs_invalidate_compress_page(sbi, blkaddr);
 }
 
diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
index 4ff3b2d14ddf..20af48d7f784 100644
--- a/fs/f2fs/segment.c
+++ b/fs/f2fs/segment.c
@@ -3741,8 +3741,7 @@ int f2fs_inplace_write_data(struct f2fs_io_info *fio)
}
 
if (fio->post_read)
-   invalidate_mapping_pages(META_MAPPING(sbi),
-   fio->new_blkaddr, fio->new_blkaddr);
+   f2fs_truncate_meta_inode_pages(sbi, fio->new_blkaddr, 1);
 
stat_inc_inplace_blocks(fio->sbi);
 
@@ -3932,7 +3931,7 @@ void f2fs_wait_on_block_writeback_range(struct inode 
*inode, block_t blkaddr,
for (i = 0; i < len; i++)
f2fs_wait_on_block_writeback(inode, blkaddr + i);
 
-   invalidate_mapping_pages(META_MAPPING(sbi), blkaddr, blkaddr + len - 1);
+   f2fs_truncate_meta_inode_pages(sbi, blkaddr, len);
 }
 
 static int read_compacted_summaries(struct f2fs_sb_info *sbi)
diff --git a/include/linux/f2fs_fs.h b/include/linux/f2fs_fs.h
index 755e9a41b196..a357287eac1e 100644
--- a/include/linux/f2fs_fs.h
+++ b/include/linux/f2fs_fs.h
@@ -27,6 +27,7 @@
 
 #define F2FS_BYTES_TO_BLK(bytes)   ((bytes) >> F2FS_BLKSIZE_BITS)
 #define F2FS_BLK_TO_BYTES(blk) ((blk) << F2FS_BLKSIZE_BITS)
+#define F2FS_BLK_END_BYTES(blk)