Re: [RFC 14/26] plugins/amdgpu: Remove plugin_log_msg()

"Francis, David" <[email protected]> Mon, 2 Mar 2026 14:25:14 +0000
Newsgroups dev.linux.lists.criu
Message-ID <SA1PR12MB81448A382D608996571F0683EF7EA@SA1PR12MB8144.namprd12.prod.outlook.com>

>> I suspect the sdma_copy_bo ones will end up being spammy but they can live for now.

> Perhaps it would be enough to move those pr_debugs under the relevant
> failure paths? That is, only for the error logs which can fail due VA
> conflicts. For the success case I don't think it is too useful to have
> them. And the "entering and leaving" function remove altogether?

Sounds good to me. As far as I know, these debug prints never get enabled,
so no one will raise a fuss at their removal.

________________________________________
From: Tvrtko Ursulin <[email protected]>
Sent: Monday, March 2, 2026 4:09 AM
To: Francis, David; [email protected]
Subject: Re: [RFC 14/26] plugins/amdgpu: Remove plugin_log_msg()


On 27/02/2026 20:28, Francis, David wrote:
> I suspect the sdma_copy_bo ones will end up being spammy but they can live for now.

Perhaps it would be enough to move those pr_debugs under the relevant
failure paths? That is, only for the error logs which can fail due VA
conflicts. For the success case I don't think it is too useful to have
them. And the "entering and leaving" function remove altogether?

Regards,

Tvrtko

> This patch is
> Reviewed-By: David Francis <[email protected]>
>
> ________________________________________
> From: Tvrtko Ursulin <[email protected]>
> Sent: Friday, February 20, 2026 7:05 AM
> To: [email protected]
> Cc: Francis, David; Tvrtko Ursulin
> Subject: [RFC 14/26] plugins/amdgpu: Remove plugin_log_msg()
>
> The extra debug build log helper does not seem very useful and some of
> them do not even compile. Lets just remove it and replace with the
> standard pr_debug. In case of too much noise, we can later re-evaluate
> to remove some of the not very useful log messages.
>
> Signed-off-by: Tvrtko Ursulin <[email protected]>
> ---
>   plugins/amdgpu/amdgpu_plugin.c      | 42 ++++++++++++++++-------------
>   plugins/amdgpu/amdgpu_plugin_drm.c  |  5 ++--
>   plugins/amdgpu/amdgpu_plugin_util.h |  9 -------
>   3 files changed, 27 insertions(+), 29 deletions(-)
>
> diff --git a/plugins/amdgpu/amdgpu_plugin.c b/plugins/amdgpu/amdgpu_plugin.c
> index 06df63de2434..b3cc3a12ce2e 100644
> --- a/plugins/amdgpu/amdgpu_plugin.c
> +++ b/plugins/amdgpu/amdgpu_plugin.c
> @@ -570,7 +570,7 @@ int sdma_copy_bo(int shared_fd, uint64_t size, FILE *storage_fp,
>          src_bo_size = (type == SDMA_OP_VRAM_WRITE) ? buffer_bo_size : size;
>          dst_bo_size = (type == SDMA_OP_VRAM_READ) ? buffer_bo_size : size;
>
> -       plugin_log_msg("Enter %s\n", __func__);
> +       pr_debug("Enter %s\n", __func__);
>
>          /* prepare src buffer */
>          switch (type) {
> @@ -605,7 +605,8 @@ int sdma_copy_bo(int shared_fd, uint64_t size, FILE *storage_fp,
>                  pr_perror("failed to GPU map the src BO");
>                  goto err_src_bo_map;
>          }
> -       plugin_log_msg("Source BO: GPU VA: %lx, size: %lx\n", gpu_addr_src, src_bo_size);
> +       pr_debug("Source BO: GPU VA: %lx, size: %lx\n",
> +                gpu_addr_src, src_bo_size);
>
>          /* prepare dest buffer */
>          switch (type) {
> @@ -640,7 +641,8 @@ int sdma_copy_bo(int shared_fd, uint64_t size, FILE *storage_fp,
>                  pr_perror("failed to GPU map the dest BO");
>                  goto err_dst_bo_map;
>          }
> -       plugin_log_msg("Dest BO: GPU VA: %lx, size: %lx\n", gpu_addr_dst, dst_bo_size);
> +       pr_debug("Dest BO: GPU VA: %lx, size: %lx\n",
> +                gpu_addr_dst, dst_bo_size);
>
>          /* prepare ring buffer/indirect buffer for command submission
>           * each copy packet is 7 dwords so we need to alloc 28x size for ib
> @@ -651,7 +653,8 @@ int sdma_copy_bo(int shared_fd, uint64_t size, FILE *storage_fp,
>                  pr_perror("failed to allocate and map ib/rb");
>                  goto err_ib_gpu_alloc;
>          }
> -       plugin_log_msg("Indirect BO: GPU VA: %lx, size: %lx\n", gpu_addr_ib, packets_per_buffer * 28);
> +       pr_debug("Indirect BO: GPU VA: %" PRIx64 ", size: %d\n",
> +                gpu_addr_ib, packets_per_buffer * 28);
>
>          resources[0] = h_bo_src;
>          resources[1] = h_bo_dst;
> @@ -792,7 +795,7 @@ err_src_va:
>          err2 = amdgpu_bo_free(h_bo_src);
>          if (err2)
>                  pr_perror("src bo free failed");
> -       plugin_log_msg("Leaving sdma_copy_bo, err = %d\n", err);
> +       pr_debug("Leaving sdma_copy_bo, err = %d\n", err);
>          return err;
>   }
>
> @@ -818,7 +821,7 @@ void *dump_bo_contents(void *_thread_data)
>                  pr_perror("failed to initialize device");
>                  goto exit;
>          }
> -       plugin_log_msg("libdrm initialized successfully\n");
> +       pr_debug("libdrm initialized successfully\n");
>
>          ret = amdgpu_query_gpu_info(h_dev, &gpu_info);
>          if (ret) {
> @@ -910,7 +913,7 @@ void *restore_bo_contents(void *_thread_data)
>                  pr_perror("failed to initialize device");
>                  goto exit;
>          }
> -       plugin_log_msg("libdrm initialized successfully\n");
> +       pr_debug("libdrm initialized successfully\n");
>
>          ret = amdgpu_query_gpu_info(h_dev, &gpu_info);
>          if (ret) {
> @@ -970,7 +973,8 @@ void *restore_bo_contents(void *_thread_data)
>                          pr_err("Failed to fill the BO using sDMA: bo_buckets[%d]\n", i);
>                          break;
>                  }
> -               plugin_log_msg("** Successfully filled the BO using sDMA: bo_buckets[%d] **\n", i);
> +               pr_debug("** Successfully filled the BO using sDMA: bo_buckets[%d] **\n",
> +                        i);
>          }
>
>   exit:
> @@ -1004,7 +1008,7 @@ int check_hsakmt_shared_mem(uint64_t *shared_mem_size, uint32_t *shared_mem_magi
>          if (ret)
>                  pr_perror("Failed to read shared mem magic");
>          else
> -               plugin_log_msg("Shared mem magic:0x%x\n", *shared_mem_magic);
> +               pr_debug("Shared mem magic:0x%x\n", *shared_mem_magic);
>
>          return 0;
>   }
> @@ -1644,8 +1648,9 @@ static int restore_bos(struct kfd_ioctl_criu_args *args, CriuKfd *e)
>                  bo_bucket->offset = bo_entry->offset;
>                  bo_bucket->alloc_flags = bo_entry->alloc_flags;
>
> -               plugin_log_msg("BO [%d] gpu_id:%x addr:%llx size:%llx offset:%llx\n", i, bo_bucket->gpu_id,
> -                              bo_bucket->addr, bo_bucket->size, bo_bucket->offset);
> +               pr_debug("BO [%d] gpu_id:%x addr:%" PRIx64 " size:%" PRIx64 " offset:%" PRIx64 "\n",
> +                        i, bo_bucket->gpu_id, bo_bucket->addr, bo_bucket->size,
> +                        bo_bucket->offset);
>          }
>
>          pr_info("Restore BOs Ok\n");
> @@ -1713,9 +1718,9 @@ static int restore_bo_data(int id, struct kfd_criu_bo_bucket *bo_buckets, CriuKf
>                          vma_md->new_pgoff = bo_bucket->restored_offset;
>                          vma_md->fd = node_get_drm_render_device(tp_node);
>
> -                       plugin_log_msg("adding vma_entry:addr:0x%lx old-off:0x%lx "
> -                                      "new_off:0x%lx new_minor:%d\n",
> -                                      vma_md->vma_entry, vma_md->old_pgoff, vma_md->new_pgoff, tp_node->drm_render_minor);
> +                       pr_debug("adding vma_entry:addr:0x%lx old-off:0x%lx new_off:0x%lx new_minor:%d\n",
> +                                vma_md->vma_entry, vma_md->old_pgoff,
> +                                vma_md->new_pgoff, tp_node->drm_render_minor);
>
>                          list_add_tail(&vma_md->list, &update_vma_info_list);
>                  }
> @@ -1987,7 +1992,7 @@ int amdgpu_plugin_restore_file(int id, bool *retry_needed)
>                  return -1;
>          }
>
> -       plugin_log_msg("read image file data\n");
> +       pr_debug("read image file data\n");
>
>          /*
>           * Initialize fd_next to be 1 greater than the biggest file descriptor in use by the target restore process.
> @@ -2094,7 +2099,7 @@ int amdgpu_plugin_update_vmamap(const char *in_path, const uint64_t addr, const
>          if (plugin_disabled)
>                  return -ENOTSUP;
>
> -       plugin_log_msg("Enter %s\n", __func__);
> +       pr_debug("Enter %s\n", __func__);
>
>          strncpy(path, in_path, sizeof(path));
>
> @@ -2137,8 +2142,9 @@ int amdgpu_plugin_update_vmamap(const char *in_path, const uint64_t addr, const
>                                  *updated_fd = fd;
>                          }
>
> -                       plugin_log_msg("old_pgoff=0x%lx new_pgoff=0x%lx fd=%d\n", vma_md->old_pgoff, vma_md->new_pgoff,
> -                                      *updated_fd);
> +                       pr_debug("old_pgoff=0x%lx new_pgoff=0x%lx fd=%d\n",
> +                                vma_md->old_pgoff, vma_md->new_pgoff,
> +                                *updated_fd);
>
>                          return 1;
>                  }
> diff --git a/plugins/amdgpu/amdgpu_plugin_drm.c b/plugins/amdgpu/amdgpu_plugin_drm.c
> index baa560537813..d0f84b46997c 100644
> --- a/plugins/amdgpu/amdgpu_plugin_drm.c
> +++ b/plugins/amdgpu/amdgpu_plugin_drm.c
> @@ -173,7 +173,7 @@ static int restore_bo_contents_drm(int drm_render_minor, CriuRenderNode *rd, int
>                  pr_perror("failed to initialize device");
>                  goto exit;
>          }
> -       plugin_log_msg("libdrm initialized successfully\n");
> +       pr_debug("libdrm initialized successfully\n");
>
>          ret = amdgpu_query_gpu_info(h_dev, &gpu_info);
>          if (ret) {
> @@ -222,7 +222,8 @@ static int restore_bo_contents_drm(int drm_render_minor, CriuRenderNode *rd, int
>                          pr_err("Failed to fill the BO using sDMA: bo_buckets[%d]\n", i);
>                          break;
>                  }
> -               plugin_log_msg("** Successfully filled the BO using sDMA: bo_buckets[%d] **\n", i);
> +               pr_debug("** Successfully filled the BO using sDMA: bo_buckets[%d] **\n",
> +                        i);
>
>                  if (bo_contents_fp)
>                          fclose(bo_contents_fp);
> diff --git a/plugins/amdgpu/amdgpu_plugin_util.h b/plugins/amdgpu/amdgpu_plugin_util.h
> index f5f752d0b0a6..014abf35ccb6 100644
> --- a/plugins/amdgpu/amdgpu_plugin_util.h
> +++ b/plugins/amdgpu/amdgpu_plugin_util.h
> @@ -25,15 +25,6 @@
>   #endif
>   #define LOG_PREFIX "amdgpu_plugin: "
>
> -#ifdef DEBUG
> -#define plugin_log_msg(fmt, ...) pr_debug(fmt, ##__VA_ARGS__)
> -#else
> -#define plugin_log_msg(fmt, ...) \
> -       {                        \
> -       }
> -#endif
> -
> -
>   /* Path where KFD device is surfaced */
>   #define AMDGPU_KFD_DEVICE              "/dev/kfd"
>
> --
> 2.52.0
>