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 <[email protected]>
---
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
