2023-06-27 16:25:01

by Gao Xiang

[permalink] [raw]
Subject: [PATCH 2/2] erofs: simplify z_erofs_transform_plain()

Use memcpy_to_page() instead of open-coding them.

In addition, add a missing flush_dcache_page() even though almost all
modern architectures clear `PG_dcache_clean` flag for new file cache
pages so that it doesn't change anything in practice.

Signed-off-by: Gao Xiang <[email protected]>
---
preliminary tested with silesia dataset.

fs/erofs/decompressor.c | 19 ++++++++-----------
1 file changed, 8 insertions(+), 11 deletions(-)

diff --git a/fs/erofs/decompressor.c b/fs/erofs/decompressor.c
index ad53cf52d899..cfad1eac7fd9 100644
--- a/fs/erofs/decompressor.c
+++ b/fs/erofs/decompressor.c
@@ -328,7 +328,7 @@ static int z_erofs_transform_plain(struct z_erofs_decompress_req *rq,
const unsigned int lefthalf = rq->outputsize - righthalf;
const unsigned int interlaced_offset =
rq->alg == Z_EROFS_COMPRESSION_SHIFTED ? 0 : rq->pageofs_out;
- unsigned char *src, *dst;
+ u8 *src;

if (outpages > 2 && rq->alg == Z_EROFS_COMPRESSION_SHIFTED) {
DBG_BUGON(1);
@@ -341,22 +341,19 @@ static int z_erofs_transform_plain(struct z_erofs_decompress_req *rq,
}

src = kmap_local_page(rq->in[inpages - 1]) + rq->pageofs_in;
- if (rq->out[0]) {
- dst = kmap_local_page(rq->out[0]);
- memcpy(dst + rq->pageofs_out, src + interlaced_offset,
- righthalf);
- kunmap_local(dst);
- }
+ if (rq->out[0])
+ memcpy_to_page(rq->out[0], rq->pageofs_out,
+ src + interlaced_offset, righthalf);

if (outpages > inpages) {
DBG_BUGON(!rq->out[outpages - 1]);
if (rq->out[outpages - 1] != rq->in[inpages - 1]) {
- dst = kmap_local_page(rq->out[outpages - 1]);
- memcpy(dst, interlaced_offset ? src :
- (src + righthalf), lefthalf);
- kunmap_local(dst);
+ memcpy_to_page(rq->out[outpages - 1], 0, src +
+ (interlaced_offset ? 0 : righthalf),
+ lefthalf);
} else if (!interlaced_offset) {
memmove(src, src + righthalf, lefthalf);
+ flush_dcache_page(rq->in[inpages - 1]);
}
}
kunmap_local(src);
--
2.24.4



2023-06-28 08:34:17

by Yue Hu

[permalink] [raw]
Subject: Re: [PATCH 2/2] erofs: simplify z_erofs_transform_plain()

On Wed, 28 Jun 2023 00:12:40 +0800
Gao Xiang <[email protected]> wrote:

> Use memcpy_to_page() instead of open-coding them.
>
> In addition, add a missing flush_dcache_page() even though almost all
> modern architectures clear `PG_dcache_clean` flag for new file cache
> pages so that it doesn't change anything in practice.
>
> Signed-off-by: Gao Xiang <[email protected]>

Reviewed-by: Yue Hu <[email protected]>

> ---
> preliminary tested with silesia dataset.
>
> fs/erofs/decompressor.c | 19 ++++++++-----------
> 1 file changed, 8 insertions(+), 11 deletions(-)
>
> diff --git a/fs/erofs/decompressor.c b/fs/erofs/decompressor.c
> index ad53cf52d899..cfad1eac7fd9 100644
> --- a/fs/erofs/decompressor.c
> +++ b/fs/erofs/decompressor.c
> @@ -328,7 +328,7 @@ static int z_erofs_transform_plain(struct z_erofs_decompress_req *rq,
> const unsigned int lefthalf = rq->outputsize - righthalf;
> const unsigned int interlaced_offset =
> rq->alg == Z_EROFS_COMPRESSION_SHIFTED ? 0 : rq->pageofs_out;
> - unsigned char *src, *dst;
> + u8 *src;
>
> if (outpages > 2 && rq->alg == Z_EROFS_COMPRESSION_SHIFTED) {
> DBG_BUGON(1);
> @@ -341,22 +341,19 @@ static int z_erofs_transform_plain(struct z_erofs_decompress_req *rq,
> }
>
> src = kmap_local_page(rq->in[inpages - 1]) + rq->pageofs_in;
> - if (rq->out[0]) {
> - dst = kmap_local_page(rq->out[0]);
> - memcpy(dst + rq->pageofs_out, src + interlaced_offset,
> - righthalf);
> - kunmap_local(dst);
> - }
> + if (rq->out[0])
> + memcpy_to_page(rq->out[0], rq->pageofs_out,
> + src + interlaced_offset, righthalf);
>
> if (outpages > inpages) {
> DBG_BUGON(!rq->out[outpages - 1]);
> if (rq->out[outpages - 1] != rq->in[inpages - 1]) {
> - dst = kmap_local_page(rq->out[outpages - 1]);
> - memcpy(dst, interlaced_offset ? src :
> - (src + righthalf), lefthalf);
> - kunmap_local(dst);
> + memcpy_to_page(rq->out[outpages - 1], 0, src +
> + (interlaced_offset ? 0 : righthalf),
> + lefthalf);
> } else if (!interlaced_offset) {
> memmove(src, src + righthalf, lefthalf);
> + flush_dcache_page(rq->in[inpages - 1]);
> }
> }
> kunmap_local(src);


2023-07-11 16:00:17

by Chao Yu

[permalink] [raw]
Subject: Re: [PATCH 2/2] erofs: simplify z_erofs_transform_plain()

On 2023/6/28 0:12, Gao Xiang wrote:
> Use memcpy_to_page() instead of open-coding them.
>
> In addition, add a missing flush_dcache_page() even though almost all
> modern architectures clear `PG_dcache_clean` flag for new file cache
> pages so that it doesn't change anything in practice.
>
> Signed-off-by: Gao Xiang <[email protected]>

Reviewed-by: Chao Yu <[email protected]>

Thanks,