On 21 July 2017 at 14:24, Ard Biesheuvel <ard.biesheuvel@xxxxxxxxxx> wrote: > 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. I mean CRYPTO_ACOMP_ALLOC_OUTPUT should not be set if CRYPTO_TFM_REQ_MAY_SLEEP is cleared. > - 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.
