On Tue, Nov 02, 2021 at 12:31:39PM +0100, Maksym Wezdecki wrote:
> From: mwezdeck <maksym.wezde...@collabora.co.uk>
> 
> The idea behind the commit:
>   1. not pin the pages during resource_create ioctl
>   2. pin the pages on the first use during:
>       - transfer_*_host ioctl
>         - map ioctl

i.e. basically lazy pinning.  Approach looks sane to me.

>   3. introduce new ioctl for pinning pages on demand

What is the use case for this ioctl?
In any case this should be a separate patch.

> +     struct virtio_gpu_object_array *objs;
> +     struct virtio_gpu_object *bo;
> +     struct virtio_gpu_object_shmem *shmem;
> +
> +     objs = virtio_gpu_array_from_handles(file, &virtio_gpu_map->handle, 1);
> +     if (objs == NULL)
> +             return -ENOENT;
> +
> +     bo = gem_to_virtio_gpu_obj(objs->objs[0]);
> +     if (bo == NULL)
> +             return -ENOENT;
> +     
> +     shmem = to_virtio_gpu_shmem(bo);
> +     if (shmem == NULL)
> +             return -ENOENT;
> +
> +     if (!shmem->pages) {
> +             virtio_gpu_object_pin(vgdev, objs, 1);
> +     }

Move this into virtio_gpu_object_pin(),
or create a helper function for it ...

> +     objs = virtio_gpu_array_from_handles(file, &virtio_gpu_pin->handle, 1);
> +     if (objs == NULL)
> +             return -ENOENT;
> +
> +     bo = gem_to_virtio_gpu_obj(objs->objs[0]);
> +     if (bo == NULL)
> +             return -ENOENT;
> +     
> +     shmem = to_virtio_gpu_shmem(bo);
> +     if (shmem == NULL)
> +             return -ENOENT;
> +
> +     if (!shmem->pages) {
> +             return virtio_gpu_object_pin(vgdev, objs, 1);
> +     }

... to avoid this code duplication?

> +int virtio_gpu_object_pin(struct virtio_gpu_device *vgdev,
> +                       struct virtio_gpu_object_array *objs,
> +                       int num_gem_objects)
> +{
> +     int i, ret;
> +
> +     for (i = 0; i < num_gem_objects; i++) {

> +             ret = virtio_gpu_object_shmem_init(vgdev, bo, &ents, &nents);
> +             if (ret != 0) {
> +                     return -EFAULT;
> +             }
> +
> +             virtio_gpu_object_attach(vgdev, bo, ents, nents);

I think it is enough to do the virtio_gpu_object_attach() call lazily.
virtio_gpu_object_shmem_init() should not actually allocate pages, that
only happens when virtio_gpu_object_attach() goes ask for a scatter
list.

take care,
  Gerd

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Reply via email to