[PATCH] drm/virtio: delay pinning the pages till first use

[Gerd Hoffmann]

On Tue, Nov 02, 2021 at 12:31:39PM +0100, Maksym Wezdecki wrote:
> From: mwezdeck <maksym.wezdecki at 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