Break up iommu from monolithic lock for the map to multiple pools/locks

In iperf experiments running linux as the Tx side (TCP client) with 10 threads results in a severe performance drop when TSO is disabled, indicating a weakness in the software that turns out to be avoidable after this patch.

Baseline numbers before this patch:
with default settings (TSO enabled) : 9-9.5 Gbps
Disable TSO using ethtool- drops badly: 2-3 Gbps. (!)

What this patch does:
Output from lockstat flags the iommu->lock as the hottest lock, showing something of the order of 21M contentions out of 27M acquisitions, and an average wait time of 26 us for the lock. This is not efficient. A better design is to follow the ppc model, where the iommu_table has multiple pools, each stretching over a segment of the map, and with a separate lock for each pool. This model allows for better parallelization of the iommu map search.

After this patch, iperf client with 10 threads, can give a throughput of at least 8.5 Gbps, even when TSO is disabled.

---
 arch/sparc/include/asm/iommu_64.h |  34 ++++++++
 arch/sparc/kernel/iommu.c         | 138 +++++++++++++++++++++++++++++
 arch/sparc/kernel/iommu_common.h  |  11 +++
 arch/sparc/kernel/pci_impl.h      |   5 ++
 arch/sparc/kernel/pci_sun4v.c     | 179 +++++++++++++++++++-------------------
 5 files changed, 276 insertions(+), 91 deletions(-)

index 2b9321a..f12287a 100644
@@ -17,12 +17,22 @@
 
 #define IOMMU_NUM_CTXS	4096
 
+#define IOMMU_POOL_HASHBITS     4
+#define IOMMU_NR_POOLS          (1 << IOMMU_POOL_HASHBITS)
+
 struct iommu_arena {
 	unsigned long	*map;
 	unsigned int	hint;
 	unsigned int	limit;
 };
 
+struct iommu_pool {
+	unsigned long	start;
+	unsigned long	end;
+	unsigned long	hint;
+	spinlock_t	lock;
+};
+
 struct iommu {
 	spinlock_t		lock;
 	struct iommu_arena	arena;

@@ -43,6 +53,30 @@ struct iommu {
 	u32			dma_addr_mask;
 };
 
+struct iommu_table {
+	spinlock_t		table_lock;
+	struct iommu_arena	notused2;
+	void			(*flush_all)(struct iommu *);
+	iopte_t			*page_table;
+	u32			page_table_map_base;
+	unsigned long		iommu_control;
+	unsigned long		iommu_tsbbase;
+	unsigned long		iommu_flush;
+	unsigned long		iommu_flushinv;
+	unsigned long		iommu_tags;
+	unsigned long		iommu_ctxflush;
+	unsigned long		write_complete_reg;
+	unsigned long		dummy_page;
+	unsigned long		dummy_page_pa;
+	unsigned long		ctx_lowest_free;
+	DECLARE_BITMAP(ctx_bitmap, IOMMU_NUM_CTXS);
+	u32			dma_addr_mask;
+	unsigned long		nr_pools;
+	struct iommu_pool	arena_pool[IOMMU_NR_POOLS];
+	unsigned long		poolsize;
+	unsigned long		*map;
+};
+
 struct strbuf {
 	int			strbuf_enabled;
 	unsigned long		strbuf_control;

index 1ec0de4..fbaa3df 100644
@@ -56,4 +56,15 @@ void iommu_range_free(struct iommu *iommu,
 		      dma_addr_t dma_addr,
 		      unsigned long npages);
 
+unsigned long iommu_tbl_range_alloc(struct device *dev,
+				struct iommu_table *iommu,
+				unsigned long npages,
+				unsigned long *handle,
+				unsigned int pool_hash);
+void iommu_tbl_range_free(struct iommu_table *iommu,
+			  dma_addr_t dma_addr,
+			  unsigned long npages,
+    			  void (*demap)(void *, unsigned long, unsigned long),
+			  void *demap_arg);
+
 #endif /* _IOMMU_COMMON_H */

index 75803c7..315257d 100644
@@ -142,7 +142,12 @@ struct pci_pbm_info {
 	struct strbuf			stc;
 
 	/* IOMMU state, potentially shared by both PBM segments. */
+#ifdef notdef
 	struct iommu			*iommu;
+#else
+	/* change only pci_sun4v and dma stuff first.. */
+	void				*iommu;
+#endif
 
 	/* Now things for the actual PCI bus probes. */
 	unsigned int			pci_first_busno;

1.8.4.2

From: Sowmini Varadhan <[email protected]>
Date: Fri, 19 Dec 2014 10:16:16 -0500

If this is such a better and more scalable algorithm for IOMMU arena DMA region allocation, then instead of one platform after another putting a private implementation under arch/, the generic IOMMU code should be adjusted instead.

Right?

Yes, it should.. that would be the eventual goal. But I wanted to do this incrementally (IOW, I was scared to boil the ocean just yet :-) I see pci, schizo, ldc using this as well. Even if I managed to convert all of them successfully, the patch-set would be pretty massive)

So I wanted to get some feedback on this first

--Sowmini

Then again, I just realized that you are talking about factorizing common code across multiple architectures, instead of multiple drivers within sparc.

yes, that would be even better.

I could look into that, but would need help from other arch experts for help with extraction of the platform-dependant parts, implementation and testing. Looks like pci-calgary_64.c also has an obvious copy of the powerpc algorithm, and there are other *iommu.c implementations as well, where the duplication may be there, but is not obvious (from cscope etc.).

--Sowmini

From: Sowmini Varadhan <[email protected]>
Date: Fri, 19 Dec 2014 13:31:33 -0500

I suspect that if we come up with something that should work for both powerpc and sparc, we can just let [email protected] know about it to give a heads up for other platform maintainers so they can do the conversions for their code too.

The core operation is the range allocation. So if you just put the powerpc alloc/free code (basically, the bulk of your patch) into lib/iommu-common.c or similar, that ought to be a good starting point.

The patch set would look like:

1) move powerpc iommu range alloc/free into lib/iommu-common.c

2) make sparc64 use it

sounds good, I'll do this and get back over the next week. (Some things like the demap function pointer may need to be generalized a bit, e.g., pass the iommu_pool * to the callback for other archs)

From: Sowmini Varadhan <[email protected]>
Date: Fri, 19 Dec 2014 15:06:51 -0500

It might be cleaner to have a "const struct iommu_ops *" kind of thing in the base iommu structure.

Agreed. The powerpc iommu_table also has a large_pool that I did not look too carefully into - used for allocating npages > 15 , iirc. I dont know how commonly that happens.

--Sowmini

From: Sowmini Varadhan <[email protected]>
Date: Fri, 19 Dec 2014 15:44:54 -0500

That large_pool mechanism might help prevent the other pools from getting fragmented and thus lead to premature allocation failures.

I see. Looks like the scsi driver uses sun4v_pci to ask for 7-8 pages on occasion - I'll add the large_pool hooks in and test it with scsi, using some absurd check like (npages > 5) for the purpose of testing the large_pool code.

--Sowmini