On 2026-07-27 15:39, Philip Yang wrote:
Extract svm_range_update_checkpoint_timestamp() from
svm_range_unmap_from_cpu(). The next patch calls it when the app sets
the no-access attribute.
Change checkpoint_ts in svm_range_list from uint64_t to atomic64_t so
svm_range_restore_pages() can read it from the page fault handler
without holding the svms lock.
No functional change, preparation for the next patch.
Signed-off-by: Philip Yang <[email protected]>
---
drivers/gpu/drm/amd/amdkfd/kfd_priv.h | 2 +-
drivers/gpu/drm/amd/amdkfd/kfd_svm.c | 83 +++++++++++++++------------
2 files changed, 48 insertions(+), 37 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
index 88191a4c1657..bcb929002839 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
@@ -895,7 +895,7 @@ struct svm_range_list {
DECLARE_BITMAP(bitmap_supported, MAX_GPU_INSTANCE);
struct task_struct *faulting_task;
/* check point ts decides if page fault recovery need be dropped */
- uint64_t checkpoint_ts[MAX_GPU_INSTANCE];
+ atomic64_t checkpoint_ts[MAX_GPU_INSTANCE];
/* Default granularity to use in buffer migration
* and restoration of backing memory while handling
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
index 30ad10bbd47e..c10d4edc8813 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
@@ -759,6 +759,48 @@ svm_range_check_attr(struct kfd_process *p,
return 0;
}
+static void svm_range_update_checkpoint_timestamp(struct kfd_process *p)
+{
+ struct svm_range_list *svms;
+ int i;
+
+ svms = &p->svms;
+
+ /* calculate time stamps that are used to decide which page faults need
be
+ * dropped or handled before unmap pages from gpu vm
+ */
+ for_each_set_bit(i, svms->bitmap_supported, p->n_pdds) {
+ struct kfd_process_device *pdd;
+ struct amdgpu_device *adev;
+ struct amdgpu_ih_ring *ih;
+ uint32_t checkpoint_wptr;
+
+ pdd = p->pdds[i];
+ if (!pdd)
+ continue;
+
+ adev = pdd->dev->adev;
+
+ /* Check and drain ih1 ring if cam not available */
+ if (!adev->irq.retry_cam_enabled && adev->irq.ih1.ring_size) {
+ ih = &adev->irq.ih1;
+ checkpoint_wptr = amdgpu_ih_get_wptr(adev, ih);
+ if (ih->rptr != checkpoint_wptr) {
+ atomic64_set(&svms->checkpoint_ts[i],
+ amdgpu_ih_decode_iv_ts(adev, ih,
checkpoint_wptr, -1));
+ continue;
+ }
+ }
+
+ /* check if dev->irq.ih_soft is not empty */
+ ih = &adev->irq.ih_soft;
+ checkpoint_wptr = amdgpu_ih_get_wptr(adev, ih);
+ if (ih->rptr != checkpoint_wptr)
+ atomic64_set(&svms->checkpoint_ts[i],
+ amdgpu_ih_decode_iv_ts(adev, ih,
checkpoint_wptr, -1));
+ }
+}
+
static void
svm_range_apply_attrs(struct kfd_process *p, struct svm_range *prange,
uint32_t nattr, struct kfd_ioctl_svm_attribute *attrs,
@@ -2556,7 +2598,6 @@ svm_range_unmap_from_cpu(struct mm_struct *mm, struct
svm_range *prange,
struct kfd_process *p;
unsigned long s, l;
bool unmap_parent;
- uint32_t i;
if (atomic_read(&prange->queue_refcount)) {
int r;
@@ -2576,38 +2617,7 @@ svm_range_unmap_from_cpu(struct mm_struct *mm, struct
svm_range *prange,
pr_debug("svms 0x%p prange 0x%p [0x%lx 0x%lx] [0x%lx 0x%lx]\n", svms,
prange, prange->start, prange->last, start, last);
- /* calculate time stamps that are used to decide which page faults need be
- * dropped or handled before unmap pages from gpu vm
- */
- for_each_set_bit(i, svms->bitmap_supported, p->n_pdds) {
- struct kfd_process_device *pdd;
- struct amdgpu_device *adev;
- struct amdgpu_ih_ring *ih;
- uint32_t checkpoint_wptr;
-
- pdd = p->pdds[i];
- if (!pdd)
- continue;
-
- adev = pdd->dev->adev;
-
- /* Check and drain ih1 ring if cam not available */
- if (!adev->irq.retry_cam_enabled && adev->irq.ih1.ring_size) {
- ih = &adev->irq.ih1;
- checkpoint_wptr = amdgpu_ih_get_wptr(adev, ih);
- if (ih->rptr != checkpoint_wptr) {
- svms->checkpoint_ts[i] =
- amdgpu_ih_decode_iv_ts(adev, ih,
checkpoint_wptr, -1);
- continue;
- }
- }
-
- /* check if dev->irq.ih_soft is not empty */
- ih = &adev->irq.ih_soft;
- checkpoint_wptr = amdgpu_ih_get_wptr(adev, ih);
- if (ih->rptr != checkpoint_wptr)
- svms->checkpoint_ts[i] = amdgpu_ih_decode_iv_ts(adev,
ih, checkpoint_wptr, -1);
- }
+ svm_range_update_checkpoint_timestamp(p);
unmap_parent = start <= prange->start && last >= prange->last;
@@ -3117,8 +3127,9 @@ svm_range_restore_pages(struct amdgpu_device *adev, unsigned int pasid,
mutex_lock(&svms->lock);
/* check if this page fault time stamp is before svms->checkpoint_ts */
- if (svms->checkpoint_ts[gpuidx] != 0) {
- if (amdgpu_ih_ts_after_or_equal(ts,
svms->checkpoint_ts[gpuidx])) {
+ if (atomic64_read(&svms->checkpoint_ts[gpuidx]) != 0) {
+ if (amdgpu_ih_ts_after_or_equal(ts,
+ atomic64_read(&svms->checkpoint_ts[gpuidx]))) {
You lose atomicity here by reading the checkpoint twice. It would be
better to read it into a local variable once only and then using the
same value twice.
With that fixed, the patch is
Reviewed-by: Felix Kuehling <[email protected]>
pr_debug("draining retry fault, drop fault 0x%llx\n",
addr);
if (write_locked)
mmap_write_downgrade(mm);
@@ -3128,7 +3139,7 @@ svm_range_restore_pages(struct amdgpu_device *adev,
unsigned int pasid,
/* ts is after svms->checkpoint_ts now, reset
svms->checkpoint_ts
* to zero to avoid following ts wrap around give wrong
comparing
*/
- svms->checkpoint_ts[gpuidx] = 0;
+ atomic64_set(&svms->checkpoint_ts[gpuidx], 0);
}
}
