* [f2fs-dev] [PATCH v2] f2fs: compress: fix overwrite may reduce compress ratio unproperly
@ 2021-10-14 13:12 Fengnan Chang
2021-10-21 14:33 ` Chao Yu
[not found] ` <ALgAyADvEmTgm6EcRRwmGaoi.9.1634826831858.Hmail.changfengnan@vivo.com>
0 siblings, 2 replies; 4+ messages in thread
From: Fengnan Chang @ 2021-10-14 13:12 UTC (permalink / raw)
To: jaegeuk, chao; +Cc: Fengnan Chang, linux-f2fs-devel
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 <changfengnan@vivo.com>
---
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)
+{
+ int idx = cluster_idx(cc, pvec->pages[index]->index);
+ int i = index + 1;
+
+ for (i = index + 1; i < index + cc->cluster_size; i++) {
+ if (cluster_idx(cc, pvec->pages[i]->index) != idx)
+ return false;
+ }
+
+ return true;
+}
static bool is_page_in_cluster(struct compress_ctx *cc, pgoff_t index)
{
if (cc->cluster_idx == NULL_CLUSTER)
diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
index f4fd6c246c9a..33ccabbe9f92 100644
--- a/fs/f2fs/data.c
+++ b/fs/f2fs/data.c
@@ -3025,6 +3025,13 @@ static int f2fs_write_cache_pages(struct address_space *mapping,
1)) {
retry = 1;
break;
+ } else if (ret2 && nr_pages - i < cc.cluster_size) {
+ retry = 1;
+ break;
+ } else if (ret2 && nr_pages - i >= cc.cluster_size &&
+ !is_page_same_cluster(&cc, &pvec, i)) {
+ retry = 1;
+ break;
}
} else {
goto lock_page;
diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index 039a229e11c9..f225ea36bb60 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -4031,6 +4031,7 @@ void f2fs_end_read_compressed_page(struct page *page, bool failed,
bool f2fs_cluster_is_empty(struct compress_ctx *cc);
bool f2fs_cluster_can_merge_page(struct compress_ctx *cc, pgoff_t index);
bool f2fs_sanity_check_cluster(struct dnode_of_data *dn);
+bool is_page_same_cluster(struct compress_ctx *cc, struct pagevec *pvec, int index);
void f2fs_compress_ctx_add_page(struct compress_ctx *cc, struct page *page);
int f2fs_write_multi_pages(struct compress_ctx *cc,
int *submitted,
--
2.32.0
_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [f2fs-dev] [PATCH v2] f2fs: compress: fix overwrite may reduce compress ratio unproperly
2021-10-14 13:12 [f2fs-dev] [PATCH v2] f2fs: compress: fix overwrite may reduce compress ratio unproperly Fengnan Chang
@ 2021-10-21 14:33 ` Chao Yu
[not found] ` <ALgAyADvEmTgm6EcRRwmGaoi.9.1634826831858.Hmail.changfengnan@vivo.com>
1 sibling, 0 replies; 4+ messages in thread
From: Chao Yu @ 2021-10-21 14:33 UTC (permalink / raw)
To: Fengnan Chang, jaegeuk; +Cc: linux-f2fs-devel
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 <changfengnan@vivo.com>
> ---
> 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?
---
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
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [f2fs-dev] [PATCH v2] f2fs: compress: fix overwrite may reduce compress ratio unproperly
[not found] ` <ALgAyADvEmTgm6EcRRwmGaoi.9.1634826831858.Hmail.changfengnan@vivo.com>
@ 2021-10-22 3:53 ` 常凤楠
2021-10-22 15:35 ` Chao Yu
0 siblings, 1 reply; 4+ messages in thread
From: 常凤楠 @ 2021-10-22 3:53 UTC (permalink / raw)
To: Chao Yu, jaegeuk; +Cc: linux-f2fs-devel
> -----Original Message-----
> From: changfengnan@vivo.com <changfengnan@vivo.com> On Behalf Of
> Chao Yu
> Sent: Thursday, October 21, 2021 10:34 PM
> To: 常凤楠 <changfengnan@vivo.com>; jaegeuk@kernel.org
> Cc: linux-f2fs-devel@lists.sourceforge.net
> 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 <changfengnan@vivo.com>
> > ---
> > 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.
>
> ---
> 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
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [f2fs-dev] [PATCH v2] f2fs: compress: fix overwrite may reduce compress ratio unproperly
2021-10-22 3:53 ` 常凤楠
@ 2021-10-22 15:35 ` Chao Yu
0 siblings, 0 replies; 4+ messages in thread
From: Chao Yu @ 2021-10-22 15:35 UTC (permalink / raw)
To: 常凤楠, jaegeuk; +Cc: linux-f2fs-devel
On 2021/10/22 11:53, 常凤楠 wrote:
>
>
>> -----Original Message-----
>> From: changfengnan@vivo.com <changfengnan@vivo.com> On Behalf Of
>> Chao Yu
>> Sent: Thursday, October 21, 2021 10:34 PM
>> To: 常凤楠 <changfengnan@vivo.com>; jaegeuk@kernel.org
>> Cc: linux-f2fs-devel@lists.sourceforge.net
>> 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 <changfengnan@vivo.com>
>>> ---
>>> 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
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2021-10-22 15:42 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-10-14 13:12 [f2fs-dev] [PATCH v2] f2fs: compress: fix overwrite may reduce compress ratio unproperly Fengnan Chang
2021-10-21 14:33 ` Chao Yu
[not found] ` <ALgAyADvEmTgm6EcRRwmGaoi.9.1634826831858.Hmail.changfengnan@vivo.com>
2021-10-22 3:53 ` 常凤楠
2021-10-22 15:35 ` Chao Yu
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.