[PATCH] drm/amdkfd: fix lost wakeup in interrupt drain on debugged process exit

[PATCH] drm/amdkfd: fix lost wakeup in interrupt drain on debugged process exit

[Heng Zhou]

When a debugged process exits, kfd_process_notifier_release_internal()
first removes it from kfd_processes_table and then calls
kfd_dbg_trap_disable(), which drains the process interrupts. The drain
sends a fence through the IH and waits, without a timeout, for
kfd_process_close_interrupt_drain() to wake it up. That wakeup looks
the process up by PASID in kfd_processes_table, which no longer holds
it, so the wakeup is lost and the exiting task sleeps forever. The
fatal signal has already been consumed in do_exit(), so the
interruptible wait cannot be broken either.

The wait happens inside the mmu_notifier release callback, i.e. within
the global mmu_notifier SRCU read-side critical section, so every
synchronize_srcu() on it stalls as well. Any other process releasing
its mm then hangs in D state and the system needs a reboot. This is
seen with rocgdb on MI300X.

Fix this by:

- Tracking pending drains in a list keyed by PASID instead of looking
  the process up in kfd_processes_table, so the drain fence always
  finds its waiter.
- Bounding the wait with a timeout, since the fence can be lost when
  the IH ring or the KFD interrupt FIFO overflows, and warning when the
  drain does not complete.
- Deferring the drain of an exiting debugged process from the
  mmu_notifier release path to kfd_process_wq_release(), before the
  PDDs and their PASIDs are released, so that stale interrupts cannot
  hit a process that reuses the PASID.

Signed-off-by: Heng Zhou <Heng.Zhou@amd.com>
---
 drivers/gpu/drm/amd/amdkfd/kfd_debug.c   |  3 +-
 drivers/gpu/drm/amd/amdkfd/kfd_priv.h    |  7 +-
 drivers/gpu/drm/amd/amdkfd/kfd_process.c | 89 +++++++++++++++++-------
 3 files changed, 68 insertions(+), 31 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_debug.c b/drivers/gpu/drm/amd/amdkfd/kfd_debug.c
index 0dd1fd448059..2b227d9de492 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_debug.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_debug.c
@@ -664,7 +664,8 @@ static void kfd_dbg_clean_exception_status(struct kfd_process *target)
 	for (i = 0; i < target->n_pdds; i++) {
 		struct kfd_process_device *pdd = target->pdds[i];
 
-		kfd_process_drain_interrupts(pdd);
+		if (!target->irq_drain_deferred)
+			kfd_process_drain_interrupts(pdd);
 
 		pdd->exception_status = 0;
 	}
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
index 2847a5ec5ede..c04c3a03f559 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
@@ -1030,9 +1030,7 @@ struct kfd_process {
 	uint64_t exception_enable_mask;
 	uint64_t exception_status;
 
-	/* Used to drain stale interrupts */
-	wait_queue_head_t wait_irq_drain;
-	bool irq_drain_is_open;
+	bool irq_drain_deferred;
 
 	/* shared virtual memory registered by this process */
 	struct svm_range_list svms;
@@ -1245,7 +1243,8 @@ bool enqueue_ih_ring_entry(struct kfd_node *kfd, const void *ih_ring_entry);
 bool interrupt_is_wanted(struct kfd_node *dev,
 				const uint32_t *ih_ring_entry,
 				uint32_t *patched_ihre, bool *flag);
-int kfd_process_drain_interrupts(struct kfd_process_device *pdd);
+void kfd_process_drain_interrupts(struct kfd_process_device *pdd);
+void kfd_process_drain_deferred_interrupts(struct kfd_process *p);
 void kfd_process_close_interrupt_drain(unsigned int pasid);
 
 /* amdkfd Apertures */
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_process.c b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
index 226f52626e84..b2365c21db9e 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_process.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
@@ -55,6 +55,22 @@ DEFINE_MUTEX(kfd_processes_mutex);
 
 DEFINE_SRCU(kfd_processes_srcu);
 
+/*
+ * Pending interrupt drains, keyed by PASID. Deliberately not keyed off
+ * kfd_processes_table: a draining process may already have been removed from
+ * it, and the drain fence must still be able to find its waiter.
+ */
+struct kfd_irq_drain_waiter {
+	struct list_head list;
+	u32 pasid;
+	struct completion done;
+};
+
+static DEFINE_SPINLOCK(kfd_irq_drain_lock);
+static LIST_HEAD(kfd_irq_drain_list);
+
+#define KFD_IRQ_DRAIN_TIMEOUT	msecs_to_jiffies(1000)
+
 /* For process termination handling */
 static struct workqueue_struct *kfd_process_wq;
 
@@ -1019,8 +1035,6 @@ struct kfd_process *kfd_create_process(struct task_struct *thread)
 		if (ret)
 			pr_warn("Failed to create debugfs entry for the kfd_process, ret = %d\n",
 				ret);
-
-		init_waitqueue_head(&process->wait_irq_drain);
 	}
 out:
 	mutex_unlock(&kfd_processes_mutex);
@@ -1344,6 +1358,8 @@ static void kfd_process_wq_release(struct work_struct *work)
 	kfd_process_free_outstanding_kfd_bos(p);
 	svm_range_list_fini(p);
 
+	kfd_process_drain_deferred_interrupts(p);
+
 	kfd_process_destroy_pdds(p);
 	dma_fence_put(ef);
 
@@ -1420,6 +1436,8 @@ void kfd_process_notifier_release_internal(struct kfd_process *p)
 	 * back on the MES list and, once proc_ctx_bo is freed, MES would fault
 	 * on the freed process context. Retiring the context last avoids this.
 	 */
+	if (p->debug_trap_enabled)
+		p->irq_drain_deferred = true;
 	kfd_dbg_trap_disable(p);
 
 	if (atomic_read(&p->debugged_process_count) > 0) {
@@ -2313,17 +2331,16 @@ int kfd_resume_all_processes(void)
 	return ret;
 }
 
-/* assumes caller holds process lock. */
-int kfd_process_drain_interrupts(struct kfd_process_device *pdd)
+void kfd_process_drain_interrupts(struct kfd_process_device *pdd)
 {
+	struct kfd_irq_drain_waiter waiter;
 	uint32_t irq_drain_fence[8];
+	unsigned long flags;
 	uint8_t node_id = 0;
-	int r = 0;
+	long r;
 
 	if (!KFD_IS_SOC15(pdd->dev))
-		return 0;
-
-	pdd->process->irq_drain_is_open = true;
+		return;
 
 	memset(irq_drain_fence, 0, sizeof(irq_drain_fence));
 	irq_drain_fence[0] = (KFD_IRQ_FENCE_SOURCEID << 8) |
@@ -2341,33 +2358,53 @@ int kfd_process_drain_interrupts(struct kfd_process_device *pdd)
 		irq_drain_fence[3] |= node_id << 16;
 	}
 
+	waiter.pasid = pdd->pasid;
+	init_completion(&waiter.done);
+
+	spin_lock_irqsave(&kfd_irq_drain_lock, flags);
+	list_add(&waiter.list, &kfd_irq_drain_list);
+	spin_unlock_irqrestore(&kfd_irq_drain_lock, flags);
+
 	/* ensure stale irqs scheduled KFD interrupts and send drain fence. */
-	if (amdgpu_amdkfd_send_close_event_drain_irq(pdd->dev->adev,
-						     irq_drain_fence)) {
-		pdd->process->irq_drain_is_open = false;
-		return 0;
+	if (!amdgpu_amdkfd_send_close_event_drain_irq(pdd->dev->adev,
+						      irq_drain_fence)) {
+		r = wait_for_completion_interruptible_timeout(&waiter.done,
+							      KFD_IRQ_DRAIN_TIMEOUT);
+		if (r <= 0)
+			dev_warn_ratelimited(pdd->dev->adev->dev,
+					     "irq drain on pasid 0x%x did not complete (%ld)\n",
+					     waiter.pasid, r);
 	}
 
-	r = wait_event_interruptible(pdd->process->wait_irq_drain,
-				     !READ_ONCE(pdd->process->irq_drain_is_open));
-	if (r)
-		pdd->process->irq_drain_is_open = false;
-
-	return r;
+	spin_lock_irqsave(&kfd_irq_drain_lock, flags);
+	list_del(&waiter.list);
+	spin_unlock_irqrestore(&kfd_irq_drain_lock, flags);
 }
 
-void kfd_process_close_interrupt_drain(unsigned int pasid)
+void kfd_process_drain_deferred_interrupts(struct kfd_process *p)
 {
-	struct kfd_process *p;
-
-	p = kfd_lookup_process_by_pasid(pasid, NULL);
+	int i;
 
-	if (!p)
+	if (!p->irq_drain_deferred)
 		return;
 
-	WRITE_ONCE(p->irq_drain_is_open, false);
-	wake_up_all(&p->wait_irq_drain);
-	kfd_unref_process(p);
+	p->irq_drain_deferred = false;
+
+	for (i = 0; i < p->n_pdds; i++)
+		kfd_process_drain_interrupts(p->pdds[i]);
+}
+
+void kfd_process_close_interrupt_drain(unsigned int pasid)
+{
+	struct kfd_irq_drain_waiter *waiter;
+	unsigned long flags;
+
+	spin_lock_irqsave(&kfd_irq_drain_lock, flags);
+	list_for_each_entry(waiter, &kfd_irq_drain_list, list) {
+		if (waiter->pasid == pasid)
+			complete(&waiter->done);
+	}
+	spin_unlock_irqrestore(&kfd_irq_drain_lock, flags);
 }
 
 struct send_exception_work_handler_workarea {
-- 
2.43.0

Re: [PATCH] drm/amdkfd: fix lost wakeup in interrupt drain on debugged process exit

[Kuehling, Felix]

[-- Attachment #1: Type: text/plain, Size: 9529 bytes --]

On 2026-09-30 09:42, Heng Zhou wrote:
> When a debugged process exits, kfd_process_notifier_release_internal()
> first removes it from kfd_processes_table and then calls
> kfd_dbg_trap_disable(), which drains the process interrupts. The drain
> sends a fence through the IH and waits, without a timeout, for
> kfd_process_close_interrupt_drain() to wake it up. That wakeup looks
> the process up by PASID in kfd_processes_table, which no longer holds
> it, so the wakeup is lost and the exiting task sleeps forever. The
> fatal signal has already been consumed in do_exit(), so the
> interruptible wait cannot be broken either.
>
> The wait happens inside the mmu_notifier release callback, i.e. within
> the global mmu_notifier SRCU read-side critical section, so every
> synchronize_srcu() on it stalls as well. Any other process releasing
> its mm then hangs in D state and the system needs a reboot. This is
> seen with rocgdb on MI300X.

I suspect this may have been introduced by this patch by Alex:

commit f34034ce5d2116f397008c63901c9c1946e5d665
Author: Alex Sierra<alex.sierra@amd.com>
Date:   Fri Jul 24 15:53:48 2026 -0500

     drm/amdkfd: disable debug before retiring MES process context on teardown

...

If you can confirm that, please add an appropriate Fixes: tag.


>
> Fix this by:
>
> - Tracking pending drains in a list keyed by PASID instead of looking
>    the process up in kfd_processes_table, so the drain fence always
>    finds its waiter.
> - Bounding the wait with a timeout, since the fence can be lost when
>    the IH ring or the KFD interrupt FIFO overflows, and warning when the
>    drain does not complete.
> - Deferring the drain of an exiting debugged process from the
>    mmu_notifier release path to kfd_process_wq_release(), before the
>    PDDs and their PASIDs are released, so that stale interrupts cannot
>    hit a process that reuses the PASID.
>
> Signed-off-by: Heng Zhou<Heng.Zhou@amd.com>
> ---
>   drivers/gpu/drm/amd/amdkfd/kfd_debug.c   |  3 +-
>   drivers/gpu/drm/amd/amdkfd/kfd_priv.h    |  7 +-
>   drivers/gpu/drm/amd/amdkfd/kfd_process.c | 89 +++++++++++++++++-------
>   3 files changed, 68 insertions(+), 31 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_debug.c b/drivers/gpu/drm/amd/amdkfd/kfd_debug.c
> index 0dd1fd448059..2b227d9de492 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_debug.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_debug.c
> @@ -664,7 +664,8 @@ static void kfd_dbg_clean_exception_status(struct kfd_process *target)
>   	for (i = 0; i < target->n_pdds; i++) {
>   		struct kfd_process_device *pdd = target->pdds[i];
>   
> -		kfd_process_drain_interrupts(pdd);
> +		if (!target->irq_drain_deferred)
> +			kfd_process_drain_interrupts(pdd);

The way you're setting and clearing p->irq_drain_deferred seems pretty 
fragile. I'd prefer to just pass this as a parameter to 
kfd_dbg_trap_disable and kfd_dbg_clean_exception_status. Then you also 
don't need two different drain functions.


>   
>   		pdd->exception_status = 0;
>   	}
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
> index 2847a5ec5ede..c04c3a03f559 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
> @@ -1030,9 +1030,7 @@ struct kfd_process {
>   	uint64_t exception_enable_mask;
>   	uint64_t exception_status;
>   
> -	/* Used to drain stale interrupts */
> -	wait_queue_head_t wait_irq_drain;
> -	bool irq_drain_is_open;
> +	bool irq_drain_deferred;
>   
>   	/* shared virtual memory registered by this process */
>   	struct svm_range_list svms;
> @@ -1245,7 +1243,8 @@ bool enqueue_ih_ring_entry(struct kfd_node *kfd, const void *ih_ring_entry);
>   bool interrupt_is_wanted(struct kfd_node *dev,
>   				const uint32_t *ih_ring_entry,
>   				uint32_t *patched_ihre, bool *flag);
> -int kfd_process_drain_interrupts(struct kfd_process_device *pdd);
> +void kfd_process_drain_interrupts(struct kfd_process_device *pdd);
> +void kfd_process_drain_deferred_interrupts(struct kfd_process *p);
>   void kfd_process_close_interrupt_drain(unsigned int pasid);
>   
>   /* amdkfd Apertures */
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_process.c b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
> index 226f52626e84..b2365c21db9e 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_process.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
> @@ -55,6 +55,22 @@ DEFINE_MUTEX(kfd_processes_mutex);
>   
>   DEFINE_SRCU(kfd_processes_srcu);
>   
> +/*
> + * Pending interrupt drains, keyed by PASID. Deliberately not keyed off
> + * kfd_processes_table: a draining process may already have been removed from
> + * it, and the drain fence must still be able to find its waiter.
> + */
> +struct kfd_irq_drain_waiter {
> +	struct list_head list;
> +	u32 pasid;
> +	struct completion done;
> +};
> +
> +static DEFINE_SPINLOCK(kfd_irq_drain_lock);
> +static LIST_HEAD(kfd_irq_drain_list);

If you use an XArray here, you have a more efficient lookup and you also 
get the spin-lock for free. In fact, that xarray already exists in 
amdgpu_ids.c: amdgpu_pasid_xa. It stores a pointer to the fpriv. You can 
get it with amdgpu_pasid_get_fpriv_locked. So you could just add the 
completion to struct amdgpu_fpriv. Maybe the whole IRQ draining logic 
should move to amdgpu_ids.c in that case, and KFD would just invoke it 
with a PASID.

Regards,
   Felix


> +
> +#define KFD_IRQ_DRAIN_TIMEOUT	msecs_to_jiffies(1000)
> +
>   /* For process termination handling */
>   static struct workqueue_struct *kfd_process_wq;
>   
> @@ -1019,8 +1035,6 @@ struct kfd_process *kfd_create_process(struct task_struct *thread)
>   		if (ret)
>   			pr_warn("Failed to create debugfs entry for the kfd_process, ret = %d\n",
>   				ret);
> -
> -		init_waitqueue_head(&process->wait_irq_drain);
>   	}
>   out:
>   	mutex_unlock(&kfd_processes_mutex);
> @@ -1344,6 +1358,8 @@ static void kfd_process_wq_release(struct work_struct *work)
>   	kfd_process_free_outstanding_kfd_bos(p);
>   	svm_range_list_fini(p);
>   
> +	kfd_process_drain_deferred_interrupts(p);
> +
>   	kfd_process_destroy_pdds(p);
>   	dma_fence_put(ef);
>   
> @@ -1420,6 +1436,8 @@ void kfd_process_notifier_release_internal(struct kfd_process *p)
>   	 * back on the MES list and, once proc_ctx_bo is freed, MES would fault
>   	 * on the freed process context. Retiring the context last avoids this.
>   	 */
> +	if (p->debug_trap_enabled)
> +		p->irq_drain_deferred = true;
>   	kfd_dbg_trap_disable(p);
>   
>   	if (atomic_read(&p->debugged_process_count) > 0) {
> @@ -2313,17 +2331,16 @@ int kfd_resume_all_processes(void)
>   	return ret;
>   }
>   
> -/* assumes caller holds process lock. */
> -int kfd_process_drain_interrupts(struct kfd_process_device *pdd)
> +void kfd_process_drain_interrupts(struct kfd_process_device *pdd)
>   {
> +	struct kfd_irq_drain_waiter waiter;
>   	uint32_t irq_drain_fence[8];
> +	unsigned long flags;
>   	uint8_t node_id = 0;
> -	int r = 0;
> +	long r;
>   
>   	if (!KFD_IS_SOC15(pdd->dev))
> -		return 0;
> -
> -	pdd->process->irq_drain_is_open = true;
> +		return;
>   
>   	memset(irq_drain_fence, 0, sizeof(irq_drain_fence));
>   	irq_drain_fence[0] = (KFD_IRQ_FENCE_SOURCEID << 8) |
> @@ -2341,33 +2358,53 @@ int kfd_process_drain_interrupts(struct kfd_process_device *pdd)
>   		irq_drain_fence[3] |= node_id << 16;
>   	}
>   
> +	waiter.pasid = pdd->pasid;
> +	init_completion(&waiter.done);
> +
> +	spin_lock_irqsave(&kfd_irq_drain_lock, flags);
> +	list_add(&waiter.list, &kfd_irq_drain_list);
> +	spin_unlock_irqrestore(&kfd_irq_drain_lock, flags);
> +
>   	/* ensure stale irqs scheduled KFD interrupts and send drain fence. */
> -	if (amdgpu_amdkfd_send_close_event_drain_irq(pdd->dev->adev,
> -						     irq_drain_fence)) {
> -		pdd->process->irq_drain_is_open = false;
> -		return 0;
> +	if (!amdgpu_amdkfd_send_close_event_drain_irq(pdd->dev->adev,
> +						      irq_drain_fence)) {
> +		r = wait_for_completion_interruptible_timeout(&waiter.done,
> +							      KFD_IRQ_DRAIN_TIMEOUT);
> +		if (r <= 0)
> +			dev_warn_ratelimited(pdd->dev->adev->dev,
> +					     "irq drain on pasid 0x%x did not complete (%ld)\n",
> +					     waiter.pasid, r);
>   	}
>   
> -	r = wait_event_interruptible(pdd->process->wait_irq_drain,
> -				     !READ_ONCE(pdd->process->irq_drain_is_open));
> -	if (r)
> -		pdd->process->irq_drain_is_open = false;
> -
> -	return r;
> +	spin_lock_irqsave(&kfd_irq_drain_lock, flags);
> +	list_del(&waiter.list);
> +	spin_unlock_irqrestore(&kfd_irq_drain_lock, flags);
>   }
>   
> -void kfd_process_close_interrupt_drain(unsigned int pasid)
> +void kfd_process_drain_deferred_interrupts(struct kfd_process *p)
>   {
> -	struct kfd_process *p;
> -
> -	p = kfd_lookup_process_by_pasid(pasid, NULL);
> +	int i;
>   
> -	if (!p)
> +	if (!p->irq_drain_deferred)
>   		return;
>   
> -	WRITE_ONCE(p->irq_drain_is_open, false);
> -	wake_up_all(&p->wait_irq_drain);
> -	kfd_unref_process(p);
> +	p->irq_drain_deferred = false;
> +
> +	for (i = 0; i < p->n_pdds; i++)
> +		kfd_process_drain_interrupts(p->pdds[i]);
> +}
> +
> +void kfd_process_close_interrupt_drain(unsigned int pasid)
> +{
> +	struct kfd_irq_drain_waiter *waiter;
> +	unsigned long flags;
> +
> +	spin_lock_irqsave(&kfd_irq_drain_lock, flags);
> +	list_for_each_entry(waiter, &kfd_irq_drain_list, list) {
> +		if (waiter->pasid == pasid)
> +			complete(&waiter->done);
> +	}
> +	spin_unlock_irqrestore(&kfd_irq_drain_lock, flags);
>   }
>   
>   struct send_exception_work_handler_workarea {

[-- Attachment #2: Type: text/html, Size: 10334 bytes --]

[PATCH v2] drm/amdkfd: fix lost wakeup in interrupt drain on debugged process exit

[Heng Zhou]

When a debugged process exits, kfd_process_notifier_release_internal()
first removes it from kfd_processes_table and then calls
kfd_dbg_trap_disable(), which drains the process interrupts. The drain
sends a fence through the IH and waits, without a timeout, for
kfd_process_close_interrupt_drain() to wake it up. That wakeup looks
the process up by PASID in kfd_processes_table, which no longer holds
it, so the wakeup is lost and the exiting task sleeps forever. The
fatal signal has already been consumed in do_exit(), so the
interruptible wait cannot be broken either.

The wait happens inside the mmu_notifier release callback, i.e. within
the global mmu_notifier SRCU read-side critical section, so every
synchronize_srcu() on it stalls as well. Any other process releasing
its mm then hangs in D state and the system needs a reboot. This is
seen with rocgdb on MI300X.

Fix this by:

- Waiting on a completion in struct amdgpu_fpriv, looked up by PASID
  through amdgpu_pasid_xa, instead of looking the process up in
  kfd_processes_table. The DRM file owning the PASID is still
  referenced by KFD while it drains, so the drain fence always finds
  its waiter. The drain logic moves to amdgpu_ids.c and KFD only
  passes the PASID.
- Bounding the wait with a timeout, since the fence can be lost when
  the IH ring or the KFD interrupt FIFO overflows, and warning when the
  drain does not complete.
- Passing drain_irqs to kfd_dbg_trap_disable() and skipping the drain
  when the process itself exits. Its interrupts are no longer delivered
  once it is out of kfd_processes_table, and its PASID owner is cleared
  before the PASID is freed and later reallocated cyclically.

v2: track drains with a completion in amdgpu_fpriv via amdgpu_pasid_xa
    and move the drain logic to amdgpu_ids.c; pass drain_irqs to
    kfd_dbg_trap_disable() and skip the drain on process exit instead
    of deferring it (Felix)

Fixes: 12fb1ad70d65 ("drm/amdkfd: update process interrupt handling for debug events")
Signed-off-by: Heng Zhou <Heng.Zhou@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu.h      |  3 ++
 drivers/gpu/drm/amd/amdgpu/amdgpu_ids.c  | 62 ++++++++++++++++++++++++
 drivers/gpu/drm/amd/amdgpu/amdgpu_ids.h  |  3 ++
 drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c  |  2 +
 drivers/gpu/drm/amd/amdkfd/kfd_chardev.c |  2 +-
 drivers/gpu/drm/amd/amdkfd/kfd_debug.c   | 10 ++--
 drivers/gpu/drm/amd/amdkfd/kfd_debug.h   |  2 +-
 drivers/gpu/drm/amd/amdkfd/kfd_priv.h    |  6 +--
 drivers/gpu/drm/amd/amdkfd/kfd_process.c | 46 ++++++------------
 9 files changed, 93 insertions(+), 43 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index ce671731b700..c32d3b0b4153 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -426,6 +426,9 @@ struct amdgpu_fpriv {
 
 	/** GPU partition selection */
 	uint32_t		xcp_id;
+
+	/** Signaled when a KFD interrupt drain fence for this PASID is seen */
+	struct completion	irq_drain_done;
 };
 
 int amdgpu_file_to_fpriv(struct file *filp, struct amdgpu_fpriv **fpriv);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ids.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ids.c
index 5e424e3b6e71..87cc297109a0 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ids.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ids.c
@@ -171,6 +171,68 @@ struct amdgpu_fpriv *amdgpu_pasid_get_fpriv_locked(u32 pasid)
 	return xa_load(&amdgpu_pasid_xa, pasid);
 }
 
+/**
+ * amdgpu_pasid_drain_irq - wait for pending KFD interrupts of a PASID
+ * @adev: amdgpu device the drain fence is sent to
+ * @pasid: PASID whose interrupts should be drained
+ * @ih_fence: IH entry injected as the drain fence
+ * @timeout: maximum time to wait, in jiffies
+ *
+ * Injects @ih_fence behind the interrupts already queued for KFD and waits
+ * until amdgpu_pasid_drain_irq_done() is called for @pasid.
+ *
+ * The caller must hold a reference to the DRM file that owns @pasid, so
+ * that its amdgpu_fpriv stays valid after the PASID lock is dropped.
+ *
+ * Returns:
+ * 0 on success or when no fence could be sent, -ENOENT if @pasid has no
+ * owner, -ETIMEDOUT on timeout or -ERESTARTSYS if interrupted.
+ */
+int amdgpu_pasid_drain_irq(struct amdgpu_device *adev, u32 pasid,
+			   u32 *ih_fence, unsigned long timeout)
+{
+	struct amdgpu_fpriv *fpriv;
+	unsigned long flags;
+	long r;
+
+	amdgpu_pasid_lock(&flags);
+	fpriv = amdgpu_pasid_get_fpriv_locked(pasid);
+	if (fpriv)
+		reinit_completion(&fpriv->irq_drain_done);
+	amdgpu_pasid_unlock(flags);
+	if (!fpriv)
+		return -ENOENT;
+
+	if (amdgpu_amdkfd_send_close_event_drain_irq(adev, ih_fence))
+		return 0;
+
+	r = wait_for_completion_interruptible_timeout(&fpriv->irq_drain_done,
+						      timeout);
+	if (!r)
+		return -ETIMEDOUT;
+
+	return r < 0 ? r : 0;
+}
+
+/**
+ * amdgpu_pasid_drain_irq_done - signal that a KFD interrupt drain completed
+ * @pasid: PASID carried by the drain fence
+ *
+ * Called by KFD when it processes the drain fence injected by
+ * amdgpu_pasid_drain_irq().
+ */
+void amdgpu_pasid_drain_irq_done(u32 pasid)
+{
+	struct amdgpu_fpriv *fpriv;
+	unsigned long flags;
+
+	amdgpu_pasid_lock(&flags);
+	fpriv = amdgpu_pasid_get_fpriv_locked(pasid);
+	if (fpriv)
+		complete(&fpriv->irq_drain_done);
+	amdgpu_pasid_unlock(flags);
+}
+
 /**
  * amdgpu_pasid_free_delayed - free pasid when fences signal
  *
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ids.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_ids.h
index 46b2c2160126..2af733d96d60 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ids.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ids.h
@@ -98,6 +98,9 @@ int amdgpu_pasid_alloc(unsigned int bits, struct amdgpu_fpriv *fpriv);
 void amdgpu_pasid_lock(unsigned long *flags);
 void amdgpu_pasid_unlock(unsigned long flags);
 struct amdgpu_fpriv *amdgpu_pasid_get_fpriv_locked(u32 pasid);
+int amdgpu_pasid_drain_irq(struct amdgpu_device *adev, u32 pasid,
+			   u32 *ih_fence, unsigned long timeout);
+void amdgpu_pasid_drain_irq_done(u32 pasid);
 void amdgpu_pasid_free(u32 pasid);
 void amdgpu_pasid_free_delayed(struct dma_resv *resv,
 			       u32 pasid);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
index bac072d73e16..8b080e2e1496 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
@@ -1507,6 +1507,8 @@ int amdgpu_driver_open_kms(struct drm_device *dev, struct drm_file *file_priv)
 	if (r)
 		goto error_pasid;
 
+	init_completion(&fpriv->irq_drain_done);
+
 	pasid = amdgpu_pasid_alloc(16, fpriv);
 	if (pasid < 0) {
 		dev_warn(adev->dev, "No more PASIDs available!");
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
index 1d86ddfd6bde..b3ebbee76088 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
@@ -3223,7 +3223,7 @@ static int kfd_ioctl_set_debug_trap(struct file *filep, struct kfd_process *p, v
 
 		break;
 	case KFD_IOC_DBG_TRAP_DISABLE:
-		r = kfd_dbg_trap_disable(target);
+		r = kfd_dbg_trap_disable(target, true);
 		break;
 	case KFD_IOC_DBG_TRAP_SEND_RUNTIME_EVENT:
 		r = kfd_dbg_send_exception_to_runtime(target,
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_debug.c b/drivers/gpu/drm/amd/amdkfd/kfd_debug.c
index 0dd1fd448059..8bed0d0f9bd2 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_debug.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_debug.c
@@ -655,7 +655,8 @@ void kfd_dbg_trap_deactivate(struct kfd_process *target, bool unwind, int unwind
 	kfd_dbg_set_workaround(target, false);
 }
 
-static void kfd_dbg_clean_exception_status(struct kfd_process *target)
+static void kfd_dbg_clean_exception_status(struct kfd_process *target,
+					   bool drain_irqs)
 {
 	struct process_queue_manager *pqm;
 	struct process_queue_node *pqn;
@@ -664,7 +665,8 @@ static void kfd_dbg_clean_exception_status(struct kfd_process *target)
 	for (i = 0; i < target->n_pdds; i++) {
 		struct kfd_process_device *pdd = target->pdds[i];
 
-		kfd_process_drain_interrupts(pdd);
+		if (drain_irqs)
+			kfd_process_drain_interrupts(pdd);
 
 		pdd->exception_status = 0;
 	}
@@ -680,7 +682,7 @@ static void kfd_dbg_clean_exception_status(struct kfd_process *target)
 	target->exception_status = 0;
 }
 
-int kfd_dbg_trap_disable(struct kfd_process *target)
+int kfd_dbg_trap_disable(struct kfd_process *target, bool drain_irqs)
 {
 	if (!target->debug_trap_enabled)
 		return 0;
@@ -704,7 +706,7 @@ int kfd_dbg_trap_disable(struct kfd_process *target)
 	}
 
 	target->debug_trap_enabled = false;
-	kfd_dbg_clean_exception_status(target);
+	kfd_dbg_clean_exception_status(target, drain_irqs);
 	kfd_unref_process(target);
 
 	return 0;
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_debug.h b/drivers/gpu/drm/amd/amdkfd/kfd_debug.h
index fbb751821c69..5babf9b992cd 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_debug.h
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_debug.h
@@ -43,7 +43,7 @@ bool kfd_dbg_ev_raise(uint64_t event_mask,
 			unsigned int source_id, bool use_worker,
 			void *exception_data,
 			size_t exception_data_size);
-int kfd_dbg_trap_disable(struct kfd_process *target);
+int kfd_dbg_trap_disable(struct kfd_process *target, bool drain_irqs);
 int kfd_dbg_trap_enable(struct kfd_process *target, uint32_t fd,
 			void __user *runtime_info,
 			uint32_t *runtime_info_size);
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
index 2847a5ec5ede..ef9c698bd5cb 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
@@ -1030,10 +1030,6 @@ struct kfd_process {
 	uint64_t exception_enable_mask;
 	uint64_t exception_status;
 
-	/* Used to drain stale interrupts */
-	wait_queue_head_t wait_irq_drain;
-	bool irq_drain_is_open;
-
 	/* shared virtual memory registered by this process */
 	struct svm_range_list svms;
 
@@ -1245,7 +1241,7 @@ bool enqueue_ih_ring_entry(struct kfd_node *kfd, const void *ih_ring_entry);
 bool interrupt_is_wanted(struct kfd_node *dev,
 				const uint32_t *ih_ring_entry,
 				uint32_t *patched_ihre, bool *flag);
-int kfd_process_drain_interrupts(struct kfd_process_device *pdd);
+void kfd_process_drain_interrupts(struct kfd_process_device *pdd);
 void kfd_process_close_interrupt_drain(unsigned int pasid);
 
 /* amdkfd Apertures */
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_process.c b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
index 226f52626e84..c1a01260463e 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_process.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
@@ -55,6 +55,8 @@ DEFINE_MUTEX(kfd_processes_mutex);
 
 DEFINE_SRCU(kfd_processes_srcu);
 
+#define KFD_IRQ_DRAIN_TIMEOUT	msecs_to_jiffies(1000)
+
 /* For process termination handling */
 static struct workqueue_struct *kfd_process_wq;
 
@@ -1019,8 +1021,6 @@ struct kfd_process *kfd_create_process(struct task_struct *thread)
 		if (ret)
 			pr_warn("Failed to create debugfs entry for the kfd_process, ret = %d\n",
 				ret);
-
-		init_waitqueue_head(&process->wait_irq_drain);
 	}
 out:
 	mutex_unlock(&kfd_processes_mutex);
@@ -1420,7 +1420,7 @@ void kfd_process_notifier_release_internal(struct kfd_process *p)
 	 * back on the MES list and, once proc_ctx_bo is freed, MES would fault
 	 * on the freed process context. Retiring the context last avoids this.
 	 */
-	kfd_dbg_trap_disable(p);
+	kfd_dbg_trap_disable(p, false);
 
 	if (atomic_read(&p->debugged_process_count) > 0) {
 		struct kfd_process *target;
@@ -1430,7 +1430,7 @@ void kfd_process_notifier_release_internal(struct kfd_process *p)
 		hash_for_each_rcu(kfd_processes_table, temp, target, kfd_processes) {
 			if (target->debugger_process && target->debugger_process == p) {
 				mutex_lock_nested(&target->mutex, 1);
-				kfd_dbg_trap_disable(target);
+				kfd_dbg_trap_disable(target, true);
 				mutex_unlock(&target->mutex);
 				if (atomic_read(&p->debugged_process_count) == 0)
 					break;
@@ -2313,17 +2313,14 @@ int kfd_resume_all_processes(void)
 	return ret;
 }
 
-/* assumes caller holds process lock. */
-int kfd_process_drain_interrupts(struct kfd_process_device *pdd)
+void kfd_process_drain_interrupts(struct kfd_process_device *pdd)
 {
 	uint32_t irq_drain_fence[8];
 	uint8_t node_id = 0;
-	int r = 0;
-
-	if (!KFD_IS_SOC15(pdd->dev))
-		return 0;
+	int r;
 
-	pdd->process->irq_drain_is_open = true;
+	if (!KFD_IS_SOC15(pdd->dev) || !pdd->drm_priv)
+		return;
 
 	memset(irq_drain_fence, 0, sizeof(irq_drain_fence));
 	irq_drain_fence[0] = (KFD_IRQ_FENCE_SOURCEID << 8) |
@@ -2342,32 +2339,17 @@ int kfd_process_drain_interrupts(struct kfd_process_device *pdd)
 	}
 
 	/* ensure stale irqs scheduled KFD interrupts and send drain fence. */
-	if (amdgpu_amdkfd_send_close_event_drain_irq(pdd->dev->adev,
-						     irq_drain_fence)) {
-		pdd->process->irq_drain_is_open = false;
-		return 0;
-	}
-
-	r = wait_event_interruptible(pdd->process->wait_irq_drain,
-				     !READ_ONCE(pdd->process->irq_drain_is_open));
+	r = amdgpu_pasid_drain_irq(pdd->dev->adev, pdd->pasid, irq_drain_fence,
+				   KFD_IRQ_DRAIN_TIMEOUT);
 	if (r)
-		pdd->process->irq_drain_is_open = false;
-
-	return r;
+		dev_warn_ratelimited(pdd->dev->adev->dev,
+				     "irq drain on pasid 0x%x did not complete (%d)\n",
+				     pdd->pasid, r);
 }
 
 void kfd_process_close_interrupt_drain(unsigned int pasid)
 {
-	struct kfd_process *p;
-
-	p = kfd_lookup_process_by_pasid(pasid, NULL);
-
-	if (!p)
-		return;
-
-	WRITE_ONCE(p->irq_drain_is_open, false);
-	wake_up_all(&p->wait_irq_drain);
-	kfd_unref_process(p);
+	amdgpu_pasid_drain_irq_done(pasid);
 }
 
 struct send_exception_work_handler_workarea {
-- 
2.43.0

Re: [PATCH v2] drm/amdkfd: fix lost wakeup in interrupt drain on debugged process exit

[Kuehling, Felix]

On 2026-10-08 10:06, Heng Zhou wrote:
> When a debugged process exits, kfd_process_notifier_release_internal()
> first removes it from kfd_processes_table and then calls
> kfd_dbg_trap_disable(), which drains the process interrupts. The drain
> sends a fence through the IH and waits, without a timeout, for
> kfd_process_close_interrupt_drain() to wake it up. That wakeup looks
> the process up by PASID in kfd_processes_table, which no longer holds
> it, so the wakeup is lost and the exiting task sleeps forever. The
> fatal signal has already been consumed in do_exit(), so the
> interruptible wait cannot be broken either.
>
> The wait happens inside the mmu_notifier release callback, i.e. within
> the global mmu_notifier SRCU read-side critical section, so every
> synchronize_srcu() on it stalls as well. Any other process releasing
> its mm then hangs in D state and the system needs a reboot. This is
> seen with rocgdb on MI300X.
>
> Fix this by:
>
> - Waiting on a completion in struct amdgpu_fpriv, looked up by PASID
>    through amdgpu_pasid_xa, instead of looking the process up in
>    kfd_processes_table. The DRM file owning the PASID is still
>    referenced by KFD while it drains, so the drain fence always finds
>    its waiter. The drain logic moves to amdgpu_ids.c and KFD only
>    passes the PASID.
> - Bounding the wait with a timeout, since the fence can be lost when
>    the IH ring or the KFD interrupt FIFO overflows, and warning when the
>    drain does not complete.
> - Passing drain_irqs to kfd_dbg_trap_disable() and skipping the drain
>    when the process itself exits. Its interrupts are no longer delivered
>    once it is out of kfd_processes_table, and its PASID owner is cleared
>    before the PASID is freed and later reallocated cyclically.
>
> v2: track drains with a completion in amdgpu_fpriv via amdgpu_pasid_xa
>      and move the drain logic to amdgpu_ids.c; pass drain_irqs to
>      kfd_dbg_trap_disable() and skip the drain on process exit instead
>      of deferring it (Felix)
>
> Fixes: 12fb1ad70d65 ("drm/amdkfd: update process interrupt handling for debug events")
> Signed-off-by: Heng Zhou <Heng.Zhou@amd.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu.h      |  3 ++
>   drivers/gpu/drm/amd/amdgpu/amdgpu_ids.c  | 62 ++++++++++++++++++++++++
>   drivers/gpu/drm/amd/amdgpu/amdgpu_ids.h  |  3 ++
>   drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c  |  2 +
>   drivers/gpu/drm/amd/amdkfd/kfd_chardev.c |  2 +-
>   drivers/gpu/drm/amd/amdkfd/kfd_debug.c   | 10 ++--
>   drivers/gpu/drm/amd/amdkfd/kfd_debug.h   |  2 +-
>   drivers/gpu/drm/amd/amdkfd/kfd_priv.h    |  6 +--
>   drivers/gpu/drm/amd/amdkfd/kfd_process.c | 46 ++++++------------
>   9 files changed, 93 insertions(+), 43 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> index ce671731b700..c32d3b0b4153 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> @@ -426,6 +426,9 @@ struct amdgpu_fpriv {
>   
>   	/** GPU partition selection */
>   	uint32_t		xcp_id;
> +
> +	/** Signaled when a KFD interrupt drain fence for this PASID is seen */
> +	struct completion	irq_drain_done;
>   };
>   
>   int amdgpu_file_to_fpriv(struct file *filp, struct amdgpu_fpriv **fpriv);
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ids.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ids.c
> index 5e424e3b6e71..87cc297109a0 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ids.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ids.c
> @@ -171,6 +171,68 @@ struct amdgpu_fpriv *amdgpu_pasid_get_fpriv_locked(u32 pasid)
>   	return xa_load(&amdgpu_pasid_xa, pasid);
>   }
>   
> +/**
> + * amdgpu_pasid_drain_irq - wait for pending KFD interrupts of a PASID
> + * @adev: amdgpu device the drain fence is sent to
> + * @pasid: PASID whose interrupts should be drained
> + * @ih_fence: IH entry injected as the drain fence
> + * @timeout: maximum time to wait, in jiffies
> + *
> + * Injects @ih_fence behind the interrupts already queued for KFD and waits
> + * until amdgpu_pasid_drain_irq_done() is called for @pasid.
> + *
> + * The caller must hold a reference to the DRM file that owns @pasid, so
> + * that its amdgpu_fpriv stays valid after the PASID lock is dropped.
> + *
> + * Returns:
> + * 0 on success or when no fence could be sent, -ENOENT if @pasid has no
> + * owner, -ETIMEDOUT on timeout or -ERESTARTSYS if interrupted.
> + */
> +int amdgpu_pasid_drain_irq(struct amdgpu_device *adev, u32 pasid,
> +			   u32 *ih_fence, unsigned long timeout)
> +{
> +	struct amdgpu_fpriv *fpriv;
> +	unsigned long flags;
> +	long r;
> +
> +	amdgpu_pasid_lock(&flags);
> +	fpriv = amdgpu_pasid_get_fpriv_locked(pasid);
> +	if (fpriv)
> +		reinit_completion(&fpriv->irq_drain_done);
> +	amdgpu_pasid_unlock(flags);
> +	if (!fpriv)
> +		return -ENOENT;
> +
> +	if (amdgpu_amdkfd_send_close_event_drain_irq(adev, ih_fence))
> +		return 0;
> +
> +	r = wait_for_completion_interruptible_timeout(&fpriv->irq_drain_done,
> +						      timeout);
> +	if (!r)
> +		return -ETIMEDOUT;
> +
> +	return r < 0 ? r : 0;
> +}
> +
> +/**
> + * amdgpu_pasid_drain_irq_done - signal that a KFD interrupt drain completed
> + * @pasid: PASID carried by the drain fence
> + *
> + * Called by KFD when it processes the drain fence injected by
> + * amdgpu_pasid_drain_irq().
> + */
> +void amdgpu_pasid_drain_irq_done(u32 pasid)
> +{
> +	struct amdgpu_fpriv *fpriv;
> +	unsigned long flags;
> +
> +	amdgpu_pasid_lock(&flags);
> +	fpriv = amdgpu_pasid_get_fpriv_locked(pasid);
> +	if (fpriv)
> +		complete(&fpriv->irq_drain_done);
> +	amdgpu_pasid_unlock(flags);
> +}
> +
>   /**
>    * amdgpu_pasid_free_delayed - free pasid when fences signal
>    *
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ids.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_ids.h
> index 46b2c2160126..2af733d96d60 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ids.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ids.h
> @@ -98,6 +98,9 @@ int amdgpu_pasid_alloc(unsigned int bits, struct amdgpu_fpriv *fpriv);
>   void amdgpu_pasid_lock(unsigned long *flags);
>   void amdgpu_pasid_unlock(unsigned long flags);
>   struct amdgpu_fpriv *amdgpu_pasid_get_fpriv_locked(u32 pasid);
> +int amdgpu_pasid_drain_irq(struct amdgpu_device *adev, u32 pasid,
> +			   u32 *ih_fence, unsigned long timeout);
> +void amdgpu_pasid_drain_irq_done(u32 pasid);
>   void amdgpu_pasid_free(u32 pasid);
>   void amdgpu_pasid_free_delayed(struct dma_resv *resv,
>   			       u32 pasid);
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> index bac072d73e16..8b080e2e1496 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> @@ -1507,6 +1507,8 @@ int amdgpu_driver_open_kms(struct drm_device *dev, struct drm_file *file_priv)
>   	if (r)
>   		goto error_pasid;
>   
> +	init_completion(&fpriv->irq_drain_done);
> +
>   	pasid = amdgpu_pasid_alloc(16, fpriv);
>   	if (pasid < 0) {
>   		dev_warn(adev->dev, "No more PASIDs available!");
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
> index 1d86ddfd6bde..b3ebbee76088 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
> @@ -3223,7 +3223,7 @@ static int kfd_ioctl_set_debug_trap(struct file *filep, struct kfd_process *p, v
>   
>   		break;
>   	case KFD_IOC_DBG_TRAP_DISABLE:
> -		r = kfd_dbg_trap_disable(target);
> +		r = kfd_dbg_trap_disable(target, true);
>   		break;
>   	case KFD_IOC_DBG_TRAP_SEND_RUNTIME_EVENT:
>   		r = kfd_dbg_send_exception_to_runtime(target,
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_debug.c b/drivers/gpu/drm/amd/amdkfd/kfd_debug.c
> index 0dd1fd448059..8bed0d0f9bd2 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_debug.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_debug.c
> @@ -655,7 +655,8 @@ void kfd_dbg_trap_deactivate(struct kfd_process *target, bool unwind, int unwind
>   	kfd_dbg_set_workaround(target, false);
>   }
>   
> -static void kfd_dbg_clean_exception_status(struct kfd_process *target)
> +static void kfd_dbg_clean_exception_status(struct kfd_process *target,
> +					   bool drain_irqs)
>   {
>   	struct process_queue_manager *pqm;
>   	struct process_queue_node *pqn;
> @@ -664,7 +665,8 @@ static void kfd_dbg_clean_exception_status(struct kfd_process *target)
>   	for (i = 0; i < target->n_pdds; i++) {
>   		struct kfd_process_device *pdd = target->pdds[i];
>   
> -		kfd_process_drain_interrupts(pdd);
> +		if (drain_irqs)
> +			kfd_process_drain_interrupts(pdd);
>   
>   		pdd->exception_status = 0;
>   	}
> @@ -680,7 +682,7 @@ static void kfd_dbg_clean_exception_status(struct kfd_process *target)
>   	target->exception_status = 0;
>   }
>   
> -int kfd_dbg_trap_disable(struct kfd_process *target)
> +int kfd_dbg_trap_disable(struct kfd_process *target, bool drain_irqs)
>   {
>   	if (!target->debug_trap_enabled)
>   		return 0;
> @@ -704,7 +706,7 @@ int kfd_dbg_trap_disable(struct kfd_process *target)
>   	}
>   
>   	target->debug_trap_enabled = false;
> -	kfd_dbg_clean_exception_status(target);
> +	kfd_dbg_clean_exception_status(target, drain_irqs);
>   	kfd_unref_process(target);
>   
>   	return 0;
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_debug.h b/drivers/gpu/drm/amd/amdkfd/kfd_debug.h
> index fbb751821c69..5babf9b992cd 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_debug.h
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_debug.h
> @@ -43,7 +43,7 @@ bool kfd_dbg_ev_raise(uint64_t event_mask,
>   			unsigned int source_id, bool use_worker,
>   			void *exception_data,
>   			size_t exception_data_size);
> -int kfd_dbg_trap_disable(struct kfd_process *target);
> +int kfd_dbg_trap_disable(struct kfd_process *target, bool drain_irqs);
>   int kfd_dbg_trap_enable(struct kfd_process *target, uint32_t fd,
>   			void __user *runtime_info,
>   			uint32_t *runtime_info_size);
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
> index 2847a5ec5ede..ef9c698bd5cb 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
> @@ -1030,10 +1030,6 @@ struct kfd_process {
>   	uint64_t exception_enable_mask;
>   	uint64_t exception_status;
>   
> -	/* Used to drain stale interrupts */
> -	wait_queue_head_t wait_irq_drain;
> -	bool irq_drain_is_open;
> -
>   	/* shared virtual memory registered by this process */
>   	struct svm_range_list svms;
>   
> @@ -1245,7 +1241,7 @@ bool enqueue_ih_ring_entry(struct kfd_node *kfd, const void *ih_ring_entry);
>   bool interrupt_is_wanted(struct kfd_node *dev,
>   				const uint32_t *ih_ring_entry,
>   				uint32_t *patched_ihre, bool *flag);
> -int kfd_process_drain_interrupts(struct kfd_process_device *pdd);
> +void kfd_process_drain_interrupts(struct kfd_process_device *pdd);
>   void kfd_process_close_interrupt_drain(unsigned int pasid);
>   
>   /* amdkfd Apertures */
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_process.c b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
> index 226f52626e84..c1a01260463e 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_process.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
> @@ -55,6 +55,8 @@ DEFINE_MUTEX(kfd_processes_mutex);
>   
>   DEFINE_SRCU(kfd_processes_srcu);
>   
> +#define KFD_IRQ_DRAIN_TIMEOUT	msecs_to_jiffies(1000)

Could we use AMDGPU_FENCE_JIFFIES_TIMEOUT here instead of inventing a 
new arbitrary timeout? If interrupts take longer than the fence-timeout 
to drain, surely fences will timeout, too.


> +
>   /* For process termination handling */
>   static struct workqueue_struct *kfd_process_wq;
>   
> @@ -1019,8 +1021,6 @@ struct kfd_process *kfd_create_process(struct task_struct *thread)
>   		if (ret)
>   			pr_warn("Failed to create debugfs entry for the kfd_process, ret = %d\n",
>   				ret);
> -
> -		init_waitqueue_head(&process->wait_irq_drain);
>   	}
>   out:
>   	mutex_unlock(&kfd_processes_mutex);
> @@ -1420,7 +1420,7 @@ void kfd_process_notifier_release_internal(struct kfd_process *p)
>   	 * back on the MES list and, once proc_ctx_bo is freed, MES would fault
>   	 * on the freed process context. Retiring the context last avoids this.
>   	 */
> -	kfd_dbg_trap_disable(p);
> +	kfd_dbg_trap_disable(p, false);
>   
>   	if (atomic_read(&p->debugged_process_count) > 0) {
>   		struct kfd_process *target;
> @@ -1430,7 +1430,7 @@ void kfd_process_notifier_release_internal(struct kfd_process *p)
>   		hash_for_each_rcu(kfd_processes_table, temp, target, kfd_processes) {
>   			if (target->debugger_process && target->debugger_process == p) {
>   				mutex_lock_nested(&target->mutex, 1);
> -				kfd_dbg_trap_disable(target);
> +				kfd_dbg_trap_disable(target, true);
>   				mutex_unlock(&target->mutex);
>   				if (atomic_read(&p->debugged_process_count) == 0)
>   					break;
> @@ -2313,17 +2313,14 @@ int kfd_resume_all_processes(void)
>   	return ret;
>   }
>   
> -/* assumes caller holds process lock. */
> -int kfd_process_drain_interrupts(struct kfd_process_device *pdd)
> +void kfd_process_drain_interrupts(struct kfd_process_device *pdd)
>   {
>   	uint32_t irq_drain_fence[8];
>   	uint8_t node_id = 0;
> -	int r = 0;
> -
> -	if (!KFD_IS_SOC15(pdd->dev))
> -		return 0;
> +	int r;
>   
> -	pdd->process->irq_drain_is_open = true;
> +	if (!KFD_IS_SOC15(pdd->dev) || !pdd->drm_priv)
> +		return;
>   
>   	memset(irq_drain_fence, 0, sizeof(irq_drain_fence));
>   	irq_drain_fence[0] = (KFD_IRQ_FENCE_SOURCEID << 8) |
> @@ -2342,32 +2339,17 @@ int kfd_process_drain_interrupts(struct kfd_process_device *pdd)
>   	}
>   
>   	/* ensure stale irqs scheduled KFD interrupts and send drain fence. */
> -	if (amdgpu_amdkfd_send_close_event_drain_irq(pdd->dev->adev,
> -						     irq_drain_fence)) {
> -		pdd->process->irq_drain_is_open = false;
> -		return 0;
> -	}
> -
> -	r = wait_event_interruptible(pdd->process->wait_irq_drain,
> -				     !READ_ONCE(pdd->process->irq_drain_is_open));
> +	r = amdgpu_pasid_drain_irq(pdd->dev->adev, pdd->pasid, irq_drain_fence,
> +				   KFD_IRQ_DRAIN_TIMEOUT);
>   	if (r)
> -		pdd->process->irq_drain_is_open = false;
> -
> -	return r;
> +		dev_warn_ratelimited(pdd->dev->adev->dev,
> +				     "irq drain on pasid 0x%x did not complete (%d)\n",
> +				     pdd->pasid, r);
>   }
>   
>   void kfd_process_close_interrupt_drain(unsigned int pasid)

Please get rid of this wrapper and call amdgpu_pasid_drain_irq_done 
directly from the interrupt handlers.

Thanks,
   Felix


>   {
> -	struct kfd_process *p;
> -
> -	p = kfd_lookup_process_by_pasid(pasid, NULL);
> -
> -	if (!p)
> -		return;
> -
> -	WRITE_ONCE(p->irq_drain_is_open, false);
> -	wake_up_all(&p->wait_irq_drain);
> -	kfd_unref_process(p);
> +	amdgpu_pasid_drain_irq_done(pasid);
>   }
>   
>   struct send_exception_work_handler_workarea {

[PATCH v3] drm/amdkfd: fix lost wakeup in interrupt drain on debugged process exit

[Heng Zhou]

When a debugged process exits, kfd_process_notifier_release_internal()
first removes it from kfd_processes_table and then calls
kfd_dbg_trap_disable(), which drains the process interrupts. The drain
sends a fence through the IH and waits, without a timeout, for
kfd_process_close_interrupt_drain() to wake it up. That wakeup looks
the process up by PASID in kfd_processes_table, which no longer holds
it, so the wakeup is lost and the exiting task sleeps forever. The
fatal signal has already been consumed in do_exit(), so the
interruptible wait cannot be broken either.

The wait happens inside the mmu_notifier release callback, i.e. within
the global mmu_notifier SRCU read-side critical section, so every
synchronize_srcu() on it stalls as well. Any other process releasing
its mm then hangs in D state and the system needs a reboot. This is
seen with rocgdb on MI300X.

Fix this by:

- Waiting on a completion in struct amdgpu_fpriv, looked up by PASID
  through amdgpu_pasid_xa, instead of looking the process up in
  kfd_processes_table. The DRM file owning the PASID is still
  referenced by KFD while it drains, so the drain fence always finds
  its waiter. The drain logic moves to amdgpu_ids.c and KFD only
  passes the PASID.
- Bounding the wait with a timeout, since the fence can be lost when
  the IH ring or the KFD interrupt FIFO overflows, and warning when the
  drain does not complete.
- Passing drain_irqs to kfd_dbg_trap_disable() and skipping the drain
  when the process itself exits. Its interrupts are no longer delivered
  once it is out of kfd_processes_table, and its PASID owner is cleared
  before the PASID is freed and later reallocated cyclically.

v2: track drains with a completion in amdgpu_fpriv via amdgpu_pasid_xa
    and move the drain logic to amdgpu_ids.c; pass drain_irqs to
    kfd_dbg_trap_disable() and skip the drain on process exit instead
    of deferring it (Felix)

v3: use AMDGPU_FENCE_JIFFIES_TIMEOUT for the drain timeout; drop
    kfd_process_close_interrupt_drain() and call
    amdgpu_pasid_drain_irq_done() directly from the interrupt handlers
    (Felix)

Fixes: 12fb1ad70d65 ("drm/amdkfd: update process interrupt handling for debug events")
Signed-off-by: Heng Zhou <Heng.Zhou@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu.h           |  3 +
 drivers/gpu/drm/amd/amdgpu/amdgpu_ids.c       | 62 +++++++++++++++++++
 drivers/gpu/drm/amd/amdgpu/amdgpu_ids.h       |  3 +
 drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c       |  2 +
 drivers/gpu/drm/amd/amdkfd/kfd_chardev.c      |  2 +-
 drivers/gpu/drm/amd/amdkfd/kfd_debug.c        | 10 +--
 drivers/gpu/drm/amd/amdkfd/kfd_debug.h        |  2 +-
 .../gpu/drm/amd/amdkfd/kfd_int_process_v10.c  |  2 +-
 .../gpu/drm/amd/amdkfd/kfd_int_process_v11.c  |  2 +-
 .../drm/amd/amdkfd/kfd_int_process_v12_1.c    |  2 +-
 .../gpu/drm/amd/amdkfd/kfd_int_process_v9.c   |  2 +-
 drivers/gpu/drm/amd/amdkfd/kfd_priv.h         |  8 +--
 drivers/gpu/drm/amd/amdkfd/kfd_process.c      | 47 ++++----------
 13 files changed, 94 insertions(+), 53 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index ce671731b700..c32d3b0b4153 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -426,6 +426,9 @@ struct amdgpu_fpriv {
 
 	/** GPU partition selection */
 	uint32_t		xcp_id;
+
+	/** Signaled when a KFD interrupt drain fence for this PASID is seen */
+	struct completion	irq_drain_done;
 };
 
 int amdgpu_file_to_fpriv(struct file *filp, struct amdgpu_fpriv **fpriv);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ids.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ids.c
index 5e424e3b6e71..87cc297109a0 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ids.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ids.c
@@ -171,6 +171,68 @@ struct amdgpu_fpriv *amdgpu_pasid_get_fpriv_locked(u32 pasid)
 	return xa_load(&amdgpu_pasid_xa, pasid);
 }
 
+/**
+ * amdgpu_pasid_drain_irq - wait for pending KFD interrupts of a PASID
+ * @adev: amdgpu device the drain fence is sent to
+ * @pasid: PASID whose interrupts should be drained
+ * @ih_fence: IH entry injected as the drain fence
+ * @timeout: maximum time to wait, in jiffies
+ *
+ * Injects @ih_fence behind the interrupts already queued for KFD and waits
+ * until amdgpu_pasid_drain_irq_done() is called for @pasid.
+ *
+ * The caller must hold a reference to the DRM file that owns @pasid, so
+ * that its amdgpu_fpriv stays valid after the PASID lock is dropped.
+ *
+ * Returns:
+ * 0 on success or when no fence could be sent, -ENOENT if @pasid has no
+ * owner, -ETIMEDOUT on timeout or -ERESTARTSYS if interrupted.
+ */
+int amdgpu_pasid_drain_irq(struct amdgpu_device *adev, u32 pasid,
+			   u32 *ih_fence, unsigned long timeout)
+{
+	struct amdgpu_fpriv *fpriv;
+	unsigned long flags;
+	long r;
+
+	amdgpu_pasid_lock(&flags);
+	fpriv = amdgpu_pasid_get_fpriv_locked(pasid);
+	if (fpriv)
+		reinit_completion(&fpriv->irq_drain_done);
+	amdgpu_pasid_unlock(flags);
+	if (!fpriv)
+		return -ENOENT;
+
+	if (amdgpu_amdkfd_send_close_event_drain_irq(adev, ih_fence))
+		return 0;
+
+	r = wait_for_completion_interruptible_timeout(&fpriv->irq_drain_done,
+						      timeout);
+	if (!r)
+		return -ETIMEDOUT;
+
+	return r < 0 ? r : 0;
+}
+
+/**
+ * amdgpu_pasid_drain_irq_done - signal that a KFD interrupt drain completed
+ * @pasid: PASID carried by the drain fence
+ *
+ * Called by KFD when it processes the drain fence injected by
+ * amdgpu_pasid_drain_irq().
+ */
+void amdgpu_pasid_drain_irq_done(u32 pasid)
+{
+	struct amdgpu_fpriv *fpriv;
+	unsigned long flags;
+
+	amdgpu_pasid_lock(&flags);
+	fpriv = amdgpu_pasid_get_fpriv_locked(pasid);
+	if (fpriv)
+		complete(&fpriv->irq_drain_done);
+	amdgpu_pasid_unlock(flags);
+}
+
 /**
  * amdgpu_pasid_free_delayed - free pasid when fences signal
  *
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ids.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_ids.h
index 46b2c2160126..2af733d96d60 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ids.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ids.h
@@ -98,6 +98,9 @@ int amdgpu_pasid_alloc(unsigned int bits, struct amdgpu_fpriv *fpriv);
 void amdgpu_pasid_lock(unsigned long *flags);
 void amdgpu_pasid_unlock(unsigned long flags);
 struct amdgpu_fpriv *amdgpu_pasid_get_fpriv_locked(u32 pasid);
+int amdgpu_pasid_drain_irq(struct amdgpu_device *adev, u32 pasid,
+			   u32 *ih_fence, unsigned long timeout);
+void amdgpu_pasid_drain_irq_done(u32 pasid);
 void amdgpu_pasid_free(u32 pasid);
 void amdgpu_pasid_free_delayed(struct dma_resv *resv,
 			       u32 pasid);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
index bac072d73e16..8b080e2e1496 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
@@ -1507,6 +1507,8 @@ int amdgpu_driver_open_kms(struct drm_device *dev, struct drm_file *file_priv)
 	if (r)
 		goto error_pasid;
 
+	init_completion(&fpriv->irq_drain_done);
+
 	pasid = amdgpu_pasid_alloc(16, fpriv);
 	if (pasid < 0) {
 		dev_warn(adev->dev, "No more PASIDs available!");
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
index 1d86ddfd6bde..b3ebbee76088 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
@@ -3223,7 +3223,7 @@ static int kfd_ioctl_set_debug_trap(struct file *filep, struct kfd_process *p, v
 
 		break;
 	case KFD_IOC_DBG_TRAP_DISABLE:
-		r = kfd_dbg_trap_disable(target);
+		r = kfd_dbg_trap_disable(target, true);
 		break;
 	case KFD_IOC_DBG_TRAP_SEND_RUNTIME_EVENT:
 		r = kfd_dbg_send_exception_to_runtime(target,
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_debug.c b/drivers/gpu/drm/amd/amdkfd/kfd_debug.c
index 0dd1fd448059..8bed0d0f9bd2 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_debug.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_debug.c
@@ -655,7 +655,8 @@ void kfd_dbg_trap_deactivate(struct kfd_process *target, bool unwind, int unwind
 	kfd_dbg_set_workaround(target, false);
 }
 
-static void kfd_dbg_clean_exception_status(struct kfd_process *target)
+static void kfd_dbg_clean_exception_status(struct kfd_process *target,
+					   bool drain_irqs)
 {
 	struct process_queue_manager *pqm;
 	struct process_queue_node *pqn;
@@ -664,7 +665,8 @@ static void kfd_dbg_clean_exception_status(struct kfd_process *target)
 	for (i = 0; i < target->n_pdds; i++) {
 		struct kfd_process_device *pdd = target->pdds[i];
 
-		kfd_process_drain_interrupts(pdd);
+		if (drain_irqs)
+			kfd_process_drain_interrupts(pdd);
 
 		pdd->exception_status = 0;
 	}
@@ -680,7 +682,7 @@ static void kfd_dbg_clean_exception_status(struct kfd_process *target)
 	target->exception_status = 0;
 }
 
-int kfd_dbg_trap_disable(struct kfd_process *target)
+int kfd_dbg_trap_disable(struct kfd_process *target, bool drain_irqs)
 {
 	if (!target->debug_trap_enabled)
 		return 0;
@@ -704,7 +706,7 @@ int kfd_dbg_trap_disable(struct kfd_process *target)
 	}
 
 	target->debug_trap_enabled = false;
-	kfd_dbg_clean_exception_status(target);
+	kfd_dbg_clean_exception_status(target, drain_irqs);
 	kfd_unref_process(target);
 
 	return 0;
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_debug.h b/drivers/gpu/drm/amd/amdkfd/kfd_debug.h
index fbb751821c69..5babf9b992cd 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_debug.h
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_debug.h
@@ -43,7 +43,7 @@ bool kfd_dbg_ev_raise(uint64_t event_mask,
 			unsigned int source_id, bool use_worker,
 			void *exception_data,
 			size_t exception_data_size);
-int kfd_dbg_trap_disable(struct kfd_process *target);
+int kfd_dbg_trap_disable(struct kfd_process *target, bool drain_irqs);
 int kfd_dbg_trap_enable(struct kfd_process *target, uint32_t fd,
 			void __user *runtime_info,
 			uint32_t *runtime_info_size);
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_int_process_v10.c b/drivers/gpu/drm/amd/amdkfd/kfd_int_process_v10.c
index 19406ab92c5b..45ac77bb2102 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_int_process_v10.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_int_process_v10.c
@@ -376,7 +376,7 @@ static void event_interrupt_wq_v10(struct kfd_node *dev,
 						&exception_data,
 						sizeof(exception_data));
 	} else if (KFD_IRQ_IS_FENCE(client_id, source_id)) {
-		kfd_process_close_interrupt_drain(pasid);
+		amdgpu_pasid_drain_irq_done(pasid);
 	}
 }
 
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_int_process_v11.c b/drivers/gpu/drm/amd/amdkfd/kfd_int_process_v11.c
index 12d81abed748..24d13a74a181 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_int_process_v11.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_int_process_v11.c
@@ -408,7 +408,7 @@ static void event_interrupt_wq_v11(struct kfd_node *dev,
 		}
 
 	} else if (KFD_IRQ_IS_FENCE(client_id, source_id)) {
-		kfd_process_close_interrupt_drain(pasid);
+		amdgpu_pasid_drain_irq_done(pasid);
 	}
 }
 
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_int_process_v12_1.c b/drivers/gpu/drm/amd/amdkfd/kfd_int_process_v12_1.c
index 0da7e1db55c9..4c7ad5aaaebc 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_int_process_v12_1.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_int_process_v12_1.c
@@ -395,7 +395,7 @@ static void event_interrupt_wq_v12_1(struct kfd_node *node,
 		}
 
 	} else if (KFD_IRQ_IS_FENCE(client_id, source_id)) {
-		kfd_process_close_interrupt_drain(pasid);
+		amdgpu_pasid_drain_irq_done(pasid);
 	}
 }
 
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_int_process_v9.c b/drivers/gpu/drm/amd/amdkfd/kfd_int_process_v9.c
index 2ae1129fea55..e241ad7d55df 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_int_process_v9.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_int_process_v9.c
@@ -596,7 +596,7 @@ static void event_interrupt_wq_v9(struct kfd_node *dev,
 						sizeof(exception_data));
 		kfd_smi_event_update_vmfault(dev, pasid);
 	} else if (KFD_IRQ_IS_FENCE(client_id, source_id)) {
-		kfd_process_close_interrupt_drain(pasid);
+		amdgpu_pasid_drain_irq_done(pasid);
 	}
 }
 
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
index 2847a5ec5ede..4777d6b7911c 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
@@ -1030,10 +1030,6 @@ struct kfd_process {
 	uint64_t exception_enable_mask;
 	uint64_t exception_status;
 
-	/* Used to drain stale interrupts */
-	wait_queue_head_t wait_irq_drain;
-	bool irq_drain_is_open;
-
 	/* shared virtual memory registered by this process */
 	struct svm_range_list svms;
 
@@ -1245,9 +1241,7 @@ bool enqueue_ih_ring_entry(struct kfd_node *kfd, const void *ih_ring_entry);
 bool interrupt_is_wanted(struct kfd_node *dev,
 				const uint32_t *ih_ring_entry,
 				uint32_t *patched_ihre, bool *flag);
-int kfd_process_drain_interrupts(struct kfd_process_device *pdd);
-void kfd_process_close_interrupt_drain(unsigned int pasid);
-
+void kfd_process_drain_interrupts(struct kfd_process_device *pdd);
 /* amdkfd Apertures */
 int kfd_init_apertures(struct kfd_process *process);
 
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_process.c b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
index 226f52626e84..e742ef206abd 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_process.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
@@ -1019,8 +1019,6 @@ struct kfd_process *kfd_create_process(struct task_struct *thread)
 		if (ret)
 			pr_warn("Failed to create debugfs entry for the kfd_process, ret = %d\n",
 				ret);
-
-		init_waitqueue_head(&process->wait_irq_drain);
 	}
 out:
 	mutex_unlock(&kfd_processes_mutex);
@@ -1420,7 +1418,7 @@ void kfd_process_notifier_release_internal(struct kfd_process *p)
 	 * back on the MES list and, once proc_ctx_bo is freed, MES would fault
 	 * on the freed process context. Retiring the context last avoids this.
 	 */
-	kfd_dbg_trap_disable(p);
+	kfd_dbg_trap_disable(p, false);
 
 	if (atomic_read(&p->debugged_process_count) > 0) {
 		struct kfd_process *target;
@@ -1430,7 +1428,7 @@ void kfd_process_notifier_release_internal(struct kfd_process *p)
 		hash_for_each_rcu(kfd_processes_table, temp, target, kfd_processes) {
 			if (target->debugger_process && target->debugger_process == p) {
 				mutex_lock_nested(&target->mutex, 1);
-				kfd_dbg_trap_disable(target);
+				kfd_dbg_trap_disable(target, true);
 				mutex_unlock(&target->mutex);
 				if (atomic_read(&p->debugged_process_count) == 0)
 					break;
@@ -2313,17 +2311,14 @@ int kfd_resume_all_processes(void)
 	return ret;
 }
 
-/* assumes caller holds process lock. */
-int kfd_process_drain_interrupts(struct kfd_process_device *pdd)
+void kfd_process_drain_interrupts(struct kfd_process_device *pdd)
 {
 	uint32_t irq_drain_fence[8];
 	uint8_t node_id = 0;
-	int r = 0;
+	int r;
 
-	if (!KFD_IS_SOC15(pdd->dev))
-		return 0;
-
-	pdd->process->irq_drain_is_open = true;
+	if (!KFD_IS_SOC15(pdd->dev) || !pdd->drm_priv)
+		return;
 
 	memset(irq_drain_fence, 0, sizeof(irq_drain_fence));
 	irq_drain_fence[0] = (KFD_IRQ_FENCE_SOURCEID << 8) |
@@ -2342,32 +2337,12 @@ int kfd_process_drain_interrupts(struct kfd_process_device *pdd)
 	}
 
 	/* ensure stale irqs scheduled KFD interrupts and send drain fence. */
-	if (amdgpu_amdkfd_send_close_event_drain_irq(pdd->dev->adev,
-						     irq_drain_fence)) {
-		pdd->process->irq_drain_is_open = false;
-		return 0;
-	}
-
-	r = wait_event_interruptible(pdd->process->wait_irq_drain,
-				     !READ_ONCE(pdd->process->irq_drain_is_open));
+	r = amdgpu_pasid_drain_irq(pdd->dev->adev, pdd->pasid, irq_drain_fence,
+				   AMDGPU_FENCE_JIFFIES_TIMEOUT);
 	if (r)
-		pdd->process->irq_drain_is_open = false;
-
-	return r;
-}
-
-void kfd_process_close_interrupt_drain(unsigned int pasid)
-{
-	struct kfd_process *p;
-
-	p = kfd_lookup_process_by_pasid(pasid, NULL);
-
-	if (!p)
-		return;
-
-	WRITE_ONCE(p->irq_drain_is_open, false);
-	wake_up_all(&p->wait_irq_drain);
-	kfd_unref_process(p);
+		dev_warn_ratelimited(pdd->dev->adev->dev,
+				     "irq drain on pasid 0x%x did not complete (%d)\n",
+				     pdd->pasid, r);
 }
 
 struct send_exception_work_handler_workarea {
-- 
2.43.0

Re: [PATCH v3] drm/amdkfd: fix lost wakeup in interrupt drain on debugged process exit

[Kuehling, Felix]

On 2026-10-09 02:45, Heng Zhou wrote:
> When a debugged process exits, kfd_process_notifier_release_internal()
> first removes it from kfd_processes_table and then calls
> kfd_dbg_trap_disable(), which drains the process interrupts. The drain
> sends a fence through the IH and waits, without a timeout, for
> kfd_process_close_interrupt_drain() to wake it up. That wakeup looks
> the process up by PASID in kfd_processes_table, which no longer holds
> it, so the wakeup is lost and the exiting task sleeps forever. The
> fatal signal has already been consumed in do_exit(), so the
> interruptible wait cannot be broken either.
>
> The wait happens inside the mmu_notifier release callback, i.e. within
> the global mmu_notifier SRCU read-side critical section, so every
> synchronize_srcu() on it stalls as well. Any other process releasing
> its mm then hangs in D state and the system needs a reboot. This is
> seen with rocgdb on MI300X.
>
> Fix this by:
>
> - Waiting on a completion in struct amdgpu_fpriv, looked up by PASID
>    through amdgpu_pasid_xa, instead of looking the process up in
>    kfd_processes_table. The DRM file owning the PASID is still
>    referenced by KFD while it drains, so the drain fence always finds
>    its waiter. The drain logic moves to amdgpu_ids.c and KFD only
>    passes the PASID.
> - Bounding the wait with a timeout, since the fence can be lost when
>    the IH ring or the KFD interrupt FIFO overflows, and warning when the
>    drain does not complete.
> - Passing drain_irqs to kfd_dbg_trap_disable() and skipping the drain
>    when the process itself exits. Its interrupts are no longer delivered
>    once it is out of kfd_processes_table, and its PASID owner is cleared
>    before the PASID is freed and later reallocated cyclically.
>
> v2: track drains with a completion in amdgpu_fpriv via amdgpu_pasid_xa
>      and move the drain logic to amdgpu_ids.c; pass drain_irqs to
>      kfd_dbg_trap_disable() and skip the drain on process exit instead
>      of deferring it (Felix)
>
> v3: use AMDGPU_FENCE_JIFFIES_TIMEOUT for the drain timeout; drop
>      kfd_process_close_interrupt_drain() and call
>      amdgpu_pasid_drain_irq_done() directly from the interrupt handlers
>      (Felix)
>
> Fixes: 12fb1ad70d65 ("drm/amdkfd: update process interrupt handling for debug events")
> Signed-off-by: Heng Zhou <Heng.Zhou@amd.com>

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


> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu.h           |  3 +
>   drivers/gpu/drm/amd/amdgpu/amdgpu_ids.c       | 62 +++++++++++++++++++
>   drivers/gpu/drm/amd/amdgpu/amdgpu_ids.h       |  3 +
>   drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c       |  2 +
>   drivers/gpu/drm/amd/amdkfd/kfd_chardev.c      |  2 +-
>   drivers/gpu/drm/amd/amdkfd/kfd_debug.c        | 10 +--
>   drivers/gpu/drm/amd/amdkfd/kfd_debug.h        |  2 +-
>   .../gpu/drm/amd/amdkfd/kfd_int_process_v10.c  |  2 +-
>   .../gpu/drm/amd/amdkfd/kfd_int_process_v11.c  |  2 +-
>   .../drm/amd/amdkfd/kfd_int_process_v12_1.c    |  2 +-
>   .../gpu/drm/amd/amdkfd/kfd_int_process_v9.c   |  2 +-
>   drivers/gpu/drm/amd/amdkfd/kfd_priv.h         |  8 +--
>   drivers/gpu/drm/amd/amdkfd/kfd_process.c      | 47 ++++----------
>   13 files changed, 94 insertions(+), 53 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> index ce671731b700..c32d3b0b4153 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> @@ -426,6 +426,9 @@ struct amdgpu_fpriv {
>   
>   	/** GPU partition selection */
>   	uint32_t		xcp_id;
> +
> +	/** Signaled when a KFD interrupt drain fence for this PASID is seen */
> +	struct completion	irq_drain_done;
>   };
>   
>   int amdgpu_file_to_fpriv(struct file *filp, struct amdgpu_fpriv **fpriv);
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ids.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ids.c
> index 5e424e3b6e71..87cc297109a0 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ids.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ids.c
> @@ -171,6 +171,68 @@ struct amdgpu_fpriv *amdgpu_pasid_get_fpriv_locked(u32 pasid)
>   	return xa_load(&amdgpu_pasid_xa, pasid);
>   }
>   
> +/**
> + * amdgpu_pasid_drain_irq - wait for pending KFD interrupts of a PASID
> + * @adev: amdgpu device the drain fence is sent to
> + * @pasid: PASID whose interrupts should be drained
> + * @ih_fence: IH entry injected as the drain fence
> + * @timeout: maximum time to wait, in jiffies
> + *
> + * Injects @ih_fence behind the interrupts already queued for KFD and waits
> + * until amdgpu_pasid_drain_irq_done() is called for @pasid.
> + *
> + * The caller must hold a reference to the DRM file that owns @pasid, so
> + * that its amdgpu_fpriv stays valid after the PASID lock is dropped.
> + *
> + * Returns:
> + * 0 on success or when no fence could be sent, -ENOENT if @pasid has no
> + * owner, -ETIMEDOUT on timeout or -ERESTARTSYS if interrupted.
> + */
> +int amdgpu_pasid_drain_irq(struct amdgpu_device *adev, u32 pasid,
> +			   u32 *ih_fence, unsigned long timeout)
> +{
> +	struct amdgpu_fpriv *fpriv;
> +	unsigned long flags;
> +	long r;
> +
> +	amdgpu_pasid_lock(&flags);
> +	fpriv = amdgpu_pasid_get_fpriv_locked(pasid);
> +	if (fpriv)
> +		reinit_completion(&fpriv->irq_drain_done);
> +	amdgpu_pasid_unlock(flags);
> +	if (!fpriv)
> +		return -ENOENT;
> +
> +	if (amdgpu_amdkfd_send_close_event_drain_irq(adev, ih_fence))
> +		return 0;
> +
> +	r = wait_for_completion_interruptible_timeout(&fpriv->irq_drain_done,
> +						      timeout);
> +	if (!r)
> +		return -ETIMEDOUT;
> +
> +	return r < 0 ? r : 0;
> +}
> +
> +/**
> + * amdgpu_pasid_drain_irq_done - signal that a KFD interrupt drain completed
> + * @pasid: PASID carried by the drain fence
> + *
> + * Called by KFD when it processes the drain fence injected by
> + * amdgpu_pasid_drain_irq().
> + */
> +void amdgpu_pasid_drain_irq_done(u32 pasid)
> +{
> +	struct amdgpu_fpriv *fpriv;
> +	unsigned long flags;
> +
> +	amdgpu_pasid_lock(&flags);
> +	fpriv = amdgpu_pasid_get_fpriv_locked(pasid);
> +	if (fpriv)
> +		complete(&fpriv->irq_drain_done);
> +	amdgpu_pasid_unlock(flags);
> +}
> +
>   /**
>    * amdgpu_pasid_free_delayed - free pasid when fences signal
>    *
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ids.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_ids.h
> index 46b2c2160126..2af733d96d60 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ids.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ids.h
> @@ -98,6 +98,9 @@ int amdgpu_pasid_alloc(unsigned int bits, struct amdgpu_fpriv *fpriv);
>   void amdgpu_pasid_lock(unsigned long *flags);
>   void amdgpu_pasid_unlock(unsigned long flags);
>   struct amdgpu_fpriv *amdgpu_pasid_get_fpriv_locked(u32 pasid);
> +int amdgpu_pasid_drain_irq(struct amdgpu_device *adev, u32 pasid,
> +			   u32 *ih_fence, unsigned long timeout);
> +void amdgpu_pasid_drain_irq_done(u32 pasid);
>   void amdgpu_pasid_free(u32 pasid);
>   void amdgpu_pasid_free_delayed(struct dma_resv *resv,
>   			       u32 pasid);
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> index bac072d73e16..8b080e2e1496 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> @@ -1507,6 +1507,8 @@ int amdgpu_driver_open_kms(struct drm_device *dev, struct drm_file *file_priv)
>   	if (r)
>   		goto error_pasid;
>   
> +	init_completion(&fpriv->irq_drain_done);
> +
>   	pasid = amdgpu_pasid_alloc(16, fpriv);
>   	if (pasid < 0) {
>   		dev_warn(adev->dev, "No more PASIDs available!");
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
> index 1d86ddfd6bde..b3ebbee76088 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
> @@ -3223,7 +3223,7 @@ static int kfd_ioctl_set_debug_trap(struct file *filep, struct kfd_process *p, v
>   
>   		break;
>   	case KFD_IOC_DBG_TRAP_DISABLE:
> -		r = kfd_dbg_trap_disable(target);
> +		r = kfd_dbg_trap_disable(target, true);
>   		break;
>   	case KFD_IOC_DBG_TRAP_SEND_RUNTIME_EVENT:
>   		r = kfd_dbg_send_exception_to_runtime(target,
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_debug.c b/drivers/gpu/drm/amd/amdkfd/kfd_debug.c
> index 0dd1fd448059..8bed0d0f9bd2 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_debug.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_debug.c
> @@ -655,7 +655,8 @@ void kfd_dbg_trap_deactivate(struct kfd_process *target, bool unwind, int unwind
>   	kfd_dbg_set_workaround(target, false);
>   }
>   
> -static void kfd_dbg_clean_exception_status(struct kfd_process *target)
> +static void kfd_dbg_clean_exception_status(struct kfd_process *target,
> +					   bool drain_irqs)
>   {
>   	struct process_queue_manager *pqm;
>   	struct process_queue_node *pqn;
> @@ -664,7 +665,8 @@ static void kfd_dbg_clean_exception_status(struct kfd_process *target)
>   	for (i = 0; i < target->n_pdds; i++) {
>   		struct kfd_process_device *pdd = target->pdds[i];
>   
> -		kfd_process_drain_interrupts(pdd);
> +		if (drain_irqs)
> +			kfd_process_drain_interrupts(pdd);
>   
>   		pdd->exception_status = 0;
>   	}
> @@ -680,7 +682,7 @@ static void kfd_dbg_clean_exception_status(struct kfd_process *target)
>   	target->exception_status = 0;
>   }
>   
> -int kfd_dbg_trap_disable(struct kfd_process *target)
> +int kfd_dbg_trap_disable(struct kfd_process *target, bool drain_irqs)
>   {
>   	if (!target->debug_trap_enabled)
>   		return 0;
> @@ -704,7 +706,7 @@ int kfd_dbg_trap_disable(struct kfd_process *target)
>   	}
>   
>   	target->debug_trap_enabled = false;
> -	kfd_dbg_clean_exception_status(target);
> +	kfd_dbg_clean_exception_status(target, drain_irqs);
>   	kfd_unref_process(target);
>   
>   	return 0;
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_debug.h b/drivers/gpu/drm/amd/amdkfd/kfd_debug.h
> index fbb751821c69..5babf9b992cd 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_debug.h
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_debug.h
> @@ -43,7 +43,7 @@ bool kfd_dbg_ev_raise(uint64_t event_mask,
>   			unsigned int source_id, bool use_worker,
>   			void *exception_data,
>   			size_t exception_data_size);
> -int kfd_dbg_trap_disable(struct kfd_process *target);
> +int kfd_dbg_trap_disable(struct kfd_process *target, bool drain_irqs);
>   int kfd_dbg_trap_enable(struct kfd_process *target, uint32_t fd,
>   			void __user *runtime_info,
>   			uint32_t *runtime_info_size);
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_int_process_v10.c b/drivers/gpu/drm/amd/amdkfd/kfd_int_process_v10.c
> index 19406ab92c5b..45ac77bb2102 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_int_process_v10.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_int_process_v10.c
> @@ -376,7 +376,7 @@ static void event_interrupt_wq_v10(struct kfd_node *dev,
>   						&exception_data,
>   						sizeof(exception_data));
>   	} else if (KFD_IRQ_IS_FENCE(client_id, source_id)) {
> -		kfd_process_close_interrupt_drain(pasid);
> +		amdgpu_pasid_drain_irq_done(pasid);
>   	}
>   }
>   
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_int_process_v11.c b/drivers/gpu/drm/amd/amdkfd/kfd_int_process_v11.c
> index 12d81abed748..24d13a74a181 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_int_process_v11.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_int_process_v11.c
> @@ -408,7 +408,7 @@ static void event_interrupt_wq_v11(struct kfd_node *dev,
>   		}
>   
>   	} else if (KFD_IRQ_IS_FENCE(client_id, source_id)) {
> -		kfd_process_close_interrupt_drain(pasid);
> +		amdgpu_pasid_drain_irq_done(pasid);
>   	}
>   }
>   
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_int_process_v12_1.c b/drivers/gpu/drm/amd/amdkfd/kfd_int_process_v12_1.c
> index 0da7e1db55c9..4c7ad5aaaebc 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_int_process_v12_1.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_int_process_v12_1.c
> @@ -395,7 +395,7 @@ static void event_interrupt_wq_v12_1(struct kfd_node *node,
>   		}
>   
>   	} else if (KFD_IRQ_IS_FENCE(client_id, source_id)) {
> -		kfd_process_close_interrupt_drain(pasid);
> +		amdgpu_pasid_drain_irq_done(pasid);
>   	}
>   }
>   
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_int_process_v9.c b/drivers/gpu/drm/amd/amdkfd/kfd_int_process_v9.c
> index 2ae1129fea55..e241ad7d55df 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_int_process_v9.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_int_process_v9.c
> @@ -596,7 +596,7 @@ static void event_interrupt_wq_v9(struct kfd_node *dev,
>   						sizeof(exception_data));
>   		kfd_smi_event_update_vmfault(dev, pasid);
>   	} else if (KFD_IRQ_IS_FENCE(client_id, source_id)) {
> -		kfd_process_close_interrupt_drain(pasid);
> +		amdgpu_pasid_drain_irq_done(pasid);
>   	}
>   }
>   
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
> index 2847a5ec5ede..4777d6b7911c 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
> @@ -1030,10 +1030,6 @@ struct kfd_process {
>   	uint64_t exception_enable_mask;
>   	uint64_t exception_status;
>   
> -	/* Used to drain stale interrupts */
> -	wait_queue_head_t wait_irq_drain;
> -	bool irq_drain_is_open;
> -
>   	/* shared virtual memory registered by this process */
>   	struct svm_range_list svms;
>   
> @@ -1245,9 +1241,7 @@ bool enqueue_ih_ring_entry(struct kfd_node *kfd, const void *ih_ring_entry);
>   bool interrupt_is_wanted(struct kfd_node *dev,
>   				const uint32_t *ih_ring_entry,
>   				uint32_t *patched_ihre, bool *flag);
> -int kfd_process_drain_interrupts(struct kfd_process_device *pdd);
> -void kfd_process_close_interrupt_drain(unsigned int pasid);
> -
> +void kfd_process_drain_interrupts(struct kfd_process_device *pdd);
>   /* amdkfd Apertures */
>   int kfd_init_apertures(struct kfd_process *process);
>   
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_process.c b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
> index 226f52626e84..e742ef206abd 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_process.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
> @@ -1019,8 +1019,6 @@ struct kfd_process *kfd_create_process(struct task_struct *thread)
>   		if (ret)
>   			pr_warn("Failed to create debugfs entry for the kfd_process, ret = %d\n",
>   				ret);
> -
> -		init_waitqueue_head(&process->wait_irq_drain);
>   	}
>   out:
>   	mutex_unlock(&kfd_processes_mutex);
> @@ -1420,7 +1418,7 @@ void kfd_process_notifier_release_internal(struct kfd_process *p)
>   	 * back on the MES list and, once proc_ctx_bo is freed, MES would fault
>   	 * on the freed process context. Retiring the context last avoids this.
>   	 */
> -	kfd_dbg_trap_disable(p);
> +	kfd_dbg_trap_disable(p, false);
>   
>   	if (atomic_read(&p->debugged_process_count) > 0) {
>   		struct kfd_process *target;
> @@ -1430,7 +1428,7 @@ void kfd_process_notifier_release_internal(struct kfd_process *p)
>   		hash_for_each_rcu(kfd_processes_table, temp, target, kfd_processes) {
>   			if (target->debugger_process && target->debugger_process == p) {
>   				mutex_lock_nested(&target->mutex, 1);
> -				kfd_dbg_trap_disable(target);
> +				kfd_dbg_trap_disable(target, true);
>   				mutex_unlock(&target->mutex);
>   				if (atomic_read(&p->debugged_process_count) == 0)
>   					break;
> @@ -2313,17 +2311,14 @@ int kfd_resume_all_processes(void)
>   	return ret;
>   }
>   
> -/* assumes caller holds process lock. */
> -int kfd_process_drain_interrupts(struct kfd_process_device *pdd)
> +void kfd_process_drain_interrupts(struct kfd_process_device *pdd)
>   {
>   	uint32_t irq_drain_fence[8];
>   	uint8_t node_id = 0;
> -	int r = 0;
> +	int r;
>   
> -	if (!KFD_IS_SOC15(pdd->dev))
> -		return 0;
> -
> -	pdd->process->irq_drain_is_open = true;
> +	if (!KFD_IS_SOC15(pdd->dev) || !pdd->drm_priv)
> +		return;
>   
>   	memset(irq_drain_fence, 0, sizeof(irq_drain_fence));
>   	irq_drain_fence[0] = (KFD_IRQ_FENCE_SOURCEID << 8) |
> @@ -2342,32 +2337,12 @@ int kfd_process_drain_interrupts(struct kfd_process_device *pdd)
>   	}
>   
>   	/* ensure stale irqs scheduled KFD interrupts and send drain fence. */
> -	if (amdgpu_amdkfd_send_close_event_drain_irq(pdd->dev->adev,
> -						     irq_drain_fence)) {
> -		pdd->process->irq_drain_is_open = false;
> -		return 0;
> -	}
> -
> -	r = wait_event_interruptible(pdd->process->wait_irq_drain,
> -				     !READ_ONCE(pdd->process->irq_drain_is_open));
> +	r = amdgpu_pasid_drain_irq(pdd->dev->adev, pdd->pasid, irq_drain_fence,
> +				   AMDGPU_FENCE_JIFFIES_TIMEOUT);
>   	if (r)
> -		pdd->process->irq_drain_is_open = false;
> -
> -	return r;
> -}
> -
> -void kfd_process_close_interrupt_drain(unsigned int pasid)
> -{
> -	struct kfd_process *p;
> -
> -	p = kfd_lookup_process_by_pasid(pasid, NULL);
> -
> -	if (!p)
> -		return;
> -
> -	WRITE_ONCE(p->irq_drain_is_open, false);
> -	wake_up_all(&p->wait_irq_drain);
> -	kfd_unref_process(p);
> +		dev_warn_ratelimited(pdd->dev->adev->dev,
> +				     "irq drain on pasid 0x%x did not complete (%d)\n",
> +				     pdd->pasid, r);
>   }
>   
>   struct send_exception_work_handler_workarea {

回复: [PATCH] drm/amdkfd: fix lost wakeup in interrupt drain on debugged process exit

[Zhou, Heng]

[-- Attachment #1: Type: text/plain, Size: 13791 bytes --]

AMD General

On 2026-09-30, Felix Kuehling wrote:
> I suspect this may have been introduced by this patch by Alex:
> commit f34034ce5d2116f397008c63901c9c1946e5d665
> ("drm/amdkfd: disable debug before retiring MES process context on teardown")
> If you can confirm that, please add an appropriate Fixes: tag.

I looked into it and I think the root cause predates f34034ce. That commit
only reorders dequeue vs. kfd_dbg_trap_disable() in the teardown path;
kfd_dbg_trap_disable() was already called from the mmu_notifier release
path, and the process is removed from kfd_processes_table before we get
there in either case. So reverting f34034ce would not avoid the hang.

The lost wakeup is inherent to the drain design: the wakeup
(kfd_process_close_interrupt_drain()) looks the process up by PASID in
kfd_processes_table, which no longer holds an exiting process. That was
introduced in:

  Fixes: 12fb1ad70d65 ("drm/amdkfd: update process interrupt handling for debug events")

I added that tag in v2. Please let me know if you'd still prefer the
f34034ce reference.

> The way you're setting and clearing p->irq_drain_deferred seems pretty
> fragile. I'd prefer to just pass this as a parameter to
> kfd_dbg_trap_disable and kfd_dbg_clean_exception_status. Then you also
> don't need two different drain functions.

Done. kfd_dbg_trap_disable() and kfd_dbg_clean_exception_status() now take
a drain_irqs argument, and kfd_process_drain_deferred_interrupts() and the
irq_drain_deferred field are gone.

While doing this I decided to skip the drain entirely for a process exiting
on its own, instead of deferring it to kfd_process_wq_release(): the
self-exit path now passes drain_irqs=false. Once the process is out of
kfd_processes_table its interrupts are no longer delivered to it, and its
PASID owner is cleared before the PASID is freed and later reallocated, so
there is nothing left to drain. The debugger disabling a target still
passes true. Please shout if you think the exiting process still needs a
real drain here.

> If you use an XArray here, you have a more efficient lookup and you also
> get the spin-lock for free. In fact, that xarray already exists in
> amdgpu_ids.c: amdgpu_pasid_xa. [...] you could just add the completion to
> struct amdgpu_fpriv. Maybe the whole IRQ draining logic should move to
> amdgpu_ids.c in that case, and KFD would just invoke it with a PASID.

Done. I added a struct completion irq_drain_done to struct amdgpu_fpriv and
moved the drain/wait and the wakeup into amdgpu_ids.c as
amdgpu_pasid_drain_irq() / amdgpu_pasid_drain_irq_done(), both looking the
fpriv up via amdgpu_pasid_get_fpriv_locked() under the PASID lock. KFD now
just passes the PASID. The kfd_irq_drain_list/lock and the per-process
wait_irq_drain/irq_drain_is_open fields are removed.

KFD holds a reference to the DRM file that owns the PASID while it drains,
so the fpriv (and its completion) stays valid after the PASID lock is
dropped.

Thanks for the review,
Heng

发件人: Kuehling, Felix <Felix.Kuehling@amd.com>
发送时间: Thursday, October 1, 2026 5:04 AM
收件人: Zhou, Heng <Heng.Zhou@amd.com>; amd-gfx@lists.freedesktop.org; Sierra Guiza, Alejandro (Alex) <Alex.Sierra@amd.com>
抄送: Lazar, Lijo <Lijo.Lazar@amd.com>; Koenig, Christian <Christian.Koenig@amd.com>; Deng, Emily <Emily.Deng@amd.com>; Zhao, Victor <Victor.Zhao@amd.com>; phasta@kernel.org; Ma, Qing (Mark) <Qing.Ma@amd.com>; Chang, HaiJun <HaiJun.Chang@amd.com>; Kim, Jonathan <Jonathan.Kim@amd.com>
主题: Re: [PATCH] drm/amdkfd: fix lost wakeup in interrupt drain on debugged process exit

On 2026-09-30 09:42, Heng Zhou wrote:

When a debugged process exits, kfd_process_notifier_release_internal()

first removes it from kfd_processes_table and then calls

kfd_dbg_trap_disable(), which drains the process interrupts. The drain

sends a fence through the IH and waits, without a timeout, for

kfd_process_close_interrupt_drain() to wake it up. That wakeup looks

the process up by PASID in kfd_processes_table, which no longer holds

it, so the wakeup is lost and the exiting task sleeps forever. The

fatal signal has already been consumed in do_exit(), so the

interruptible wait cannot be broken either.



The wait happens inside the mmu_notifier release callback, i.e. within

the global mmu_notifier SRCU read-side critical section, so every

synchronize_srcu() on it stalls as well. Any other process releasing

its mm then hangs in D state and the system needs a reboot. This is

seen with rocgdb on MI300X.

I suspect this may have been introduced by this patch by Alex:

commit f34034ce5d2116f397008c63901c9c1946e5d665

Author: Alex Sierra <alex.sierra@amd.com><mailto:alex.sierra@amd.com>

Date:   Fri Jul 24 15:53:48 2026 -0500



    drm/amdkfd: disable debug before retiring MES process context on teardown



...

If you can confirm that, please add an appropriate Fixes: tag.







Fix this by:



- Tracking pending drains in a list keyed by PASID instead of looking

  the process up in kfd_processes_table, so the drain fence always

  finds its waiter.

- Bounding the wait with a timeout, since the fence can be lost when

  the IH ring or the KFD interrupt FIFO overflows, and warning when the

  drain does not complete.

- Deferring the drain of an exiting debugged process from the

  mmu_notifier release path to kfd_process_wq_release(), before the

  PDDs and their PASIDs are released, so that stale interrupts cannot

  hit a process that reuses the PASID.



Signed-off-by: Heng Zhou <Heng.Zhou@amd.com><mailto:Heng.Zhou@amd.com>

---

 drivers/gpu/drm/amd/amdkfd/kfd_debug.c   |  3 +-

 drivers/gpu/drm/amd/amdkfd/kfd_priv.h    |  7 +-

 drivers/gpu/drm/amd/amdkfd/kfd_process.c | 89 +++++++++++++++++-------

 3 files changed, 68 insertions(+), 31 deletions(-)



diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_debug.c b/drivers/gpu/drm/amd/amdkfd/kfd_debug.c

index 0dd1fd448059..2b227d9de492 100644

--- a/drivers/gpu/drm/amd/amdkfd/kfd_debug.c

+++ b/drivers/gpu/drm/amd/amdkfd/kfd_debug.c

@@ -664,7 +664,8 @@ static void kfd_dbg_clean_exception_status(struct kfd_process *target)

  for (i = 0; i < target->n_pdds; i++) {

         struct kfd_process_device *pdd = target->pdds[i];



-        kfd_process_drain_interrupts(pdd);

+        if (!target->irq_drain_deferred)

+          kfd_process_drain_interrupts(pdd);

The way you're setting and clearing p->irq_drain_deferred seems pretty fragile. I'd prefer to just pass this as a parameter to kfd_dbg_trap_disable and kfd_dbg_clean_exception_status. Then you also don't need two different drain functions.







         pdd->exception_status = 0;

  }

diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h

index 2847a5ec5ede..c04c3a03f559 100644

--- a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h

+++ b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h

@@ -1030,9 +1030,7 @@ struct kfd_process {

  uint64_t exception_enable_mask;

  uint64_t exception_status;



- /* Used to drain stale interrupts */

- wait_queue_head_t wait_irq_drain;

- bool irq_drain_is_open;

+ bool irq_drain_deferred;



  /* shared virtual memory registered by this process */

  struct svm_range_list svms;

@@ -1245,7 +1243,8 @@ bool enqueue_ih_ring_entry(struct kfd_node *kfd, const void *ih_ring_entry);

 bool interrupt_is_wanted(struct kfd_node *dev,

                         const uint32_t *ih_ring_entry,

                         uint32_t *patched_ihre, bool *flag);

-int kfd_process_drain_interrupts(struct kfd_process_device *pdd);

+void kfd_process_drain_interrupts(struct kfd_process_device *pdd);

+void kfd_process_drain_deferred_interrupts(struct kfd_process *p);

 void kfd_process_close_interrupt_drain(unsigned int pasid);



 /* amdkfd Apertures */

diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_process.c b/drivers/gpu/drm/amd/amdkfd/kfd_process.c

index 226f52626e84..b2365c21db9e 100644

--- a/drivers/gpu/drm/amd/amdkfd/kfd_process.c

+++ b/drivers/gpu/drm/amd/amdkfd/kfd_process.c

@@ -55,6 +55,22 @@ DEFINE_MUTEX(kfd_processes_mutex);



 DEFINE_SRCU(kfd_processes_srcu);



+/*

+ * Pending interrupt drains, keyed by PASID. Deliberately not keyed off

+ * kfd_processes_table: a draining process may already have been removed from

+ * it, and the drain fence must still be able to find its waiter.

+ */

+struct kfd_irq_drain_waiter {

+ struct list_head list;

+ u32 pasid;

+ struct completion done;

+};

+

+static DEFINE_SPINLOCK(kfd_irq_drain_lock);

+static LIST_HEAD(kfd_irq_drain_list);

If you use an XArray here, you have a more efficient lookup and you also get the spin-lock for free. In fact, that xarray already exists in amdgpu_ids.c: amdgpu_pasid_xa. It stores a pointer to the fpriv. You can get it with amdgpu_pasid_get_fpriv_locked. So you could just add the completion to struct amdgpu_fpriv. Maybe the whole IRQ draining logic should move to amdgpu_ids.c in that case, and KFD would just invoke it with a PASID.

Regards,
  Felix





+

+#define KFD_IRQ_DRAIN_TIMEOUT  msecs_to_jiffies(1000)

+

 /* For process termination handling */

 static struct workqueue_struct *kfd_process_wq;



@@ -1019,8 +1035,6 @@ struct kfd_process *kfd_create_process(struct task_struct *thread)

         if (ret)

                 pr_warn("Failed to create debugfs entry for the kfd_process, ret = %d\n",

                         ret);

-

-  init_waitqueue_head(&process->wait_irq_drain);

  }

 out:

  mutex_unlock(&kfd_processes_mutex);

@@ -1344,6 +1358,8 @@ static void kfd_process_wq_release(struct work_struct *work)

  kfd_process_free_outstanding_kfd_bos(p);

  svm_range_list_fini(p);



+ kfd_process_drain_deferred_interrupts(p);

+

  kfd_process_destroy_pdds(p);

  dma_fence_put(ef);



@@ -1420,6 +1436,8 @@ void kfd_process_notifier_release_internal(struct kfd_process *p)

   * back on the MES list and, once proc_ctx_bo is freed, MES would fault

   * on the freed process context. Retiring the context last avoids this.

   */

+ if (p->debug_trap_enabled)

+        p->irq_drain_deferred = true;

  kfd_dbg_trap_disable(p);



  if (atomic_read(&p->debugged_process_count) > 0) {

@@ -2313,17 +2331,16 @@ int kfd_resume_all_processes(void)

  return ret;

 }



-/* assumes caller holds process lock. */

-int kfd_process_drain_interrupts(struct kfd_process_device *pdd)

+void kfd_process_drain_interrupts(struct kfd_process_device *pdd)

 {

+ struct kfd_irq_drain_waiter waiter;

  uint32_t irq_drain_fence[8];

+ unsigned long flags;

  uint8_t node_id = 0;

- int r = 0;

+ long r;



  if (!KFD_IS_SOC15(pdd->dev))

-        return 0;

-

- pdd->process->irq_drain_is_open = true;

+        return;



  memset(irq_drain_fence, 0, sizeof(irq_drain_fence));

  irq_drain_fence[0] = (KFD_IRQ_FENCE_SOURCEID << 8) |

@@ -2341,33 +2358,53 @@ int kfd_process_drain_interrupts(struct kfd_process_device *pdd)

         irq_drain_fence[3] |= node_id << 16;

  }



+ waiter.pasid = pdd->pasid;

+ init_completion(&waiter.done);

+

+ spin_lock_irqsave(&kfd_irq_drain_lock, flags);

+ list_add(&waiter.list, &kfd_irq_drain_list);

+  spin_unlock_irqrestore(&kfd_irq_drain_lock, flags);

+

  /* ensure stale irqs scheduled KFD interrupts and send drain fence. */

- if (amdgpu_amdkfd_send_close_event_drain_irq(pdd->dev->adev,

-                                            irq_drain_fence)) {

-        pdd->process->irq_drain_is_open = false;

-        return 0;

+ if (!amdgpu_amdkfd_send_close_event_drain_irq(pdd->dev->adev,

+                                             irq_drain_fence)) {

+         r = wait_for_completion_interruptible_timeout(&waiter.done,

+                                                    KFD_IRQ_DRAIN_TIMEOUT);

+        if (r <= 0)

+          dev_warn_ratelimited(pdd->dev->adev->dev,

+                                    "irq drain on pasid 0x%x did not complete (%ld)\n",

+                                    waiter.pasid, r);

  }



- r = wait_event_interruptible(pdd->process->wait_irq_drain,

-                      !READ_ONCE(pdd->process->irq_drain_is_open));

- if (r)

-        pdd->process->irq_drain_is_open = false;

-

- return r;

+ spin_lock_irqsave(&kfd_irq_drain_lock, flags);

+ list_del(&waiter.list);

+  spin_unlock_irqrestore(&kfd_irq_drain_lock, flags);

 }



-void kfd_process_close_interrupt_drain(unsigned int pasid)

+void kfd_process_drain_deferred_interrupts(struct kfd_process *p)

 {

- struct kfd_process *p;

-

- p = kfd_lookup_process_by_pasid(pasid, NULL);

+ int i;



- if (!p)

+ if (!p->irq_drain_deferred)

         return;



- WRITE_ONCE(p->irq_drain_is_open, false);

- wake_up_all(&p->wait_irq_drain);

- kfd_unref_process(p);

+ p->irq_drain_deferred = false;

+

+ for (i = 0; i < p->n_pdds; i++)

+   kfd_process_drain_interrupts(p->pdds[i]);

+}

+

+void kfd_process_close_interrupt_drain(unsigned int pasid)

+{

+ struct kfd_irq_drain_waiter *waiter;

+ unsigned long flags;

+

+ spin_lock_irqsave(&kfd_irq_drain_lock, flags);

+ list_for_each_entry(waiter, &kfd_irq_drain_list, list) {

+        if (waiter->pasid == pasid)

+                complete(&waiter->done);

+ }

+  spin_unlock_irqrestore(&kfd_irq_drain_lock, flags);

 }



 struct send_exception_work_handler_workarea {

[-- Attachment #2: Type: text/html, Size: 35272 bytes --]