All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 01/10] drm/amdkfd: device pgmap owner at the svm migrate init
@ 2021-06-21 16:04 Alex Sierra
  2021-06-21 16:04 ` [PATCH 02/10] drm/amdkfd: add owner ref param to get hmm pages Alex Sierra
                   ` (9 more replies)
  0 siblings, 10 replies; 21+ messages in thread
From: Alex Sierra @ 2021-06-21 16:04 UTC (permalink / raw)
  To: amd-gfx

pgmap owner member at the svm migrate init could be referenced
to either adev or hive, depending on device topology.

Signed-off-by: Alex Sierra <alex.sierra@amd.com>
---
 drivers/gpu/drm/amd/amdkfd/kfd_migrate.c | 6 +++---
 drivers/gpu/drm/amd/amdkfd/kfd_svm.h     | 3 +++
 2 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c b/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c
index fd8f544f0de2..11f7f590c6ec 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c
@@ -426,7 +426,7 @@ svm_migrate_vma_to_vram(struct amdgpu_device *adev, struct svm_range *prange,
 	migrate.start = start;
 	migrate.end = end;
 	migrate.flags = MIGRATE_VMA_SELECT_SYSTEM;
-	migrate.pgmap_owner = adev;
+	migrate.pgmap_owner = SVM_ADEV_PGMAP_OWNER(adev);
 
 	size = 2 * sizeof(*migrate.src) + sizeof(uint64_t) + sizeof(dma_addr_t);
 	size *= npages;
@@ -641,7 +641,7 @@ svm_migrate_vma_to_ram(struct amdgpu_device *adev, struct svm_range *prange,
 	migrate.start = start;
 	migrate.end = end;
 	migrate.flags = MIGRATE_VMA_SELECT_DEVICE_PRIVATE;
-	migrate.pgmap_owner = adev;
+	migrate.pgmap_owner = SVM_ADEV_PGMAP_OWNER(adev);
 
 	size = 2 * sizeof(*migrate.src) + sizeof(uint64_t) + sizeof(dma_addr_t);
 	size *= npages;
@@ -907,7 +907,7 @@ int svm_migrate_init(struct amdgpu_device *adev)
 	pgmap->range.start = res->start;
 	pgmap->range.end = res->end;
 	pgmap->ops = &svm_migrate_pgmap_ops;
-	pgmap->owner = adev;
+	pgmap->owner = SVM_ADEV_PGMAP_OWNER(adev);
 	pgmap->flags = MIGRATE_VMA_SELECT_DEVICE_PRIVATE;
 	r = devm_memremap_pages(adev->dev, pgmap);
 	if (IS_ERR(r)) {
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_svm.h b/drivers/gpu/drm/amd/amdkfd/kfd_svm.h
index 573f984b81fe..4297250f259d 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_svm.h
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_svm.h
@@ -35,6 +35,9 @@
 #include "amdgpu.h"
 #include "kfd_priv.h"
 
+#define SVM_ADEV_PGMAP_OWNER(adev)\
+			((adev)->hive ? (void *)(adev)->hive : (void *)(adev))
+
 struct svm_range_bo {
 	struct amdgpu_bo		*bo;
 	struct kref			kref;
-- 
2.32.0

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH 02/10] drm/amdkfd: add owner ref param to get hmm pages
  2021-06-21 16:04 [PATCH 01/10] drm/amdkfd: device pgmap owner at the svm migrate init Alex Sierra
@ 2021-06-21 16:04 ` Alex Sierra
  2021-06-21 20:00   ` Felix Kuehling
  2021-06-21 16:04 ` [PATCH 03/10] drm/amdkfd: set owner ref to svm range prefault Alex Sierra
                   ` (8 subsequent siblings)
  9 siblings, 1 reply; 21+ messages in thread
From: Alex Sierra @ 2021-06-21 16:04 UTC (permalink / raw)
  To: amd-gfx

The parameter is used in the dev_private_owner to decide if device
pages in the range require to be migrated back to system memory, based
if they are or not in the same memory domain.
In this case, this reference could come from the same memory domain
with devices connected to the same hive.

Signed-off-by: Alex Sierra <alex.sierra@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c  | 3 ++-
 drivers/gpu/drm/amd/amdgpu/amdgpu_mn.h  | 2 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 2 +-
 drivers/gpu/drm/amd/amdkfd/kfd_svm.c    | 4 ++--
 4 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c
index 2741c28ff1b5..378c238c2099 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c
@@ -160,7 +160,7 @@ int amdgpu_hmm_range_get_pages(struct mmu_interval_notifier *notifier,
 			       struct mm_struct *mm, struct page **pages,
 			       uint64_t start, uint64_t npages,
 			       struct hmm_range **phmm_range, bool readonly,
-			       bool mmap_locked)
+			       bool mmap_locked, void *owner)
 {
 	struct hmm_range *hmm_range;
 	unsigned long timeout;
@@ -185,6 +185,7 @@ int amdgpu_hmm_range_get_pages(struct mmu_interval_notifier *notifier,
 	hmm_range->hmm_pfns = pfns;
 	hmm_range->start = start;
 	hmm_range->end = start + npages * PAGE_SIZE;
+	hmm_range->dev_private_owner = owner;
 
 	/* Assuming 512MB takes maxmium 1 second to fault page address */
 	timeout = max(npages >> 17, 1ULL) * HMM_RANGE_DEFAULT_TIMEOUT;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_mn.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_mn.h
index 7f7d37a457c3..14a3c1864085 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_mn.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_mn.h
@@ -34,7 +34,7 @@ int amdgpu_hmm_range_get_pages(struct mmu_interval_notifier *notifier,
 			       struct mm_struct *mm, struct page **pages,
 			       uint64_t start, uint64_t npages,
 			       struct hmm_range **phmm_range, bool readonly,
-			       bool mmap_locked);
+			       bool mmap_locked, void *owner);
 int amdgpu_hmm_range_get_pages_done(struct hmm_range *hmm_range);
 
 #if defined(CONFIG_HMM_MIRROR)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index 7e7d8330d64b..c13f7fbfc070 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -709,7 +709,7 @@ int amdgpu_ttm_tt_get_user_pages(struct amdgpu_bo *bo, struct page **pages)
 	readonly = amdgpu_ttm_tt_is_readonly(ttm);
 	r = amdgpu_hmm_range_get_pages(&bo->notifier, mm, pages, start,
 				       ttm->num_pages, &gtt->range, readonly,
-				       false);
+				       false, NULL);
 out_putmm:
 	mmput(mm);
 
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
index b665e9ff77e3..b939f353ac8c 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
@@ -1392,7 +1392,7 @@ static int svm_range_validate_and_map(struct mm_struct *mm,
 		r = amdgpu_hmm_range_get_pages(&prange->notifier, mm, NULL,
 					       prange->start << PAGE_SHIFT,
 					       prange->npages, &hmm_range,
-					       false, true);
+					       false, true, NULL);
 		if (r) {
 			pr_debug("failed %d to get svm range pages\n", r);
 			goto unreserve_out;
@@ -2657,7 +2657,7 @@ void svm_range_prefault(struct svm_range *prange, struct mm_struct *mm)
 	r = amdgpu_hmm_range_get_pages(&prange->notifier, mm, NULL,
 				       prange->start << PAGE_SHIFT,
 				       prange->npages, &hmm_range,
-				       false, true);
+				       false, true, NULL);
 	if (!r) {
 		amdgpu_hmm_range_get_pages_done(hmm_range);
 		prange->validated_once = true;
-- 
2.32.0

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH 03/10] drm/amdkfd: set owner ref to svm range prefault
  2021-06-21 16:04 [PATCH 01/10] drm/amdkfd: device pgmap owner at the svm migrate init Alex Sierra
  2021-06-21 16:04 ` [PATCH 02/10] drm/amdkfd: add owner ref param to get hmm pages Alex Sierra
@ 2021-06-21 16:04 ` Alex Sierra
  2021-06-21 20:02   ` Felix Kuehling
  2021-06-21 16:04 ` [PATCH 04/10] drm/amdgpu: get owner ref in validate and map Alex Sierra
                   ` (7 subsequent siblings)
  9 siblings, 1 reply; 21+ messages in thread
From: Alex Sierra @ 2021-06-21 16:04 UTC (permalink / raw)
  To: amd-gfx

svm_range_prefault is called right before migrations to VRAM,
to make sure pages are resident in system memory before the migration.
With partial migrations, this reference is used by hmm range get pages
to avoid migrating pages that are already in the same VRAM domain.

Signed-off-by: Alex Sierra <alex.sierra@amd.com>
---
 drivers/gpu/drm/amd/amdkfd/kfd_migrate.c | 2 +-
 drivers/gpu/drm/amd/amdkfd/kfd_svm.c     | 5 +++--
 drivers/gpu/drm/amd/amdkfd/kfd_svm.h     | 3 ++-
 3 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c b/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c
index 11f7f590c6ec..b298aa8dea4d 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c
@@ -512,7 +512,7 @@ svm_migrate_ram_to_vram(struct svm_range *prange, uint32_t best_loc,
 		 prange->start, prange->last, best_loc);
 
 	/* FIXME: workaround for page locking bug with invalid pages */
-	svm_range_prefault(prange, mm);
+	svm_range_prefault(prange, mm, SVM_ADEV_PGMAP_OWNER(adev));
 
 	start = prange->start << PAGE_SHIFT;
 	end = (prange->last + 1) << PAGE_SHIFT;
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
index b939f353ac8c..54f47b09b14a 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
@@ -2646,7 +2646,8 @@ svm_range_best_prefetch_location(struct svm_range *prange)
 /* FIXME: This is a workaround for page locking bug when some pages are
  * invalid during migration to VRAM
  */
-void svm_range_prefault(struct svm_range *prange, struct mm_struct *mm)
+void svm_range_prefault(struct svm_range *prange, struct mm_struct *mm,
+			void *owner)
 {
 	struct hmm_range *hmm_range;
 	int r;
@@ -2657,7 +2658,7 @@ void svm_range_prefault(struct svm_range *prange, struct mm_struct *mm)
 	r = amdgpu_hmm_range_get_pages(&prange->notifier, mm, NULL,
 				       prange->start << PAGE_SHIFT,
 				       prange->npages, &hmm_range,
-				       false, true, NULL);
+				       false, true, owner);
 	if (!r) {
 		amdgpu_hmm_range_get_pages_done(hmm_range);
 		prange->validated_once = true;
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_svm.h b/drivers/gpu/drm/amd/amdkfd/kfd_svm.h
index 4297250f259d..08542fe39303 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_svm.h
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_svm.h
@@ -176,7 +176,8 @@ void schedule_deferred_list_work(struct svm_range_list *svms);
 void svm_range_dma_unmap(struct device *dev, dma_addr_t *dma_addr,
 			 unsigned long offset, unsigned long npages);
 void svm_range_free_dma_mappings(struct svm_range *prange);
-void svm_range_prefault(struct svm_range *prange, struct mm_struct *mm);
+void svm_range_prefault(struct svm_range *prange, struct mm_struct *mm,
+			void *owner);
 
 #else
 
-- 
2.32.0

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH 04/10] drm/amdgpu: get owner ref in validate and map
  2021-06-21 16:04 [PATCH 01/10] drm/amdkfd: device pgmap owner at the svm migrate init Alex Sierra
  2021-06-21 16:04 ` [PATCH 02/10] drm/amdkfd: add owner ref param to get hmm pages Alex Sierra
  2021-06-21 16:04 ` [PATCH 03/10] drm/amdkfd: set owner ref to svm range prefault Alex Sierra
@ 2021-06-21 16:04 ` Alex Sierra
  2021-06-21 20:04   ` Felix Kuehling
  2021-06-21 16:04 ` [PATCH 05/10] drm/amdkfd: classify and map mixed svm range pages in GPU Alex Sierra
                   ` (6 subsequent siblings)
  9 siblings, 1 reply; 21+ messages in thread
From: Alex Sierra @ 2021-06-21 16:04 UTC (permalink / raw)
  To: amd-gfx

Get the proper owner reference for amdgpu_hmm_range_get_pages function.
This is useful for partial migrations. To avoid migrating back to
system memory, VRAM pages, that are accessible by all devices in the
same memory domain.
Ex. multiple devices in the same hive.

Signed-off-by: Alex Sierra <alex.sierra@amd.com>
---
 drivers/gpu/drm/amd/amdkfd/kfd_svm.c | 25 ++++++++++++++++++++++++-
 1 file changed, 24 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
index 54f47b09b14a..2b4318646a75 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
@@ -1313,6 +1313,17 @@ static void svm_range_unreserve_bos(struct svm_validate_context *ctx)
 	ttm_eu_backoff_reservation(&ctx->ticket, &ctx->validate_list);
 }
 
+static void *kfd_svm_page_owner(struct kfd_process *p, int32_t gpuidx)
+{
+	struct kfd_process_device *pdd;
+	struct amdgpu_device *adev;
+
+	pdd = kfd_process_device_from_gpuidx(p, gpuidx);
+	adev = (struct amdgpu_device *)pdd->dev->kgd;
+
+	return SVM_ADEV_PGMAP_OWNER(adev);
+}
+
 /*
  * Validation+GPU mapping with concurrent invalidation (MMU notifiers)
  *
@@ -1343,6 +1354,9 @@ static int svm_range_validate_and_map(struct mm_struct *mm,
 {
 	struct svm_validate_context ctx;
 	struct hmm_range *hmm_range;
+	struct kfd_process *p;
+	void *owner;
+	int32_t idx;
 	int r = 0;
 
 	ctx.process = container_of(prange->svms, struct kfd_process, svms);
@@ -1389,10 +1403,19 @@ static int svm_range_validate_and_map(struct mm_struct *mm,
 	svm_range_reserve_bos(&ctx);
 
 	if (!prange->actual_loc) {
+		p = container_of(prange->svms, struct kfd_process, svms);
+		owner = kfd_svm_page_owner(p, find_first_bit(ctx.bitmap,
+							MAX_GPU_INSTANCE));
+		for_each_set_bit(idx, ctx.bitmap, MAX_GPU_INSTANCE) {
+			if (kfd_svm_page_owner(p, idx) != owner) {
+				owner = NULL;
+				break;
+			}
+		}
 		r = amdgpu_hmm_range_get_pages(&prange->notifier, mm, NULL,
 					       prange->start << PAGE_SHIFT,
 					       prange->npages, &hmm_range,
-					       false, true, NULL);
+					       false, true, owner);
 		if (r) {
 			pr_debug("failed %d to get svm range pages\n", r);
 			goto unreserve_out;
-- 
2.32.0

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH 05/10] drm/amdkfd: classify and map mixed svm range pages in GPU
  2021-06-21 16:04 [PATCH 01/10] drm/amdkfd: device pgmap owner at the svm migrate init Alex Sierra
                   ` (2 preceding siblings ...)
  2021-06-21 16:04 ` [PATCH 04/10] drm/amdgpu: get owner ref in validate and map Alex Sierra
@ 2021-06-21 16:04 ` Alex Sierra
  2021-06-21 20:26   ` Felix Kuehling
  2021-06-21 16:04 ` [PATCH 06/10] drm/amdkfd: skip invalid pages during migrations Alex Sierra
                   ` (5 subsequent siblings)
  9 siblings, 1 reply; 21+ messages in thread
From: Alex Sierra @ 2021-06-21 16:04 UTC (permalink / raw)
  To: amd-gfx

[Why]
svm ranges can have mixed pages from device or system memory.
A good example is, after a prange has been allocated in VRAM and a
copy-on-write is triggered by a fork. This invalidates some pages
inside the prange. Endding up in mixed pages.

[How]
By classifying each page inside a prange, based on its type. Device or
system memory, during dma mapping call. If page corresponds
to VRAM domain, a flag is set to its dma_addr entry for each GPU.
Then, at the GPU page table mapping. All group of contiguous pages within
the same type are mapped with their proper pte flags.

v2:
Instead of using ttm_res to calculate vram pfns in the svm_range. It is now
done by setting the vram real physical address into drm_addr array.
This makes more flexible VRAM management, plus removes the need to have
a BO reference in the svm_range.

v3:
Remove mapping member from svm_range

Signed-off-by: Alex Sierra <alex.sierra@amd.com>
---
 drivers/gpu/drm/amd/amdkfd/kfd_svm.c | 73 ++++++++++++++++++----------
 drivers/gpu/drm/amd/amdkfd/kfd_svm.h |  2 +-
 2 files changed, 47 insertions(+), 28 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
index 2b4318646a75..3b05bc270732 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
@@ -119,11 +119,12 @@ static void svm_range_remove_notifier(struct svm_range *prange)
 }
 
 static int
-svm_range_dma_map_dev(struct device *dev, dma_addr_t **dma_addr,
+svm_range_dma_map_dev(struct amdgpu_device *adev, dma_addr_t **dma_addr,
 		      unsigned long *hmm_pfns, uint64_t npages)
 {
 	enum dma_data_direction dir = DMA_BIDIRECTIONAL;
 	dma_addr_t *addr = *dma_addr;
+	struct device *dev = adev->dev;
 	struct page *page;
 	int i, r;
 
@@ -141,6 +142,14 @@ svm_range_dma_map_dev(struct device *dev, dma_addr_t **dma_addr,
 			dma_unmap_page(dev, addr[i], PAGE_SIZE, dir);
 
 		page = hmm_pfn_to_page(hmm_pfns[i]);
+		if (is_zone_device_page(page)) {
+			addr[i] = (hmm_pfns[i] << PAGE_SHIFT) +
+				   adev->vm_manager.vram_base_offset -
+				   adev->kfd.dev->pgmap.range.start;
+			addr[i] |= SVM_RANGE_VRAM_DOMAIN;
+			pr_debug("vram address detected: 0x%llx\n", addr[i]);
+			continue;
+		}
 		addr[i] = dma_map_page(dev, page, 0, PAGE_SIZE, dir);
 		r = dma_mapping_error(dev, addr[i]);
 		if (r) {
@@ -175,7 +184,7 @@ svm_range_dma_map(struct svm_range *prange, unsigned long *bitmap,
 		}
 		adev = (struct amdgpu_device *)pdd->dev->kgd;
 
-		r = svm_range_dma_map_dev(adev->dev, &prange->dma_addr[gpuidx],
+		r = svm_range_dma_map_dev(adev, &prange->dma_addr[gpuidx],
 					  hmm_pfns, prange->npages);
 		if (r)
 			break;
@@ -1003,21 +1012,22 @@ svm_range_split_by_granularity(struct kfd_process *p, struct mm_struct *mm,
 }
 
 static uint64_t
-svm_range_get_pte_flags(struct amdgpu_device *adev, struct svm_range *prange)
+svm_range_get_pte_flags(struct amdgpu_device *adev, struct svm_range *prange,
+			int domain)
 {
 	struct amdgpu_device *bo_adev;
 	uint32_t flags = prange->flags;
 	uint32_t mapping_flags = 0;
 	uint64_t pte_flags;
-	bool snoop = !prange->ttm_res;
+	bool snoop = (domain != SVM_RANGE_VRAM_DOMAIN);
 	bool coherent = flags & KFD_IOCTL_SVM_FLAG_COHERENT;
 
-	if (prange->svm_bo && prange->ttm_res)
+	if (domain == SVM_RANGE_VRAM_DOMAIN)
 		bo_adev = amdgpu_ttm_adev(prange->svm_bo->bo->tbo.bdev);
 
 	switch (adev->asic_type) {
 	case CHIP_ARCTURUS:
-		if (prange->svm_bo && prange->ttm_res) {
+		if (domain == SVM_RANGE_VRAM_DOMAIN) {
 			if (bo_adev == adev) {
 				mapping_flags |= coherent ?
 					AMDGPU_VM_MTYPE_CC : AMDGPU_VM_MTYPE_RW;
@@ -1032,7 +1042,7 @@ svm_range_get_pte_flags(struct amdgpu_device *adev, struct svm_range *prange)
 		}
 		break;
 	case CHIP_ALDEBARAN:
-		if (prange->svm_bo && prange->ttm_res) {
+		if (domain == SVM_RANGE_VRAM_DOMAIN) {
 			if (bo_adev == adev) {
 				mapping_flags |= coherent ?
 					AMDGPU_VM_MTYPE_CC : AMDGPU_VM_MTYPE_RW;
@@ -1061,14 +1071,14 @@ svm_range_get_pte_flags(struct amdgpu_device *adev, struct svm_range *prange)
 		mapping_flags |= AMDGPU_VM_PAGE_EXECUTABLE;
 
 	pte_flags = AMDGPU_PTE_VALID;
-	pte_flags |= prange->ttm_res ? 0 : AMDGPU_PTE_SYSTEM;
+	pte_flags |= (domain == SVM_RANGE_VRAM_DOMAIN) ? 0 : AMDGPU_PTE_SYSTEM;
 	pte_flags |= snoop ? AMDGPU_PTE_SNOOPED : 0;
 
 	pte_flags |= amdgpu_gem_va_map_flags(adev, mapping_flags);
 
 	pr_debug("svms 0x%p [0x%lx 0x%lx] vram %d PTE 0x%llx mapping 0x%x\n",
 		 prange->svms, prange->start, prange->last,
-		 prange->ttm_res ? 1:0, pte_flags, mapping_flags);
+		 (domain == SVM_RANGE_VRAM_DOMAIN) ? 1:0, pte_flags, mapping_flags);
 
 	return pte_flags;
 }
@@ -1138,31 +1148,41 @@ svm_range_map_to_gpu(struct amdgpu_device *adev, struct amdgpu_vm *vm,
 {
 	struct amdgpu_bo_va bo_va;
 	uint64_t pte_flags;
+	unsigned long last_start;
+	int last_domain;
 	int r = 0;
+	int64_t i;
 
 	pr_debug("svms 0x%p [0x%lx 0x%lx]\n", prange->svms, prange->start,
 		 prange->last);
 
-	if (prange->svm_bo && prange->ttm_res) {
+	if (prange->svm_bo && prange->ttm_res)
 		bo_va.is_xgmi = amdgpu_xgmi_same_hive(adev, bo_adev);
-		prange->mapping.bo_va = &bo_va;
-	}
 
-	prange->mapping.start = prange->start;
-	prange->mapping.last = prange->last;
-	prange->mapping.offset = prange->offset;
-	pte_flags = svm_range_get_pte_flags(adev, prange);
+	last_start = prange->start;
+	for (i = 0; i < prange->npages; i++) {
+		last_domain = dma_addr[i] & SVM_RANGE_VRAM_DOMAIN;
+		dma_addr[i] &= ~SVM_RANGE_VRAM_DOMAIN;
+		if ((prange->start + i) < prange->last &&
+		    last_domain == (dma_addr[i + 1] & SVM_RANGE_VRAM_DOMAIN))
+			continue;
 
-	r = amdgpu_vm_bo_update_mapping(adev, bo_adev, vm, false, false, NULL,
-					prange->mapping.start,
-					prange->mapping.last, pte_flags,
-					prange->mapping.offset,
-					prange->ttm_res ?
-						prange->ttm_res->mm_node : NULL,
-					dma_addr, &vm->last_update);
-	if (r) {
-		pr_debug("failed %d to map to gpu 0x%lx\n", r, prange->start);
-		goto out;
+		pr_debug("Mapping range [0x%lx 0x%llx] on domain: %s\n",
+			 last_start, prange->start + i, last_domain ? "GPU" : "CPU");
+		pte_flags = svm_range_get_pte_flags(adev, prange, last_domain);
+		r = amdgpu_vm_bo_update_mapping(adev, bo_adev, vm, false, false, NULL,
+						last_start,
+						prange->start + i, pte_flags,
+						prange->offset +
+						((last_start - prange->start) << PAGE_SHIFT),
+						NULL,
+						dma_addr,
+						&vm->last_update);
+		if (r) {
+			pr_debug("failed %d to map to gpu 0x%lx\n", r, prange->start);
+			goto out;
+		}
+		last_start = prange->start + i + 1;
 	}
 
 	r = amdgpu_vm_update_pdes(adev, vm, false);
@@ -1176,7 +1196,6 @@ svm_range_map_to_gpu(struct amdgpu_device *adev, struct amdgpu_vm *vm,
 		*fence = dma_fence_get(vm->last_update);
 
 out:
-	prange->mapping.bo_va = NULL;
 	return r;
 }
 
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_svm.h b/drivers/gpu/drm/amd/amdkfd/kfd_svm.h
index 08542fe39303..27fbe1936493 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_svm.h
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_svm.h
@@ -35,6 +35,7 @@
 #include "amdgpu.h"
 #include "kfd_priv.h"
 
+#define SVM_RANGE_VRAM_DOMAIN (1UL << 0)
 #define SVM_ADEV_PGMAP_OWNER(adev)\
 			((adev)->hive ? (void *)(adev)->hive : (void *)(adev))
 
@@ -113,7 +114,6 @@ struct svm_range {
 	struct list_head		update_list;
 	struct list_head		remove_list;
 	struct list_head		insert_list;
-	struct amdgpu_bo_va_mapping	mapping;
 	uint64_t			npages;
 	dma_addr_t			*dma_addr[MAX_GPU_INSTANCE];
 	struct ttm_resource		*ttm_res;
-- 
2.32.0

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH 06/10] drm/amdkfd: skip invalid pages during migrations
  2021-06-21 16:04 [PATCH 01/10] drm/amdkfd: device pgmap owner at the svm migrate init Alex Sierra
                   ` (3 preceding siblings ...)
  2021-06-21 16:04 ` [PATCH 05/10] drm/amdkfd: classify and map mixed svm range pages in GPU Alex Sierra
@ 2021-06-21 16:04 ` Alex Sierra
  2021-06-21 16:04 ` [PATCH 07/10] drm/amdkfd: skip migration for pages already in VRAM Alex Sierra
                   ` (4 subsequent siblings)
  9 siblings, 0 replies; 21+ messages in thread
From: Alex Sierra @ 2021-06-21 16:04 UTC (permalink / raw)
  To: amd-gfx

Invalid pages can be the result of pages that have been migrated
already due to copy-on-write procedure or pages that were never
migrated to VRAM in first place. This is not an issue anymore,
as pranges now support mixed memory domains (CPU/GPU).

Signed-off-by: Alex Sierra <alex.sierra@amd.com>
Reviewed-by: Felix Kuehling <Felix.Kuehling@amd.com>
---
 drivers/gpu/drm/amd/amdkfd/kfd_migrate.c | 38 +++++++++++-------------
 1 file changed, 18 insertions(+), 20 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c b/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c
index b298aa8dea4d..6fd68528c425 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c
@@ -419,7 +419,6 @@ svm_migrate_vma_to_vram(struct amdgpu_device *adev, struct svm_range *prange,
 	size_t size;
 	void *buf;
 	int r = -ENOMEM;
-	int retry = 0;
 
 	memset(&migrate, 0, sizeof(migrate));
 	migrate.vma = vma;
@@ -438,7 +437,6 @@ svm_migrate_vma_to_vram(struct amdgpu_device *adev, struct svm_range *prange,
 	migrate.dst = migrate.src + npages;
 	scratch = (dma_addr_t *)(migrate.dst + npages);
 
-retry:
 	r = migrate_vma_setup(&migrate);
 	if (r) {
 		pr_debug("failed %d prepare migrate svms 0x%p [0x%lx 0x%lx]\n",
@@ -446,17 +444,9 @@ svm_migrate_vma_to_vram(struct amdgpu_device *adev, struct svm_range *prange,
 		goto out_free;
 	}
 	if (migrate.cpages != npages) {
-		pr_debug("collect 0x%lx/0x%llx pages, retry\n", migrate.cpages,
+		pr_debug("Partial migration. 0x%lx/0x%llx pages can be migrated\n",
+			 migrate.cpages,
 			 npages);
-		migrate_vma_finalize(&migrate);
-		if (retry++ >= 3) {
-			r = -ENOMEM;
-			pr_debug("failed %d migrate svms 0x%p [0x%lx 0x%lx]\n",
-				 r, prange->svms, prange->start, prange->last);
-			goto out_free;
-		}
-
-		goto retry;
 	}
 
 	if (migrate.cpages) {
@@ -547,9 +537,8 @@ static void svm_migrate_page_free(struct page *page)
 static int
 svm_migrate_copy_to_ram(struct amdgpu_device *adev, struct svm_range *prange,
 			struct migrate_vma *migrate, struct dma_fence **mfence,
-			dma_addr_t *scratch)
+			dma_addr_t *scratch, uint64_t npages)
 {
-	uint64_t npages = migrate->cpages;
 	struct device *dev = adev->dev;
 	uint64_t *src;
 	dma_addr_t *dst;
@@ -566,15 +555,23 @@ svm_migrate_copy_to_ram(struct amdgpu_device *adev, struct svm_range *prange,
 	src = (uint64_t *)(scratch + npages);
 	dst = scratch;
 
-	for (i = 0, j = 0; i < npages; i++, j++, addr += PAGE_SIZE) {
+	for (i = 0, j = 0; i < npages; i++, addr += PAGE_SIZE) {
 		struct page *spage;
 
 		spage = migrate_pfn_to_page(migrate->src[i]);
-		if (!spage) {
-			pr_debug("failed get spage svms 0x%p [0x%lx 0x%lx]\n",
+		if (!spage || !is_zone_device_page(spage)) {
+			pr_debug("invalid page. Could be in CPU already svms 0x%p [0x%lx 0x%lx]\n",
 				 prange->svms, prange->start, prange->last);
-			r = -ENOMEM;
-			goto out_oom;
+			if (j) {
+				r = svm_migrate_copy_memory_gart(adev, dst + i - j,
+								 src + i - j, j,
+								 FROM_VRAM_TO_RAM,
+								 mfence);
+				if (r)
+					goto out_oom;
+				j = 0;
+			}
+			continue;
 		}
 		src[i] = svm_migrate_addr(adev, spage);
 		if (i > 0 && src[i] != src[i - 1] + PAGE_SIZE) {
@@ -607,6 +604,7 @@ svm_migrate_copy_to_ram(struct amdgpu_device *adev, struct svm_range *prange,
 
 		migrate->dst[i] = migrate_pfn(page_to_pfn(dpage));
 		migrate->dst[i] |= MIGRATE_PFN_LOCKED;
+		j++;
 	}
 
 	r = svm_migrate_copy_memory_gart(adev, dst + i - j, src + i - j, j,
@@ -664,7 +662,7 @@ svm_migrate_vma_to_ram(struct amdgpu_device *adev, struct svm_range *prange,
 
 	if (migrate.cpages) {
 		r = svm_migrate_copy_to_ram(adev, prange, &migrate, &mfence,
-					    scratch);
+					    scratch, npages);
 		migrate_vma_pages(&migrate);
 		svm_migrate_copy_done(adev, mfence);
 		migrate_vma_finalize(&migrate);
-- 
2.32.0

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH 07/10] drm/amdkfd: skip migration for pages already in VRAM
  2021-06-21 16:04 [PATCH 01/10] drm/amdkfd: device pgmap owner at the svm migrate init Alex Sierra
                   ` (4 preceding siblings ...)
  2021-06-21 16:04 ` [PATCH 06/10] drm/amdkfd: skip invalid pages during migrations Alex Sierra
@ 2021-06-21 16:04 ` Alex Sierra
  2021-06-21 21:01   ` Felix Kuehling
  2021-06-21 16:04 ` [PATCH 08/10] drm/amdkfd: add invalid pages debug at vram migration Alex Sierra
                   ` (3 subsequent siblings)
  9 siblings, 1 reply; 21+ messages in thread
From: Alex Sierra @ 2021-06-21 16:04 UTC (permalink / raw)
  To: amd-gfx

Migration skipped for pages that are already in VRAM
domain. These could be the result of previous partial
migrations to SYS RAM, and prefetch back to VRAM.
Ex. Coherent pages in VRAM that were not written/invalidated after
a copy-on-write.

Signed-off-by: Alex Sierra <alex.sierra@amd.com>
---
 drivers/gpu/drm/amd/amdkfd/kfd_migrate.c | 17 +++++++++--------
 1 file changed, 9 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c b/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c
index 6fd68528c425..8a3f21d76915 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c
@@ -329,14 +329,15 @@ svm_migrate_copy_to_vram(struct amdgpu_device *adev, struct svm_range *prange,
 	for (i = j = 0; i < npages; i++) {
 		struct page *spage;
 
-		dst[i] = vram_addr + (j << PAGE_SHIFT);
-		migrate->dst[i] = svm_migrate_addr_to_pfn(adev, dst[i]);
-		svm_migrate_get_vram_page(prange, migrate->dst[i]);
-
-		migrate->dst[i] = migrate_pfn(migrate->dst[i]);
-		migrate->dst[i] |= MIGRATE_PFN_LOCKED;
-
-		if (migrate->src[i] & MIGRATE_PFN_VALID) {
+		spage = migrate_pfn_to_page(migrate->src[i]);
+		if (spage && !is_zone_device_page(spage)) {
+			dst[i] = vram_addr + (j << PAGE_SHIFT);
+			migrate->dst[i] = svm_migrate_addr_to_pfn(adev, dst[i]);
+			svm_migrate_get_vram_page(prange, migrate->dst[i]);
+			migrate->dst[i] = migrate_pfn(migrate->dst[i]);
+			migrate->dst[i] |= MIGRATE_PFN_LOCKED;
+		}
+		if (migrate->dst[i] & MIGRATE_PFN_VALID) {
 			spage = migrate_pfn_to_page(migrate->src[i]);
 			src[i] = dma_map_page(dev, spage, 0, PAGE_SIZE,
 					      DMA_TO_DEVICE);
-- 
2.32.0

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH 08/10] drm/amdkfd: add invalid pages debug at vram migration
  2021-06-21 16:04 [PATCH 01/10] drm/amdkfd: device pgmap owner at the svm migrate init Alex Sierra
                   ` (5 preceding siblings ...)
  2021-06-21 16:04 ` [PATCH 07/10] drm/amdkfd: skip migration for pages already in VRAM Alex Sierra
@ 2021-06-21 16:04 ` Alex Sierra
  2021-06-21 21:02   ` Felix Kuehling
  2021-06-21 16:04 ` [PATCH 09/10] drm/amdkfd: partially actual_loc removed Alex Sierra
                   ` (2 subsequent siblings)
  9 siblings, 1 reply; 21+ messages in thread
From: Alex Sierra @ 2021-06-21 16:04 UTC (permalink / raw)
  To: amd-gfx

This is for debug purposes only.
It conditionally generates partial migrations to test mixed
CPU/GPU memory domain pages in a prange easily.

Signed-off-by: Alex Sierra <alex.sierra@amd.com>
---
 drivers/gpu/drm/amd/amdkfd/kfd_migrate.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c b/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c
index 8a3f21d76915..f71f8d7e2b72 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c
@@ -404,6 +404,20 @@ svm_migrate_copy_to_vram(struct amdgpu_device *adev, struct svm_range *prange,
 		}
 	}
 
+#ifdef DEBUG_FORCE_MIXED_DOMAINS
+	for (i = 0, j = 0; i < npages; i += 4, j++) {
+		if (j & 1)
+			continue;
+		svm_migrate_put_vram_page(adev, dst[i]);
+		migrate->dst[i] = 0;
+		svm_migrate_put_vram_page(adev, dst[i + 1]);
+		migrate->dst[i + 1] = 0;
+		svm_migrate_put_vram_page(adev, dst[i + 2]);
+		migrate->dst[i + 2] = 0;
+		svm_migrate_put_vram_page(adev, dst[i + 3]);
+		migrate->dst[i + 3] = 0;
+	}
+#endif
 out:
 	return r;
 }
-- 
2.32.0

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH 09/10] drm/amdkfd: partially actual_loc removed
  2021-06-21 16:04 [PATCH 01/10] drm/amdkfd: device pgmap owner at the svm migrate init Alex Sierra
                   ` (6 preceding siblings ...)
  2021-06-21 16:04 ` [PATCH 08/10] drm/amdkfd: add invalid pages debug at vram migration Alex Sierra
@ 2021-06-21 16:04 ` Alex Sierra
  2021-06-21 21:24   ` Felix Kuehling
  2021-06-21 16:04 ` [PATCH 10/10] drm/amdkfd: protect svm_bo ref in case prange has forked Alex Sierra
  2021-06-21 19:59 ` [PATCH 01/10] drm/amdkfd: device pgmap owner at the svm migrate init Felix Kuehling
  9 siblings, 1 reply; 21+ messages in thread
From: Alex Sierra @ 2021-06-21 16:04 UTC (permalink / raw)
  To: amd-gfx

actual_loc should not be used anymore, as pranges
could have mixed locations (VRAM & SYSRAM) at the
same time.

Signed-off-by: Alex Sierra <alex.sierra@amd.com>
---
 drivers/gpu/drm/amd/amdkfd/kfd_migrate.c | 12 +---
 drivers/gpu/drm/amd/amdkfd/kfd_svm.c     | 71 ++++++++++--------------
 2 files changed, 29 insertions(+), 54 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c b/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c
index f71f8d7e2b72..acb9f64577a0 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c
@@ -501,12 +501,6 @@ svm_migrate_ram_to_vram(struct svm_range *prange, uint32_t best_loc,
 	struct amdgpu_device *adev;
 	int r = 0;
 
-	if (prange->actual_loc == best_loc) {
-		pr_debug("svms 0x%p [0x%lx 0x%lx] already on best_loc 0x%x\n",
-			 prange->svms, prange->start, prange->last, best_loc);
-		return 0;
-	}
-
 	adev = svm_range_get_adev_by_id(prange, best_loc);
 	if (!adev) {
 		pr_debug("failed to get device by id 0x%x\n", best_loc);
@@ -791,11 +785,7 @@ int
 svm_migrate_to_vram(struct svm_range *prange, uint32_t best_loc,
 		    struct mm_struct *mm)
 {
-	if  (!prange->actual_loc)
-		return svm_migrate_ram_to_vram(prange, best_loc, mm);
-	else
-		return svm_migrate_vram_to_vram(prange, best_loc, mm);
-
+	return svm_migrate_ram_to_vram(prange, best_loc, mm);
 }
 
 /**
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
index 3b05bc270732..ebc1ae7e5193 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
@@ -1421,42 +1421,38 @@ static int svm_range_validate_and_map(struct mm_struct *mm,
 
 	svm_range_reserve_bos(&ctx);
 
-	if (!prange->actual_loc) {
-		p = container_of(prange->svms, struct kfd_process, svms);
-		owner = kfd_svm_page_owner(p, find_first_bit(ctx.bitmap,
-							MAX_GPU_INSTANCE));
-		for_each_set_bit(idx, ctx.bitmap, MAX_GPU_INSTANCE) {
-			if (kfd_svm_page_owner(p, idx) != owner) {
-				owner = NULL;
-				break;
-			}
-		}
-		r = amdgpu_hmm_range_get_pages(&prange->notifier, mm, NULL,
-					       prange->start << PAGE_SHIFT,
-					       prange->npages, &hmm_range,
-					       false, true, owner);
-		if (r) {
-			pr_debug("failed %d to get svm range pages\n", r);
-			goto unreserve_out;
-		}
-
-		r = svm_range_dma_map(prange, ctx.bitmap,
-				      hmm_range->hmm_pfns);
-		if (r) {
-			pr_debug("failed %d to dma map range\n", r);
-			goto unreserve_out;
+	p = container_of(prange->svms, struct kfd_process, svms);
+	owner = kfd_svm_page_owner(p, find_first_bit(ctx.bitmap,
+						MAX_GPU_INSTANCE));
+	for_each_set_bit(idx, ctx.bitmap, MAX_GPU_INSTANCE) {
+		if (kfd_svm_page_owner(p, idx) != owner) {
+			owner = NULL;
+			break;
 		}
+	}
+	r = amdgpu_hmm_range_get_pages(&prange->notifier, mm, NULL,
+				       prange->start << PAGE_SHIFT,
+				       prange->npages, &hmm_range,
+				       false, true, owner);
+	if (r) {
+		pr_debug("failed %d to get svm range pages\n", r);
+		goto unreserve_out;
+	}
 
-		prange->validated_once = true;
+	r = svm_range_dma_map(prange, ctx.bitmap,
+			      hmm_range->hmm_pfns);
+	if (r) {
+		pr_debug("failed %d to dma map range\n", r);
+		goto unreserve_out;
 	}
 
+	prange->validated_once = true;
+
 	svm_range_lock(prange);
-	if (!prange->actual_loc) {
-		if (amdgpu_hmm_range_get_pages_done(hmm_range)) {
-			pr_debug("hmm update the range, need validate again\n");
-			r = -EAGAIN;
-			goto unlock_out;
-		}
+	if (amdgpu_hmm_range_get_pages_done(hmm_range)) {
+		pr_debug("hmm update the range, need validate again\n");
+		r = -EAGAIN;
+		goto unlock_out;
 	}
 	if (!list_empty(&prange->child_list)) {
 		pr_debug("range split by unmap in parallel, validate again\n");
@@ -2741,20 +2737,9 @@ svm_range_trigger_migration(struct mm_struct *mm, struct svm_range *prange,
 	*migrated = false;
 	best_loc = svm_range_best_prefetch_location(prange);
 
-	if (best_loc == KFD_IOCTL_SVM_LOCATION_UNDEFINED ||
-	    best_loc == prange->actual_loc)
+	if (best_loc == KFD_IOCTL_SVM_LOCATION_UNDEFINED)
 		return 0;
 
-	/*
-	 * Prefetch to GPU without host access flag, set actual_loc to gpu, then
-	 * validate on gpu and map to gpus will be handled afterwards.
-	 */
-	if (best_loc && !prange->actual_loc &&
-	    !(prange->flags & KFD_IOCTL_SVM_FLAG_HOST_ACCESS)) {
-		prange->actual_loc = best_loc;
-		return 0;
-	}
-
 	if (!best_loc) {
 		r = svm_migrate_vram_to_ram(prange, mm);
 		*migrated = !r;
-- 
2.32.0

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH 10/10] drm/amdkfd: protect svm_bo ref in case prange has forked
  2021-06-21 16:04 [PATCH 01/10] drm/amdkfd: device pgmap owner at the svm migrate init Alex Sierra
                   ` (7 preceding siblings ...)
  2021-06-21 16:04 ` [PATCH 09/10] drm/amdkfd: partially actual_loc removed Alex Sierra
@ 2021-06-21 16:04 ` Alex Sierra
  2021-06-21 21:46   ` Felix Kuehling
  2021-06-21 19:59 ` [PATCH 01/10] drm/amdkfd: device pgmap owner at the svm migrate init Felix Kuehling
  9 siblings, 1 reply; 21+ messages in thread
From: Alex Sierra @ 2021-06-21 16:04 UTC (permalink / raw)
  To: amd-gfx

Keep track of all the pages inside of pranges referenced to
the same svm_bo. This is done, by using the ref count inside this object.
This makes sure the object has freed after the last prange is not longer
at any GPU. Including references shared between a parent and child during
a fork.

Signed-off-by: Alex Sierra <alex.sierra@amd.com>
Change-Id: Ibfe5efbfed28c2d7681fe091264a5d0d5f3657b2
---
 drivers/gpu/drm/amd/amdkfd/kfd_migrate.c | 10 ++++++++--
 drivers/gpu/drm/amd/amdkfd/kfd_svm.c     | 10 +---------
 drivers/gpu/drm/amd/amdkfd/kfd_svm.h     | 10 +++++++++-
 3 files changed, 18 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c b/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c
index acb9f64577a0..c8ca3252cbc2 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c
@@ -245,7 +245,7 @@ svm_migrate_get_vram_page(struct svm_range *prange, unsigned long pfn)
 	struct page *page;
 
 	page = pfn_to_page(pfn);
-	page->zone_device_data = prange;
+	page->zone_device_data = prange->svm_bo;
 	get_page(page);
 	lock_page(page);
 }
@@ -336,6 +336,7 @@ svm_migrate_copy_to_vram(struct amdgpu_device *adev, struct svm_range *prange,
 			svm_migrate_get_vram_page(prange, migrate->dst[i]);
 			migrate->dst[i] = migrate_pfn(migrate->dst[i]);
 			migrate->dst[i] |= MIGRATE_PFN_LOCKED;
+			svm_range_bo_ref(prange->svm_bo);
 		}
 		if (migrate->dst[i] & MIGRATE_PFN_VALID) {
 			spage = migrate_pfn_to_page(migrate->src[i]);
@@ -540,7 +541,12 @@ svm_migrate_ram_to_vram(struct svm_range *prange, uint32_t best_loc,
 
 static void svm_migrate_page_free(struct page *page)
 {
-	/* Keep this function to avoid warning */
+	struct svm_range_bo *svm_bo = page->zone_device_data;
+
+	if (svm_bo) {
+		pr_debug("svm_bo ref left: %d\n", kref_read(&svm_bo->kref));
+		svm_range_bo_unref(svm_bo);
+	}
 }
 
 static int
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
index ebc1ae7e5193..4b5fc2375641 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
@@ -309,14 +309,6 @@ static bool svm_bo_ref_unless_zero(struct svm_range_bo *svm_bo)
 	return true;
 }
 
-static struct svm_range_bo *svm_range_bo_ref(struct svm_range_bo *svm_bo)
-{
-	if (svm_bo)
-		kref_get(&svm_bo->kref);
-
-	return svm_bo;
-}
-
 static void svm_range_bo_release(struct kref *kref)
 {
 	struct svm_range_bo *svm_bo;
@@ -355,7 +347,7 @@ static void svm_range_bo_release(struct kref *kref)
 	kfree(svm_bo);
 }
 
-static void svm_range_bo_unref(struct svm_range_bo *svm_bo)
+void svm_range_bo_unref(struct svm_range_bo *svm_bo)
 {
 	if (!svm_bo)
 		return;
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_svm.h b/drivers/gpu/drm/amd/amdkfd/kfd_svm.h
index 27fbe1936493..21f693767a0d 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_svm.h
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_svm.h
@@ -150,6 +150,14 @@ static inline void svm_range_unlock(struct svm_range *prange)
 	mutex_unlock(&prange->lock);
 }
 
+static inline struct svm_range_bo *svm_range_bo_ref(struct svm_range_bo *svm_bo)
+{
+	if (svm_bo)
+		kref_get(&svm_bo->kref);
+
+	return svm_bo;
+}
+
 int svm_range_list_init(struct kfd_process *p);
 void svm_range_list_fini(struct kfd_process *p);
 int svm_ioctl(struct kfd_process *p, enum kfd_ioctl_svm_op op, uint64_t start,
@@ -178,7 +186,7 @@ void svm_range_dma_unmap(struct device *dev, dma_addr_t *dma_addr,
 void svm_range_free_dma_mappings(struct svm_range *prange);
 void svm_range_prefault(struct svm_range *prange, struct mm_struct *mm,
 			void *owner);
-
+void svm_range_bo_unref(struct svm_range_bo *svm_bo);
 #else
 
 struct kfd_process;
-- 
2.32.0

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* Re: [PATCH 01/10] drm/amdkfd: device pgmap owner at the svm migrate init
  2021-06-21 16:04 [PATCH 01/10] drm/amdkfd: device pgmap owner at the svm migrate init Alex Sierra
                   ` (8 preceding siblings ...)
  2021-06-21 16:04 ` [PATCH 10/10] drm/amdkfd: protect svm_bo ref in case prange has forked Alex Sierra
@ 2021-06-21 19:59 ` Felix Kuehling
  9 siblings, 0 replies; 21+ messages in thread
From: Felix Kuehling @ 2021-06-21 19:59 UTC (permalink / raw)
  To: amd-gfx, Sierra Guiza, Alejandro (Alex)

On 2021-06-21 12:04 p.m., Alex Sierra wrote:
> pgmap owner member at the svm migrate init could be referenced
> to either adev or hive, depending on device topology.

The reasoning for this change is, that GPUs in the same XGMI hive have 
direct access to all members' VRAM. When mapping memory to a GPU, we 
don't need hmm_range_fault to fault device-private pages in the same 
hive back to the host. Identifying the page owner as the hive, rather 
than the individual GPU, accomplishes this.

With this explanation in the commit description, the patch is

Reviewed-by: Felix Kuehling <Felix.Kuehling@amd.com>

>
> Signed-off-by: Alex Sierra <alex.sierra@amd.com>
> ---
>   drivers/gpu/drm/amd/amdkfd/kfd_migrate.c | 6 +++---
>   drivers/gpu/drm/amd/amdkfd/kfd_svm.h     | 3 +++
>   2 files changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c b/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c
> index fd8f544f0de2..11f7f590c6ec 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c
> @@ -426,7 +426,7 @@ svm_migrate_vma_to_vram(struct amdgpu_device *adev, struct svm_range *prange,
>   	migrate.start = start;
>   	migrate.end = end;
>   	migrate.flags = MIGRATE_VMA_SELECT_SYSTEM;
> -	migrate.pgmap_owner = adev;
> +	migrate.pgmap_owner = SVM_ADEV_PGMAP_OWNER(adev);
>   
>   	size = 2 * sizeof(*migrate.src) + sizeof(uint64_t) + sizeof(dma_addr_t);
>   	size *= npages;
> @@ -641,7 +641,7 @@ svm_migrate_vma_to_ram(struct amdgpu_device *adev, struct svm_range *prange,
>   	migrate.start = start;
>   	migrate.end = end;
>   	migrate.flags = MIGRATE_VMA_SELECT_DEVICE_PRIVATE;
> -	migrate.pgmap_owner = adev;
> +	migrate.pgmap_owner = SVM_ADEV_PGMAP_OWNER(adev);
>   
>   	size = 2 * sizeof(*migrate.src) + sizeof(uint64_t) + sizeof(dma_addr_t);
>   	size *= npages;
> @@ -907,7 +907,7 @@ int svm_migrate_init(struct amdgpu_device *adev)
>   	pgmap->range.start = res->start;
>   	pgmap->range.end = res->end;
>   	pgmap->ops = &svm_migrate_pgmap_ops;
> -	pgmap->owner = adev;
> +	pgmap->owner = SVM_ADEV_PGMAP_OWNER(adev);
>   	pgmap->flags = MIGRATE_VMA_SELECT_DEVICE_PRIVATE;
>   	r = devm_memremap_pages(adev->dev, pgmap);
>   	if (IS_ERR(r)) {
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_svm.h b/drivers/gpu/drm/amd/amdkfd/kfd_svm.h
> index 573f984b81fe..4297250f259d 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_svm.h
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_svm.h
> @@ -35,6 +35,9 @@
>   #include "amdgpu.h"
>   #include "kfd_priv.h"
>   
> +#define SVM_ADEV_PGMAP_OWNER(adev)\
> +			((adev)->hive ? (void *)(adev)->hive : (void *)(adev))
> +
>   struct svm_range_bo {
>   	struct amdgpu_bo		*bo;
>   	struct kref			kref;
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 02/10] drm/amdkfd: add owner ref param to get hmm pages
  2021-06-21 16:04 ` [PATCH 02/10] drm/amdkfd: add owner ref param to get hmm pages Alex Sierra
@ 2021-06-21 20:00   ` Felix Kuehling
  0 siblings, 0 replies; 21+ messages in thread
From: Felix Kuehling @ 2021-06-21 20:00 UTC (permalink / raw)
  To: Alex Sierra, amd-gfx

On 2021-06-21 12:04 p.m., Alex Sierra wrote:
> The parameter is used in the dev_private_owner to decide if device
> pages in the range require to be migrated back to system memory, based
> if they are or not in the same memory domain.
> In this case, this reference could come from the same memory domain
> with devices connected to the same hive.
>
> Signed-off-by: Alex Sierra <alex.sierra@amd.com>

Reviewed-by: Felix Kuehling <Felix.Kuehling@amd.com>


> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c  | 3 ++-
>   drivers/gpu/drm/amd/amdgpu/amdgpu_mn.h  | 2 +-
>   drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 2 +-
>   drivers/gpu/drm/amd/amdkfd/kfd_svm.c    | 4 ++--
>   4 files changed, 6 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c
> index 2741c28ff1b5..378c238c2099 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c
> @@ -160,7 +160,7 @@ int amdgpu_hmm_range_get_pages(struct mmu_interval_notifier *notifier,
>   			       struct mm_struct *mm, struct page **pages,
>   			       uint64_t start, uint64_t npages,
>   			       struct hmm_range **phmm_range, bool readonly,
> -			       bool mmap_locked)
> +			       bool mmap_locked, void *owner)
>   {
>   	struct hmm_range *hmm_range;
>   	unsigned long timeout;
> @@ -185,6 +185,7 @@ int amdgpu_hmm_range_get_pages(struct mmu_interval_notifier *notifier,
>   	hmm_range->hmm_pfns = pfns;
>   	hmm_range->start = start;
>   	hmm_range->end = start + npages * PAGE_SIZE;
> +	hmm_range->dev_private_owner = owner;
>   
>   	/* Assuming 512MB takes maxmium 1 second to fault page address */
>   	timeout = max(npages >> 17, 1ULL) * HMM_RANGE_DEFAULT_TIMEOUT;
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_mn.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_mn.h
> index 7f7d37a457c3..14a3c1864085 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_mn.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_mn.h
> @@ -34,7 +34,7 @@ int amdgpu_hmm_range_get_pages(struct mmu_interval_notifier *notifier,
>   			       struct mm_struct *mm, struct page **pages,
>   			       uint64_t start, uint64_t npages,
>   			       struct hmm_range **phmm_range, bool readonly,
> -			       bool mmap_locked);
> +			       bool mmap_locked, void *owner);
>   int amdgpu_hmm_range_get_pages_done(struct hmm_range *hmm_range);
>   
>   #if defined(CONFIG_HMM_MIRROR)
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> index 7e7d8330d64b..c13f7fbfc070 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> @@ -709,7 +709,7 @@ int amdgpu_ttm_tt_get_user_pages(struct amdgpu_bo *bo, struct page **pages)
>   	readonly = amdgpu_ttm_tt_is_readonly(ttm);
>   	r = amdgpu_hmm_range_get_pages(&bo->notifier, mm, pages, start,
>   				       ttm->num_pages, &gtt->range, readonly,
> -				       false);
> +				       false, NULL);
>   out_putmm:
>   	mmput(mm);
>   
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
> index b665e9ff77e3..b939f353ac8c 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
> @@ -1392,7 +1392,7 @@ static int svm_range_validate_and_map(struct mm_struct *mm,
>   		r = amdgpu_hmm_range_get_pages(&prange->notifier, mm, NULL,
>   					       prange->start << PAGE_SHIFT,
>   					       prange->npages, &hmm_range,
> -					       false, true);
> +					       false, true, NULL);
>   		if (r) {
>   			pr_debug("failed %d to get svm range pages\n", r);
>   			goto unreserve_out;
> @@ -2657,7 +2657,7 @@ void svm_range_prefault(struct svm_range *prange, struct mm_struct *mm)
>   	r = amdgpu_hmm_range_get_pages(&prange->notifier, mm, NULL,
>   				       prange->start << PAGE_SHIFT,
>   				       prange->npages, &hmm_range,
> -				       false, true);
> +				       false, true, NULL);
>   	if (!r) {
>   		amdgpu_hmm_range_get_pages_done(hmm_range);
>   		prange->validated_once = true;
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 03/10] drm/amdkfd: set owner ref to svm range prefault
  2021-06-21 16:04 ` [PATCH 03/10] drm/amdkfd: set owner ref to svm range prefault Alex Sierra
@ 2021-06-21 20:02   ` Felix Kuehling
  0 siblings, 0 replies; 21+ messages in thread
From: Felix Kuehling @ 2021-06-21 20:02 UTC (permalink / raw)
  To: Alex Sierra, amd-gfx

On 2021-06-21 12:04 p.m., Alex Sierra wrote:
> svm_range_prefault is called right before migrations to VRAM,
> to make sure pages are resident in system memory before the migration.
> With partial migrations, this reference is used by hmm range get pages
> to avoid migrating pages that are already in the same VRAM domain.
>
> Signed-off-by: Alex Sierra <alex.sierra@amd.com>

Reviewed-by: Felix Kuehling <Felix.Kuehling@amd.com>


> ---
>   drivers/gpu/drm/amd/amdkfd/kfd_migrate.c | 2 +-
>   drivers/gpu/drm/amd/amdkfd/kfd_svm.c     | 5 +++--
>   drivers/gpu/drm/amd/amdkfd/kfd_svm.h     | 3 ++-
>   3 files changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c b/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c
> index 11f7f590c6ec..b298aa8dea4d 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c
> @@ -512,7 +512,7 @@ svm_migrate_ram_to_vram(struct svm_range *prange, uint32_t best_loc,
>   		 prange->start, prange->last, best_loc);
>   
>   	/* FIXME: workaround for page locking bug with invalid pages */
> -	svm_range_prefault(prange, mm);
> +	svm_range_prefault(prange, mm, SVM_ADEV_PGMAP_OWNER(adev));
>   
>   	start = prange->start << PAGE_SHIFT;
>   	end = (prange->last + 1) << PAGE_SHIFT;
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
> index b939f353ac8c..54f47b09b14a 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
> @@ -2646,7 +2646,8 @@ svm_range_best_prefetch_location(struct svm_range *prange)
>   /* FIXME: This is a workaround for page locking bug when some pages are
>    * invalid during migration to VRAM
>    */
> -void svm_range_prefault(struct svm_range *prange, struct mm_struct *mm)
> +void svm_range_prefault(struct svm_range *prange, struct mm_struct *mm,
> +			void *owner)
>   {
>   	struct hmm_range *hmm_range;
>   	int r;
> @@ -2657,7 +2658,7 @@ void svm_range_prefault(struct svm_range *prange, struct mm_struct *mm)
>   	r = amdgpu_hmm_range_get_pages(&prange->notifier, mm, NULL,
>   				       prange->start << PAGE_SHIFT,
>   				       prange->npages, &hmm_range,
> -				       false, true, NULL);
> +				       false, true, owner);
>   	if (!r) {
>   		amdgpu_hmm_range_get_pages_done(hmm_range);
>   		prange->validated_once = true;
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_svm.h b/drivers/gpu/drm/amd/amdkfd/kfd_svm.h
> index 4297250f259d..08542fe39303 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_svm.h
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_svm.h
> @@ -176,7 +176,8 @@ void schedule_deferred_list_work(struct svm_range_list *svms);
>   void svm_range_dma_unmap(struct device *dev, dma_addr_t *dma_addr,
>   			 unsigned long offset, unsigned long npages);
>   void svm_range_free_dma_mappings(struct svm_range *prange);
> -void svm_range_prefault(struct svm_range *prange, struct mm_struct *mm);
> +void svm_range_prefault(struct svm_range *prange, struct mm_struct *mm,
> +			void *owner);
>   
>   #else
>   
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 04/10] drm/amdgpu: get owner ref in validate and map
  2021-06-21 16:04 ` [PATCH 04/10] drm/amdgpu: get owner ref in validate and map Alex Sierra
@ 2021-06-21 20:04   ` Felix Kuehling
  0 siblings, 0 replies; 21+ messages in thread
From: Felix Kuehling @ 2021-06-21 20:04 UTC (permalink / raw)
  To: Alex Sierra, amd-gfx

On 2021-06-21 12:04 p.m., Alex Sierra wrote:
> Get the proper owner reference for amdgpu_hmm_range_get_pages function.
> This is useful for partial migrations. To avoid migrating back to
> system memory, VRAM pages, that are accessible by all devices in the
> same memory domain.
> Ex. multiple devices in the same hive.
>
> Signed-off-by: Alex Sierra <alex.sierra@amd.com>

Reviewed-by: Felix Kuehling <Felix.Kuehling@amd.com>


> ---
>   drivers/gpu/drm/amd/amdkfd/kfd_svm.c | 25 ++++++++++++++++++++++++-
>   1 file changed, 24 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
> index 54f47b09b14a..2b4318646a75 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
> @@ -1313,6 +1313,17 @@ static void svm_range_unreserve_bos(struct svm_validate_context *ctx)
>   	ttm_eu_backoff_reservation(&ctx->ticket, &ctx->validate_list);
>   }
>   
> +static void *kfd_svm_page_owner(struct kfd_process *p, int32_t gpuidx)
> +{
> +	struct kfd_process_device *pdd;
> +	struct amdgpu_device *adev;
> +
> +	pdd = kfd_process_device_from_gpuidx(p, gpuidx);
> +	adev = (struct amdgpu_device *)pdd->dev->kgd;
> +
> +	return SVM_ADEV_PGMAP_OWNER(adev);
> +}
> +
>   /*
>    * Validation+GPU mapping with concurrent invalidation (MMU notifiers)
>    *
> @@ -1343,6 +1354,9 @@ static int svm_range_validate_and_map(struct mm_struct *mm,
>   {
>   	struct svm_validate_context ctx;
>   	struct hmm_range *hmm_range;
> +	struct kfd_process *p;
> +	void *owner;
> +	int32_t idx;
>   	int r = 0;
>   
>   	ctx.process = container_of(prange->svms, struct kfd_process, svms);
> @@ -1389,10 +1403,19 @@ static int svm_range_validate_and_map(struct mm_struct *mm,
>   	svm_range_reserve_bos(&ctx);
>   
>   	if (!prange->actual_loc) {
> +		p = container_of(prange->svms, struct kfd_process, svms);
> +		owner = kfd_svm_page_owner(p, find_first_bit(ctx.bitmap,
> +							MAX_GPU_INSTANCE));
> +		for_each_set_bit(idx, ctx.bitmap, MAX_GPU_INSTANCE) {
> +			if (kfd_svm_page_owner(p, idx) != owner) {
> +				owner = NULL;
> +				break;
> +			}
> +		}
>   		r = amdgpu_hmm_range_get_pages(&prange->notifier, mm, NULL,
>   					       prange->start << PAGE_SHIFT,
>   					       prange->npages, &hmm_range,
> -					       false, true, NULL);
> +					       false, true, owner);
>   		if (r) {
>   			pr_debug("failed %d to get svm range pages\n", r);
>   			goto unreserve_out;
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 05/10] drm/amdkfd: classify and map mixed svm range pages in GPU
  2021-06-21 16:04 ` [PATCH 05/10] drm/amdkfd: classify and map mixed svm range pages in GPU Alex Sierra
@ 2021-06-21 20:26   ` Felix Kuehling
  0 siblings, 0 replies; 21+ messages in thread
From: Felix Kuehling @ 2021-06-21 20:26 UTC (permalink / raw)
  To: Alex Sierra, amd-gfx

On 2021-06-21 12:04 p.m., Alex Sierra wrote:
> [Why]
> svm ranges can have mixed pages from device or system memory.
> A good example is, after a prange has been allocated in VRAM and a
> copy-on-write is triggered by a fork. This invalidates some pages
> inside the prange. Endding up in mixed pages.
>
> [How]
> By classifying each page inside a prange, based on its type. Device or
> system memory, during dma mapping call. If page corresponds
> to VRAM domain, a flag is set to its dma_addr entry for each GPU.
> Then, at the GPU page table mapping. All group of contiguous pages within
> the same type are mapped with their proper pte flags.
>
> v2:
> Instead of using ttm_res to calculate vram pfns in the svm_range. It is now
> done by setting the vram real physical address into drm_addr array.
> This makes more flexible VRAM management, plus removes the need to have
> a BO reference in the svm_range.
>
> v3:
> Remove mapping member from svm_range
>
> Signed-off-by: Alex Sierra <alex.sierra@amd.com>

Reviewed-by: Felix Kuehling <Felix.Kuehling@amd.com>


> ---
>   drivers/gpu/drm/amd/amdkfd/kfd_svm.c | 73 ++++++++++++++++++----------
>   drivers/gpu/drm/amd/amdkfd/kfd_svm.h |  2 +-
>   2 files changed, 47 insertions(+), 28 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
> index 2b4318646a75..3b05bc270732 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
> @@ -119,11 +119,12 @@ static void svm_range_remove_notifier(struct svm_range *prange)
>   }
>   
>   static int
> -svm_range_dma_map_dev(struct device *dev, dma_addr_t **dma_addr,
> +svm_range_dma_map_dev(struct amdgpu_device *adev, dma_addr_t **dma_addr,
>   		      unsigned long *hmm_pfns, uint64_t npages)
>   {
>   	enum dma_data_direction dir = DMA_BIDIRECTIONAL;
>   	dma_addr_t *addr = *dma_addr;
> +	struct device *dev = adev->dev;
>   	struct page *page;
>   	int i, r;
>   
> @@ -141,6 +142,14 @@ svm_range_dma_map_dev(struct device *dev, dma_addr_t **dma_addr,
>   			dma_unmap_page(dev, addr[i], PAGE_SIZE, dir);
>   
>   		page = hmm_pfn_to_page(hmm_pfns[i]);
> +		if (is_zone_device_page(page)) {
> +			addr[i] = (hmm_pfns[i] << PAGE_SHIFT) +
> +				   adev->vm_manager.vram_base_offset -
> +				   adev->kfd.dev->pgmap.range.start;
> +			addr[i] |= SVM_RANGE_VRAM_DOMAIN;
> +			pr_debug("vram address detected: 0x%llx\n", addr[i]);
> +			continue;
> +		}
>   		addr[i] = dma_map_page(dev, page, 0, PAGE_SIZE, dir);
>   		r = dma_mapping_error(dev, addr[i]);
>   		if (r) {
> @@ -175,7 +184,7 @@ svm_range_dma_map(struct svm_range *prange, unsigned long *bitmap,
>   		}
>   		adev = (struct amdgpu_device *)pdd->dev->kgd;
>   
> -		r = svm_range_dma_map_dev(adev->dev, &prange->dma_addr[gpuidx],
> +		r = svm_range_dma_map_dev(adev, &prange->dma_addr[gpuidx],
>   					  hmm_pfns, prange->npages);
>   		if (r)
>   			break;
> @@ -1003,21 +1012,22 @@ svm_range_split_by_granularity(struct kfd_process *p, struct mm_struct *mm,
>   }
>   
>   static uint64_t
> -svm_range_get_pte_flags(struct amdgpu_device *adev, struct svm_range *prange)
> +svm_range_get_pte_flags(struct amdgpu_device *adev, struct svm_range *prange,
> +			int domain)
>   {
>   	struct amdgpu_device *bo_adev;
>   	uint32_t flags = prange->flags;
>   	uint32_t mapping_flags = 0;
>   	uint64_t pte_flags;
> -	bool snoop = !prange->ttm_res;
> +	bool snoop = (domain != SVM_RANGE_VRAM_DOMAIN);
>   	bool coherent = flags & KFD_IOCTL_SVM_FLAG_COHERENT;
>   
> -	if (prange->svm_bo && prange->ttm_res)
> +	if (domain == SVM_RANGE_VRAM_DOMAIN)
>   		bo_adev = amdgpu_ttm_adev(prange->svm_bo->bo->tbo.bdev);
>   
>   	switch (adev->asic_type) {
>   	case CHIP_ARCTURUS:
> -		if (prange->svm_bo && prange->ttm_res) {
> +		if (domain == SVM_RANGE_VRAM_DOMAIN) {
>   			if (bo_adev == adev) {
>   				mapping_flags |= coherent ?
>   					AMDGPU_VM_MTYPE_CC : AMDGPU_VM_MTYPE_RW;
> @@ -1032,7 +1042,7 @@ svm_range_get_pte_flags(struct amdgpu_device *adev, struct svm_range *prange)
>   		}
>   		break;
>   	case CHIP_ALDEBARAN:
> -		if (prange->svm_bo && prange->ttm_res) {
> +		if (domain == SVM_RANGE_VRAM_DOMAIN) {
>   			if (bo_adev == adev) {
>   				mapping_flags |= coherent ?
>   					AMDGPU_VM_MTYPE_CC : AMDGPU_VM_MTYPE_RW;
> @@ -1061,14 +1071,14 @@ svm_range_get_pte_flags(struct amdgpu_device *adev, struct svm_range *prange)
>   		mapping_flags |= AMDGPU_VM_PAGE_EXECUTABLE;
>   
>   	pte_flags = AMDGPU_PTE_VALID;
> -	pte_flags |= prange->ttm_res ? 0 : AMDGPU_PTE_SYSTEM;
> +	pte_flags |= (domain == SVM_RANGE_VRAM_DOMAIN) ? 0 : AMDGPU_PTE_SYSTEM;
>   	pte_flags |= snoop ? AMDGPU_PTE_SNOOPED : 0;
>   
>   	pte_flags |= amdgpu_gem_va_map_flags(adev, mapping_flags);
>   
>   	pr_debug("svms 0x%p [0x%lx 0x%lx] vram %d PTE 0x%llx mapping 0x%x\n",
>   		 prange->svms, prange->start, prange->last,
> -		 prange->ttm_res ? 1:0, pte_flags, mapping_flags);
> +		 (domain == SVM_RANGE_VRAM_DOMAIN) ? 1:0, pte_flags, mapping_flags);
>   
>   	return pte_flags;
>   }
> @@ -1138,31 +1148,41 @@ svm_range_map_to_gpu(struct amdgpu_device *adev, struct amdgpu_vm *vm,
>   {
>   	struct amdgpu_bo_va bo_va;
>   	uint64_t pte_flags;
> +	unsigned long last_start;
> +	int last_domain;
>   	int r = 0;
> +	int64_t i;
>   
>   	pr_debug("svms 0x%p [0x%lx 0x%lx]\n", prange->svms, prange->start,
>   		 prange->last);
>   
> -	if (prange->svm_bo && prange->ttm_res) {
> +	if (prange->svm_bo && prange->ttm_res)
>   		bo_va.is_xgmi = amdgpu_xgmi_same_hive(adev, bo_adev);
> -		prange->mapping.bo_va = &bo_va;
> -	}
>   
> -	prange->mapping.start = prange->start;
> -	prange->mapping.last = prange->last;
> -	prange->mapping.offset = prange->offset;
> -	pte_flags = svm_range_get_pte_flags(adev, prange);
> +	last_start = prange->start;
> +	for (i = 0; i < prange->npages; i++) {
> +		last_domain = dma_addr[i] & SVM_RANGE_VRAM_DOMAIN;
> +		dma_addr[i] &= ~SVM_RANGE_VRAM_DOMAIN;
> +		if ((prange->start + i) < prange->last &&
> +		    last_domain == (dma_addr[i + 1] & SVM_RANGE_VRAM_DOMAIN))
> +			continue;
>   
> -	r = amdgpu_vm_bo_update_mapping(adev, bo_adev, vm, false, false, NULL,
> -					prange->mapping.start,
> -					prange->mapping.last, pte_flags,
> -					prange->mapping.offset,
> -					prange->ttm_res ?
> -						prange->ttm_res->mm_node : NULL,
> -					dma_addr, &vm->last_update);
> -	if (r) {
> -		pr_debug("failed %d to map to gpu 0x%lx\n", r, prange->start);
> -		goto out;
> +		pr_debug("Mapping range [0x%lx 0x%llx] on domain: %s\n",
> +			 last_start, prange->start + i, last_domain ? "GPU" : "CPU");
> +		pte_flags = svm_range_get_pte_flags(adev, prange, last_domain);
> +		r = amdgpu_vm_bo_update_mapping(adev, bo_adev, vm, false, false, NULL,
> +						last_start,
> +						prange->start + i, pte_flags,
> +						prange->offset +
> +						((last_start - prange->start) << PAGE_SHIFT),
> +						NULL,
> +						dma_addr,
> +						&vm->last_update);
> +		if (r) {
> +			pr_debug("failed %d to map to gpu 0x%lx\n", r, prange->start);
> +			goto out;
> +		}
> +		last_start = prange->start + i + 1;
>   	}
>   
>   	r = amdgpu_vm_update_pdes(adev, vm, false);
> @@ -1176,7 +1196,6 @@ svm_range_map_to_gpu(struct amdgpu_device *adev, struct amdgpu_vm *vm,
>   		*fence = dma_fence_get(vm->last_update);
>   
>   out:
> -	prange->mapping.bo_va = NULL;
>   	return r;
>   }
>   
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_svm.h b/drivers/gpu/drm/amd/amdkfd/kfd_svm.h
> index 08542fe39303..27fbe1936493 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_svm.h
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_svm.h
> @@ -35,6 +35,7 @@
>   #include "amdgpu.h"
>   #include "kfd_priv.h"
>   
> +#define SVM_RANGE_VRAM_DOMAIN (1UL << 0)
>   #define SVM_ADEV_PGMAP_OWNER(adev)\
>   			((adev)->hive ? (void *)(adev)->hive : (void *)(adev))
>   
> @@ -113,7 +114,6 @@ struct svm_range {
>   	struct list_head		update_list;
>   	struct list_head		remove_list;
>   	struct list_head		insert_list;
> -	struct amdgpu_bo_va_mapping	mapping;
>   	uint64_t			npages;
>   	dma_addr_t			*dma_addr[MAX_GPU_INSTANCE];
>   	struct ttm_resource		*ttm_res;
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 07/10] drm/amdkfd: skip migration for pages already in VRAM
  2021-06-21 16:04 ` [PATCH 07/10] drm/amdkfd: skip migration for pages already in VRAM Alex Sierra
@ 2021-06-21 21:01   ` Felix Kuehling
  0 siblings, 0 replies; 21+ messages in thread
From: Felix Kuehling @ 2021-06-21 21:01 UTC (permalink / raw)
  To: Alex Sierra, amd-gfx

On 2021-06-21 12:04 p.m., Alex Sierra wrote:
> Migration skipped for pages that are already in VRAM
> domain. These could be the result of previous partial
> migrations to SYS RAM, and prefetch back to VRAM.
> Ex. Coherent pages in VRAM that were not written/invalidated after
> a copy-on-write.
>
> Signed-off-by: Alex Sierra <alex.sierra@amd.com>
> ---
>   drivers/gpu/drm/amd/amdkfd/kfd_migrate.c | 17 +++++++++--------
>   1 file changed, 9 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c b/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c
> index 6fd68528c425..8a3f21d76915 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c
> @@ -329,14 +329,15 @@ svm_migrate_copy_to_vram(struct amdgpu_device *adev, struct svm_range *prange,
>   	for (i = j = 0; i < npages; i++) {
>   		struct page *spage;
>   
> -		dst[i] = vram_addr + (j << PAGE_SHIFT);
> -		migrate->dst[i] = svm_migrate_addr_to_pfn(adev, dst[i]);
> -		svm_migrate_get_vram_page(prange, migrate->dst[i]);
> -
> -		migrate->dst[i] = migrate_pfn(migrate->dst[i]);
> -		migrate->dst[i] |= MIGRATE_PFN_LOCKED;
> -
> -		if (migrate->src[i] & MIGRATE_PFN_VALID) {
> +		spage = migrate_pfn_to_page(migrate->src[i]);
> +		if (spage && !is_zone_device_page(spage)) {
> +			dst[i] = vram_addr + (j << PAGE_SHIFT);
> +			migrate->dst[i] = svm_migrate_addr_to_pfn(adev, dst[i]);
> +			svm_migrate_get_vram_page(prange, migrate->dst[i]);
> +			migrate->dst[i] = migrate_pfn(migrate->dst[i]);
> +			migrate->dst[i] |= MIGRATE_PFN_LOCKED;
> +		}
> +		if (migrate->dst[i] & MIGRATE_PFN_VALID) {
>   			spage = migrate_pfn_to_page(migrate->src[i]);

I think spage is already set correctly here. You shouldn't need to 
assign it again.

Also, is this condition (migrate->dst[i] & MIGRATE_PFN_VALID) really 
needed? It seems to me, migrate_pfn sets that flag unconditionally. So 
you can just continue the previous if-block.

Regards,
   Felix


>   			src[i] = dma_map_page(dev, spage, 0, PAGE_SIZE,
>   					      DMA_TO_DEVICE);
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 08/10] drm/amdkfd: add invalid pages debug at vram migration
  2021-06-21 16:04 ` [PATCH 08/10] drm/amdkfd: add invalid pages debug at vram migration Alex Sierra
@ 2021-06-21 21:02   ` Felix Kuehling
  0 siblings, 0 replies; 21+ messages in thread
From: Felix Kuehling @ 2021-06-21 21:02 UTC (permalink / raw)
  To: Alex Sierra, amd-gfx

On 2021-06-21 12:04 p.m., Alex Sierra wrote:
> This is for debug purposes only.
> It conditionally generates partial migrations to test mixed
> CPU/GPU memory domain pages in a prange easily.
>
> Signed-off-by: Alex Sierra <alex.sierra@amd.com>

Reviewed-by: Felix Kuehling <Felix.Kuehling@amd.com>


> ---
>   drivers/gpu/drm/amd/amdkfd/kfd_migrate.c | 14 ++++++++++++++
>   1 file changed, 14 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c b/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c
> index 8a3f21d76915..f71f8d7e2b72 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c
> @@ -404,6 +404,20 @@ svm_migrate_copy_to_vram(struct amdgpu_device *adev, struct svm_range *prange,
>   		}
>   	}
>   
> +#ifdef DEBUG_FORCE_MIXED_DOMAINS
> +	for (i = 0, j = 0; i < npages; i += 4, j++) {
> +		if (j & 1)
> +			continue;
> +		svm_migrate_put_vram_page(adev, dst[i]);
> +		migrate->dst[i] = 0;
> +		svm_migrate_put_vram_page(adev, dst[i + 1]);
> +		migrate->dst[i + 1] = 0;
> +		svm_migrate_put_vram_page(adev, dst[i + 2]);
> +		migrate->dst[i + 2] = 0;
> +		svm_migrate_put_vram_page(adev, dst[i + 3]);
> +		migrate->dst[i + 3] = 0;
> +	}
> +#endif
>   out:
>   	return r;
>   }
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 09/10] drm/amdkfd: partially actual_loc removed
  2021-06-21 16:04 ` [PATCH 09/10] drm/amdkfd: partially actual_loc removed Alex Sierra
@ 2021-06-21 21:24   ` Felix Kuehling
  0 siblings, 0 replies; 21+ messages in thread
From: Felix Kuehling @ 2021-06-21 21:24 UTC (permalink / raw)
  To: Alex Sierra, amd-gfx

On 2021-06-21 12:04 p.m., Alex Sierra wrote:
> actual_loc should not be used anymore, as pranges
> could have mixed locations (VRAM & SYSRAM) at the
> same time.
>
> Signed-off-by: Alex Sierra <alex.sierra@amd.com>
> ---
>   drivers/gpu/drm/amd/amdkfd/kfd_migrate.c | 12 +---
>   drivers/gpu/drm/amd/amdkfd/kfd_svm.c     | 71 ++++++++++--------------
>   2 files changed, 29 insertions(+), 54 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c b/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c
> index f71f8d7e2b72..acb9f64577a0 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c
> @@ -501,12 +501,6 @@ svm_migrate_ram_to_vram(struct svm_range *prange, uint32_t best_loc,
>   	struct amdgpu_device *adev;
>   	int r = 0;
>   
> -	if (prange->actual_loc == best_loc) {
> -		pr_debug("svms 0x%p [0x%lx 0x%lx] already on best_loc 0x%x\n",
> -			 prange->svms, prange->start, prange->last, best_loc);
> -		return 0;
> -	}
> -
>   	adev = svm_range_get_adev_by_id(prange, best_loc);
>   	if (!adev) {
>   		pr_debug("failed to get device by id 0x%x\n", best_loc);
> @@ -791,11 +785,7 @@ int
>   svm_migrate_to_vram(struct svm_range *prange, uint32_t best_loc,
>   		    struct mm_struct *mm)
>   {
> -	if  (!prange->actual_loc)
> -		return svm_migrate_ram_to_vram(prange, best_loc, mm);
> -	else
> -		return svm_migrate_vram_to_vram(prange, best_loc, mm);
> -
> +	return svm_migrate_ram_to_vram(prange, best_loc, mm);

Can you remove svm_migrate_vram_to_vram in this case? I guess we're 
relying on the svm_range_prefault call in svm_migrate_ram_to_vram now to 
migrate VRAM in a different XGMI hive to system memory now. But 
eventually we want to get rid of that pre-fault hack.


>   }
>   
>   /**
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
> index 3b05bc270732..ebc1ae7e5193 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
> @@ -1421,42 +1421,38 @@ static int svm_range_validate_and_map(struct mm_struct *mm,
>   
>   	svm_range_reserve_bos(&ctx);
>   
> -	if (!prange->actual_loc) {
> -		p = container_of(prange->svms, struct kfd_process, svms);
> -		owner = kfd_svm_page_owner(p, find_first_bit(ctx.bitmap,
> -							MAX_GPU_INSTANCE));
> -		for_each_set_bit(idx, ctx.bitmap, MAX_GPU_INSTANCE) {
> -			if (kfd_svm_page_owner(p, idx) != owner) {
> -				owner = NULL;
> -				break;
> -			}
> -		}
> -		r = amdgpu_hmm_range_get_pages(&prange->notifier, mm, NULL,
> -					       prange->start << PAGE_SHIFT,
> -					       prange->npages, &hmm_range,
> -					       false, true, owner);
> -		if (r) {
> -			pr_debug("failed %d to get svm range pages\n", r);
> -			goto unreserve_out;
> -		}
> -
> -		r = svm_range_dma_map(prange, ctx.bitmap,
> -				      hmm_range->hmm_pfns);
> -		if (r) {
> -			pr_debug("failed %d to dma map range\n", r);
> -			goto unreserve_out;
> +	p = container_of(prange->svms, struct kfd_process, svms);
> +	owner = kfd_svm_page_owner(p, find_first_bit(ctx.bitmap,
> +						MAX_GPU_INSTANCE));
> +	for_each_set_bit(idx, ctx.bitmap, MAX_GPU_INSTANCE) {
> +		if (kfd_svm_page_owner(p, idx) != owner) {
> +			owner = NULL;
> +			break;
>   		}
> +	}
> +	r = amdgpu_hmm_range_get_pages(&prange->notifier, mm, NULL,
> +				       prange->start << PAGE_SHIFT,
> +				       prange->npages, &hmm_range,
> +				       false, true, owner);
> +	if (r) {
> +		pr_debug("failed %d to get svm range pages\n", r);
> +		goto unreserve_out;
> +	}
>   
> -		prange->validated_once = true;
> +	r = svm_range_dma_map(prange, ctx.bitmap,
> +			      hmm_range->hmm_pfns);
> +	if (r) {
> +		pr_debug("failed %d to dma map range\n", r);
> +		goto unreserve_out;
>   	}
>   
> +	prange->validated_once = true;
> +
>   	svm_range_lock(prange);
> -	if (!prange->actual_loc) {
> -		if (amdgpu_hmm_range_get_pages_done(hmm_range)) {
> -			pr_debug("hmm update the range, need validate again\n");
> -			r = -EAGAIN;
> -			goto unlock_out;
> -		}
> +	if (amdgpu_hmm_range_get_pages_done(hmm_range)) {
> +		pr_debug("hmm update the range, need validate again\n");
> +		r = -EAGAIN;
> +		goto unlock_out;

IMO, this is the most important part of this commit, and it should be 
called out the the path description. Here we use hmm_range_fault for 
getting VRAM addresses. This is what enables mixed mappings in the first 
place.

Regards,
    Felix


>   	}
>   	if (!list_empty(&prange->child_list)) {
>   		pr_debug("range split by unmap in parallel, validate again\n");
> @@ -2741,20 +2737,9 @@ svm_range_trigger_migration(struct mm_struct *mm, struct svm_range *prange,
>   	*migrated = false;
>   	best_loc = svm_range_best_prefetch_location(prange);
>   
> -	if (best_loc == KFD_IOCTL_SVM_LOCATION_UNDEFINED ||
> -	    best_loc == prange->actual_loc)
> +	if (best_loc == KFD_IOCTL_SVM_LOCATION_UNDEFINED)
>   		return 0;
>   
> -	/*
> -	 * Prefetch to GPU without host access flag, set actual_loc to gpu, then
> -	 * validate on gpu and map to gpus will be handled afterwards.
> -	 */
> -	if (best_loc && !prange->actual_loc &&
> -	    !(prange->flags & KFD_IOCTL_SVM_FLAG_HOST_ACCESS)) {
> -		prange->actual_loc = best_loc;
> -		return 0;
> -	}
> -
>   	if (!best_loc) {
>   		r = svm_migrate_vram_to_ram(prange, mm);
>   		*migrated = !r;
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 10/10] drm/amdkfd: protect svm_bo ref in case prange has forked
  2021-06-21 16:04 ` [PATCH 10/10] drm/amdkfd: protect svm_bo ref in case prange has forked Alex Sierra
@ 2021-06-21 21:46   ` Felix Kuehling
  0 siblings, 0 replies; 21+ messages in thread
From: Felix Kuehling @ 2021-06-21 21:46 UTC (permalink / raw)
  To: Alex Sierra, amd-gfx

On 2021-06-21 12:04 p.m., Alex Sierra wrote:
> Keep track of all the pages inside of pranges referenced to
> the same svm_bo.

This description is a bit confusing because you're not tracking page 
references but BO references.


>   This is done, by using the ref count inside this object.
> This makes sure the object has freed after the last prange is not longer
> at any GPU. Including references shared between a parent and child during
> a fork.

References to the BO are not really shared between parent and child 
processes. They share page references. What we're doing here is, we're 
removing assumptions about the lifetime of the svm_bo by tying it to the 
lifetime of any pages referencing it.

I'd suggest this description:

drm/amdkfd: Maintain svm_bo reference in page->zone_device_data

Each zone-device page holds a reference to the SVM BO that manages its 
backing storage. This is necessary to correctly hold on to the BO in 
case zone_device pages are shared with a child-process.


>
> Signed-off-by: Alex Sierra <alex.sierra@amd.com>
> Change-Id: Ibfe5efbfed28c2d7681fe091264a5d0d5f3657b2

Please remove the Change-Id.


> ---
>   drivers/gpu/drm/amd/amdkfd/kfd_migrate.c | 10 ++++++++--
>   drivers/gpu/drm/amd/amdkfd/kfd_svm.c     | 10 +---------
>   drivers/gpu/drm/amd/amdkfd/kfd_svm.h     | 10 +++++++++-
>   3 files changed, 18 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c b/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c
> index acb9f64577a0..c8ca3252cbc2 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c
> @@ -245,7 +245,7 @@ svm_migrate_get_vram_page(struct svm_range *prange, unsigned long pfn)
>   	struct page *page;
>   
>   	page = pfn_to_page(pfn);
> -	page->zone_device_data = prange;
> +	page->zone_device_data = prange->svm_bo;
>   	get_page(page);
>   	lock_page(page);
>   }
> @@ -336,6 +336,7 @@ svm_migrate_copy_to_vram(struct amdgpu_device *adev, struct svm_range *prange,
>   			svm_migrate_get_vram_page(prange, migrate->dst[i]);
>   			migrate->dst[i] = migrate_pfn(migrate->dst[i]);
>   			migrate->dst[i] |= MIGRATE_PFN_LOCKED;
> +			svm_range_bo_ref(prange->svm_bo);

It would be cleaner to move this into svm_migrate_get_vram_page, where 
you assign the prange->svm_bo reference to page->zonde_device_data.

Regards,
   Felix


>   		}
>   		if (migrate->dst[i] & MIGRATE_PFN_VALID) {
>   			spage = migrate_pfn_to_page(migrate->src[i]);
> @@ -540,7 +541,12 @@ svm_migrate_ram_to_vram(struct svm_range *prange, uint32_t best_loc,
>   
>   static void svm_migrate_page_free(struct page *page)
>   {
> -	/* Keep this function to avoid warning */
> +	struct svm_range_bo *svm_bo = page->zone_device_data;
> +
> +	if (svm_bo) {
> +		pr_debug("svm_bo ref left: %d\n", kref_read(&svm_bo->kref));
> +		svm_range_bo_unref(svm_bo);
> +	}
>   }
>   
>   static int
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
> index ebc1ae7e5193..4b5fc2375641 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
> @@ -309,14 +309,6 @@ static bool svm_bo_ref_unless_zero(struct svm_range_bo *svm_bo)
>   	return true;
>   }
>   
> -static struct svm_range_bo *svm_range_bo_ref(struct svm_range_bo *svm_bo)
> -{
> -	if (svm_bo)
> -		kref_get(&svm_bo->kref);
> -
> -	return svm_bo;
> -}
> -
>   static void svm_range_bo_release(struct kref *kref)
>   {
>   	struct svm_range_bo *svm_bo;
> @@ -355,7 +347,7 @@ static void svm_range_bo_release(struct kref *kref)
>   	kfree(svm_bo);
>   }
>   
> -static void svm_range_bo_unref(struct svm_range_bo *svm_bo)
> +void svm_range_bo_unref(struct svm_range_bo *svm_bo)
>   {
>   	if (!svm_bo)
>   		return;
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_svm.h b/drivers/gpu/drm/amd/amdkfd/kfd_svm.h
> index 27fbe1936493..21f693767a0d 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_svm.h
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_svm.h
> @@ -150,6 +150,14 @@ static inline void svm_range_unlock(struct svm_range *prange)
>   	mutex_unlock(&prange->lock);
>   }
>   
> +static inline struct svm_range_bo *svm_range_bo_ref(struct svm_range_bo *svm_bo)
> +{
> +	if (svm_bo)
> +		kref_get(&svm_bo->kref);
> +
> +	return svm_bo;
> +}
> +
>   int svm_range_list_init(struct kfd_process *p);
>   void svm_range_list_fini(struct kfd_process *p);
>   int svm_ioctl(struct kfd_process *p, enum kfd_ioctl_svm_op op, uint64_t start,
> @@ -178,7 +186,7 @@ void svm_range_dma_unmap(struct device *dev, dma_addr_t *dma_addr,
>   void svm_range_free_dma_mappings(struct svm_range *prange);
>   void svm_range_prefault(struct svm_range *prange, struct mm_struct *mm,
>   			void *owner);
> -
> +void svm_range_bo_unref(struct svm_range_bo *svm_bo);
>   #else
>   
>   struct kfd_process;
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH 06/10] drm/amdkfd: skip invalid pages during migrations
  2021-05-27 20:55 Felix Kuehling
@ 2021-05-27 20:56   ` Felix Kuehling
  0 siblings, 0 replies; 21+ messages in thread
From: Felix Kuehling @ 2021-05-27 20:56 UTC (permalink / raw)
  To: amd-gfx, dri-devel; +Cc: Alex Sierra

From: Alex Sierra <alex.sierra@amd.com>

Invalid pages can be the result of pages that have been migrated
already due to copy-on-write procedure or pages that were never
migrated to VRAM in first place. This is not an issue anymore,
as pranges now support mixed memory domains (CPU/GPU).

Signed-off-by: Alex Sierra <alex.sierra@amd.com>
Reviewed-by: Felix Kuehling <Felix.Kuehling@amd.com>
---
 drivers/gpu/drm/amd/amdkfd/kfd_migrate.c | 38 +++++++++++-------------
 1 file changed, 18 insertions(+), 20 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c b/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c
index b298aa8dea4d..6fd68528c425 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c
@@ -419,7 +419,6 @@ svm_migrate_vma_to_vram(struct amdgpu_device *adev, struct svm_range *prange,
 	size_t size;
 	void *buf;
 	int r = -ENOMEM;
-	int retry = 0;
 
 	memset(&migrate, 0, sizeof(migrate));
 	migrate.vma = vma;
@@ -438,7 +437,6 @@ svm_migrate_vma_to_vram(struct amdgpu_device *adev, struct svm_range *prange,
 	migrate.dst = migrate.src + npages;
 	scratch = (dma_addr_t *)(migrate.dst + npages);
 
-retry:
 	r = migrate_vma_setup(&migrate);
 	if (r) {
 		pr_debug("failed %d prepare migrate svms 0x%p [0x%lx 0x%lx]\n",
@@ -446,17 +444,9 @@ svm_migrate_vma_to_vram(struct amdgpu_device *adev, struct svm_range *prange,
 		goto out_free;
 	}
 	if (migrate.cpages != npages) {
-		pr_debug("collect 0x%lx/0x%llx pages, retry\n", migrate.cpages,
+		pr_debug("Partial migration. 0x%lx/0x%llx pages can be migrated\n",
+			 migrate.cpages,
 			 npages);
-		migrate_vma_finalize(&migrate);
-		if (retry++ >= 3) {
-			r = -ENOMEM;
-			pr_debug("failed %d migrate svms 0x%p [0x%lx 0x%lx]\n",
-				 r, prange->svms, prange->start, prange->last);
-			goto out_free;
-		}
-
-		goto retry;
 	}
 
 	if (migrate.cpages) {
@@ -547,9 +537,8 @@ static void svm_migrate_page_free(struct page *page)
 static int
 svm_migrate_copy_to_ram(struct amdgpu_device *adev, struct svm_range *prange,
 			struct migrate_vma *migrate, struct dma_fence **mfence,
-			dma_addr_t *scratch)
+			dma_addr_t *scratch, uint64_t npages)
 {
-	uint64_t npages = migrate->cpages;
 	struct device *dev = adev->dev;
 	uint64_t *src;
 	dma_addr_t *dst;
@@ -566,15 +555,23 @@ svm_migrate_copy_to_ram(struct amdgpu_device *adev, struct svm_range *prange,
 	src = (uint64_t *)(scratch + npages);
 	dst = scratch;
 
-	for (i = 0, j = 0; i < npages; i++, j++, addr += PAGE_SIZE) {
+	for (i = 0, j = 0; i < npages; i++, addr += PAGE_SIZE) {
 		struct page *spage;
 
 		spage = migrate_pfn_to_page(migrate->src[i]);
-		if (!spage) {
-			pr_debug("failed get spage svms 0x%p [0x%lx 0x%lx]\n",
+		if (!spage || !is_zone_device_page(spage)) {
+			pr_debug("invalid page. Could be in CPU already svms 0x%p [0x%lx 0x%lx]\n",
 				 prange->svms, prange->start, prange->last);
-			r = -ENOMEM;
-			goto out_oom;
+			if (j) {
+				r = svm_migrate_copy_memory_gart(adev, dst + i - j,
+								 src + i - j, j,
+								 FROM_VRAM_TO_RAM,
+								 mfence);
+				if (r)
+					goto out_oom;
+				j = 0;
+			}
+			continue;
 		}
 		src[i] = svm_migrate_addr(adev, spage);
 		if (i > 0 && src[i] != src[i - 1] + PAGE_SIZE) {
@@ -607,6 +604,7 @@ svm_migrate_copy_to_ram(struct amdgpu_device *adev, struct svm_range *prange,
 
 		migrate->dst[i] = migrate_pfn(page_to_pfn(dpage));
 		migrate->dst[i] |= MIGRATE_PFN_LOCKED;
+		j++;
 	}
 
 	r = svm_migrate_copy_memory_gart(adev, dst + i - j, src + i - j, j,
@@ -664,7 +662,7 @@ svm_migrate_vma_to_ram(struct amdgpu_device *adev, struct svm_range *prange,
 
 	if (migrate.cpages) {
 		r = svm_migrate_copy_to_ram(adev, prange, &migrate, &mfence,
-					    scratch);
+					    scratch, npages);
 		migrate_vma_pages(&migrate);
 		svm_migrate_copy_done(adev, mfence);
 		migrate_vma_finalize(&migrate);
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH 06/10] drm/amdkfd: skip invalid pages during migrations
@ 2021-05-27 20:56   ` Felix Kuehling
  0 siblings, 0 replies; 21+ messages in thread
From: Felix Kuehling @ 2021-05-27 20:56 UTC (permalink / raw)
  To: amd-gfx, dri-devel; +Cc: Alex Sierra

From: Alex Sierra <alex.sierra@amd.com>

Invalid pages can be the result of pages that have been migrated
already due to copy-on-write procedure or pages that were never
migrated to VRAM in first place. This is not an issue anymore,
as pranges now support mixed memory domains (CPU/GPU).

Signed-off-by: Alex Sierra <alex.sierra@amd.com>
Reviewed-by: Felix Kuehling <Felix.Kuehling@amd.com>
---
 drivers/gpu/drm/amd/amdkfd/kfd_migrate.c | 38 +++++++++++-------------
 1 file changed, 18 insertions(+), 20 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c b/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c
index b298aa8dea4d..6fd68528c425 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c
@@ -419,7 +419,6 @@ svm_migrate_vma_to_vram(struct amdgpu_device *adev, struct svm_range *prange,
 	size_t size;
 	void *buf;
 	int r = -ENOMEM;
-	int retry = 0;
 
 	memset(&migrate, 0, sizeof(migrate));
 	migrate.vma = vma;
@@ -438,7 +437,6 @@ svm_migrate_vma_to_vram(struct amdgpu_device *adev, struct svm_range *prange,
 	migrate.dst = migrate.src + npages;
 	scratch = (dma_addr_t *)(migrate.dst + npages);
 
-retry:
 	r = migrate_vma_setup(&migrate);
 	if (r) {
 		pr_debug("failed %d prepare migrate svms 0x%p [0x%lx 0x%lx]\n",
@@ -446,17 +444,9 @@ svm_migrate_vma_to_vram(struct amdgpu_device *adev, struct svm_range *prange,
 		goto out_free;
 	}
 	if (migrate.cpages != npages) {
-		pr_debug("collect 0x%lx/0x%llx pages, retry\n", migrate.cpages,
+		pr_debug("Partial migration. 0x%lx/0x%llx pages can be migrated\n",
+			 migrate.cpages,
 			 npages);
-		migrate_vma_finalize(&migrate);
-		if (retry++ >= 3) {
-			r = -ENOMEM;
-			pr_debug("failed %d migrate svms 0x%p [0x%lx 0x%lx]\n",
-				 r, prange->svms, prange->start, prange->last);
-			goto out_free;
-		}
-
-		goto retry;
 	}
 
 	if (migrate.cpages) {
@@ -547,9 +537,8 @@ static void svm_migrate_page_free(struct page *page)
 static int
 svm_migrate_copy_to_ram(struct amdgpu_device *adev, struct svm_range *prange,
 			struct migrate_vma *migrate, struct dma_fence **mfence,
-			dma_addr_t *scratch)
+			dma_addr_t *scratch, uint64_t npages)
 {
-	uint64_t npages = migrate->cpages;
 	struct device *dev = adev->dev;
 	uint64_t *src;
 	dma_addr_t *dst;
@@ -566,15 +555,23 @@ svm_migrate_copy_to_ram(struct amdgpu_device *adev, struct svm_range *prange,
 	src = (uint64_t *)(scratch + npages);
 	dst = scratch;
 
-	for (i = 0, j = 0; i < npages; i++, j++, addr += PAGE_SIZE) {
+	for (i = 0, j = 0; i < npages; i++, addr += PAGE_SIZE) {
 		struct page *spage;
 
 		spage = migrate_pfn_to_page(migrate->src[i]);
-		if (!spage) {
-			pr_debug("failed get spage svms 0x%p [0x%lx 0x%lx]\n",
+		if (!spage || !is_zone_device_page(spage)) {
+			pr_debug("invalid page. Could be in CPU already svms 0x%p [0x%lx 0x%lx]\n",
 				 prange->svms, prange->start, prange->last);
-			r = -ENOMEM;
-			goto out_oom;
+			if (j) {
+				r = svm_migrate_copy_memory_gart(adev, dst + i - j,
+								 src + i - j, j,
+								 FROM_VRAM_TO_RAM,
+								 mfence);
+				if (r)
+					goto out_oom;
+				j = 0;
+			}
+			continue;
 		}
 		src[i] = svm_migrate_addr(adev, spage);
 		if (i > 0 && src[i] != src[i - 1] + PAGE_SIZE) {
@@ -607,6 +604,7 @@ svm_migrate_copy_to_ram(struct amdgpu_device *adev, struct svm_range *prange,
 
 		migrate->dst[i] = migrate_pfn(page_to_pfn(dpage));
 		migrate->dst[i] |= MIGRATE_PFN_LOCKED;
+		j++;
 	}
 
 	r = svm_migrate_copy_memory_gart(adev, dst + i - j, src + i - j, j,
@@ -664,7 +662,7 @@ svm_migrate_vma_to_ram(struct amdgpu_device *adev, struct svm_range *prange,
 
 	if (migrate.cpages) {
 		r = svm_migrate_copy_to_ram(adev, prange, &migrate, &mfence,
-					    scratch);
+					    scratch, npages);
 		migrate_vma_pages(&migrate);
 		svm_migrate_copy_done(adev, mfence);
 		migrate_vma_finalize(&migrate);
-- 
2.31.1

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply related	[flat|nested] 21+ messages in thread

end of thread, other threads:[~2021-06-21 21:46 UTC | newest]

Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-06-21 16:04 [PATCH 01/10] drm/amdkfd: device pgmap owner at the svm migrate init Alex Sierra
2021-06-21 16:04 ` [PATCH 02/10] drm/amdkfd: add owner ref param to get hmm pages Alex Sierra
2021-06-21 20:00   ` Felix Kuehling
2021-06-21 16:04 ` [PATCH 03/10] drm/amdkfd: set owner ref to svm range prefault Alex Sierra
2021-06-21 20:02   ` Felix Kuehling
2021-06-21 16:04 ` [PATCH 04/10] drm/amdgpu: get owner ref in validate and map Alex Sierra
2021-06-21 20:04   ` Felix Kuehling
2021-06-21 16:04 ` [PATCH 05/10] drm/amdkfd: classify and map mixed svm range pages in GPU Alex Sierra
2021-06-21 20:26   ` Felix Kuehling
2021-06-21 16:04 ` [PATCH 06/10] drm/amdkfd: skip invalid pages during migrations Alex Sierra
2021-06-21 16:04 ` [PATCH 07/10] drm/amdkfd: skip migration for pages already in VRAM Alex Sierra
2021-06-21 21:01   ` Felix Kuehling
2021-06-21 16:04 ` [PATCH 08/10] drm/amdkfd: add invalid pages debug at vram migration Alex Sierra
2021-06-21 21:02   ` Felix Kuehling
2021-06-21 16:04 ` [PATCH 09/10] drm/amdkfd: partially actual_loc removed Alex Sierra
2021-06-21 21:24   ` Felix Kuehling
2021-06-21 16:04 ` [PATCH 10/10] drm/amdkfd: protect svm_bo ref in case prange has forked Alex Sierra
2021-06-21 21:46   ` Felix Kuehling
2021-06-21 19:59 ` [PATCH 01/10] drm/amdkfd: device pgmap owner at the svm migrate init Felix Kuehling
  -- strict thread matches above, loose matches on Subject: below --
2021-05-27 20:55 Felix Kuehling
2021-05-27 20:56 ` [PATCH 06/10] drm/amdkfd: skip invalid pages during migrations Felix Kuehling
2021-05-27 20:56   ` Felix Kuehling

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.