Hi Robin,
Signed-off-by: John Garry<john.ga...@huawei.com>
Mangled patch? (no "---" separator here)
hmm... not sure. As an experiment, I just downloaded this patch from
lore.kernel.org and it applies ok.
Overall this looks great, just a few comments further down...
...
+}
+EXPORT_SYMBOL_GPL(iova_domain_init_rcaches);
+
+void iova_domain_free_rcaches(struct iova_domain *iovad)
+{
+ cpuhp_state_remove_instance_nocalls(CPUHP_IOMMU_IOVA_DEAD,
+ &iovad->cpuhp_dead);
+ free_iova_rcaches(iovad);
}
+EXPORT_SYMBOL_GPL(iova_domain_free_rcaches);
I think we should continue to expect external callers to clean up with
put_iova_domain().
ok, fine, makes sense
If they aren't doing that already they have a bug
(albeit minor), and we don't want to give the impression that it's OK to
free the caches at any point*other* than tearing down the whole
iova_domain, since the implementation really wouldn't expect that.
/*
* Try inserting IOVA range starting with 'iova_pfn' into 'rcache', and
@@ -831,7 +872,7 @@ static unsigned long iova_rcache_get(struct iova_domain
*iovad,
{
unsigned int log_size = order_base_2(size);
- if (log_size >= IOVA_RANGE_CACHE_MAX_SIZE)
+ if (log_size >= IOVA_RANGE_CACHE_MAX_SIZE || !iovad->rcaches)
return 0;
..
@@ -102,6 +92,8 @@ struct iova *reserve_iova(struct iova_domain *iovad,
unsigned long pfn_lo,
unsigned long pfn_hi);
void init_iova_domain(struct iova_domain *iovad, unsigned long granule,
unsigned long start_pfn);
+int iova_domain_init_rcaches(struct iova_domain *iovad);
+void iova_domain_free_rcaches(struct iova_domain *iovad);
As above, I vote for just forward-declaring the free routine in iova.c
and keeping it entirely private.
ok
struct iova *find_iova(struct iova_domain *iovad, unsigned long pfn);
void put_iova_domain(struct iova_domain *iovad);
#else
@@ -157,6 +149,15 @@ static inline void init_iova_domain(struct iova_domain
*iovad,
{
}
+static inline int iova_domain_init_rcaches(struct iova_domain *iovad)
+{
+ return -ENOTSUPP;
+}
+
+static inline void iova_domain_free_rcaches(struct iova_domain *iovad)
+{
+}
+
I'd be inclined not to add stubs at all - I think it's a reasonable
assumption that anyone involved enough to care about rcaches has a hard
dependency on IOMMU_IOVA already.
So iova_domain_free_rcaches() would disappear from here as a result of
the changes discussed above.
As for iova_domain_init_rcaches(), I was just following the IOMMU/IOVA
coding practice - that is, always stub.
It's certainly the case today, and I'd
hardly want to encourage more users anyway.
I think that stronger deterrents would be needed :)
Anyway, I can remove it.
Thanks,
John
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
