Re: [f2fs-dev] [PATCH v3 1/6] f2fs: compress: fix to guarantee persisting compressed blocks by CP

[Chao Yu]

On 2024/1/11 8:55, Daeho Jeong wrote:

On Thu, Dec 28, 2023 at 6:33 AM Chao Yu <c...@kernel.org> wrote:

If data block in compressed cluster is not persisted with metadata
during checkpoint, after SPOR, the data may be corrupted, let's
guarantee to write compressed page by checkpoint.

Fixes: 4c8ff7095bef ("f2fs: support data compression")
Signed-off-by: Chao Yu <c...@kernel.org>
---
v3:
- treat compressed page as CP guaranteed data explictly.
  fs/f2fs/compress.c |  4 +++-
  fs/f2fs/data.c     | 17 +++++++++--------
  fs/f2fs/f2fs.h     |  4 +++-
  3 files changed, 15 insertions(+), 10 deletions(-)

diff --git a/fs/f2fs/compress.c b/fs/f2fs/compress.c
index c5a4364c4482..9940b7886e5d 100644
--- a/fs/f2fs/compress.c
+++ b/fs/f2fs/compress.c
@@ -1418,6 +1418,8 @@ void f2fs_compress_write_end_io(struct bio *bio, struct 
page *page)
         struct f2fs_sb_info *sbi = bio->bi_private;
         struct compress_io_ctx *cic =
                         (struct compress_io_ctx *)page_private(page);
+       enum count_type type = WB_DATA_TYPE(page,
+                               f2fs_is_compressed_page(page));
         int i;

         if (unlikely(bio->bi_status))
@@ -1425,7 +1427,7 @@ void f2fs_compress_write_end_io(struct bio *bio, struct 
page *page)

         f2fs_compress_free_page(page);

-       dec_page_count(sbi, F2FS_WB_DATA);
+       dec_page_count(sbi, type);

         if (atomic_dec_return(&cic->pending_pages))
                 return;
diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
index dce8defdf4c7..81f9e2cc49e2 100644
--- a/fs/f2fs/data.c
+++ b/fs/f2fs/data.c
@@ -48,7 +48,7 @@ void f2fs_destroy_bioset(void)
         bioset_exit(&f2fs_bioset);
  }

-static bool __is_cp_guaranteed(struct page *page)
+bool f2fs_is_cp_guaranteed(struct page *page)
  {
         struct address_space *mapping = page->mapping;
         struct inode *inode;
@@ -65,8 +65,6 @@ static bool __is_cp_guaranteed(struct page *page)
                         S_ISDIR(inode->i_mode))
                 return true;

-       if (f2fs_is_compressed_page(page))
-               return false;

Out of curiosity, why don't we simply change the above to "return true"?

Daeho,

I used the implementation, please check v1 and related comments
from Jaegeuk and me, let me know if that was not clear enough. :)

https://lore.kernel.org/linux-f2fs-devel/aae654e7-8a7e-478d-9f5a-65807a0e0...@kernel.org/

         if ((S_ISREG(inode->i_mode) && IS_NOQUOTA(inode)) ||
                         page_private_gcing(page))
                 return true;
@@ -338,7 +336,7 @@ static void f2fs_write_end_io(struct bio *bio)

         bio_for_each_segment_all(bvec, bio, iter_all) {
                 struct page *page = bvec->bv_page;
-               enum count_type type = WB_DATA_TYPE(page);
+               enum count_type type = WB_DATA_TYPE(page, false);

                 if (page_private_dummy(page)) {
                         clear_page_private_dummy(page);
@@ -762,7 +760,7 @@ int f2fs_submit_page_bio(struct f2fs_io_info *fio)
                 wbc_account_cgroup_owner(fio->io_wbc, fio->page, PAGE_SIZE);

         inc_page_count(fio->sbi, is_read_io(fio->op) ?
-                       __read_io_type(page) : WB_DATA_TYPE(fio->page));
+                       __read_io_type(page) : WB_DATA_TYPE(fio->page, false));

         if (is_read_io(bio_op(bio)))
                 f2fs_submit_read_bio(fio->sbi, bio, fio->type);
@@ -973,7 +971,7 @@ int f2fs_merge_page_bio(struct f2fs_io_info *fio)
         if (fio->io_wbc)
                 wbc_account_cgroup_owner(fio->io_wbc, fio->page, PAGE_SIZE);

-       inc_page_count(fio->sbi, WB_DATA_TYPE(page));
+       inc_page_count(fio->sbi, WB_DATA_TYPE(page, false));

         *fio->last_block = fio->new_blkaddr;
         *fio->bio = bio;
@@ -1007,6 +1005,7 @@ void f2fs_submit_page_write(struct f2fs_io_info *fio)
         enum page_type btype = PAGE_TYPE_OF_BIO(fio->type);
         struct f2fs_bio_info *io = sbi->write_io[btype] + fio->temp;
         struct page *bio_page;
+       enum count_type type;

         f2fs_bug_on(sbi, is_read_io(fio->op));

@@ -1046,7 +1045,8 @@ void f2fs_submit_page_write(struct f2fs_io_info *fio)
         /* set submitted = true as a return value */
         fio->submitted = 1;

-       inc_page_count(sbi, WB_DATA_TYPE(bio_page));
+       type = WB_DATA_TYPE(bio_page, fio->compressed_page);
+       inc_page_count(sbi, type);

         if (io->bio &&
             (!io_is_mergeable(sbi, io->bio, io, fio, io->last_block_in_bio,
@@ -1059,7 +1059,8 @@ void f2fs_submit_page_write(struct f2fs_io_info *fio)
                 if (F2FS_IO_ALIGNED(sbi) &&
                                 (fio->type == DATA || fio->type == NODE) &&
                                 fio->new_blkaddr & F2FS_IO_SIZE_MASK(sbi)) {
-                       dec_page_count(sbi, WB_DATA_TYPE(bio_page));
+                       dec_page_count(sbi, WB_DATA_TYPE(bio_page,
+                                               fio->compressed_page));
                         fio->retry = 1;
                         goto skip;
                 }
diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index 65294e3b0bef..50f3d546ded8 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -1080,7 +1080,8 @@ struct f2fs_sm_info {
   * f2fs monitors the number of several block types such as on-writeback,
   * dirty dentry blocks, dirty node blocks, and dirty meta blocks.
   */
-#define WB_DATA_TYPE(p)        (__is_cp_guaranteed(p) ? F2FS_WB_CP_DATA : 
F2FS_WB_DATA)
+#define WB_DATA_TYPE(p, f)                     \
+       (f || f2fs_is_cp_guaranteed(p) ? F2FS_WB_CP_DATA : F2FS_WB_DATA)
  enum count_type {
         F2FS_DIRTY_DENTS,
         F2FS_DIRTY_DATA,
@@ -3804,6 +3805,7 @@ void f2fs_init_ckpt_req_control(struct f2fs_sb_info *sbi);
   */
  int __init f2fs_init_bioset(void);
  void f2fs_destroy_bioset(void);
+bool f2fs_is_cp_guaranteed(struct page *page);
  int f2fs_init_bio_entry_cache(void);
  void f2fs_destroy_bio_entry_cache(void);
  void f2fs_submit_read_bio(struct f2fs_sb_info *sbi, struct bio *bio,
--
2.40.1



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

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