Re: [PATCH 2/2] iommu/iova: Free global iova rcache on iova alloc failure

[John Garry]

On 13/08/2020 06:36, Vijayanand Jitta wrote:
> On 8/12/2020 8:46 PM, Joerg Roedel wrote:
> > On Mon, Aug 03, 2020 at 03:30:48PM +0530, Vijayanand Jitta wrote:
> > > ping?
> >
> > Please repost when v5.9-rc1 is released and add
> >
> > 	Robin Murphy <robin.murphy@xxxxxxx>
> >
> > on your Cc list.
> >
> > Thanks,
> >
> > 	Joerg
>
> Sure, will do.
>
> Thanks,
> Vijay

And a cover letter would be useful also, to at least us know what 
changes have been made per version.

>
> diff --git a/drivers/iommu/iova.c b/drivers/iommu/iova.c
> index 4e77116..5836c87 100644
> --- a/drivers/iommu/iova.c
> +++ b/drivers/iommu/iova.c
> @@ -442,6 +442,7 @@ struct iova *find_iova(struct iova_domain *iovad, 
unsigned long pfn)
>   		flush_rcache = false;
>   		for_each_online_cpu(cpu)
>   			free_cpu_cached_iovas(cpu, iovad);
> +		free_global_cached_iovas(iovad);

Have you seen an issue where this is needed?

If we have filled the IOVA space, then as a measure we flush all the CPU 
rcaches, and then there should be free IOVA space and we can make 
progress. And it may be useful to still have the global depots to use 
straightaway then to swap into empty CPU rcaches.

>   		goto retry;
>   	}
>
> @@ -1055,5 +1056,27 @@ void free_cpu_cached_iovas(unsigned int cpu, 
struct iova_domain *iovad)
>   	}
>   }
>
> +/*
> + * free all the IOVA ranges of global cache
> + */
> +void free_global_cached_iovas(struct iova_domain *iovad)
> +{
> +	struct iova_rcache *rcache;
> +	unsigned long flags;
> +	int i, j;
> +
> +	for (i = 0; i < IOVA_RANGE_CACHE_MAX_SIZE; ++i) {
> +		rcache = &iovad->rcaches[i];
> +		spin_lock_irqsave(&rcache->lock, flags);
> +		for (j = 0; j < rcache->depot_size; ++j) {
> +			iova_magazine_free_pfns(rcache->depot[j], iovad);
> +			iova_magazine_free(rcache->depot[j]);
> +			rcache->depot[j] = NULL;
> +		}
> +		rcache->depot_size = 0;
> +		spin_unlock_irqrestore(&rcache->lock, flags);
> +	}
> +}
> +
>   MODULE_AUTHOR("Anil S Keshavamurthy <anil.s.keshavamurthy@xxxxxxxxx>");
>   MODULE_LICENSE("GPL");
> diff --git a/include/linux/iova.h b/include/linux/iova.h
> index a0637ab..a905726 100644
> --- a/include/linux/iova.h
> +++ b/include/linux/iova.h

why is this in the iova.h, when it is only used internally in iova.c?

> @@ -163,6 +163,7 @@ int init_iova_flush_queue(struct iova_domain *iovad,
>   struct iova *split_and_remove_iova(struct iova_domain *iovad,
>   	struct iova *iova, unsigned long pfn_lo, unsigned long pfn_hi);
>   void free_cpu_cached_iovas(unsigned int cpu, struct iova_domain 
*iovad);
> +void free_global_cached_iovas(struct iova_domain *iovad);
>   #else
>   static inline int iova_cache_get(void)
>   {
> @@ -270,6 +271,11 @@ static inline void 
free_cpu_cached_iovas(unsigned int cpu,
>   					 struct iova_domain *iovad)
>   {
>   }
> +
> +static inline void free_global_cached_iovas(struct iova_domain *iovad)
> +{
> +}
> +
>   #endif
>
>   #endif