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@xxxxxxxxxxxxxx > > > > 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
