Re: [PATCH v2 RESEND] iommu/iova: Move CPU magazine init to first insert

[Robin Murphy <robin.murphy@arm.com>]

On 24/07/2026 2:44 pm, Michal Clapinski wrote:
> From: Logan Odell <loganodell@google.com>
> 
> A large amount of memory may be allocated for these magazines on
> machines with a lot of IOMMU groups and CPU cores. Not all may be used
> as some devices may be unused or be bound to drivers that do not use the
> DMA-API. Furthermore, some drivers may not use all levels or CPUs.
> 
> Move the initialization of the loaded and prev magazines for each CPU on
> the first attempt to try to insert a freed IOVA to them.
> 
> Signed-off-by: Logan Odell <loganodell@google.com>
> Signed-off-by: Michal Clapinski <mclapinski@google.com>
> ---
> v2: only rebase
> ---
>   drivers/iommu/iova.c | 27 ++++++++++++++++++---------
>   1 file changed, 18 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/iommu/iova.c b/drivers/iommu/iova.c
> index 021daf6528de..1a4bbf45dbb9 100644
> --- a/drivers/iommu/iova.c
> +++ b/drivers/iommu/iova.c
> @@ -621,6 +621,9 @@ iova_magazine_free_pfns(struct iova_magazine *mag, struct iova_domain *iovad)
>   	unsigned long flags;
>   	int i;
>   
> +	if (!mag)
> +		return;
> +
>   	spin_lock_irqsave(&iovad->iova_rbtree_lock, flags);
>   
>   	for (i = 0 ; i < mag->size; ++i) {
> @@ -737,14 +740,7 @@ int iova_domain_init_rcaches(struct iova_domain *iovad)
>   		}
>   		for_each_possible_cpu(cpu) {
>   			cpu_rcache = per_cpu_ptr(rcache->cpu_rcaches, cpu);
> -
>   			spin_lock_init(&cpu_rcache->lock);
> -			cpu_rcache->loaded = iova_magazine_alloc(GFP_KERNEL);
> -			cpu_rcache->prev = iova_magazine_alloc(GFP_KERNEL);
> -			if (!cpu_rcache->loaded || !cpu_rcache->prev) {
> -				ret = -ENOMEM;
> -				goto out_err;
> -			}
>   		}
>   	}
>   
> @@ -777,8 +773,20 @@ static bool __iova_rcache_insert(struct iova_domain *iovad,
>   	cpu_rcache = raw_cpu_ptr(rcache->cpu_rcaches);
>   	spin_lock_irqsave(&cpu_rcache->lock, flags);
>   
> +	if (!cpu_rcache->loaded) {
> +		cpu_rcache->loaded = iova_magazine_alloc(GFP_ATOMIC | __GFP_NOWARN);
> +		if (!cpu_rcache->loaded)
> +			goto unlock;

I think it would make sense to at least maintain the existing allocation 
pattern to preserve the invariant that if loaded is valid then prev must 
be valid as well. I can accept the argument for not allocating them at 
all, but as soon as we _do_ start using the mechanism then there is 
really no reason to ever have one without the other.

Furthermore it would seem more logical to either do this in 
__iova_rcache_get() if you want late-initialisation to be an explicit 
special case. Or alternatively, just fold it even more into the existing 
flow - AFAICS the only real difference should be that if both loaded and 
prev are "full" (i.e. unable to accept the PFN) due to not existing at 
all, then there's obviously nothing to push to the depot, but otherwise 
that path should work just the same to initialise loaded at least. I 
guess it might take a bit more fiddling to ensure prev ends up ever 
getting allocated though...

Thanks,
Robin.

> +	}
> +
>   	if (!iova_magazine_full(cpu_rcache->loaded)) {
>   		can_insert = true;
> +	} else if (!cpu_rcache->prev) {
> +		cpu_rcache->prev = iova_magazine_alloc(GFP_ATOMIC | __GFP_NOWARN);
> +		if (!cpu_rcache->prev)
> +			goto unlock;
> +		swap(cpu_rcache->prev, cpu_rcache->loaded);
> +		can_insert = true;
>   	} else if (!iova_magazine_full(cpu_rcache->prev)) {
>   		swap(cpu_rcache->prev, cpu_rcache->loaded);
>   		can_insert = true;
> @@ -799,6 +807,7 @@ static bool __iova_rcache_insert(struct iova_domain *iovad,
>   	if (can_insert)
>   		iova_magazine_push(cpu_rcache->loaded, iova_pfn);
>   
> +unlock:
>   	spin_unlock_irqrestore(&cpu_rcache->lock, flags);
>   
>   	return can_insert;
> @@ -831,9 +840,9 @@ static unsigned long __iova_rcache_get(struct iova_rcache *rcache,
>   	cpu_rcache = raw_cpu_ptr(rcache->cpu_rcaches);
>   	spin_lock_irqsave(&cpu_rcache->lock, flags);
>   
> -	if (!iova_magazine_empty(cpu_rcache->loaded)) {
> +	if (cpu_rcache->loaded && !iova_magazine_empty(cpu_rcache->loaded)) {
>   		has_pfn = true;
> -	} else if (!iova_magazine_empty(cpu_rcache->prev)) {
> +	} else if (cpu_rcache->prev && !iova_magazine_empty(cpu_rcache->prev)) {
>   		swap(cpu_rcache->prev, cpu_rcache->loaded);
>   		has_pfn = true;
>   	} else {