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

On Tue, Nov 02, 2021 at 12:31:39PM +0100, Maksym Wezdecki wrote:

From: mwezdeck maksym.wezdecki@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