On 2021/10/22 11:53, 常凤楠 wrote:


-----Original Message-----
From: [email protected] <[email protected]> On Behalf Of
Chao Yu
Sent: Thursday, October 21, 2021 10:34 PM
To: 常凤楠 <[email protected]>; [email protected]
Cc: [email protected]
Subject: Re: [PATCH v2] f2fs: compress: fix overwrite may reduce compress
ratio unproperly

On 2021/10/14 21:12, Fengnan Chang wrote:
when overwrite only first block of cluster, since cluster is not full,
it will call f2fs_write_raw_pages when f2fs_write_multi_pages, and
cause the whole cluster become uncompressed eventhough data can be
compressed.
this may will make random write bench score reduce a lot.

root# dd if=/dev/zero of=./fio-test bs=1M count=1

root# sync

root# echo 3 > /proc/sys/vm/drop_caches

root# f2fs_io get_cblocks ./fio-test

root# dd if=/dev/zero of=./fio-test bs=4K count=1 oflag=direct
conv=notrunc

w/o patch:
root# f2fs_io get_cblocks ./fio-test
189

w/ patch:
root# f2fs_io get_cblocks ./fio-test
192

Signed-off-by: Fengnan Chang <[email protected]>
---
   fs/f2fs/compress.c | 12 ++++++++++++
   fs/f2fs/data.c     |  7 +++++++
   fs/f2fs/f2fs.h     |  1 +
   3 files changed, 20 insertions(+)

diff --git a/fs/f2fs/compress.c b/fs/f2fs/compress.c index
c1bf9ad4c220..c4f36ead6f17 100644
--- a/fs/f2fs/compress.c
+++ b/fs/f2fs/compress.c
@@ -857,6 +857,18 @@ void f2fs_end_read_compressed_page(struct page
*page, bool failed,
                f2fs_decompress_cluster(dic);
   }

+bool is_page_same_cluster(struct compress_ctx *cc, struct pagevec
+*pvec, int index)

Fengnan,

It needs add f2fs prefix for non-static symbol to avoid global namespace
pollution.

Anyway, how about this?

It looks good for me, you can make this as V3.

Well, could you please send v3 with below updates?

Thanks,


---
   fs/f2fs/compress.c | 19 +++++++++++++++++++
   fs/f2fs/data.c     |  7 ++++---
   fs/f2fs/f2fs.h     |  2 ++
   3 files changed, 25 insertions(+), 3 deletions(-)

diff --git a/fs/f2fs/compress.c b/fs/f2fs/compress.c index
c1bf9ad4c220..15d9b89d4df0 100644
--- a/fs/f2fs/compress.c
+++ b/fs/f2fs/compress.c
@@ -881,6 +881,25 @@ bool f2fs_cluster_can_merge_page(struct
compress_ctx *cc, pgoff_t index)
        return is_page_in_cluster(cc, index);
   }

+bool f2fs_all_cluster_page_loaded(struct compress_ctx *cc, struct pagevec
*pvec,
+                                               int index, int nr_pages)
+{
+       unsigned long pgidx;
+       int i;
+
+       if (nr_pages - index < cc->cluster_size)
+               return false;
+
+       pgidx = pvec->pages[index]->index;
+
+       for (i = 1; i < cc->cluster_size; i++) {
+               if (pvec->pages[index + i]->index != pgidx + i)
+                       return false;
+       }
+
+       return true;
+}
+
   static bool cluster_has_invalid_data(struct compress_ctx *cc)
   {
        loff_t i_size = i_size_read(cc->inode); diff --git a/fs/f2fs/data.c
b/fs/f2fs/data.c index 14fe5c6216cc..b0665f69c093 100644
--- a/fs/f2fs/data.c
+++ b/fs/f2fs/data.c
@@ -3075,9 +3075,10 @@ static int f2fs_write_cache_pages(struct
address_space *mapping,
                                                done = 1;
                                                break;
                                        } else if (ret2 &&
-                                               !f2fs_compress_write_end(inode,
-                                                               fsdata, 
page->index,
-                                                               1)) {
+                                               (!f2fs_compress_write_end(inode,
+                                                       fsdata, page->index, 1) 
||
+                                               
!f2fs_all_cluster_page_loaded(&cc,
+                                                       &pvec, i, nr_pages))) {
                                                retry = 1;
                                                break;
                                        }
diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h index 7a32c2127945..b8da34198ce0
100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -4047,6 +4047,8 @@ void f2fs_end_read_compressed_page(struct page
*page, bool failed,
                                                        block_t blkaddr);
   bool f2fs_cluster_is_empty(struct compress_ctx *cc);
   bool f2fs_cluster_can_merge_page(struct compress_ctx *cc, pgoff_t index);
+bool f2fs_all_cluster_page_loaded(struct compress_ctx *cc, struct pagevec
*pvec,
+                                               int index, int nr_pages);
   bool f2fs_sanity_check_cluster(struct dnode_of_data *dn);
   void f2fs_compress_ctx_add_page(struct compress_ctx *cc, struct page
*page);
   int f2fs_write_multi_pages(struct compress_ctx *cc,
--
2.32.0




_______________________________________________
Linux-f2fs-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

Reply via email to