* Re: [PATCH] drm/amdkfd: AIP mGPUs best prefetch location for xnack on
2021-08-02 16:28 [PATCH] drm/amdkfd: AIP mGPUs best prefetch location for xnack on Philip Yang
@ 2021-08-03 21:06 ` Felix Kuehling
2021-08-09 22:21 ` [PATCH v2] " Philip Yang
2021-08-11 14:35 ` [PATCH v3] " Philip Yang
2 siblings, 0 replies; 6+ messages in thread
From: Felix Kuehling @ 2021-08-03 21:06 UTC (permalink / raw)
To: amd-gfx, Yang, Philip
Am 2021-08-02 um 12:28 p.m. schrieb Philip Yang:
> For xnack on, if range ACCESS or ACCESS_IN_PLACE (AIP) by single GPU, or
> range is ACCESS_IN_PLACE by mGPUs and all mGPUs connection on xgmi same
> hive, the best prefetch location is prefetch_loc GPU. Otherwise, the best
> prefetch location is always CPU because GPU can not map vram of other
> GPUs through small bar PCIe.
>
> Signed-off-by: Philip Yang <Philip.Yang@amd.com>
> ---
> drivers/gpu/drm/amd/amdkfd/kfd_svm.c | 44 +++++++++++++++++-----------
> 1 file changed, 27 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
> index 69237d2ab2ad..6160c301f78b 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
> @@ -2754,22 +2754,29 @@ svm_range_add(struct kfd_process *p, uint64_t start, uint64_t size,
> return 0;
> }
>
> -/* svm_range_best_prefetch_location - decide the best prefetch location
> +/**
> + * svm_range_best_prefetch_location - decide the best prefetch location
> * @prange: svm range structure
> *
> * For xnack off:
> - * If range map to single GPU, the best acutal location is prefetch loc, which
> + * If range map to single GPU, the best prefetch location is prefetch_loc, which
> * can be CPU or GPU.
> *
> - * If range map to multiple GPUs, only if mGPU connection on xgmi same hive,
> - * the best actual location could be prefetch_loc GPU. If mGPU connection on
> - * PCIe, the best actual location is always CPU, because GPU cannot access vram
> - * of other GPUs, assuming PCIe small bar (large bar support is not upstream).
> + * If range is ACCESS or ACCESS_IN_PLACE by mGPUs, only if mGPU connection on
> + * xgmi same hive, the best prefetch location is prefetch_loc GPU. If mGPUs
> + * connection on PCIe, the best prefetch location is always CPU, because GPU
> + * cannot map vram of other GPUs, assuming PCIe small bar (large bar support is
> + * not upstream).
> *
> * For xnack on:
> - * The best actual location is prefetch location. If mGPU connection on xgmi
> - * same hive, range map to multiple GPUs. Otherwise, the range only map to
> - * actual location GPU. Other GPU access vm fault will trigger migration.
> + * If range map to single GPU or is not ACCESS_IN_PLACE by mGPUs, the best
> + * prefetch location is prefetch_loc, other GPU access vm fault will trigger
> + * migration.
> + *
> + * If range is ACCESS_IN_PLACE by mGPUs, only if mGPU connection on xgmi same
> + * hive, the best prefetch location is prefetch_loc GPU. If mGPUs connection on
> + * PCIe, the best prefetch location is always CPU, because GPU cannot map vram
> + * of other GPUs.
> *
> * Context: Process context
> *
> @@ -2787,14 +2794,13 @@ svm_range_best_prefetch_location(struct svm_range *prange)
> struct kfd_process *p;
> uint32_t gpuidx;
>
> - p = container_of(prange->svms, struct kfd_process, svms);
> -
> - /* xnack on */
> - if (p->xnack_enabled)
> + if (!best_loc || best_loc == KFD_IOCTL_SVM_LOCATION_UNDEFINED)
> goto out;
>
> - /* xnack off */
> - if (!best_loc || best_loc == KFD_IOCTL_SVM_LOCATION_UNDEFINED)
> + p = container_of(prange->svms, struct kfd_process, svms);
> +
> + if (p->xnack_enabled &&
> + bitmap_weight(prange->bitmap_aip, MAX_GPU_INSTANCE) <= 1)
I'm not sure why this condition is needed. I think there is an implicit
assumption that the prefetch_location is the one GPU that's set in the
bitmap_aip. But is that really a safe assumption? If you're prefetching
to a different GPU than the one that needs to access the memory
in-place, then the access is still going to trigger a page fault, so
it's not "in-place".
I think you can just drop this condition. It's a questionable short-cut
for something that's handled more correctly in the loop below.
Regards,
Felix
> goto out;
>
> bo_adev = svm_range_get_adev_by_id(prange, best_loc);
> @@ -2803,8 +2809,12 @@ svm_range_best_prefetch_location(struct svm_range *prange)
> best_loc = 0;
> goto out;
> }
> - bitmap_or(bitmap, prange->bitmap_access, prange->bitmap_aip,
> - MAX_GPU_INSTANCE);
> +
> + if (p->xnack_enabled)
> + bitmap_copy(bitmap, prange->bitmap_aip, MAX_GPU_INSTANCE);
> + else
> + bitmap_or(bitmap, prange->bitmap_access, prange->bitmap_aip,
> + MAX_GPU_INSTANCE);
>
> for_each_set_bit(gpuidx, bitmap, MAX_GPU_INSTANCE) {
> pdd = kfd_process_device_from_gpuidx(p, gpuidx);
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH v2] drm/amdkfd: AIP mGPUs best prefetch location for xnack on
2021-08-02 16:28 [PATCH] drm/amdkfd: AIP mGPUs best prefetch location for xnack on Philip Yang
2021-08-03 21:06 ` Felix Kuehling
@ 2021-08-09 22:21 ` Philip Yang
2021-08-10 3:52 ` Felix Kuehling
2021-08-11 14:35 ` [PATCH v3] " Philip Yang
2 siblings, 1 reply; 6+ messages in thread
From: Philip Yang @ 2021-08-09 22:21 UTC (permalink / raw)
To: amd-gfx; +Cc: Philip Yang
For xnack on, if range ACCESS or ACCESS_IN_PLACE (AIP) by single GPU, or
range is ACCESS_IN_PLACE by mGPUs and all mGPUs connection on xgmi same
hive, the best prefetch location is prefetch_loc GPU. Otherwise, the best
prefetch location is always CPU because GPU can not map vram of other
GPUs through small bar PCIe.
Signed-off-by: Philip Yang <Philip.Yang@amd.com>
---
drivers/gpu/drm/amd/amdkfd/kfd_svm.c | 35 +++++++++++++++-------------
1 file changed, 19 insertions(+), 16 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
index f811a3a24cd2..5bd51a15fb00 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
@@ -2719,22 +2719,26 @@ svm_range_add(struct kfd_process *p, uint64_t start, uint64_t size,
return 0;
}
-/* svm_range_best_prefetch_location - decide the best prefetch location
+/**
+ * svm_range_best_prefetch_location - decide the best prefetch location
* @prange: svm range structure
*
* For xnack off:
- * If range map to single GPU, the best acutal location is prefetch loc, which
+ * If range map to single GPU, the best prefetch location is prefetch_loc, which
* can be CPU or GPU.
*
- * If range map to multiple GPUs, only if mGPU connection on xgmi same hive,
- * the best actual location could be prefetch_loc GPU. If mGPU connection on
- * PCIe, the best actual location is always CPU, because GPU cannot access vram
- * of other GPUs, assuming PCIe small bar (large bar support is not upstream).
+ * If range is ACCESS or ACCESS_IN_PLACE by mGPUs, only if mGPU connection on
+ * XGMI same hive, the best prefetch location is prefetch_loc GPU, othervise
+ * the best prefetch location is always CPU, because GPU can not map vram of
+ * other GPUs, assuming PCIe small bar (large bar support is not upstream).
*
* For xnack on:
- * The best actual location is prefetch location. If mGPU connection on xgmi
- * same hive, range map to multiple GPUs. Otherwise, the range only map to
- * actual location GPU. Other GPU access vm fault will trigger migration.
+ * If range is not ACCESS_IN_PLACE by mGPUs, the best prefetch location is
+ * prefetch_loc, other GPU access will generate vm fault and trigger migration.
+ *
+ * If range is ACCESS_IN_PLACE by mGPUs, only if mGPU connection on XGMI same
+ * hive, the best prefetch location is prefetch_loc GPU, otherwise the best
+ * prefetch location is always CPU, because GPU cannot map vram of other GPUs.
*
* Context: Process context
*
@@ -2754,11 +2758,6 @@ svm_range_best_prefetch_location(struct svm_range *prange)
p = container_of(prange->svms, struct kfd_process, svms);
- /* xnack on */
- if (p->xnack_enabled)
- goto out;
-
- /* xnack off */
if (!best_loc || best_loc == KFD_IOCTL_SVM_LOCATION_UNDEFINED)
goto out;
@@ -2768,8 +2767,12 @@ svm_range_best_prefetch_location(struct svm_range *prange)
best_loc = 0;
goto out;
}
- bitmap_or(bitmap, prange->bitmap_access, prange->bitmap_aip,
- MAX_GPU_INSTANCE);
+
+ if (p->xnack_enabled)
+ bitmap_copy(bitmap, prange->bitmap_aip, MAX_GPU_INSTANCE);
+ else
+ bitmap_or(bitmap, prange->bitmap_access, prange->bitmap_aip,
+ MAX_GPU_INSTANCE);
for_each_set_bit(gpuidx, bitmap, MAX_GPU_INSTANCE) {
pdd = kfd_process_device_from_gpuidx(p, gpuidx);
--
2.17.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH v2] drm/amdkfd: AIP mGPUs best prefetch location for xnack on
2021-08-09 22:21 ` [PATCH v2] " Philip Yang
@ 2021-08-10 3:52 ` Felix Kuehling
0 siblings, 0 replies; 6+ messages in thread
From: Felix Kuehling @ 2021-08-10 3:52 UTC (permalink / raw)
To: Philip Yang, amd-gfx
Am 2021-08-09 um 6:21 p.m. schrieb Philip Yang:
> For xnack on, if range ACCESS or ACCESS_IN_PLACE (AIP) by single GPU, or
> range is ACCESS_IN_PLACE by mGPUs and all mGPUs connection on xgmi same
> hive, the best prefetch location is prefetch_loc GPU. Otherwise, the best
> prefetch location is always CPU because GPU can not map vram of other
> GPUs through small bar PCIe.
I don't think small-bar is really a factor here. Even with large-BAR,
our P2P mappings are not coherent like XGMI mappings are. So we wouldn't
be able to use P2P even on large-BAR systems. So I would modify this
sentence:
> Otherwise, the best
> prefetch location is always CPU because GPU can not coherently map vram
> of other GPUs through PCIe.
>
> Signed-off-by: Philip Yang <Philip.Yang@amd.com>
> ---
> drivers/gpu/drm/amd/amdkfd/kfd_svm.c | 35 +++++++++++++++-------------
> 1 file changed, 19 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
> index f811a3a24cd2..5bd51a15fb00 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
> @@ -2719,22 +2719,26 @@ svm_range_add(struct kfd_process *p, uint64_t start, uint64_t size,
> return 0;
> }
>
> -/* svm_range_best_prefetch_location - decide the best prefetch location
> +/**
> + * svm_range_best_prefetch_location - decide the best prefetch location
> * @prange: svm range structure
> *
> * For xnack off:
> - * If range map to single GPU, the best acutal location is prefetch loc, which
> + * If range map to single GPU, the best prefetch location is prefetch_loc, which
> * can be CPU or GPU.
> *
> - * If range map to multiple GPUs, only if mGPU connection on xgmi same hive,
> - * the best actual location could be prefetch_loc GPU. If mGPU connection on
> - * PCIe, the best actual location is always CPU, because GPU cannot access vram
> - * of other GPUs, assuming PCIe small bar (large bar support is not upstream).
> + * If range is ACCESS or ACCESS_IN_PLACE by mGPUs, only if mGPU connection on
> + * XGMI same hive, the best prefetch location is prefetch_loc GPU, othervise
> + * the best prefetch location is always CPU, because GPU can not map vram of
> + * other GPUs, assuming PCIe small bar (large bar support is not upstream).
Same as above. With that fixed, the patch is
Reviewed-by: Felix Kuehling <Felix.Kuehling@amd.com>
> *
> * For xnack on:
> - * The best actual location is prefetch location. If mGPU connection on xgmi
> - * same hive, range map to multiple GPUs. Otherwise, the range only map to
> - * actual location GPU. Other GPU access vm fault will trigger migration.
> + * If range is not ACCESS_IN_PLACE by mGPUs, the best prefetch location is
> + * prefetch_loc, other GPU access will generate vm fault and trigger migration.
> + *
> + * If range is ACCESS_IN_PLACE by mGPUs, only if mGPU connection on XGMI same
> + * hive, the best prefetch location is prefetch_loc GPU, otherwise the best
> + * prefetch location is always CPU, because GPU cannot map vram of other GPUs.
> *
> * Context: Process context
> *
> @@ -2754,11 +2758,6 @@ svm_range_best_prefetch_location(struct svm_range *prange)
>
> p = container_of(prange->svms, struct kfd_process, svms);
>
> - /* xnack on */
> - if (p->xnack_enabled)
> - goto out;
> -
> - /* xnack off */
> if (!best_loc || best_loc == KFD_IOCTL_SVM_LOCATION_UNDEFINED)
> goto out;
>
> @@ -2768,8 +2767,12 @@ svm_range_best_prefetch_location(struct svm_range *prange)
> best_loc = 0;
> goto out;
> }
> - bitmap_or(bitmap, prange->bitmap_access, prange->bitmap_aip,
> - MAX_GPU_INSTANCE);
> +
> + if (p->xnack_enabled)
> + bitmap_copy(bitmap, prange->bitmap_aip, MAX_GPU_INSTANCE);
> + else
> + bitmap_or(bitmap, prange->bitmap_access, prange->bitmap_aip,
> + MAX_GPU_INSTANCE);
>
> for_each_set_bit(gpuidx, bitmap, MAX_GPU_INSTANCE) {
> pdd = kfd_process_device_from_gpuidx(p, gpuidx);
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v3] drm/amdkfd: AIP mGPUs best prefetch location for xnack on
2021-08-02 16:28 [PATCH] drm/amdkfd: AIP mGPUs best prefetch location for xnack on Philip Yang
2021-08-03 21:06 ` Felix Kuehling
2021-08-09 22:21 ` [PATCH v2] " Philip Yang
@ 2021-08-11 14:35 ` Philip Yang
2021-08-11 15:54 ` Felix Kuehling
2 siblings, 1 reply; 6+ messages in thread
From: Philip Yang @ 2021-08-11 14:35 UTC (permalink / raw)
To: amd-gfx; +Cc: Philip Yang
For xnack on, if range ACCESS or ACCESS_IN_PLACE (AIP) by single GPU, or
range is ACCESS_IN_PLACE by mGPUs and all mGPUs connection on XGMI same
hive, the best prefetch location is prefetch_loc GPU. Otherwise, the best
prefetch location is always CPU because GPU does not have coherent
mapping VRAM of other GPUs even with large-BAR PCIe connection.
Signed-off-by: Philip Yang <Philip.Yang@amd.com>
---
drivers/gpu/drm/amd/amdkfd/kfd_svm.c | 35 +++++++++++++++-------------
1 file changed, 19 insertions(+), 16 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
index f811a3a24cd2..4cdca8604d69 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
@@ -2719,22 +2719,26 @@ svm_range_add(struct kfd_process *p, uint64_t start, uint64_t size,
return 0;
}
-/* svm_range_best_prefetch_location - decide the best prefetch location
+/**
+ * svm_range_best_prefetch_location - decide the best prefetch location
* @prange: svm range structure
*
* For xnack off:
- * If range map to single GPU, the best acutal location is prefetch loc, which
+ * If range map to single GPU, the best prefetch location is prefetch_loc, which
* can be CPU or GPU.
*
- * If range map to multiple GPUs, only if mGPU connection on xgmi same hive,
- * the best actual location could be prefetch_loc GPU. If mGPU connection on
- * PCIe, the best actual location is always CPU, because GPU cannot access vram
- * of other GPUs, assuming PCIe small bar (large bar support is not upstream).
+ * If range is ACCESS or ACCESS_IN_PLACE by mGPUs, only if mGPU connection on
+ * XGMI same hive, the best prefetch location is prefetch_loc GPU, othervise
+ * the best prefetch location is always CPU, because GPU can not have coherent
+ * mapping VRAM of other GPUs even with large-BAR PCIe connection.
*
* For xnack on:
- * The best actual location is prefetch location. If mGPU connection on xgmi
- * same hive, range map to multiple GPUs. Otherwise, the range only map to
- * actual location GPU. Other GPU access vm fault will trigger migration.
+ * If range is not ACCESS_IN_PLACE by mGPUs, the best prefetch location is
+ * prefetch_loc, other GPU access will generate vm fault and trigger migration.
+ *
+ * If range is ACCESS_IN_PLACE by mGPUs, only if mGPU connection on XGMI same
+ * hive, the best prefetch location is prefetch_loc GPU, otherwise the best
+ * prefetch location is always CPU.
*
* Context: Process context
*
@@ -2754,11 +2758,6 @@ svm_range_best_prefetch_location(struct svm_range *prange)
p = container_of(prange->svms, struct kfd_process, svms);
- /* xnack on */
- if (p->xnack_enabled)
- goto out;
-
- /* xnack off */
if (!best_loc || best_loc == KFD_IOCTL_SVM_LOCATION_UNDEFINED)
goto out;
@@ -2768,8 +2767,12 @@ svm_range_best_prefetch_location(struct svm_range *prange)
best_loc = 0;
goto out;
}
- bitmap_or(bitmap, prange->bitmap_access, prange->bitmap_aip,
- MAX_GPU_INSTANCE);
+
+ if (p->xnack_enabled)
+ bitmap_copy(bitmap, prange->bitmap_aip, MAX_GPU_INSTANCE);
+ else
+ bitmap_or(bitmap, prange->bitmap_access, prange->bitmap_aip,
+ MAX_GPU_INSTANCE);
for_each_set_bit(gpuidx, bitmap, MAX_GPU_INSTANCE) {
pdd = kfd_process_device_from_gpuidx(p, gpuidx);
--
2.17.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH v3] drm/amdkfd: AIP mGPUs best prefetch location for xnack on
2021-08-11 14:35 ` [PATCH v3] " Philip Yang
@ 2021-08-11 15:54 ` Felix Kuehling
0 siblings, 0 replies; 6+ messages in thread
From: Felix Kuehling @ 2021-08-11 15:54 UTC (permalink / raw)
To: Philip Yang, amd-gfx
Am 2021-08-11 um 10:35 a.m. schrieb Philip Yang:
> For xnack on, if range ACCESS or ACCESS_IN_PLACE (AIP) by single GPU, or
> range is ACCESS_IN_PLACE by mGPUs and all mGPUs connection on XGMI same
> hive, the best prefetch location is prefetch_loc GPU. Otherwise, the best
> prefetch location is always CPU because GPU does not have coherent
> mapping VRAM of other GPUs even with large-BAR PCIe connection.
>
> Signed-off-by: Philip Yang <Philip.Yang@amd.com>
Reviewed-by: Felix Kuehling <Felix.Kuehling@amd.com>
> ---
> drivers/gpu/drm/amd/amdkfd/kfd_svm.c | 35 +++++++++++++++-------------
> 1 file changed, 19 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
> index f811a3a24cd2..4cdca8604d69 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
> @@ -2719,22 +2719,26 @@ svm_range_add(struct kfd_process *p, uint64_t start, uint64_t size,
> return 0;
> }
>
> -/* svm_range_best_prefetch_location - decide the best prefetch location
> +/**
> + * svm_range_best_prefetch_location - decide the best prefetch location
> * @prange: svm range structure
> *
> * For xnack off:
> - * If range map to single GPU, the best acutal location is prefetch loc, which
> + * If range map to single GPU, the best prefetch location is prefetch_loc, which
> * can be CPU or GPU.
> *
> - * If range map to multiple GPUs, only if mGPU connection on xgmi same hive,
> - * the best actual location could be prefetch_loc GPU. If mGPU connection on
> - * PCIe, the best actual location is always CPU, because GPU cannot access vram
> - * of other GPUs, assuming PCIe small bar (large bar support is not upstream).
> + * If range is ACCESS or ACCESS_IN_PLACE by mGPUs, only if mGPU connection on
> + * XGMI same hive, the best prefetch location is prefetch_loc GPU, othervise
> + * the best prefetch location is always CPU, because GPU can not have coherent
> + * mapping VRAM of other GPUs even with large-BAR PCIe connection.
> *
> * For xnack on:
> - * The best actual location is prefetch location. If mGPU connection on xgmi
> - * same hive, range map to multiple GPUs. Otherwise, the range only map to
> - * actual location GPU. Other GPU access vm fault will trigger migration.
> + * If range is not ACCESS_IN_PLACE by mGPUs, the best prefetch location is
> + * prefetch_loc, other GPU access will generate vm fault and trigger migration.
> + *
> + * If range is ACCESS_IN_PLACE by mGPUs, only if mGPU connection on XGMI same
> + * hive, the best prefetch location is prefetch_loc GPU, otherwise the best
> + * prefetch location is always CPU.
> *
> * Context: Process context
> *
> @@ -2754,11 +2758,6 @@ svm_range_best_prefetch_location(struct svm_range *prange)
>
> p = container_of(prange->svms, struct kfd_process, svms);
>
> - /* xnack on */
> - if (p->xnack_enabled)
> - goto out;
> -
> - /* xnack off */
> if (!best_loc || best_loc == KFD_IOCTL_SVM_LOCATION_UNDEFINED)
> goto out;
>
> @@ -2768,8 +2767,12 @@ svm_range_best_prefetch_location(struct svm_range *prange)
> best_loc = 0;
> goto out;
> }
> - bitmap_or(bitmap, prange->bitmap_access, prange->bitmap_aip,
> - MAX_GPU_INSTANCE);
> +
> + if (p->xnack_enabled)
> + bitmap_copy(bitmap, prange->bitmap_aip, MAX_GPU_INSTANCE);
> + else
> + bitmap_or(bitmap, prange->bitmap_access, prange->bitmap_aip,
> + MAX_GPU_INSTANCE);
>
> for_each_set_bit(gpuidx, bitmap, MAX_GPU_INSTANCE) {
> pdd = kfd_process_device_from_gpuidx(p, gpuidx);
^ permalink raw reply [flat|nested] 6+ messages in thread