Hi all,
After merging the crypto tree, today's linux-next build (powerpc
ppc64_defconfig) failed like this:
In file included from mm/zswap.c:30:
include/crypto/acompress.h: In function 'acomp_is_async':
include/crypto/acompress.h:124:16: error: implicit declaration of function 'crypto_comp_alg_common'; did you mean 'crypto_tfm_alg_name'? [-Werror=implicit-function-declaration]
124 | return crypto_comp_alg_common(tfm)->base.cra_flags &
| ^~~~~~~~~~~~~~~~~~~~~~
| crypto_tfm_alg_name
include/crypto/acompress.h:124:43: error: invalid type argument of '->' (have 'int')
124 | return crypto_comp_alg_common(tfm)->base.cra_flags &
| ^~
include/crypto/acompress.h:126:1: error: control reaches end of non-void function [-Werror=return-type]
126 | }
| ^
cc1: some warnings being treated as errors
Caused by commit
86464db929ca ("crypto: introduce: acomp_is_async to expose if comp drivers might sleep")
from the mm-unstable branch of the mm tree interacting with commit
2beb81fbf0c0 ("crypto: remove CONFIG_CRYPTO_STATS")
from the crypto tree.
I have reverted these commits from the mm-unstable branch for today:
86464db929ca ("crypto: introduce: acomp_is_async to expose if comp drivers might sleep")
791f798331bc ("mm/zswap: remove the memcpy if acomp is not sleepable")
I will stop merging the -unstable parts of the mm tree from tomorrow.
--
Cheers,
Stephen Rothwell
Hi Herbert,
On Wed, Mar 13, 2024 at 09:45:15AM +0800, Herbert Xu wrote:
> On Wed, Mar 13, 2024 at 09:43:52AM +0800, Herbert Xu wrote:
> >
> > Instead of using the non-existent crypto_comp_alg_common, it
> > should do something like
> >
> > return crypto_acomp_tfm(tfm)->__crt_alg->cra_flags & ...
>
> Nevermind, the stats revert should not have removed acomp infrastructure
> like this.
>
> I'll revert it.
>
> Thanks,
The only user of comp_alg_common was the crypto stats, and it was introduced by
a refactoring of the crypto stats (commit 0a742389bcc0, "crypto: acomp - Count
error stats differently"), so it seems appropriate to remove it for now.
If you could go through my patch and explain what other unused code related to
the crypto stats you might consider to be "infrastructure" that should not be
removed, that would be helpful.
- Eric
On Wed, Mar 13, 2024 at 03:12:11PM +1300, Barry Song wrote:
> > The only user of comp_alg_common was the crypto stats, and it was introduced by
> > a refactoring of the crypto stats (commit 0a742389bcc0, "crypto: acomp - Count
> > error stats differently"), so it seems appropriate to remove it for now.
> >
> > If you could go through my patch and explain what other unused code related to
> > the crypto stats you might consider to be "infrastructure" that should not be
> > removed, that would be helpful.
>
> Hi Eric,
> that user is mm/zswap [1], apologize it was not in crypto tree. I guess you might
> just need a new version to keep that one :-)
>
> [1] https://lore.kernel.org/linux-mm/[email protected]/
>
That code is just reading the algorithm flags, which exist regardless of whether
struct comp_alg_common does or not. The exact way to get at those flags just
changes slightly.
- Eric
On Tue, Mar 12, 2024 at 07:01:12PM -0700, Eric Biggers wrote:
>
> The only user of comp_alg_common was the crypto stats, and it was introduced by
> a refactoring of the crypto stats (commit 0a742389bcc0, "crypto: acomp - Count
> error stats differently"), so it seems appropriate to remove it for now.
>
> If you could go through my patch and explain what other unused code related to
> the crypto stats you might consider to be "infrastructure" that should not be
> removed, that would be helpful.
The first patch should only remove code directly related to
STATS. Any removal of code that is rendered useless should
be done in one or more subsequent patches.
Thanks,
--
Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
On Wed, Mar 13, 2024 at 09:43:52AM +0800, Herbert Xu wrote:
>
> Instead of using the non-existent crypto_comp_alg_common, it
> should do something like
>
> return crypto_acomp_tfm(tfm)->__crt_alg->cra_flags & ...
Nevermind, the stats revert should not have removed acomp infrastructure
like this.
I'll revert it.
Thanks,
--
Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
On Wed, Mar 13, 2024 at 11:57:51AM +1100, Stephen Rothwell wrote:
> Hi all,
>
> After merging the crypto tree, today's linux-next build (powerpc
> ppc64_defconfig) failed like this:
>
> In file included from mm/zswap.c:30:
> include/crypto/acompress.h: In function 'acomp_is_async':
> include/crypto/acompress.h:124:16: error: implicit declaration of function 'crypto_comp_alg_common'; did you mean 'crypto_tfm_alg_name'? [-Werror=implicit-function-declaration]
> 124 | return crypto_comp_alg_common(tfm)->base.cra_flags &
> | ^~~~~~~~~~~~~~~~~~~~~~
> | crypto_tfm_alg_name
> include/crypto/acompress.h:124:43: error: invalid type argument of '->' (have 'int')
> 124 | return crypto_comp_alg_common(tfm)->base.cra_flags &
> | ^~
> include/crypto/acompress.h:126:1: error: control reaches end of non-void function [-Werror=return-type]
> 126 | }
> | ^
> cc1: some warnings being treated as errors
>
> Caused by commit
>
> 86464db929ca ("crypto: introduce: acomp_is_async to expose if comp drivers might sleep")
Instead of using the non-existent crypto_comp_alg_common, it
should do something like
return crypto_acomp_tfm(tfm)->__crt_alg->cra_flags & ...
Thanks,
--
Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
On Tue, Mar 12, 2024 at 07:29:12PM -0700, Eric Biggers wrote:
>
> comp_alg_common was part of the implementation of STATS.
I understand that comp_alg_common becomes trivial without stats,
but it is still something that is useful in the API because for
a given acomp tfm, we don't know whether it's an acomp_alg or
scomp_alg.
> I'll do the best I can to interpret your request, but if you could actually
> review my original patch and let me know if there are any other specific changes
> you'd like besides keeping comp_alg_common, that would be helpful as then maybe
> the next version won't get reverted.
I think comp_alg_common is the only part that should be excised.
Thanks,
--
Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
> On Wed, Mar 13, 2024 at 09:43:52AM +0800, Herbert Xu wrote:
>>
>> Instead of using the non-existent crypto_comp_alg_common, it
>> should do something like
>>
>> return crypto_acomp_tfm(tfm)->__crt_alg->cra_flags & ...
>
> Nevermind, the stats revert should not have removed acomp infrastructure
> like this.
>
> I'll revert it.
>
I assume mm-unstable doesn't need to do anything since Herbert is going
to revert the crypto commit "crypto: remove CONFIG_CRYPTO_STATS" which
accidently removed the API. once that is done, linux-next is going to
be ok.
> Thanks,
> --
> Email: Herbert Xu <[email protected]>
> Home Page: http://gondor.apana.org.au/~herbert/
> PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
Thanks
Barry