Re: [PATCH RFC 0/2] dma-pool: allow user to disable atomic pool

From: Robin Murphy
Date: Thu Jun 24 2021 - 06:47:40 EST


On 2021-06-24 10:29, Baoquan He wrote:
On 06/24/21 at 08:40am, Christoph Hellwig wrote:
So reduce the amount allocated. But the pool is needed for proper
operation on systems with memory encryption. And please add the right
maintainer or at least mailing list for the code you're touching next
time.

Oh, I thoutht it's memory issue only, should have run
./scripts/get_maintainer.pl. sorry.

About reducing the amount allocated, it may not help. Because on x86_64,
kdump kernel doesn't put any page of memory into buddy allocator of DMA
zone. Means it will defenitely OOM for atomic_pool_dma initialization.

Wondering in which case or on which device the atomic pool is needed on
AMD system with mem encrytion enabled. As we can see, the OOM will
happen too in kdump kernel on Intel system, even though it's not
necessary.

Hmm, I think the Kconfig reshuffle has actually left a slight wrinkle here. For DMA_DIRECT_REMAP=y we can assume an atomic pool is always needed, since that was the original behaviour anyway. However the implications of AMD_MEM_ENCRYPT=y are different - even if support is enabled, it still should only be relevant if mem_encrypt_active(), so it probably does make sense to have an additional runtime gate on that.

From a quick scan, use of dma_alloc_from_pool() already depends on force_dma_unencrypted() so that's probably fine already, but I think we'd need a bit of extra protection around dma_free_from_pool() to prevent gen_pool_has_addr() dereferencing NULL if the pools are uninitialised, even with your proposed patch as it is. Presumably nothing actually called dma_direct_free() when you tested this?

Robin.