Re: [PATCH 07/10] crypto: Use ARCH_DMA_MINALIGN instead of ARCH_KMALLOC_MINALIGN

[Catalin Marinas <catalin.marinas@arm.com>]

On Mon, Apr 18, 2022 at 04:37:17PM +0800, Herbert Xu wrote:
> On Sun, Apr 17, 2022 at 05:30:27PM +0100, Catalin Marinas wrote:
> > Do you mean as per Ard's proposal here:
> > 
> > https://lore.kernel.org/r/CAMj1kXH0x5Va7Wgs+mU1ONDwwsazOBuN4z4ihVzO2uG-n41Kbg@mail.gmail.com
> > 
> > struct crypto_request {
> > 	union {
> > 		struct {
> > 			... fields ...
> > 		};
> > 		u8 __padding[ARCH_DMA_MINALIGN];
> > 	};
> > 	void __ctx[]  __aligned(CRYPTO_MINALIGN);
> > };
> > 
> > If CRYPTO_MINALIGN is lowered to, say, 8 (to be the same as lowest
> > ARCH_KMALLOC_MINALIGN), the __alignof__(req->__ctx) would be 8.
> > Functions like crypto_tfm_ctx_alignment() will return 8 when what you
> > need is 128. We can change those functions to return ARCH_DMA_MINALIGN
> > instead or always bump cra_alignmask to ARCH_DMA_MINALIGN-1.
> 
> OK, at this point I think we need to let the code do the talking :)
> 
> I've seen Ard's patches already and I think I understand what your
> needs are.  So let me whip up some code to show you guys what I
> think needs to be done.

BTW before you have a go at this, there's also Linus' idea that does not
change the crypto code (at least not functionally). Of course, you and
Ard can still try to figure out how to reduce the padding but if we go
with Linus' idea of a new GFP_NODMA flag, there won't be any changes to
the crypto code as long as it doesn't pass such flag. So, the options:

1. Change ARCH_KMALLOC_MINALIGN to 8 (or ARCH_SLAB_MINALIGN if higher)
   while keeping ARCH_DMA_MINALIGN to 128. By default kmalloc() will
   honour the 128-byte alignment, unless GDP_NODMA is passed. This still
   requires changing CRYPTO_MINALIGN to ARCH_DMA_MINALIGN but there is
   no functional change, kmalloc() without the new flag will return
   CRYPTO_MINALIGN-aligned pointers.

2. Leave ARCH_KMALLOC_MINALIGN as ARCH_DMA_MINALIGN (128) and introduce
   a new GFP_PACKED (I think it fits better than 'NODMA') flag that
   reduces the minimum kmalloc() below ARCH_KMALLOC_MINALIGN (and
   probably at least ARCH_SLAB_MINALIGN). It's equivalent to (1) but
   does not touch the crypto code at all.

(1) and (2) are the same, just minor naming difference. Happy to go with
any of them. They still have the downside that we need to add the new
GFP_ flag to those hotspots that allocate small objects (Arnd provided
an idea on how to find them with ftrace) but at least we know it won't
inadvertently break anything.

-- 
Catalin