[PATCH 1/2] drm/amdkfd: Back off SVM restore work retry

[PATCH 1/2] drm/amdkfd: Back off SVM restore work retry

[Philip Yang]

svm_range_restore_work retries every AMDGPU_SVM_RANGE_RESTORE_DELAY_MS,
1ms, as long as validation keeps failing. Each attempt takes
process_info->lock, the mmap write lock and svms->lock, so a restore
that cannot converge, for example while memory reclaim keeps
invalidating the ranges, starves everything waiting for the same locks.
svm_range_set_attr cannot add a new range until the retries stop.

Double the retry delay on each failed attempt, up to 128ms, and reset it
when a new eviction cycle starts. Retrying 1000 times per second does
not help a restore that is failing, and the backoff leaves gaps for
other users of these locks to make progress.

Signed-off-by: Philip Yang <Philip.Yang@amd.com>
---
 drivers/gpu/drm/amd/amdkfd/kfd_priv.h |  2 ++
 drivers/gpu/drm/amd/amdkfd/kfd_svm.c  | 16 ++++++++++++++--
 2 files changed, 16 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
index 4ba72c6069e5..8ed25a92cd89 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
@@ -1060,6 +1060,8 @@ struct svm_range_list {
 	atomic_t			evicted_ranges;
 	atomic_t			drain_pagefaults;
 	struct delayed_work		restore_work;
+	/* Retry delay of restore_work, doubled on each failed attempt */
+	u32				restore_delay_ms;
 	DECLARE_BITMAP(bitmap_supported, MAX_GPU_INSTANCE);
 	struct task_struct		*faulting_task;
 	/* check point ts decides if page fault recovery need be dropped */
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
index 40f5b2ee0cfd..28b687133cf8 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
@@ -46,6 +46,7 @@
 #define dev_fmt(fmt) "kfd_svm: %s: " fmt, __func__
 
 #define AMDGPU_SVM_RANGE_RESTORE_DELAY_MS 1
+#define AMDGPU_SVM_RANGE_RESTORE_DELAY_MAX_MS 128
 
 /* Long enough to ensure no retry fault comes after svm range is restored and
  * page table is updated.
@@ -1964,6 +1965,7 @@ static void svm_range_restore_work(struct work_struct *work)
 	struct mm_struct *mm;
 	int evicted_ranges;
 	int invalid;
+	u32 delay;
 	int r;
 
 	svms = container_of(dwork, struct svm_range_list, restore_work);
@@ -2034,15 +2036,23 @@ static void svm_range_restore_work(struct work_struct *work)
 	pr_debug("restore svm ranges successfully\n");
 
 out_reschedule:
+	/* Back off on repeated failures, retrying at the shortest delay keeps
+	 * retaking the same locks and starves everything else waiting for them.
+	 */
+	delay = svms->restore_delay_ms;
+	if (evicted_ranges)
+		svms->restore_delay_ms = min_t(u32, delay * 2,
+					       AMDGPU_SVM_RANGE_RESTORE_DELAY_MAX_MS);
+
 	mutex_unlock(&svms->lock);
 	mmap_write_unlock(mm);
 	mutex_unlock(&process_info->lock);
 
 	/* If validation failed, reschedule another attempt */
 	if (evicted_ranges) {
-		pr_debug("reschedule to restore svm range\n");
+		pr_debug("reschedule to restore svm range in %u ms\n", delay);
 		queue_delayed_work(system_freezable_wq, &svms->restore_work,
-			msecs_to_jiffies(AMDGPU_SVM_RANGE_RESTORE_DELAY_MS));
+				   msecs_to_jiffies(delay));
 
 		kfd_smi_event_queue_restore_rescheduled(mm);
 	}
@@ -2117,6 +2127,7 @@ svm_range_evict(struct svm_range *prange, struct mm_struct *mm,
 			pr_debug("failed to quiesce KFD\n");
 
 		pr_debug("schedule to restore svm %p ranges\n", svms);
+		svms->restore_delay_ms = AMDGPU_SVM_RANGE_RESTORE_DELAY_MS;
 		queue_delayed_work(system_freezable_wq, &svms->restore_work,
 			msecs_to_jiffies(AMDGPU_SVM_RANGE_RESTORE_DELAY_MS));
 	} else {
@@ -3420,6 +3431,7 @@ int svm_range_list_init(struct kfd_process *p)
 	INIT_LIST_HEAD(&svms->list);
 	atomic_set(&svms->evicted_ranges, 0);
 	atomic_set(&svms->drain_pagefaults, 0);
+	svms->restore_delay_ms = AMDGPU_SVM_RANGE_RESTORE_DELAY_MS;
 	INIT_DELAYED_WORK(&svms->restore_work, svm_range_restore_work);
 	INIT_WORK(&svms->deferred_list_work, svm_range_deferred_list_work);
 	INIT_LIST_HEAD(&svms->deferred_range_list);
-- 
2.50.1

[PATCH 2/2] drm/amdkfd: Downgrade mmap lock in SVM restore work

[Philip Yang]

svm_range_restore_work holds the mmap write lock across the entire loop
validating and mapping every invalid range, which blocks all mmap,
munmap and page fault activity of the process, not only the KFD paths
waiting for the same locks.

Only the deferred work flush handshake in
svm_range_list_lock_and_flush_work needs the write lock.
svm_range_validate_and_map runs under the read lock, as
svm_range_set_attr already does after its mmap_write_downgrade. The
flush stays valid because svm_range_deferred_list_work takes the write
lock, so it cannot run while the read lock is held.

Downgrade the mmap lock before walking the range list.

Signed-off-by: Philip Yang <Philip.Yang@amd.com>
---
 drivers/gpu/drm/amd/amdkfd/kfd_svm.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
index 28b687133cf8..f63bd8cf4174 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
@@ -1987,6 +1987,10 @@ static void svm_range_restore_work(struct work_struct *work)
 
 	mutex_lock(&process_info->lock);
 	svm_range_list_lock_and_flush_work(svms, mm);
+	/* Validate and map needs the read lock only, and deferred list work
+	 * takes the write lock, so the flush above stays valid.
+	 */
+	mmap_write_downgrade(mm);
 	mutex_lock(&svms->lock);
 
 	evicted_ranges = atomic_read(&svms->evicted_ranges);
@@ -2045,7 +2049,7 @@ static void svm_range_restore_work(struct work_struct *work)
 					       AMDGPU_SVM_RANGE_RESTORE_DELAY_MAX_MS);
 
 	mutex_unlock(&svms->lock);
-	mmap_write_unlock(mm);
+	mmap_read_unlock(mm);
 	mutex_unlock(&process_info->lock);
 
 	/* If validation failed, reschedule another attempt */
-- 
2.50.1

Re: [PATCH 1/2] drm/amdkfd: Back off SVM restore work retry

[Felix Kuehling]

On 2026-10-20 12:31, Philip Yang wrote:
> svm_range_restore_work retries every AMDGPU_SVM_RANGE_RESTORE_DELAY_MS,
> 1ms, as long as validation keeps failing. Each attempt takes
> process_info->lock, the mmap write lock and svms->lock, so a restore
> that cannot converge, for example while memory reclaim keeps
> invalidating the ranges, starves everything waiting for the same locks.
> svm_range_set_attr cannot add a new range until the retries stop.
>
> Double the retry delay on each failed attempt, up to 128ms, and reset it
> when a new eviction cycle starts. Retrying 1000 times per second does
> not help a restore that is failing, and the backoff leaves gaps for
> other users of these locks to make progress.
>
> Signed-off-by: Philip Yang <Philip.Yang@amd.com>
> ---
>   drivers/gpu/drm/amd/amdkfd/kfd_priv.h |  2 ++
>   drivers/gpu/drm/amd/amdkfd/kfd_svm.c  | 16 ++++++++++++++--
>   2 files changed, 16 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
> index 4ba72c6069e5..8ed25a92cd89 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
> @@ -1060,6 +1060,8 @@ struct svm_range_list {
>   	atomic_t			evicted_ranges;
>   	atomic_t			drain_pagefaults;
>   	struct delayed_work		restore_work;
> +	/* Retry delay of restore_work, doubled on each failed attempt */
> +	u32				restore_delay_ms;
>   	DECLARE_BITMAP(bitmap_supported, MAX_GPU_INSTANCE);
>   	struct task_struct		*faulting_task;
>   	/* check point ts decides if page fault recovery need be dropped */
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
> index 40f5b2ee0cfd..28b687133cf8 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
> @@ -46,6 +46,7 @@
>   #define dev_fmt(fmt) "kfd_svm: %s: " fmt, __func__
>   
>   #define AMDGPU_SVM_RANGE_RESTORE_DELAY_MS 1
> +#define AMDGPU_SVM_RANGE_RESTORE_DELAY_MAX_MS 128
>   
>   /* Long enough to ensure no retry fault comes after svm range is restored and
>    * page table is updated.
> @@ -1964,6 +1965,7 @@ static void svm_range_restore_work(struct work_struct *work)
>   	struct mm_struct *mm;
>   	int evicted_ranges;
>   	int invalid;
> +	u32 delay;
>   	int r;
>   
>   	svms = container_of(dwork, struct svm_range_list, restore_work);
> @@ -2034,15 +2036,23 @@ static void svm_range_restore_work(struct work_struct *work)
>   	pr_debug("restore svm ranges successfully\n");
>   
>   out_reschedule:
> +	/* Back off on repeated failures, retrying at the shortest delay keeps
> +	 * retaking the same locks and starves everything else waiting for them.
> +	 */
> +	delay = svms->restore_delay_ms;
> +	if (evicted_ranges)
> +		svms->restore_delay_ms = min_t(u32, delay * 2,
> +					       AMDGPU_SVM_RANGE_RESTORE_DELAY_MAX_MS);

Why not do this inside the "if (evicted_ranges)" block below?


> +
>   	mutex_unlock(&svms->lock);
>   	mmap_write_unlock(mm);
>   	mutex_unlock(&process_info->lock);
>   
>   	/* If validation failed, reschedule another attempt */
>   	if (evicted_ranges) {
> -		pr_debug("reschedule to restore svm range\n");
> +		pr_debug("reschedule to restore svm range in %u ms\n", delay);
>   		queue_delayed_work(system_freezable_wq, &svms->restore_work,
> -			msecs_to_jiffies(AMDGPU_SVM_RANGE_RESTORE_DELAY_MS));
> +				   msecs_to_jiffies(delay));
>   
>   		kfd_smi_event_queue_restore_rescheduled(mm);
>   	}
> @@ -2117,6 +2127,7 @@ svm_range_evict(struct svm_range *prange, struct mm_struct *mm,
>   			pr_debug("failed to quiesce KFD\n");
>   
>   		pr_debug("schedule to restore svm %p ranges\n", svms);
> +		svms->restore_delay_ms = AMDGPU_SVM_RANGE_RESTORE_DELAY_MS;

Is this the right place to reset the delay? You can have a lot of 
invalidations happening that cause failed restores. So that seems like 
the wrong place to reset the back-off that's supposed to solve that very 
problem. I would think the delay should be reset after a successful 
restore, not after another invalidation.

Regards,
   Felix


>   		queue_delayed_work(system_freezable_wq, &svms->restore_work,
>   			msecs_to_jiffies(AMDGPU_SVM_RANGE_RESTORE_DELAY_MS));
>   	} else {
> @@ -3420,6 +3431,7 @@ int svm_range_list_init(struct kfd_process *p)
>   	INIT_LIST_HEAD(&svms->list);
>   	atomic_set(&svms->evicted_ranges, 0);
>   	atomic_set(&svms->drain_pagefaults, 0);
> +	svms->restore_delay_ms = AMDGPU_SVM_RANGE_RESTORE_DELAY_MS;
>   	INIT_DELAYED_WORK(&svms->restore_work, svm_range_restore_work);
>   	INIT_WORK(&svms->deferred_list_work, svm_range_deferred_list_work);
>   	INIT_LIST_HEAD(&svms->deferred_range_list);

Re: [PATCH 2/2] drm/amdkfd: Downgrade mmap lock in SVM restore work

[Felix Kuehling]

On 2026-10-20 12:31, Philip Yang wrote:
> svm_range_restore_work holds the mmap write lock across the entire loop
> validating and mapping every invalid range, which blocks all mmap,
> munmap and page fault activity of the process, not only the KFD paths
> waiting for the same locks.
>
> Only the deferred work flush handshake in
> svm_range_list_lock_and_flush_work needs the write lock.
> svm_range_validate_and_map runs under the read lock, as
> svm_range_set_attr already does after its mmap_write_downgrade. The
> flush stays valid because svm_range_deferred_list_work takes the write
> lock, so it cannot run while the read lock is held.
>
> Downgrade the mmap lock before walking the range list.
>
> 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 | 6 +++++-
>   1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
> index 28b687133cf8..f63bd8cf4174 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
> @@ -1987,6 +1987,10 @@ static void svm_range_restore_work(struct work_struct *work)
>   
>   	mutex_lock(&process_info->lock);
>   	svm_range_list_lock_and_flush_work(svms, mm);
> +	/* Validate and map needs the read lock only, and deferred list work
> +	 * takes the write lock, so the flush above stays valid.
> +	 */
> +	mmap_write_downgrade(mm);
>   	mutex_lock(&svms->lock);
>   
>   	evicted_ranges = atomic_read(&svms->evicted_ranges);
> @@ -2045,7 +2049,7 @@ static void svm_range_restore_work(struct work_struct *work)
>   					       AMDGPU_SVM_RANGE_RESTORE_DELAY_MAX_MS);
>   
>   	mutex_unlock(&svms->lock);
> -	mmap_write_unlock(mm);
> +	mmap_read_unlock(mm);
>   	mutex_unlock(&process_info->lock);
>   
>   	/* If validation failed, reschedule another attempt */