Re: [PATCH 3/3] drm/amdkfd: destroy_pdds release pdd->drm_file at end

[Philip Yang]

On 2025-05-15 10:29, Chen, Xiaogang wrote:

Does this patch fix a bug or just make code look more reasonable? 
kfd_process_destroy_pdds releases pdd related buffers, not related to 
operations on vm. So vm tear down dose not affect this function.

This change doesn't fix anything currently, as fput(pdd->drm_file) to 
free vm is right between free vm mapping qpd->cwsr_mem, qpd->ib_mem and 
free kernel bo qpd->proc_doorbells, pdd->proc_ctx_bo, to make it clear 
for future change.

Regards,

Philip

Regards

Xiaogang

On 5/14/2025 12:10 PM, Philip Yang wrote:
Release pdd->drm_file may free the vm if this is the last reference,
move it to the last step after memory is unmapped.

Signed-off-by: Philip Yang<philip.y...@amd.com>
---
  drivers/gpu/drm/amd/amdkfd/kfd_process.c | 10 +++++++---
  1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_process.c 
b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
index e868cc8da46f..b009c852180d 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_process.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
@@ -1063,9 +1063,6 @@ static void kfd_process_destroy_pdds(struct kfd_process 
*p)
                kfd_process_device_destroy_cwsr_dgpu(pdd);
                kfd_process_device_destroy_ib_mem(pdd);
  
-		if (pdd->drm_file)
-                       fput(pdd->drm_file);
-
                if (pdd->qpd.cwsr_kaddr && !pdd->qpd.cwsr_base)
                        free_pages((unsigned long)pdd->qpd.cwsr_kaddr,
                                get_order(KFD_CWSR_TBA_TMA_SIZE));
@@ -1088,6 +1085,13 @@ static void kfd_process_destroy_pdds(struct kfd_process 
*p)
                        pdd->runtime_inuse = false;
                }
  
+		/*
+                * This may release the vm if application already close the drm 
node,
+                * do it as last step after memory unmapped.
+                */
+               if (pdd->drm_file)
+                       fput(pdd->drm_file);
+
                kfree(pdd);
                p->pdds[i] = NULL;
        }