Re: [PATCH v4 1/3] crypto: introduce crypto_acomp_get_alg_flags to expose algorithm flags

[Barry Song]

On Tue, Feb 20, 2024 at 5:14 PM Herbert Xu <herbert@xxxxxxxxxxxxxxxxxxx> wrote:
>
> On Tue, Feb 20, 2024 at 03:55:43PM +1300, Barry Song wrote:
> >
> > diff --git a/drivers/crypto/hisilicon/zip/zip_crypto.c b/drivers/crypto/hisilicon/zip/zip_crypto.c
> > index c650c741a18d..94e2d66b04b6 100644
> > --- a/drivers/crypto/hisilicon/zip/zip_crypto.c
> > +++ b/drivers/crypto/hisilicon/zip/zip_crypto.c
> > @@ -591,6 +591,7 @@ static struct acomp_alg hisi_zip_acomp_deflate = {
> >       .base                   = {
> >               .cra_name               = "deflate",
> >               .cra_driver_name        = "hisi-deflate-acomp",
> > +             .cra_flags              = CRYPTO_ALG_ASYNC,
> >               .cra_module             = THIS_MODULE,
> >               .cra_priority           = HZIP_ALG_PRIORITY,
> >               .cra_ctxsize            = sizeof(struct hisi_zip_ctx),
> > diff --git a/drivers/crypto/intel/iaa/iaa_crypto_main.c b/drivers/crypto/intel/iaa/iaa_crypto_main.c
> > index dfd3baf0a8d8..91adf9d76a2e 100644
> > --- a/drivers/crypto/intel/iaa/iaa_crypto_main.c
> > +++ b/drivers/crypto/intel/iaa/iaa_crypto_main.c
> > @@ -1916,6 +1916,7 @@ static struct acomp_alg iaa_acomp_fixed_deflate = {
> >       .base                   = {
> >               .cra_name               = "deflate",
> >               .cra_driver_name        = "deflate-iaa",
> > +             .cra_flags              = CRYPTO_ALG_ASYNC,
> >               .cra_ctxsize            = sizeof(struct iaa_compression_ctx),
> >               .cra_module             = THIS_MODULE,
> >               .cra_priority           = IAA_ALG_PRIORITY,
>
> Good catch.  I think this should go into a separate bug-fix patch.

done.

>
> > diff --git a/include/crypto/acompress.h b/include/crypto/acompress.h
> > index 574cffc90730..07bd8f6bc79a 100644
> > --- a/include/crypto/acompress.h
> > +++ b/include/crypto/acompress.h
> > @@ -160,6 +160,11 @@ static inline void acomp_request_set_tfm(struct acomp_req *req,
> >       req->base.tfm = crypto_acomp_tfm(tfm);
> >  }
> >
> > +static inline u32 crypto_acomp_get_alg_flags(struct crypto_acomp *tfm)
> > +{
> > +     return crypto_tfm_alg_flags(crypto_acomp_tfm(tfm));
> > +}
>
> Sorry, my mistake.  I shouldn't have suggested copying skcipher
> since that gets the tfm flags as opposed to the alg flags which
> you've found out.

no worries, Herbert :-)

>
> I think you should just go with your original function acomp_is_async
> but do it like this:
>
> static inline bool acomp_is_async(struct crypto_acomp *tfm)
> {
>         return crypto_comp_alg_common(tfm)->base.cra_flags &
>                CRYPTO_ALG_ASYNC;
> }

ok, I will do that. Besides, i'm also curious if we can open a
discussion, for example, letting offload drivers be able to work
in both sync mode and async.  A scenario I can imagine is as
below,

for zswap, and page size is configured as 4KiB. in this
case, offload hardware might be able to compress/decompress
much faster than CPU, but the event-wait, wake-up, schedule
latency might add the total time for a compression and
decompression. thus offload might work worse than CPU
though hardware-accelerator is much faster than CPU.
In this case, it seems good to let offload drivers poll
the completion of compression and decompression
compared with sleep and wait.

On the other hand, when PAGE SIZE is big, for example,
ARM64's PAGE_SIZE could be 64KiB. in the future,
mTHP/large folios project might also ask for larger data
to be swapped as a whole. we will only need a schedule/
wake-up for 64KiB or larger data,  the  compression/
decompression time is much longer, in this case, async
mode might help to decrease CPU usage while providing
lower comp/decomp latency.

So it could be something like:
if data is short, acomp driver works by polling; if data is
long, acomp driver works by sleeping and waiting.

it would be perfect if Zhou or Yang Shen can help collect some
data to back up the discussion.

>
> Thanks,
> --
> Email: Herbert Xu <herbert@xxxxxxxxxxxxxxxxxxx>
> Home Page: http://gondor.apana.org.au/~herbert/
> PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Thanks
Barry