Re: [RFC 13/26] plugins/amdgpu: Consolidate vm_info collection

On 27/02/2026 20:11, Francis, David wrote:
> (Re-sending, hopefully without html content)
> 
> This ioctl shouldn't ever need more than two calls (the first call should give the right size for the second) but this is cleaner.

Right, I also thought it is cleaner to to repeat the args init block and 
all twice, glad you agree.

> Nitpick: criu seems to use "while (1)" more than "for (;;)". Could this be changed here?

Ack, I marked this as a todo for when it is clearer how things will proceed.

> With that change, this is
> 
> Reviewed-By: David Francis <[email protected]>

Thank you!

Tvrtko

> ________________________________________
> From: Tvrtko Ursulin <[email protected]>
> Sent: Friday, February 20, 2026 7:05 AM
> To: [email protected]
> Cc: Francis, David; Tvrtko Ursulin
> Subject: [RFC 13/26] plugins/amdgpu: Consolidate vm_info collection
> 
> Instead of open coding the same ioctl twice we can put it in a loop from
> which we break out once we have allocated enough space for all objects.
> 
> While at it we add error handling for the memory allocation.
> 
> Signed-off-by: Tvrtko Ursulin <[email protected]>
> ---
>   plugins/amdgpu/amdgpu_plugin_drm.c | 47 ++++++++++++++++--------------
>   1 file changed, 25 insertions(+), 22 deletions(-)
> 
> diff --git a/plugins/amdgpu/amdgpu_plugin_drm.c b/plugins/amdgpu/amdgpu_plugin_drm.c
> index 9df755450dc9..baa560537813 100644
> --- a/plugins/amdgpu/amdgpu_plugin_drm.c
> +++ b/plugins/amdgpu/amdgpu_plugin_drm.c
> @@ -302,8 +302,7 @@ int amdgpu_plugin_drm_dump_file(int fd, int id, struct stat *drm)
> 
>          for (int i = 0; i < num_bos; i++) {
>                  int num_vm_entries = 8;
> -               struct drm_amdgpu_gem_vm_entry *vm_info_entries;
> -               struct drm_amdgpu_gem_op vm_info_args = { 0 };
> +               struct drm_amdgpu_gem_vm_entry *vm_info_entries = NULL;
>                  DrmBoEntry *boinfo = rd->bo_entries[i];
>                  struct drm_amdgpu_gem_list_handles_entry handle_entry = list_handles_entries[i];
>                  union drm_amdgpu_gem_mmap mmap_args = { 0 };
> @@ -333,32 +332,36 @@ int amdgpu_plugin_drm_dump_file(int fd, int id, struct stat *drm)
> 
>                  boinfo->offset = mmap_args.out.addr_ptr;
> 
> -               vm_info_entries = xzalloc(sizeof(struct drm_amdgpu_gem_vm_entry) * num_vm_entries);
> -               vm_info_args.handle = handle_entry.gem_handle;
> -               vm_info_args.num_entries = num_vm_entries;
> -               vm_info_args.value = (uintptr_t)vm_info_entries;
> -               vm_info_args.op = AMDGPU_GEM_OP_GET_MAPPING_INFO;
> -               ret = drmIoctl(fd, DRM_IOCTL_AMDGPU_GEM_OP, &vm_info_args);
> -               if (ret) {
> -                       pr_perror("Failed to call vm info ioctl");
> -                       goto exit;
> -               }
> +               for (;;) {
> +                       struct drm_amdgpu_gem_op vm_info_args = {
> +                               .handle = handle_entry.gem_handle,
> +                               .num_entries = num_vm_entries,
> +                               .op = AMDGPU_GEM_OP_GET_MAPPING_INFO,
> +                       };
> +
> +                       if (vm_info_entries)
> +                               xfree(vm_info_entries);
> +                       vm_info_entries = xzalloc(sizeof(*vm_info_entries) *
> +                                                 num_vm_entries);
> +                       if (!vm_info_entries) {
> +                               ret = -ENOMEM;
> +                               goto exit;
> +                       }
> 
> -               if (vm_info_args.num_entries > num_vm_entries) {
> -                       num_vm_entries = vm_info_args.num_entries;
> -                       xfree(vm_info_entries);
> -                       vm_info_entries = xzalloc(sizeof(struct drm_amdgpu_gem_vm_entry) * num_vm_entries);
> -                       vm_info_args.handle = handle_entry.gem_handle;
> -                       vm_info_args.num_entries = num_vm_entries;
>                          vm_info_args.value = (uintptr_t)vm_info_entries;
> -                       vm_info_args.op = AMDGPU_GEM_OP_GET_MAPPING_INFO;
> -                       ret = drmIoctl(fd, DRM_IOCTL_AMDGPU_GEM_OP, &vm_info_args);
> +                       ret = drmIoctl(fd, DRM_IOCTL_AMDGPU_GEM_OP,
> +                                      &vm_info_args);
>                          if (ret) {
>                                  pr_perror("Failed to call vm info ioctl");
>                                  goto exit;
>                          }
> -               } else {
> -                       num_vm_entries = vm_info_args.num_entries;
> +
> +                       if (vm_info_args.num_entries <= num_vm_entries) {
> +                               num_vm_entries = vm_info_args.num_entries;
> +                               break;
> +                       } else {
> +                               num_vm_entries = vm_info_args.num_entries;
> +                       }
>                  }
> 
>                  boinfo->num_of_vms = num_vm_entries;
> --
> 2.52.0
>