git: 815db559eccc - stable/13 - busdma_iommu: Fine-grained locking for the dmamap's map list

From: Doug Moore <dougm_at_FreeBSD.org>
Date: Wed, 06 Jul 2022 17:11:31 UTC
The branch stable/13 has been updated by dougm:

URL: https://cgit.FreeBSD.org/src/commit/?id=815db559eccceecd9293cd174291e59d145b8d51

commit 815db559eccceecd9293cd174291e59d145b8d51
Author:     Alan Cox <alc@FreeBSD.org>
AuthorDate: 2022-06-22 21:51:47 +0000
Commit:     Doug Moore <dougm@FreeBSD.org>
CommitDate: 2022-07-06 17:11:10 +0000

    busdma_iommu: Fine-grained locking for the dmamap's map list
    
    Introduce fine-grained locking on the dmamap's list of map entries,
    replacing the use of the domain lock.  This is not the most significant
    source of lock contention, but it is the easiest to address.
    
    Reviewed by:    kib
    MFC after:      2 weeks
    Differential Revision:  https://reviews.freebsd.org/D35557
    
    (cherry picked from commit eeb46578c21ad37866f49f3bbb3ac738b44abbf6)
---
 sys/dev/iommu/busdma_iommu.c | 56 +++++++++++++++++++++-----------------------
 sys/dev/iommu/busdma_iommu.h |  7 ++++++
 2 files changed, 34 insertions(+), 29 deletions(-)

diff --git a/sys/dev/iommu/busdma_iommu.c b/sys/dev/iommu/busdma_iommu.c
index a8ba98469345..ab3b9ec4829a 100644
--- a/sys/dev/iommu/busdma_iommu.c
+++ b/sys/dev/iommu/busdma_iommu.c
@@ -457,6 +457,7 @@ iommu_bus_dmamap_create(bus_dma_tag_t dmat, int flags, bus_dmamap_t *mapp)
 			return (ENOMEM);
 		}
 	}
+	IOMMU_DMAMAP_INIT(map);
 	TAILQ_INIT(&map->map_entries);
 	map->tag = tag;
 	map->locked = true;
@@ -472,18 +473,16 @@ iommu_bus_dmamap_destroy(bus_dma_tag_t dmat, bus_dmamap_t map1)
 {
 	struct bus_dma_tag_iommu *tag;
 	struct bus_dmamap_iommu *map;
-	struct iommu_domain *domain;
 
 	tag = (struct bus_dma_tag_iommu *)dmat;
 	map = (struct bus_dmamap_iommu *)map1;
 	if (map != NULL) {
-		domain = tag->ctx->domain;
-		IOMMU_DOMAIN_LOCK(domain);
+		IOMMU_DMAMAP_LOCK(map);
 		if (!TAILQ_EMPTY(&map->map_entries)) {
-			IOMMU_DOMAIN_UNLOCK(domain);
+			IOMMU_DMAMAP_UNLOCK(map);
 			return (EBUSY);
 		}
-		IOMMU_DOMAIN_UNLOCK(domain);
+		IOMMU_DMAMAP_DESTROY(map);
 		free(map, M_IOMMU_DMAMAP);
 	}
 	tag->map_count--;
@@ -624,10 +623,11 @@ iommu_bus_dmamap_load_something1(struct bus_dma_tag_iommu *tag,
 		    (uintmax_t)entry->start, (uintmax_t)entry->end,
 		    (uintmax_t)buflen1, (uintmax_t)tag->common.maxsegsz));
 
-		IOMMU_DOMAIN_LOCK(domain);
+		KASSERT((entry->flags & IOMMU_MAP_ENTRY_MAP) != 0,
+		    ("entry %p missing IOMMU_MAP_ENTRY_MAP", entry));
+		IOMMU_DMAMAP_LOCK(map);
 		TAILQ_INSERT_TAIL(&map->map_entries, entry, dmamap_link);
-		entry->flags |= IOMMU_MAP_ENTRY_MAP;
-		IOMMU_DOMAIN_UNLOCK(domain);
+		IOMMU_DMAMAP_UNLOCK(map);
 		TAILQ_INSERT_TAIL(unroll_list, entry, unroll_link);
 
 		segs[seg].ds_addr = entry->start + offset;
@@ -650,8 +650,8 @@ iommu_bus_dmamap_load_something(struct bus_dma_tag_iommu *tag,
 {
 	struct iommu_ctx *ctx;
 	struct iommu_domain *domain;
-	struct iommu_map_entry *entry, *entry1;
-	struct iommu_map_entries_tailq unroll_list;
+	struct iommu_map_entry *entry;
+	struct iommu_map_entries_tailq entries, unroll_list;
 	int error;
 
 	ctx = tag->ctx;
@@ -661,15 +661,15 @@ iommu_bus_dmamap_load_something(struct bus_dma_tag_iommu *tag,
 	TAILQ_INIT(&unroll_list);
 	error = iommu_bus_dmamap_load_something1(tag, map, ma, offset,
 	    buflen, flags, segs, segp, &unroll_list);
-	if (error != 0) {
+	if (error != 0 && !TAILQ_EMPTY(&unroll_list)) {
 		/*
 		 * The busdma interface does not allow us to report
 		 * partial buffer load, so unfortunately we have to
 		 * revert all work done.
 		 */
-		IOMMU_DOMAIN_LOCK(domain);
-		TAILQ_FOREACH_SAFE(entry, &unroll_list, unroll_link,
-		    entry1) {
+		TAILQ_INIT(&entries);
+		IOMMU_DMAMAP_LOCK(map);
+		TAILQ_FOREACH(entry, &unroll_list, unroll_link) {
 			/*
 			 * No entries other than what we have created
 			 * during the failed run might have been
@@ -677,10 +677,11 @@ iommu_bus_dmamap_load_something(struct bus_dma_tag_iommu *tag,
 			 * pglock.
 			 */
 			TAILQ_REMOVE(&map->map_entries, entry, dmamap_link);
-			TAILQ_REMOVE(&unroll_list, entry, unroll_link);
-			TAILQ_INSERT_TAIL(&domain->unload_entries, entry,
-			    dmamap_link);
+			TAILQ_INSERT_TAIL(&entries, entry, dmamap_link);
 		}
+		IOMMU_DMAMAP_UNLOCK(map);
+		IOMMU_DOMAIN_LOCK(domain);
+		TAILQ_CONCAT(&domain->unload_entries, &entries, dmamap_link);
 		IOMMU_DOMAIN_UNLOCK(domain);
 		taskqueue_enqueue(domain->iommu->delayed_taskqueue,
 		    &domain->unload_task);
@@ -871,9 +872,7 @@ iommu_bus_dmamap_unload(bus_dma_tag_t dmat, bus_dmamap_t map1)
 	struct bus_dmamap_iommu *map;
 	struct iommu_ctx *ctx;
 	struct iommu_domain *domain;
-#ifndef IOMMU_DOMAIN_UNLOAD_SLEEP
 	struct iommu_map_entries_tailq entries;
-#endif
 
 	tag = (struct bus_dma_tag_iommu *)dmat;
 	map = (struct bus_dmamap_iommu *)map1;
@@ -881,17 +880,17 @@ iommu_bus_dmamap_unload(bus_dma_tag_t dmat, bus_dmamap_t map1)
 	domain = ctx->domain;
 	atomic_add_long(&ctx->unloads, 1);
 
+	TAILQ_INIT(&entries);
+	IOMMU_DMAMAP_LOCK(map);
+	TAILQ_CONCAT(&entries, &map->map_entries, dmamap_link);
+	IOMMU_DMAMAP_UNLOCK(map);
 #if defined(IOMMU_DOMAIN_UNLOAD_SLEEP)
 	IOMMU_DOMAIN_LOCK(domain);
-	TAILQ_CONCAT(&domain->unload_entries, &map->map_entries, dmamap_link);
+	TAILQ_CONCAT(&domain->unload_entries, &entries, dmamap_link);
 	IOMMU_DOMAIN_UNLOCK(domain);
 	taskqueue_enqueue(domain->iommu->delayed_taskqueue,
 	    &domain->unload_task);
 #else
-	TAILQ_INIT(&entries);
-	IOMMU_DOMAIN_LOCK(domain);
-	TAILQ_CONCAT(&entries, &map->map_entries, dmamap_link);
-	IOMMU_DOMAIN_UNLOCK(domain);
 	THREAD_NO_SLEEPING();
 	iommu_domain_unload(domain, &entries, false);
 	THREAD_SLEEPING_OK();
@@ -1041,13 +1040,12 @@ bus_dma_iommu_load_ident(bus_dma_tag_t dmat, bus_dmamap_t map1,
 		    VM_MEMATTR_DEFAULT);
 	}
 	error = iommu_gas_map_region(domain, entry, IOMMU_MAP_ENTRY_READ |
-	    ((flags & BUS_DMA_NOWRITE) ? 0 : IOMMU_MAP_ENTRY_WRITE),
-	    waitok ? IOMMU_MF_CANWAIT : 0, ma);
+	    ((flags & BUS_DMA_NOWRITE) ? 0 : IOMMU_MAP_ENTRY_WRITE) |
+	    IOMMU_MAP_ENTRY_MAP, waitok ? IOMMU_MF_CANWAIT : 0, ma);
 	if (error == 0) {
-		IOMMU_DOMAIN_LOCK(domain);
+		IOMMU_DMAMAP_LOCK(map);
 		TAILQ_INSERT_TAIL(&map->map_entries, entry, dmamap_link);
-		entry->flags |= IOMMU_MAP_ENTRY_MAP;
-		IOMMU_DOMAIN_UNLOCK(domain);
+		IOMMU_DMAMAP_UNLOCK(map);
 	} else {
 		iommu_domain_unload_entry(entry, true);
 	}
diff --git a/sys/dev/iommu/busdma_iommu.h b/sys/dev/iommu/busdma_iommu.h
index f911f26fa0f7..98e7b94d4ed9 100644
--- a/sys/dev/iommu/busdma_iommu.h
+++ b/sys/dev/iommu/busdma_iommu.h
@@ -49,6 +49,7 @@ struct bus_dmamap_iommu {
 	struct memdesc mem;
 	bus_dmamap_callback_t *callback;
 	void *callback_arg;
+	struct mtx lock;
 	struct iommu_map_entries_tailq map_entries;
 	TAILQ_ENTRY(bus_dmamap_iommu) delay_link;
 	bool locked;
@@ -56,6 +57,12 @@ struct bus_dmamap_iommu {
 	int flags;
 };
 
+#define	IOMMU_DMAMAP_INIT(map)		mtx_init(&(map)->lock, \
+					    "iommu dmamap", NULL, MTX_DEF)
+#define	IOMMU_DMAMAP_DESTROY(map)	mtx_destroy(&(map)->lock)
+#define	IOMMU_DMAMAP_LOCK(map)		mtx_lock(&(map)->lock)
+#define	IOMMU_DMAMAP_UNLOCK(map)	mtx_unlock(&(map)->lock)
+
 #define	BUS_DMAMAP_IOMMU_MALLOC	0x0001
 #define	BUS_DMAMAP_IOMMU_KMEM_ALLOC 0x0002