erofs: get rid of the remaining kmap_atomic()
PATCH 1/2 erofs: get rid of the remaining kmap_atomic()
It's unnecessary to use kmap_atomic() compared with kmap_local_page(). In addition, kmap_atomic() is deprecated now.
---
preliminary tested with silesia dataset.
fs/erofs/decompressor.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-)
fs/erofs/decompressor.c +9 -9
@@ -148,7 +148,7 @@ static void *z_erofs_lz4_handle_overlap(struct z_erofs_lz4_decompress_ctx *ctx, *maptype =0 ;return inpage; } -kunmap_atomic (inpage); +kunmap_local (inpage);might_sleep (); src =erofs_vm_map_ram (rq->in, ctx->inpages);if (!src)
@@ -162,7 +162,7 @@ static void *z_erofs_lz4_handle_overlap(struct z_erofs_lz4_decompress_ctx *ctx, src =erofs_get_pcpubuf (ctx->inpages);if (!src) {DBG_BUGON (1 ); -kunmap_atomic (inpage); +kunmap_local (inpage);return ERR_PTR (-EFAULT ); }
@@ -173,9 +173,9 @@ static void *z_erofs_lz4_handle_overlap(struct z_erofs_lz4_decompress_ctx *ctx, min_t(unsigned int, total, PAGE_SIZE - *inputmargin);if (!inpage) - inpage =kmap_atomic (*in); + inpage =kmap_local_page (*in);memcpy (tmp, inpage + *inputmargin, page_copycnt); -kunmap_atomic (inpage); +kunmap_local (inpage); inpage =NULL ; tmp += page_copycnt; total -= page_copycnt;
@@ -214,7 +214,7 @@ static int z_erofs_lz4_decompress_mem(struct z_erofs_lz4_decompress_ctx *ctx,int ret, maptype;DBG_BUGON (*rq->in ==NULL ); - headpage =kmap_atomic (*rq->in); + headpage =kmap_local_page (*rq->in);/* LZ4 decompression inplace is only safe if zero_padding is enabled */ if (erofs_sb_has_zero_padding (EROFS_SB (rq->sb))) {
@@ -223,7 +223,7 @@ static int z_erofs_lz4_decompress_mem(struct z_erofs_lz4_decompress_ctx *ctx, min_t(unsigned int, rq->inputsize, rq->sb->s_blocksize - rq->pageofs_in));if (ret) { -kunmap_atomic (headpage); +kunmap_local (headpage);return ret; } may_inplace = !((rq->pageofs_in + rq->inputsize) &
@@ -261,7 +261,7 @@ static int z_erofs_lz4_decompress_mem(struct z_erofs_lz4_decompress_ctx *ctx, }if (maptype ==0 ) { -kunmap_atomic (headpage); +kunmap_local (headpage); }else if (maptype ==1 ) {vm_unmap_ram (src, ctx->inpages); }else if (maptype ==2 ) {
@@ -289,7 +289,7 @@ static int z_erofs_lz4_decompress(struct z_erofs_decompress_req *rq,/* one optimized fast path only for non bigpcluster cases yet */ if (ctx.inpages ==1 && ctx.outpages ==1 && !rq->inplace_io) {DBG_BUGON (!*rq->out); - dst =kmap_atomic (*rq->out); + dst =kmap_local_page (*rq->out); dst_maptype =0 ; goto dstmap_out; }
@@ -311,7 +311,7 @@ static int z_erofs_lz4_decompress(struct z_erofs_decompress_req *rq, dstmap_out: ret =z_erofs_lz4_decompress_mem (&ctx, dst + rq->pageofs_out);if (!dst_maptype) -kunmap_atomic (dst); +kunmap_local (dst);else if (dst_maptype ==2 )vm_unmap_ram (dst, ctx.outpages);return ret;
Signature · 1 line
2.24.4
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.
---
preliminary tested with silesia dataset.
fs/erofs/decompressor.c | 19 ++++++++----------- 1 file changed, 8 insertions(+), 11 deletions(-)
fs/erofs/decompressor.c +8 -11
@@ -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);
Signature · 1 line
2.24.4
Re: [PATCH 2/2] erofs: simplify z_erofs_transform_plain()
Quoted 3 lines
Use
memcpy_to_page()instead of open-coding them.In addition, add a missing
flush_dcache_page()even though …
Does such a wording really fit to the known requirement “Solve only one problem per patch.”?
How do you think about to add the tag “Fixes”?
Regards,
Markus
Re: [PATCH 2/2] erofs: simplify z_erofs_transform_plain()
In reply to Gao XiangQuoted 7 lines · On Wed, 28 Jun 2023 00:12:40 +0800 Gao Xiang <[email protected]>…
Use
memcpy_to_page()instead of open-coding them.In addition, add a missing
flush_dcache_page()even though almost all modern architectures clearPG_dcache_cleanflag for new file cache pages so that it doesn't change anything in practice.Signed-off-by Gao Xiang <[email protected]>
Quoted 49 lines
---
preliminary tested with silesia dataset.fs/erofs/decompressor.c | 19 ++++++++----------- 1 file changed, 8 insertions(+), 11 deletions(-)fs/erofs/decompressor.c +8
-11@@ -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);
Re: [PATCH 2/2] erofs: simplify z_erofs_transform_plain()
In reply to Gao XiangQuoted 7 lines · Gao Xiang
Use
memcpy_to_page()instead of open-coding them.In addition, add a missing
flush_dcache_page()even though almost all modern architectures clearPG_dcache_cleanflag for new file cache pages so that it doesn't change anything in practice.Signed-off-by Gao Xiang <[email protected]>
Thanks,
Can such a small patch series become a bit nicer also with a corresponding cover letter?
Quoted 1 line
It's unnecessary to use
kmap_atomic()compared withkmap_local_page().
Can the desired comparison be improved any further?
Quoted 1 line
In addition,
kmap_atomic()is deprecated now.
Please choose another imperative change suggestion.
Regards,
Markus
Quoted 4 lines · On Wed, 28 Jun 2023 00:12:39 +0800 Gao Xiang <[email protected]>…
It's unnecessary to use
kmap_atomic()compared withkmap_local_page(). In addition,kmap_atomic()is deprecated now.Signed-off-by Gao Xiang <[email protected]>
Quoted 4 lines · Gao Xiang
It's unnecessary to use
kmap_atomic()compared withkmap_local_page(). In addition,kmap_atomic()is deprecated now.Signed-off-by Gao Xiang <[email protected]>
Thanks,