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

["Kuehling, Felix" <felix.kuehling@amd.com>]

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 {