* [PATCH] erofs: fix the per-CPU buffer decompression for small output size
@ 2021-10-13 9:29 Yue Hu
2021-10-13 11:51 ` Gao Xiang
0 siblings, 1 reply; 6+ messages in thread
From: Yue Hu @ 2021-10-13 9:29 UTC (permalink / raw)
To: xiang, chao; +Cc: linux-erofs, linux-kernel, huyue2, zhangwen, zbestahu
From: Yue Hu <huyue2@yulong.com>
Note that z_erofs_lz4_decompress() will return a positive value if
decompression succeeds. However, we do not copy_from_pcpubuf() due
to !ret. Let's fix it.
Signed-off-by: Yue Hu <huyue2@yulong.com>
---
fs/erofs/decompressor.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/fs/erofs/decompressor.c b/fs/erofs/decompressor.c
index a5bc4b1..e4cab4e 100644
--- a/fs/erofs/decompressor.c
+++ b/fs/erofs/decompressor.c
@@ -326,7 +326,7 @@ static int z_erofs_decompress_generic(struct z_erofs_decompress_req *rq,
rq->inplace_io = false;
ret = alg->decompress(rq, dst);
- if (!ret)
+ if (ret > 0)
copy_from_pcpubuf(rq->out, dst, rq->pageofs_out,
rq->outputsize);
--
1.9.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] erofs: fix the per-CPU buffer decompression for small output size
2021-10-13 9:29 [PATCH] erofs: fix the per-CPU buffer decompression for small output size Yue Hu
@ 2021-10-13 11:51 ` Gao Xiang
2021-10-13 11:58 ` Gao Xiang
2021-10-13 13:10 ` Yue Hu
0 siblings, 2 replies; 6+ messages in thread
From: Gao Xiang @ 2021-10-13 11:51 UTC (permalink / raw)
To: Yue Hu; +Cc: xiang, chao, linux-erofs, linux-kernel, huyue2, zhangwen, zbestahu
Hi Yue,
On Wed, Oct 13, 2021 at 05:29:05PM +0800, Yue Hu wrote:
> From: Yue Hu <huyue2@yulong.com>
>
> Note that z_erofs_lz4_decompress() will return a positive value if
> decompression succeeds. However, we do not copy_from_pcpubuf() due
> to !ret. Let's fix it.
>
> Signed-off-by: Yue Hu <huyue2@yulong.com>
Thanks for catching this. This is a valid issue, but it has no real
impact to the current kernels since such pcluster in practice will be
!inplace_io and trigger "if (nrpages_out == 1 && !rq->inplace_io) {"
above for upstream strategies.
Our customized lz4 implementation will return 0 if success instead, so
it has no issue to our previous products as well.
For such cases, how about updating z_erofs_lz4_decompress() to return
0 if success instead rather than outputsize. Since I'll return 0 if
success for LZMA as well.
Thanks,
Gao Xiang
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] erofs: fix the per-CPU buffer decompression for small output size
2021-10-13 11:51 ` Gao Xiang
@ 2021-10-13 11:58 ` Gao Xiang
2021-10-13 13:10 ` Yue Hu
1 sibling, 0 replies; 6+ messages in thread
From: Gao Xiang @ 2021-10-13 11:58 UTC (permalink / raw)
To: Yue Hu; +Cc: xiang, chao, linux-erofs, linux-kernel, huyue2, zhangwen, zbestahu
On Wed, Oct 13, 2021 at 07:51:55PM +0800, Gao Xiang wrote:
> Hi Yue,
>
> On Wed, Oct 13, 2021 at 05:29:05PM +0800, Yue Hu wrote:
> > From: Yue Hu <huyue2@yulong.com>
> >
> > Note that z_erofs_lz4_decompress() will return a positive value if
> > decompression succeeds. However, we do not copy_from_pcpubuf() due
> > to !ret. Let's fix it.
> >
> > Signed-off-by: Yue Hu <huyue2@yulong.com>
>
> Thanks for catching this. This is a valid issue, but it has no real
> impact to the current kernels since such pcluster in practice will be
> !inplace_io and trigger "if (nrpages_out == 1 && !rq->inplace_io) {"
> above for upstream strategies.
>
> Our customized lz4 implementation will return 0 if success instead, so
> it has no issue to our previous products as well.
>
> For such cases, how about updating z_erofs_lz4_decompress() to return
> 0 if success instead rather than outputsize. Since I'll return 0 if
> success for LZMA as well.
In addition, I'm fine to getting rid of such path as well, since it has
no real impact to our current upstream decompression strategy.
If it has some use cases due to decompression strategy change, we could
re-add it with some evaluation (some real numbers) later.
Thanks,
Gao Xiang
>
> Thanks,
> Gao Xiang
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] erofs: fix the per-CPU buffer decompression for small output size
2021-10-13 11:51 ` Gao Xiang
2021-10-13 11:58 ` Gao Xiang
@ 2021-10-13 13:10 ` Yue Hu
2021-10-13 16:03 ` Gao Xiang
1 sibling, 1 reply; 6+ messages in thread
From: Yue Hu @ 2021-10-13 13:10 UTC (permalink / raw)
To: Gao Xiang
Cc: Yue Hu, xiang, chao, linux-erofs, linux-kernel, huyue2, zhangwen
Hi Xiang,
On Wed, 13 Oct 2021 19:51:55 +0800
Gao Xiang <hsiangkao@linux.alibaba.com> wrote:
> Hi Yue,
>
> On Wed, Oct 13, 2021 at 05:29:05PM +0800, Yue Hu wrote:
> > From: Yue Hu <huyue2@yulong.com>
> >
> > Note that z_erofs_lz4_decompress() will return a positive value if
> > decompression succeeds. However, we do not copy_from_pcpubuf() due
> > to !ret. Let's fix it.
> >
> > Signed-off-by: Yue Hu <huyue2@yulong.com>
>
> Thanks for catching this. This is a valid issue, but it has no real
> impact to the current kernels since such pcluster in practice will be
> !inplace_io and trigger "if (nrpages_out == 1 && !rq->inplace_io) {"
> above for upstream strategies.
>
> Our customized lz4 implementation will return 0 if success instead, so
> it has no issue to our previous products as well.
Yes, i just find the issue when i try to implement a new feature of
tail-packing inline compressed data. No problem in my current version.
Thanks.
>
> For such cases, how about updating z_erofs_lz4_decompress() to return
> 0 if success instead rather than outputsize. Since I'll return 0 if
> success for LZMA as well.
>
> Thanks,
> Gao Xiang
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] erofs: fix the per-CPU buffer decompression for small output size
2021-10-13 13:10 ` Yue Hu
@ 2021-10-13 16:03 ` Gao Xiang
2021-10-14 1:42 ` Yue Hu
0 siblings, 1 reply; 6+ messages in thread
From: Gao Xiang @ 2021-10-13 16:03 UTC (permalink / raw)
To: Yue Hu; +Cc: Yue Hu, xiang, chao, linux-erofs, linux-kernel, huyue2, zhangwen
On Wed, Oct 13, 2021 at 09:10:05PM +0800, Yue Hu wrote:
> Hi Xiang,
>
> On Wed, 13 Oct 2021 19:51:55 +0800
> Gao Xiang <hsiangkao@linux.alibaba.com> wrote:
>
> > Hi Yue,
> >
> > On Wed, Oct 13, 2021 at 05:29:05PM +0800, Yue Hu wrote:
> > > From: Yue Hu <huyue2@yulong.com>
> > >
> > > Note that z_erofs_lz4_decompress() will return a positive value if
> > > decompression succeeds. However, we do not copy_from_pcpubuf() due
> > > to !ret. Let's fix it.
> > >
> > > Signed-off-by: Yue Hu <huyue2@yulong.com>
> >
> > Thanks for catching this. This is a valid issue, but it has no real
> > impact to the current kernels since such pcluster in practice will be
> > !inplace_io and trigger "if (nrpages_out == 1 && !rq->inplace_io) {"
> > above for upstream strategies.
> >
> > Our customized lz4 implementation will return 0 if success instead, so
> > it has no issue to our previous products as well.
>
> Yes, i just find the issue when i try to implement a new feature of
> tail-packing inline compressed data. No problem in my current version.
Yeah, please help update the return value of z_erofs_lz4_decompress()
and get rid of such unneeded fast path.
Thanks,
Gao Xiang
>
> Thanks.
>
> >
> > For such cases, how about updating z_erofs_lz4_decompress() to return
> > 0 if success instead rather than outputsize. Since I'll return 0 if
> > success for LZMA as well.
> >
> > Thanks,
> > Gao Xiang
>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] erofs: fix the per-CPU buffer decompression for small output size
2021-10-13 16:03 ` Gao Xiang
@ 2021-10-14 1:42 ` Yue Hu
0 siblings, 0 replies; 6+ messages in thread
From: Yue Hu @ 2021-10-14 1:42 UTC (permalink / raw)
To: Gao Xiang
Cc: Yue Hu, xiang, chao, linux-erofs, linux-kernel, huyue2, zhangwen
On Thu, 14 Oct 2021 00:03:02 +0800
Gao Xiang <hsiangkao@linux.alibaba.com> wrote:
> On Wed, Oct 13, 2021 at 09:10:05PM +0800, Yue Hu wrote:
> > Hi Xiang,
> >
> > On Wed, 13 Oct 2021 19:51:55 +0800
> > Gao Xiang <hsiangkao@linux.alibaba.com> wrote:
> >
> > > Hi Yue,
> > >
> > > On Wed, Oct 13, 2021 at 05:29:05PM +0800, Yue Hu wrote:
> > > > From: Yue Hu <huyue2@yulong.com>
> > > >
> > > > Note that z_erofs_lz4_decompress() will return a positive value if
> > > > decompression succeeds. However, we do not copy_from_pcpubuf() due
> > > > to !ret. Let's fix it.
> > > >
> > > > Signed-off-by: Yue Hu <huyue2@yulong.com>
> > >
> > > Thanks for catching this. This is a valid issue, but it has no real
> > > impact to the current kernels since such pcluster in practice will be
> > > !inplace_io and trigger "if (nrpages_out == 1 && !rq->inplace_io) {"
> > > above for upstream strategies.
> > >
> > > Our customized lz4 implementation will return 0 if success instead, so
> > > it has no issue to our previous products as well.
> >
> > Yes, i just find the issue when i try to implement a new feature of
> > tail-packing inline compressed data. No problem in my current version.
>
> Yeah, please help update the return value of z_erofs_lz4_decompress()
> and get rid of such unneeded fast path.
OK, will update in next version.
Thanks.
>
> Thanks,
> Gao Xiang
>
> >
> > Thanks.
> >
> > >
> > > For such cases, how about updating z_erofs_lz4_decompress() to return
> > > 0 if success instead rather than outputsize. Since I'll return 0 if
> > > success for LZMA as well.
> > >
> > > Thanks,
> > > Gao Xiang
> >
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2021-10-14 1:43 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-10-13 9:29 [PATCH] erofs: fix the per-CPU buffer decompression for small output size Yue Hu
2021-10-13 11:51 ` Gao Xiang
2021-10-13 11:58 ` Gao Xiang
2021-10-13 13:10 ` Yue Hu
2021-10-13 16:03 ` Gao Xiang
2021-10-14 1:42 ` Yue Hu
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).