[PATCH v3 1/2] drm/amdkfd: Fix false positive queue buffer free warning

[PATCH v3 1/2] drm/amdkfd: Fix false positive queue buffer free warning

[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

[PATCH v3 2/2] drm/amdkfd: Dequeue user queues when process mm released

[Philip Yang]

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

Re: [PATCH v3 1/2] drm/amdkfd: Fix false positive queue buffer free warning

[Chen, Xiaogang]

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

RE: [PATCH v3 1/2] drm/amdkfd: Fix false positive queue buffer free warning

[Kim, Jonathan]

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

Re: [PATCH v3 1/2] drm/amdkfd: Fix false positive queue buffer free warning

[Chen, Xiaogang]

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

RE: [PATCH v3 1/2] drm/amdkfd: Fix false positive queue buffer free warning

[Kim, Jonathan]

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

Re: [PATCH v3 1/2] drm/amdkfd: Fix false positive queue buffer free warning

[Chen, Xiaogang]

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

Re: [PATCH v3 1/2] drm/amdkfd: Fix false positive queue buffer free warning

[Philip Yang]

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;

Re: [PATCH v3 1/2] drm/amdkfd: Fix false positive queue buffer free warning

[Chen, Xiaogang]

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

Re: [PATCH v3 1/2] drm/amdkfd: Fix false positive queue buffer free warning

[Philip Yang]

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;