Re: [PATCH v2] drm/amdkfd: Change svm_range_get_info return type

[Felix Kuehling]

On 2025-03-31 09:18, Ваторопин Андрей wrote:
From: Andrey Vatoropin <a.vatoro...@crpt.ru>
Static analysis shows that pointer "svms" cannot be NULL because it points
to the object "struct svm_range_list". Remove the extra NULL check. It is
meaningless and harms the readability of the code.

In the function svm_range_get_info() there is no possibility of failure.
Therefore, the caller of the function svm_range_get_info() does not need
a return value. Change the function svm_range_get_info() return type from
"int" to "void".

Since the function svm_range_get_info() has a return type of "void". The
caller of the function svm_range_get_info() does not need a return value.
Delete extra code.

Found by Linux Verification Center (linuxtesting.org) with SVACE.

Signed-off-by: Andrey Vatoropin <a.vatoro...@crpt.ru>
---
v1 -> v2: also change return type of svm_range_get_info() per Felix Kuehling 
suggestion
Thank you for the patch. It seems you lost the change in 
kfd_criu_checkpoint_svm from the first version. Was that accidental or 
were you planning to send another patch?

I'm also having trouble applying your patches automatically with git am. 
Apparently you're using DOS CR-LF line endings, and --no-keep-cr isn't 
helping. I finally made it work with "git am --quoted-cr=strip ...". 
Please check your workflow for generating patches.

Thanks,
  Felix


  drivers/gpu/drm/amd/amdkfd/kfd_chardev.c |  4 +---
  drivers/gpu/drm/amd/amdkfd/kfd_svm.c     |  7 ++-----
  drivers/gpu/drm/amd/amdkfd/kfd_svm.h     | 11 +++++------
  3 files changed, 8 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c 
b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
index 1e9dd00620bf..a2149afa5803 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
@@ -2039,9 +2039,7 @@ static int criu_get_process_object_info(struct 
kfd_process *p,
  
  	num_events = kfd_get_num_events(p);
  
-	ret = svm_range_get_info(p, &num_svm_ranges, &svm_priv_data_size);
-       if (ret)
-               return ret;
+       svm_range_get_info(p, &num_svm_ranges, &svm_priv_data_size);
  
  	*num_objects = num_queues + num_events + num_svm_ranges;
  
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
index 100717a98ec1..1b7d57a1b245 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
@@ -4076,8 +4076,8 @@ int kfd_criu_restore_svm(struct kfd_process *p,
        return ret;
  }
  
-int svm_range_get_info(struct kfd_process *p, uint32_t *num_svm_ranges,
-                      uint64_t *svm_priv_data_size)
+void svm_range_get_info(struct kfd_process *p, uint32_t *num_svm_ranges,
+                       uint64_t *svm_priv_data_size)
  {
        uint64_t total_size, accessibility_size, common_attr_size;
        int nattr_common = 4, nattr_accessibility = 1;
@@ -4089,8 +4089,6 @@ int svm_range_get_info(struct kfd_process *p, uint32_t 
*num_svm_ranges,
        *svm_priv_data_size = 0;
  
  	svms = &p->svms;
-       if (!svms)
-               return -EINVAL;
  
  	mutex_lock(&svms->lock);
        list_for_each_entry(prange, &svms->list, list) {
@@ -4132,7 +4130,6 @@ int svm_range_get_info(struct kfd_process *p, uint32_t 
*num_svm_ranges,
  
  	pr_debug("num_svm_ranges %u total_priv_size %llu\n", *num_svm_ranges,
                 *svm_priv_data_size);
-       return 0;
  }
  
  int kfd_criu_checkpoint_svm(struct kfd_process *p,
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_svm.h 
b/drivers/gpu/drm/amd/amdkfd/kfd_svm.h
index 6ea23c78009c..01c7a4877904 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_svm.h
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_svm.h
@@ -184,8 +184,8 @@ void schedule_deferred_list_work(struct svm_range_list 
*svms);
  void svm_range_dma_unmap_dev(struct device *dev, dma_addr_t *dma_addr,
                         unsigned long offset, unsigned long npages);
  void svm_range_dma_unmap(struct svm_range *prange);
-int svm_range_get_info(struct kfd_process *p, uint32_t *num_svm_ranges,
-                      uint64_t *svm_priv_data_size);
+void svm_range_get_info(struct kfd_process *p, uint32_t *num_svm_ranges,
+                       uint64_t *svm_priv_data_size);
  int kfd_criu_checkpoint_svm(struct kfd_process *p,
                            uint8_t __user *user_priv_data,
                            uint64_t *priv_offset);
@@ -237,13 +237,12 @@ static inline int svm_range_schedule_evict_svm_bo(
        return -EINVAL;
  }
  
-static inline int svm_range_get_info(struct kfd_process *p,
-                                    uint32_t *num_svm_ranges,
-                                    uint64_t *svm_priv_data_size)
+static inline void svm_range_get_info(struct kfd_process *p,
+                                     uint32_t *num_svm_ranges,
+                                     uint64_t *svm_priv_data_size)
  {
        *num_svm_ranges = 0;
        *svm_priv_data_size = 0;
-       return 0;
  }
  
  static inline int kfd_criu_checkpoint_svm(struct kfd_process *p,