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

[Philip Yang <yangp@amd.com>]

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;