2023-08-30 18:44:43

by Herbert Xu

[permalink] [raw]
Subject: [PATCH 2/4] crypto: qat - Remove zlib-deflate

Remove the implementation of zlib-deflate because it is completely
unused in the kernel.

Signed-off-by: Herbert Xu <[email protected]>
---

drivers/crypto/intel/qat/qat_common/qat_comp_algs.c | 129 --------------------
1 file changed, 1 insertion(+), 128 deletions(-)

diff --git a/drivers/crypto/intel/qat/qat_common/qat_comp_algs.c b/drivers/crypto/intel/qat/qat_common/qat_comp_algs.c
index b533984906ec..bf8c0ee62917 100644
--- a/drivers/crypto/intel/qat/qat_common/qat_comp_algs.c
+++ b/drivers/crypto/intel/qat/qat_common/qat_comp_algs.c
@@ -109,69 +109,6 @@ static void qat_comp_resubmit(struct work_struct *work)
acomp_request_complete(areq, ret);
}

-static int parse_zlib_header(u16 zlib_h)
-{
- int ret = -EINVAL;
- __be16 header;
- u8 *header_p;
- u8 cmf, flg;
-
- header = cpu_to_be16(zlib_h);
- header_p = (u8 *)&header;
-
- flg = header_p[0];
- cmf = header_p[1];
-
- if (cmf >> QAT_RFC_1950_CM_OFFSET > QAT_RFC_1950_CM_DEFLATE_CINFO_32K)
- return ret;
-
- if ((cmf & QAT_RFC_1950_CM_MASK) != QAT_RFC_1950_CM_DEFLATE)
- return ret;
-
- if (flg & QAT_RFC_1950_DICT_MASK)
- return ret;
-
- return 0;
-}
-
-static int qat_comp_rfc1950_callback(struct qat_compression_req *qat_req,
- void *resp)
-{
- struct acomp_req *areq = qat_req->acompress_req;
- enum direction dir = qat_req->dir;
- __be32 qat_produced_adler;
-
- qat_produced_adler = cpu_to_be32(qat_comp_get_produced_adler32(resp));
-
- if (dir == COMPRESSION) {
- __be16 zlib_header;
-
- zlib_header = cpu_to_be16(QAT_RFC_1950_COMP_HDR);
- scatterwalk_map_and_copy(&zlib_header, areq->dst, 0, QAT_RFC_1950_HDR_SIZE, 1);
- areq->dlen += QAT_RFC_1950_HDR_SIZE;
-
- scatterwalk_map_and_copy(&qat_produced_adler, areq->dst, areq->dlen,
- QAT_RFC_1950_FOOTER_SIZE, 1);
- areq->dlen += QAT_RFC_1950_FOOTER_SIZE;
- } else {
- __be32 decomp_adler;
- int footer_offset;
- int consumed;
-
- consumed = qat_comp_get_consumed_ctr(resp);
- footer_offset = consumed + QAT_RFC_1950_HDR_SIZE;
- if (footer_offset + QAT_RFC_1950_FOOTER_SIZE > areq->slen)
- return -EBADMSG;
-
- scatterwalk_map_and_copy(&decomp_adler, areq->src, footer_offset,
- QAT_RFC_1950_FOOTER_SIZE, 0);
-
- if (qat_produced_adler != decomp_adler)
- return -EBADMSG;
- }
- return 0;
-}
-
static void qat_comp_generic_callback(struct qat_compression_req *qat_req,
void *resp)
{
@@ -293,18 +230,6 @@ static void qat_comp_alg_exit_tfm(struct crypto_acomp *acomp_tfm)
memset(ctx, 0, sizeof(*ctx));
}

-static int qat_comp_alg_rfc1950_init_tfm(struct crypto_acomp *acomp_tfm)
-{
- struct crypto_tfm *tfm = crypto_acomp_tfm(acomp_tfm);
- struct qat_compression_ctx *ctx = crypto_tfm_ctx(tfm);
- int ret;
-
- ret = qat_comp_alg_init_tfm(acomp_tfm);
- ctx->qat_comp_callback = &qat_comp_rfc1950_callback;
-
- return ret;
-}
-
static int qat_comp_alg_compress_decompress(struct acomp_req *areq, enum direction dir,
unsigned int shdr, unsigned int sftr,
unsigned int dhdr, unsigned int dftr)
@@ -400,43 +325,6 @@ static int qat_comp_alg_decompress(struct acomp_req *req)
return qat_comp_alg_compress_decompress(req, DECOMPRESSION, 0, 0, 0, 0);
}

-static int qat_comp_alg_rfc1950_compress(struct acomp_req *req)
-{
- if (!req->dst && req->dlen != 0)
- return -EINVAL;
-
- if (req->dst && req->dlen <= QAT_RFC_1950_HDR_SIZE + QAT_RFC_1950_FOOTER_SIZE)
- return -EINVAL;
-
- return qat_comp_alg_compress_decompress(req, COMPRESSION, 0, 0,
- QAT_RFC_1950_HDR_SIZE,
- QAT_RFC_1950_FOOTER_SIZE);
-}
-
-static int qat_comp_alg_rfc1950_decompress(struct acomp_req *req)
-{
- struct crypto_acomp *acomp_tfm = crypto_acomp_reqtfm(req);
- struct crypto_tfm *tfm = crypto_acomp_tfm(acomp_tfm);
- struct qat_compression_ctx *ctx = crypto_tfm_ctx(tfm);
- struct adf_accel_dev *accel_dev = ctx->inst->accel_dev;
- u16 zlib_header;
- int ret;
-
- if (req->slen <= QAT_RFC_1950_HDR_SIZE + QAT_RFC_1950_FOOTER_SIZE)
- return -EBADMSG;
-
- scatterwalk_map_and_copy(&zlib_header, req->src, 0, QAT_RFC_1950_HDR_SIZE, 0);
-
- ret = parse_zlib_header(zlib_header);
- if (ret) {
- dev_dbg(&GET_DEV(accel_dev), "Error parsing zlib header\n");
- return ret;
- }
-
- return qat_comp_alg_compress_decompress(req, DECOMPRESSION, QAT_RFC_1950_HDR_SIZE,
- QAT_RFC_1950_FOOTER_SIZE, 0, 0);
-}
-
static struct acomp_alg qat_acomp[] = { {
.base = {
.cra_name = "deflate",
@@ -452,22 +340,7 @@ static struct acomp_alg qat_acomp[] = { {
.decompress = qat_comp_alg_decompress,
.dst_free = sgl_free,
.reqsize = sizeof(struct qat_compression_req),
-}, {
- .base = {
- .cra_name = "zlib-deflate",
- .cra_driver_name = "qat_zlib_deflate",
- .cra_priority = 4001,
- .cra_flags = CRYPTO_ALG_ASYNC,
- .cra_ctxsize = sizeof(struct qat_compression_ctx),
- .cra_module = THIS_MODULE,
- },
- .init = qat_comp_alg_rfc1950_init_tfm,
- .exit = qat_comp_alg_exit_tfm,
- .compress = qat_comp_alg_rfc1950_compress,
- .decompress = qat_comp_alg_rfc1950_decompress,
- .dst_free = sgl_free,
- .reqsize = sizeof(struct qat_compression_req),
-} };
+}};

int qat_comp_algs_register(void)
{


2023-09-06 08:39:20

by Gao Xiang

[permalink] [raw]
Subject: Re: [PATCH 2/4] crypto: qat - Remove zlib-deflate



On 2023/9/6 00:23, David Sterba wrote:
> On Tue, Sep 05, 2023 at 02:15:44PM +0100, Giovanni Cabiddu wrote:
>> Hi Herbert,
>>
>> On Wed, Aug 30, 2023 at 06:08:47PM +0800, Herbert Xu wrote:
>>> Remove the implementation of zlib-deflate because it is completely
>>> unused in the kernel.
>> We are working at a new revision of [1] which enables BTRFS to use acomp
>> for offloading zlib-deflate. We see that there is value in using QAT for
>> such use case in terms of throughput / CPU utilization / compression ratio
>> compared to software.
>> Zlib-deflate is preferred to deflate since BTRFS already uses that
>> format.
>>
>> We expect to send this patch for 6.7.
>> Can we keep zlib-deflate in the kernel?
>>
>> Thanks,
>>
>> [1] https://patchwork.kernel.org/project/linux-btrfs/patch/[email protected]/
>
> The patch is from 2016 and zlib though still supported has been
> superseded by zstd that is from 2017. It would be good to see numbers
> comparing zlib (cpu), zlib (qat) against relevant zstd levels. The
> offloading might be an improvement and worth adding the support
> otherwise I don't see much reason to add it unless there are users.
>
> I can see there's QAT support for zstd too,
> https://github.com/intel/QAT-ZSTD-Plugin, can't find one for lzo but in
> case ther's QAT for all 3 algorithms used by btrfs I wouldn't mind
> keeping the QAT support for zlib for parity.

Just my personal side note: from my own point of view, QAT actually only
has DEFLATE and LZ4 format hardware end-to-end (de)compression support [1]
(IAA actually has DEFLATE-family format only [2]).

They partially support compressing Zstd format with their internal
hardware lz4s + postprocessing pipeline (mostly a LZ77 matchfinder) by
using hardware (with hw_buffer_sz less than 128KiB. [3][4]) and Zstd
hardware decompression is currently not supported since there is no
such hardware format support.

I'm not saying new Zstd algorithm is not amazing. Yet from the hardware
perspective, I guess that due to gzip, the original zip, png, pdf, docx,
https, even pppoe all based on DEFLATE (you could see a lot other OSes
support DEFLATE-family format), so it's reasonble to resolve such
de-facto standard with limited hardware first to boost up data center
use cases. If they have abundant hardware chip room, I guess they will
consider Zstd as well.

As for zlib container format, I don't see zlib Adler-32 checksum is
useful since almost all hardware accelerator supports raw DEFLATE but
Adler-32 checksum is uncommon. If we consider better integrating
support, we should consider a more common hash (instead Adler-32
checksum) of or by using merkle tree to build the trust chain.

Actually we're working on EROFS raw deflate to enable IAA accelerator
support so we also hope IAA patchset could be landed upstream [5] so
I could upstream my work then. Therefore, I hope "hisilicon ZIP
driver" could support raw deflate too (after a quick search, their
decompression spend is 1530 MB/s compared with the original zlib
219 MB/s on their platform [6])

[1] https://github.com/intel/QATzip
[2] https://cdrdv2.intel.com/v1/dl/getContent/721858
[3] https://github.com/facebook/zstd/issues/455#issuecomment-1397204587
[4] https://github.com/intel/QATzip/blob/master/utils/qzstd.c#L211
[5] https://lore.kernel.org/r/[email protected]
[6] https://compare-intel-kunpeng.readthedocs.io/zh_CN/latest/accelerator.html

Thanks,
Gao Xiang