* [PATCH v3 1/2] drm/amdkfd: Fix false positive queue buffer free warning
@ 2025-10-20 14:30 Philip Yang
2025-10-20 14:30 ` [PATCH v3 2/2] drm/amdkfd: Dequeue user queues when process mm released Philip Yang
2025-10-20 15:59 ` [PATCH v3 1/2] drm/amdkfd: Fix false positive queue buffer free warning Chen, Xiaogang
0 siblings, 2 replies; 10+ messages in thread
From: Philip Yang @ 2025-10-20 14:30 UTC (permalink / raw)
To: amd-gfx; +Cc: Felix.Kuehling, Philip Yang
Only show warning message if process mm is still alive when queue
buffer is freed.
If kfd_lookup_process_by_mm return NULL, means the process is already
exited and mm is gone, it is fine to free queue buffer.
Fixes: b049504e211e ("drm/amdkfd: Validate user queue svm memory residency")
Signed-off-by: Philip Yang <Philip.Yang@amd.com>
---
drivers/gpu/drm/amd/amdkfd/kfd_svm.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
index 4d4a47313f5b..d1b2f8525f80 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
@@ -2487,7 +2487,9 @@ svm_range_unmap_from_cpu(struct mm_struct *mm, struct svm_range *prange,
bool unmap_parent;
uint32_t i;
- if (atomic_read(&prange->queue_refcount)) {
+ p = kfd_lookup_process_by_mm(mm);
+
+ if (p && atomic_read(&prange->queue_refcount)) {
int r;
pr_warn("Freeing queue vital buffer 0x%lx, queue evicted\n",
@@ -2497,7 +2499,6 @@ svm_range_unmap_from_cpu(struct mm_struct *mm, struct svm_range *prange,
pr_debug("failed %d to quiesce KFD queues\n", r);
}
- p = kfd_lookup_process_by_mm(mm);
if (!p)
return;
svms = &p->svms;
--
2.49.0
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v3 2/2] drm/amdkfd: Dequeue user queues when process mm released
2025-10-20 14:30 [PATCH v3 1/2] drm/amdkfd: Fix false positive queue buffer free warning Philip Yang
@ 2025-10-20 14:30 ` Philip Yang
2025-10-20 15:59 ` [PATCH v3 1/2] drm/amdkfd: Fix false positive queue buffer free warning Chen, Xiaogang
1 sibling, 0 replies; 10+ messages in thread
From: Philip Yang @ 2025-10-20 14:30 UTC (permalink / raw)
To: amd-gfx; +Cc: Felix.Kuehling, Philip Yang, Felix Kuehling
Move kfd_process_dequeue_from_all_devices from kfd_process_wq_release to
mmu notifier release callback, do it earlier to ensure no system memory
access from GPU because the process memory is going to free from CPU
after mmu release notifier callback returns.
Suggested-by: Felix Kuehling <felix.kuehling@amd.com>
Signed-off-by: Philip Yang <Philip.Yang@amd.com>
---
drivers/gpu/drm/amd/amdkfd/kfd_process.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_process.c b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
index 849456ac498b..9080d23d22ae 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_process.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
@@ -1162,7 +1162,6 @@ static void kfd_process_wq_release(struct work_struct *work)
release_work);
struct dma_fence *ef;
- kfd_process_dequeue_from_all_devices(p);
pqm_uninit(&p->pqm);
/*
@@ -1226,6 +1225,13 @@ static void kfd_process_notifier_release_internal(struct kfd_process *p)
cancel_delayed_work_sync(&p->eviction_work);
cancel_delayed_work_sync(&p->restore_work);
+ /*
+ * Evict and remove user queues because exit_mmap free process memory,
+ * it is not safe for GPU to access system memory after mmu release
+ * notifier callback returns.
+ */
+ kfd_process_dequeue_from_all_devices(p);
+
for (i = 0; i < p->n_pdds; i++) {
struct kfd_process_device *pdd = p->pdds[i];
--
2.49.0
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v3 1/2] drm/amdkfd: Fix false positive queue buffer free warning
2025-10-20 14:30 [PATCH v3 1/2] drm/amdkfd: Fix false positive queue buffer free warning Philip Yang
2025-10-20 14:30 ` [PATCH v3 2/2] drm/amdkfd: Dequeue user queues when process mm released Philip Yang
@ 2025-10-20 15:59 ` Chen, Xiaogang
2025-10-20 16:24 ` Kim, Jonathan
2025-10-20 16:34 ` Philip Yang
1 sibling, 2 replies; 10+ messages in thread
From: Chen, Xiaogang @ 2025-10-20 15:59 UTC (permalink / raw)
To: Philip Yang, amd-gfx; +Cc: Felix.Kuehling
[-- Attachment #1: Type: text/plain, Size: 1799 bytes --]
On 10/20/2025 9:30 AM, Philip Yang wrote:
> Only show warning message if process mm is still alive when queue
> buffer is freed.
>
> If kfd_lookup_process_by_mm return NULL, means the process is already
> exited and mm is gone, it is fine to free queue buffer.
>
> Fixes: b049504e211e ("drm/amdkfd: Validate user queue svm memory residency")
> Signed-off-by: Philip Yang<Philip.Yang@amd.com>
> ---
> drivers/gpu/drm/amd/amdkfd/kfd_svm.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
> index 4d4a47313f5b..d1b2f8525f80 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
> @@ -2487,7 +2487,9 @@ svm_range_unmap_from_cpu(struct mm_struct *mm, struct svm_range *prange,
> bool unmap_parent;
> uint32_t i;
>
> - if (atomic_read(&prange->queue_refcount)) {
> + p = kfd_lookup_process_by_mm(mm);
p->mm is null, not p because driver set p->mm to null. But still
prange->queue_refcount is ref of queues from this prange. If app unmap
this prange app should have destroyed the queues associated with this
prange already. If not, it is not driver issue. I think driver should
send this warning anyway to indicate there are queues not destroyed by
app before app unmap the prange. It is an app race condition, not driver.
Regards
Xiaogang
> +
> + if (p && atomic_read(&prange->queue_refcount)) {
> int r;
>
> pr_warn("Freeing queue vital buffer 0x%lx, queue evicted\n",
> @@ -2497,7 +2499,6 @@ svm_range_unmap_from_cpu(struct mm_struct *mm, struct svm_range *prange,
> pr_debug("failed %d to quiesce KFD queues\n", r);
> }
>
> - p = kfd_lookup_process_by_mm(mm);
> if (!p)
> return;
> svms = &p->svms;
[-- Attachment #2: Type: text/html, Size: 2538 bytes --]
^ permalink raw reply [flat|nested] 10+ messages in thread
* RE: [PATCH v3 1/2] drm/amdkfd: Fix false positive queue buffer free warning
2025-10-20 15:59 ` [PATCH v3 1/2] drm/amdkfd: Fix false positive queue buffer free warning Chen, Xiaogang
@ 2025-10-20 16:24 ` Kim, Jonathan
2025-10-20 17:33 ` Chen, Xiaogang
2025-10-20 16:34 ` Philip Yang
1 sibling, 1 reply; 10+ messages in thread
From: Kim, Jonathan @ 2025-10-20 16:24 UTC (permalink / raw)
To: Chen, Xiaogang, Yang, Philip, amd-gfx@lists.freedesktop.org
Cc: Kuehling, Felix
[-- Attachment #1: Type: text/plain, Size: 2795 bytes --]
[Public]
You can see this error message on CTRL-C or gpu reset conditions.
Those don’t seem like application race conditions to me i.e. the app can’t do anything about terminations it can’t see coming.
It appears any bad actor could spam dmesgs with these error messages if they wanted to with the way the driver currently is.
Jon
From: amd-gfx <amd-gfx-bounces@lists.freedesktop.org> On Behalf Of Chen, Xiaogang
Sent: Monday, October 20, 2025 12:00 PM
To: Yang, Philip <Philip.Yang@amd.com>; amd-gfx@lists.freedesktop.org
Cc: Kuehling, Felix <Felix.Kuehling@amd.com>
Subject: Re: [PATCH v3 1/2] drm/amdkfd: Fix false positive queue buffer free warning
Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding.
On 10/20/2025 9:30 AM, Philip Yang wrote:
Only show warning message if process mm is still alive when queue
buffer is freed.
If kfd_lookup_process_by_mm return NULL, means the process is already
exited and mm is gone, it is fine to free queue buffer.
Fixes: b049504e211e ("drm/amdkfd: Validate user queue svm memory residency")
Signed-off-by: Philip Yang <Philip.Yang@amd.com><mailto:Philip.Yang@amd.com>
---
drivers/gpu/drm/amd/amdkfd/kfd_svm.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
index 4d4a47313f5b..d1b2f8525f80 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
@@ -2487,7 +2487,9 @@ svm_range_unmap_from_cpu(struct mm_struct *mm, struct svm_range *prange,
bool unmap_parent;
uint32_t i;
- if (atomic_read(&prange->queue_refcount)) {
+ p = kfd_lookup_process_by_mm(mm);
p->mm is null, not p because driver set p->mm to null. But still prange->queue_refcount is ref of queues from this prange. If app unmap this prange app should have destroyed the queues associated with this prange already. If not, it is not driver issue. I think driver should send this warning anyway to indicate there are queues not destroyed by app before app unmap the prange. It is an app race condition, not driver.
Regards
Xiaogang
+
+ if (p && atomic_read(&prange->queue_refcount)) {
int r;
pr_warn("Freeing queue vital buffer 0x%lx, queue evicted\n",
@@ -2497,7 +2499,6 @@ svm_range_unmap_from_cpu(struct mm_struct *mm, struct svm_range *prange,
pr_debug("failed %d to quiesce KFD queues\n", r);
}
- p = kfd_lookup_process_by_mm(mm);
if (!p)
return;
svms = &p->svms;
[-- Attachment #2: Type: text/html, Size: 8119 bytes --]
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v3 1/2] drm/amdkfd: Fix false positive queue buffer free warning
2025-10-20 15:59 ` [PATCH v3 1/2] drm/amdkfd: Fix false positive queue buffer free warning Chen, Xiaogang
2025-10-20 16:24 ` Kim, Jonathan
@ 2025-10-20 16:34 ` Philip Yang
2025-10-20 17:29 ` Chen, Xiaogang
1 sibling, 1 reply; 10+ messages in thread
From: Philip Yang @ 2025-10-20 16:34 UTC (permalink / raw)
To: Chen, Xiaogang, Philip Yang, amd-gfx; +Cc: Felix.Kuehling
On 2025-10-20 11:59, Chen, Xiaogang wrote:
>
>
> On 10/20/2025 9:30 AM, Philip Yang wrote:
>> Only show warning message if process mm is still alive when queue
>> buffer is freed.
>>
>> If kfd_lookup_process_by_mm return NULL, means the process is already
>> exited and mm is gone, it is fine to free queue buffer.
>>
>> Fixes: b049504e211e ("drm/amdkfd: Validate user queue svm memory residency")
>> Signed-off-by: Philip Yang<Philip.Yang@amd.com>
>> ---
>> drivers/gpu/drm/amd/amdkfd/kfd_svm.c | 5 +++--
>> 1 file changed, 3 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
>> index 4d4a47313f5b..d1b2f8525f80 100644
>> --- a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
>> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
>> @@ -2487,7 +2487,9 @@ svm_range_unmap_from_cpu(struct mm_struct *mm, struct svm_range *prange,
>> bool unmap_parent;
>> uint32_t i;
>>
>> - if (atomic_read(&prange->queue_refcount)) {
>> + p = kfd_lookup_process_by_mm(mm);
>
> p->mm is null, not p because driver set p->mm to null. But still
> prange->queue_refcount is ref of queues from this prange. If app unmap
> this prange app should have destroyed the queues associated with this
> prange already. If not, it is not driver issue.
>
right, we still pr_warn if app unmap the prange with queue_refcount not
zero, p is not NULL for this case.
>
> I think driver should send this warning anyway to indicate there are
> queues not destroyed by app before app unmap the prange.
>
if app killed or segmentation fault, mm is gone first, then queue is
destroyed in scheduled release wq, then exit_mmap unmap the prange with
queue_refcount not zero, p is NULL, should not pr_warn, because warning
message "Freeing queue vital buffer...." is not related to app
segmentation fault or killed.
Regards,
Philip
> It is an app race condition, not driver.
>
> Regards
>
> Xiaogang
>
>> +
>> + if (p && atomic_read(&prange->queue_refcount)) {
>> int r;
>>
>> pr_warn("Freeing queue vital buffer 0x%lx, queue evicted\n",
>> @@ -2497,7 +2499,6 @@ svm_range_unmap_from_cpu(struct mm_struct *mm, struct svm_range *prange,
>> pr_debug("failed %d to quiesce KFD queues\n", r);
>> }
>>
>> - p = kfd_lookup_process_by_mm(mm);
>> if (!p)
>> return;
>> svms = &p->svms;
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v3 1/2] drm/amdkfd: Fix false positive queue buffer free warning
2025-10-20 16:34 ` Philip Yang
@ 2025-10-20 17:29 ` Chen, Xiaogang
2025-10-22 18:13 ` Philip Yang
0 siblings, 1 reply; 10+ messages in thread
From: Chen, Xiaogang @ 2025-10-20 17:29 UTC (permalink / raw)
To: Philip Yang, Philip Yang, amd-gfx; +Cc: Felix.Kuehling
[-- Attachment #1: Type: text/plain, Size: 3186 bytes --]
On 10/20/2025 11:34 AM, Philip Yang wrote:
>
> On 2025-10-20 11:59, Chen, Xiaogang wrote:
>>
>>
>> On 10/20/2025 9:30 AM, Philip Yang wrote:
>>> Only show warning message if process mm is still alive when queue
>>> buffer is freed.
>>>
>>> If kfd_lookup_process_by_mm return NULL, means the process is already
>>> exited and mm is gone, it is fine to free queue buffer.
>>>
>>> Fixes: b049504e211e ("drm/amdkfd: Validate user queue svm memory
>>> residency")
>>> Signed-off-by: Philip Yang<Philip.Yang@amd.com>
>>> ---
>>> drivers/gpu/drm/amd/amdkfd/kfd_svm.c | 5 +++--
>>> 1 file changed, 3 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
>>> b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
>>> index 4d4a47313f5b..d1b2f8525f80 100644
>>> --- a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
>>> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
>>> @@ -2487,7 +2487,9 @@ svm_range_unmap_from_cpu(struct mm_struct *mm,
>>> struct svm_range *prange,
>>> bool unmap_parent;
>>> uint32_t i;
>>> - if (atomic_read(&prange->queue_refcount)) {
>>> + p = kfd_lookup_process_by_mm(mm);
>>
>> p->mm is null, not p because driver set p->mm to null. But still
>> prange->queue_refcount is ref of queues from this prange. If app
>> unmap this prange app should have destroyed the queues associated
>> with this prange already. If not, it is not driver issue.
>>
> right, we still pr_warn if app unmap the prange with queue_refcount
> not zero, p is not NULL for this case.
>>
>> I think driver should send this warning anyway to indicate there are
>> queues not destroyed by app before app unmap the prange.
>>
> if app killed or segmentation fault, mm is gone first, then queue is
> destroyed in scheduled release wq, then exit_mmap unmap the prange
> with queue_refcount not zero, p is NULL, should not pr_warn, because
> warning message "Freeing queue vital buffer...." is not related to app
> segmentation fault or killed.
Then can we put pqm_uninit at kfd_process_notifier_release_internal as
you did at second patch with kfd_process_dequeue_from_all_devices:
destroy queue when mm is gone?
And if this is for the cases "app got killed or segment fault" how
important user should not see this warning message? Something was wrong
already anyway.
My thought is that prange->queue_refcount means queue number associated
with prange. If it got unmap the queues should be destroyed already, if
not, something was wrong somewhere, then need send this warning message.
Regards
Xiaogang
>
> Regards,
>
> Philip
>
>> It is an app race condition, not driver.
>>
>> Regards
>>
>> Xiaogang
>>
>>> +
>>> + if (p && atomic_read(&prange->queue_refcount)) {
>>> int r;
>>> pr_warn("Freeing queue vital buffer 0x%lx, queue
>>> evicted\n",
>>> @@ -2497,7 +2499,6 @@ svm_range_unmap_from_cpu(struct mm_struct *mm,
>>> struct svm_range *prange,
>>> pr_debug("failed %d to quiesce KFD queues\n", r);
>>> }
>>> - p = kfd_lookup_process_by_mm(mm);
>>> if (!p)
>>> return;
>>> svms = &p->svms;
[-- Attachment #2: Type: text/html, Size: 76493 bytes --]
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v3 1/2] drm/amdkfd: Fix false positive queue buffer free warning
2025-10-20 16:24 ` Kim, Jonathan
@ 2025-10-20 17:33 ` Chen, Xiaogang
2025-10-20 17:45 ` Kim, Jonathan
0 siblings, 1 reply; 10+ messages in thread
From: Chen, Xiaogang @ 2025-10-20 17:33 UTC (permalink / raw)
To: Kim, Jonathan, Yang, Philip, amd-gfx@lists.freedesktop.org
Cc: Kuehling, Felix
[-- Attachment #1: Type: text/plain, Size: 3369 bytes --]
On 10/20/2025 11:24 AM, Kim, Jonathan wrote:
>
> [Public]
>
>
> You can see this error message on CTRL-C or gpu reset conditions.
>
> Those don’t seem like application race conditions to me i.e. the app
> can’t do anything about terminations it can’t see coming.
>
> It appears any bad actor could spam dmesgs with these error messages
> if they wanted to with the way the driver currently is.
>
If this warning message comes out due to some unexpected things
happened(like ctrl-c) I think user would not be surprised to see some
warnings.
Regards
Xiaogang
> Jon
>
> *From:*amd-gfx <amd-gfx-bounces@lists.freedesktop.org> *On Behalf Of
> *Chen, Xiaogang
> *Sent:* Monday, October 20, 2025 12:00 PM
> *To:* Yang, Philip <Philip.Yang@amd.com>; amd-gfx@lists.freedesktop.org
> *Cc:* Kuehling, Felix <Felix.Kuehling@amd.com>
> *Subject:* Re: [PATCH v3 1/2] drm/amdkfd: Fix false positive queue
> buffer free warning
>
>
>
>
> *Caution:*This message originated from an External Source. Use proper
> caution when opening attachments, clicking links, or responding.
>
> On 10/20/2025 9:30 AM, Philip Yang wrote:
>
> Only show warning message if process mm is still alive when queue
>
> buffer is freed.
>
> If kfd_lookup_process_by_mm return NULL, means the process is already
>
> exited and mm is gone, it is fine to free queue buffer.
>
> Fixes: b049504e211e ("drm/amdkfd: Validate user queue svm memory residency")
>
> Signed-off-by: Philip Yang<Philip.Yang@amd.com> <mailto:Philip.Yang@amd.com>
>
> ---
>
> drivers/gpu/drm/amd/amdkfd/kfd_svm.c | 5 +++--
>
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
>
> index 4d4a47313f5b..d1b2f8525f80 100644
>
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
>
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
>
> @@ -2487,7 +2487,9 @@ svm_range_unmap_from_cpu(struct mm_struct *mm, struct svm_range *prange,
>
> bool unmap_parent;
>
> uint32_t i;
>
>
>
> - if (atomic_read(&prange->queue_refcount)) {
>
> + p = kfd_lookup_process_by_mm(mm);
>
> p->mm is null, not p because driver set p->mm to null. But
> still prange->queue_refcount is ref of queues from this prange. If app
> unmap this prange app should have destroyed the queues associated with
> this prange already. If not, it is not driver issue. I think driver
> should send this warning anyway to indicate there are queues not
> destroyed by app before app unmap the prange. It is an app race
> condition, not driver.
>
> Regards
>
> Xiaogang
>
> +
>
> + if (p && atomic_read(&prange->queue_refcount)) {
>
> int r;
>
>
>
> pr_warn("Freeing queue vital buffer 0x%lx, queue evicted\n",
>
> @@ -2497,7 +2499,6 @@ svm_range_unmap_from_cpu(struct mm_struct *mm, struct svm_range *prange,
>
> pr_debug("failed %d to quiesce KFD queues\n", r);
>
> }
>
>
>
> - p = kfd_lookup_process_by_mm(mm);
>
> if (!p)
>
> return;
>
> svms = &p->svms;
>
[-- Attachment #2: Type: text/html, Size: 10591 bytes --]
^ permalink raw reply [flat|nested] 10+ messages in thread
* RE: [PATCH v3 1/2] drm/amdkfd: Fix false positive queue buffer free warning
2025-10-20 17:33 ` Chen, Xiaogang
@ 2025-10-20 17:45 ` Kim, Jonathan
2025-10-20 17:51 ` Chen, Xiaogang
0 siblings, 1 reply; 10+ messages in thread
From: Kim, Jonathan @ 2025-10-20 17:45 UTC (permalink / raw)
To: Chen, Xiaogang, Yang, Philip, amd-gfx@lists.freedesktop.org
Cc: Kuehling, Felix
[-- Attachment #1: Type: text/plain, Size: 3757 bytes --]
[Public]
Why print something under conditions where it’s not needed then?
Could this not be a pr_debug instead?
The number of queues could be quite large.
I don’t see the merit in potentially spamming dmesgs if driver is going to clean up anyways.
Jon
From: Chen, Xiaogang <Xiaogang.Chen@amd.com>
Sent: Monday, October 20, 2025 1:34 PM
To: Kim, Jonathan <Jonathan.Kim@amd.com>; Yang, Philip <Philip.Yang@amd.com>; amd-gfx@lists.freedesktop.org
Cc: Kuehling, Felix <Felix.Kuehling@amd.com>
Subject: Re: [PATCH v3 1/2] drm/amdkfd: Fix false positive queue buffer free warning
On 10/20/2025 11:24 AM, Kim, Jonathan wrote:
[Public]
You can see this error message on CTRL-C or gpu reset conditions.
Those don’t seem like application race conditions to me i.e. the app can’t do anything about terminations it can’t see coming.
It appears any bad actor could spam dmesgs with these error messages if they wanted to with the way the driver currently is.
If this warning message comes out due to some unexpected things happened(like ctrl-c) I think user would not be surprised to see some warnings.
Regards
Xiaogang
Jon
From: amd-gfx <amd-gfx-bounces@lists.freedesktop.org><mailto:amd-gfx-bounces@lists.freedesktop.org> On Behalf Of Chen, Xiaogang
Sent: Monday, October 20, 2025 12:00 PM
To: Yang, Philip <Philip.Yang@amd.com><mailto:Philip.Yang@amd.com>; amd-gfx@lists.freedesktop.org<mailto:amd-gfx@lists.freedesktop.org>
Cc: Kuehling, Felix <Felix.Kuehling@amd.com><mailto:Felix.Kuehling@amd.com>
Subject: Re: [PATCH v3 1/2] drm/amdkfd: Fix false positive queue buffer free warning
Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding.
On 10/20/2025 9:30 AM, Philip Yang wrote:
Only show warning message if process mm is still alive when queue
buffer is freed.
If kfd_lookup_process_by_mm return NULL, means the process is already
exited and mm is gone, it is fine to free queue buffer.
Fixes: b049504e211e ("drm/amdkfd: Validate user queue svm memory residency")
Signed-off-by: Philip Yang <Philip.Yang@amd.com><mailto:Philip.Yang@amd.com>
---
drivers/gpu/drm/amd/amdkfd/kfd_svm.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
index 4d4a47313f5b..d1b2f8525f80 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
@@ -2487,7 +2487,9 @@ svm_range_unmap_from_cpu(struct mm_struct *mm, struct svm_range *prange,
bool unmap_parent;
uint32_t i;
- if (atomic_read(&prange->queue_refcount)) {
+ p = kfd_lookup_process_by_mm(mm);
p->mm is null, not p because driver set p->mm to null. But still prange->queue_refcount is ref of queues from this prange. If app unmap this prange app should have destroyed the queues associated with this prange already. If not, it is not driver issue. I think driver should send this warning anyway to indicate there are queues not destroyed by app before app unmap the prange. It is an app race condition, not driver.
Regards
Xiaogang
+
+ if (p && atomic_read(&prange->queue_refcount)) {
int r;
pr_warn("Freeing queue vital buffer 0x%lx, queue evicted\n",
@@ -2497,7 +2499,6 @@ svm_range_unmap_from_cpu(struct mm_struct *mm, struct svm_range *prange,
pr_debug("failed %d to quiesce KFD queues\n", r);
}
- p = kfd_lookup_process_by_mm(mm);
if (!p)
return;
svms = &p->svms;
[-- Attachment #2: Type: text/html, Size: 10465 bytes --]
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v3 1/2] drm/amdkfd: Fix false positive queue buffer free warning
2025-10-20 17:45 ` Kim, Jonathan
@ 2025-10-20 17:51 ` Chen, Xiaogang
0 siblings, 0 replies; 10+ messages in thread
From: Chen, Xiaogang @ 2025-10-20 17:51 UTC (permalink / raw)
To: Kim, Jonathan, Yang, Philip, amd-gfx@lists.freedesktop.org
Cc: Kuehling, Felix
[-- Attachment #1: Type: text/plain, Size: 4662 bytes --]
On 10/20/2025 12:45 PM, Kim, Jonathan wrote:
>
> [Public]
>
>
> Why print something under conditions where it’s not needed then?
>
> Could this not be a pr_debug instead?
>
> The number of queues could be quite large.
>
> I don’t see the merit in potentially spamming dmesgs if driver is
> going to clean up anyways.
>
I think the issue here is not about print this warning or not. The issue
is queues associate with prange should have been destroyed before prange
got unmapped.
Regards
Xiaogang
> Jon
>
> *From:*Chen, Xiaogang <Xiaogang.Chen@amd.com>
> *Sent:* Monday, October 20, 2025 1:34 PM
> *To:* Kim, Jonathan <Jonathan.Kim@amd.com>; Yang, Philip
> <Philip.Yang@amd.com>; amd-gfx@lists.freedesktop.org
> *Cc:* Kuehling, Felix <Felix.Kuehling@amd.com>
> *Subject:* Re: [PATCH v3 1/2] drm/amdkfd: Fix false positive queue
> buffer free warning
>
> On 10/20/2025 11:24 AM, Kim, Jonathan wrote:
>
> [Public]
>
> You can see this error message on CTRL-C or gpu reset conditions.
>
> Those don’t seem like application race conditions to me i.e. the
> app can’t do anything about terminations it can’t see coming.
>
> It appears any bad actor could spam dmesgs with these error
> messages if they wanted to with the way the driver currently is.
>
> If this warning message comes out due to some unexpected things
> happened(like ctrl-c) I think user would not be surprised to see some
> warnings.
>
> Regards
>
> Xiaogang
>
> Jon
>
> *From:*amd-gfx <amd-gfx-bounces@lists.freedesktop.org>
> <mailto:amd-gfx-bounces@lists.freedesktop.org> *On Behalf Of
> *Chen, Xiaogang
> *Sent:* Monday, October 20, 2025 12:00 PM
> *To:* Yang, Philip <Philip.Yang@amd.com>
> <mailto:Philip.Yang@amd.com>; amd-gfx@lists.freedesktop.org
> *Cc:* Kuehling, Felix <Felix.Kuehling@amd.com>
> <mailto:Felix.Kuehling@amd.com>
> *Subject:* Re: [PATCH v3 1/2] drm/amdkfd: Fix false positive queue
> buffer free warning
>
>
>
>
> *Caution:*This message originated from an External Source. Use
> proper caution when opening attachments, clicking links, or
> responding.
>
> On 10/20/2025 9:30 AM, Philip Yang wrote:
>
> Only show warning message if process mm is still alive when queue
>
> buffer is freed.
>
>
>
> If kfd_lookup_process_by_mm return NULL, means the process is already
>
> exited and mm is gone, it is fine to free queue buffer.
>
>
>
> Fixes: b049504e211e ("drm/amdkfd: Validate user queue svm memory residency")
>
> Signed-off-by: Philip Yang<Philip.Yang@amd.com> <mailto:Philip.Yang@amd.com>
>
> ---
>
> drivers/gpu/drm/amd/amdkfd/kfd_svm.c | 5 +++--
>
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
>
>
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
>
> index 4d4a47313f5b..d1b2f8525f80 100644
>
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
>
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
>
> @@ -2487,7 +2487,9 @@ svm_range_unmap_from_cpu(struct mm_struct *mm, struct svm_range *prange,
>
> bool unmap_parent;
>
> uint32_t i;
>
>
>
> - if (atomic_read(&prange->queue_refcount)) {
>
> + p = kfd_lookup_process_by_mm(mm);
>
> p->mm is null, not p because driver set p->mm to null. But
> still prange->queue_refcount is ref of queues from this prange. If
> app unmap this prange app should have destroyed the queues
> associated with this prange already. If not, it is not driver
> issue. I think driver should send this warning anyway to indicate
> there are queues not destroyed by app before app unmap the prange.
> It is an app race condition, not driver.
>
> Regards
>
> Xiaogang
>
>
>
> +
>
> + if (p && atomic_read(&prange->queue_refcount)) {
>
> int r;
>
>
>
> pr_warn("Freeing queue vital buffer 0x%lx, queue evicted\n",
>
> @@ -2497,7 +2499,6 @@ svm_range_unmap_from_cpu(struct mm_struct *mm, struct svm_range *prange,
>
> pr_debug("failed %d to quiesce KFD queues\n", r);
>
> }
>
>
>
> - p = kfd_lookup_process_by_mm(mm);
>
> if (!p)
>
> return;
>
> svms = &p->svms;
>
[-- Attachment #2: Type: text/html, Size: 14232 bytes --]
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v3 1/2] drm/amdkfd: Fix false positive queue buffer free warning
2025-10-20 17:29 ` Chen, Xiaogang
@ 2025-10-22 18:13 ` Philip Yang
0 siblings, 0 replies; 10+ messages in thread
From: Philip Yang @ 2025-10-22 18:13 UTC (permalink / raw)
To: Chen, Xiaogang, Philip Yang, amd-gfx; +Cc: Felix.Kuehling
On 2025-10-20 13:29, Chen, Xiaogang wrote:
>
>
> On 10/20/2025 11:34 AM, Philip Yang wrote:
>>
>> On 2025-10-20 11:59, Chen, Xiaogang wrote:
>>>
>>>
>>> On 10/20/2025 9:30 AM, Philip Yang wrote:
>>>> Only show warning message if process mm is still alive when queue
>>>> buffer is freed.
>>>>
>>>> If kfd_lookup_process_by_mm return NULL, means the process is already
>>>> exited and mm is gone, it is fine to free queue buffer.
>>>>
>>>> Fixes: b049504e211e ("drm/amdkfd: Validate user queue svm memory
>>>> residency")
>>>> Signed-off-by: Philip Yang<Philip.Yang@amd.com>
>>>> ---
>>>> drivers/gpu/drm/amd/amdkfd/kfd_svm.c | 5 +++--
>>>> 1 file changed, 3 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
>>>> b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
>>>> index 4d4a47313f5b..d1b2f8525f80 100644
>>>> --- a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
>>>> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
>>>> @@ -2487,7 +2487,9 @@ svm_range_unmap_from_cpu(struct mm_struct
>>>> *mm, struct svm_range *prange,
>>>> bool unmap_parent;
>>>> uint32_t i;
>>>> - if (atomic_read(&prange->queue_refcount)) {
>>>> + p = kfd_lookup_process_by_mm(mm);
>>>
>>> p->mm is null, not p because driver set p->mm to null. But still
>>> prange->queue_refcount is ref of queues from this prange. If app
>>> unmap this prange app should have destroyed the queues associated
>>> with this prange already. If not, it is not driver issue.
>>>
>> right, we still pr_warn if app unmap the prange with queue_refcount
>> not zero, p is not NULL for this case.
>>>
>>> I think driver should send this warning anyway to indicate there are
>>> queues not destroyed by app before app unmap the prange.
>>>
>> if app killed or segmentation fault, mm is gone first, then queue is
>> destroyed in scheduled release wq, then exit_mmap unmap the prange
>> with queue_refcount not zero, p is NULL, should not pr_warn, because
>> warning message "Freeing queue vital buffer...." is not related to
>> app segmentation fault or killed.
>
> Then can we put pqm_uninit at kfd_process_notifier_release_internal as
> you did at second patch with kfd_process_dequeue_from_all_devices:
> destroy queue when mm is gone?
>
This is good idea,I didn't try because it seems not safe to destroy user
queues inside mmu release notifier callback, which free and clean queue
resource, BOs.
After testing on HWS, MES, with SDMA updating page table, no circular
locking warning (have to fix one circular locking warning to be sure),
and the warning message "Freeing queue vital buffer..." is gone too. I
will send out v4 patch.
Regards,
Philip
>
> And if this is for the cases "app got killed or segment fault" how
> important user should not see this warning message? Something was
> wrong already anyway.
>
> My thought is that prange->queue_refcount means queue number
> associated with prange. If it got unmap the queues should be destroyed
> already, if not, something was wrong somewhere, then need send this
> warning message.
>
> Regards
>
> Xiaogang
>
>>
>> Regards,
>>
>> Philip
>>
>>> It is an app race condition, not driver.
>>>
>>> Regards
>>>
>>> Xiaogang
>>>
>>>> +
>>>> + if (p && atomic_read(&prange->queue_refcount)) {
>>>> int r;
>>>> pr_warn("Freeing queue vital buffer 0x%lx, queue
>>>> evicted\n",
>>>> @@ -2497,7 +2499,6 @@ svm_range_unmap_from_cpu(struct mm_struct
>>>> *mm, struct svm_range *prange,
>>>> pr_debug("failed %d to quiesce KFD queues\n", r);
>>>> }
>>>> - p = kfd_lookup_process_by_mm(mm);
>>>> if (!p)
>>>> return;
>>>> svms = &p->svms;
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2025-10-22 18:13 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-10-20 14:30 [PATCH v3 1/2] drm/amdkfd: Fix false positive queue buffer free warning Philip Yang
2025-10-20 14:30 ` [PATCH v3 2/2] drm/amdkfd: Dequeue user queues when process mm released Philip Yang
2025-10-20 15:59 ` [PATCH v3 1/2] drm/amdkfd: Fix false positive queue buffer free warning Chen, Xiaogang
2025-10-20 16:24 ` Kim, Jonathan
2025-10-20 17:33 ` Chen, Xiaogang
2025-10-20 17:45 ` Kim, Jonathan
2025-10-20 17:51 ` Chen, Xiaogang
2025-10-20 16:34 ` Philip Yang
2025-10-20 17:29 ` Chen, Xiaogang
2025-10-22 18:13 ` Philip Yang
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox