On 2021-06-16 4:35 a.m., xinhui pan wrote:
Some resource are freed even destroy queue fails.
Looks like you're keeping this behaviour for -ETIME. That is consistent
with what pqn_destroy_queue does. What you're fixing here is the
behaviour for non-timeout errors. Please make that clear in the patch
description.
Out of curiosity, what kind of error were you getting? The only other
ones that are not a fatal memory shortage, are some EINVAL cases in
pm_unmap_queues_v*. But that would indicate some internal error, that a
queue was created with an invalid type, or maybe the queue data
structure was somehow corrupted.
That will cause double
free when user-space issue another destroy_queue ioctl.
Paste some log below.
amdgpu: Can't create new usermode queue because -1 queues were already
created
refcount_t: underflow; use-after-free.
Call Trace:
kobject_put+0xe6/0x1b0
kfd_procfs_del_queue+0x37/0x50 [amdgpu]
pqm_destroy_queue+0x17a/0x390 [amdgpu]
kfd_ioctl_destroy_queue+0x57/0xc0 [amdgpu]
kfd_ioctl+0x463/0x690 [amdgpu]
BUG kmalloc-32 (Tainted: G W ): Object already free
INFO: Allocated in allocate_sdma_mqd+0x30/0xb0 [amdgpu] age=4796 cpu=2
pid=2511
__slab_alloc+0x72/0x80
kmem_cache_alloc_trace+0x81f/0x8c0
allocate_sdma_mqd+0x30/0xb0 [amdgpu]
create_queue_cpsch+0xbf/0x470 [amdgpu]
pqm_create_queue+0x28d/0x6d0 [amdgpu]
kfd_ioctl_create_queue+0x492/0xae0 [amdgpu]
INFO: Freed in free_mqd_hiq_sdma+0x20/0x60 [amdgpu] age=2537 cpu=7
pid=2511
kfree+0x322/0x340
free_mqd_hiq_sdma+0x20/0x60 [amdgpu]
destroy_queue_cpsch+0x20c/0x330 [amdgpu]
pqm_destroy_queue+0x1a3/0x390 [amdgpu]
kfd_ioctl_destroy_queue+0x57/0xc0 [amdgpu]
Signed-off-by: xinhui pan <xinhui.pan@xxxxxxx>
---
drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c | 2 ++
drivers/gpu/drm/amd/amdkfd/kfd_process.c | 4 +++-
drivers/gpu/drm/amd/amdkfd/kfd_process_queue_manager.c | 1 +
3 files changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
index e6366b408420..c24ab8f17eb6 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
@@ -1529,6 +1529,8 @@ static int destroy_queue_cpsch(struct device_queue_manager *dqm,
KFD_UNMAP_QUEUES_FILTER_DYNAMIC_QUEUES, 0);
if (retval == -ETIME)
qpd->reset_wavefronts = true;
+ else if (retval)
+ goto failed_try_destroy_debugged_queue;
if (q->properties.is_gws) {
dqm->gws_queue_count--;
qpd->mapped_gws_queue = false;
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_process.c b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
index 09b98a83f670..984197e5929f 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_process.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
@@ -607,11 +607,13 @@ static int kfd_procfs_add_sysfs_files(struct kfd_process *p)
void kfd_procfs_del_queue(struct queue *q)
{
- if (!q)
+ if (!q || !kobject_get_unless_zero(&q->kobj))
return;
kobject_del(&q->kobj);
kobject_put(&q->kobj);
+ /* paired with the get above */
+ kobject_put(&q->kobj);
}
int kfd_process_create_wq(void)
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_process_queue_manager.c b/drivers/gpu/drm/amd/amdkfd/kfd_process_queue_manager.c
index 95a6c36cea4c..4fcb64bc43dd 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_process_queue_manager.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_process_queue_manager.c
@@ -373,6 +373,7 @@ int pqm_destroy_queue(struct process_queue_manager *pqm, unsigned int qid)
dqm = pqn->kq->dev->dqm;
dqm->ops.destroy_kernel_queue(dqm, pqn->kq, &pdd->qpd);
kernel_queue_uninit(pqn->kq, false);
+ pqn->kq = NULL;
This seems unrelated to this patch. But if you're fixing this, I'd
expect a similar fix after uninit_queue(pqn->q).
Regards,
Felix
}
if (pqn->q) {
_______________________________________________
amd-gfx mailing list
amd-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
