On 21 July 2017 at 14:11, Herbert Xu <herbert@xxxxxxxxxxxxxxxxxxx> wrote: > On Fri, Jul 21, 2017 at 02:09:39PM +0100, Ard Biesheuvel wrote: >> >> Right. And is req->dst guaranteed to be assigned in that case? Because >> crypto_scomp_sg_alloc() happily allocates pages and kmalloc()s the >> scatterlist if req->dst == NULL. >> >> Is there any way we could make these scratch buffers part of the >> request structure instead? Or at least defer allocating them until the >> first call to crypto_scomp_init_tfm()? And on top of that, we should >> probably only use the per-CPU scratch buffers if >> CRYPTO_TFM_REQ_MAY_SLEEP is cleared, because in that case, we are not >> pre-emptible to begin with, and the concern does not apply. > > Well for now scomp is new code so nothing actually uses it. But > the idea is to convert IPcomp over to acomp and scomp will fill in > as the compatibility glue. > Yes, I guessed as much. > The medium/long-term plan is to convert IPcomp over to a partial > decompression model so that we can allocate pages instead of a > contiguous chunk as we do now. > OK, but that doesn't really answer any of my questions: - Should we enforce that CRYPTO_ACOMP_ALLOC_OUTPUT is mutually exclusive with CRYPTO_TFM_REQ_MAY_SLEEP, or should crypto_scomp_sg_alloc() always use GFP_ATOMIC? We need one or the other, because the current situation is buggy. - Could we move the scratch buffers to the request structure instead? Or may that be allocated from softirq context as well? - Would you mind a patch that defers the allocation of the per-CPU buffers to the first invocation of crypto_scomp_init_tfm()? - Would you mind a patch that makes the code only use the per-CPU buffers if we are running atomically to begin with? Thanks, Ard.
