[PATCH 0/4] Improve virtio ID allocation

[PATCH 0/4] Improve virtio ID allocation

[Matthew Wilcox]

I noticed you were using IDRs where you could be using the more efficient
IDAs, then while fixing that I noticed the lack of error handling,
and I decided to follow that up with an efficiency improvement.

There's probably a v2 of this to follow because I couldn't figure
out how to properly handle one of the error cases ... see the comment
embedded in one of the patches.

Matthew Wilcox (4):
  drm/virtio: Replace IDRs with IDAs
  drm/virtio: Handle context ID allocation errors
  drm/virtio: Handle object ID allocation errors
  drm/virtio: Use IDAs more efficiently

 drivers/gpu/drm/virtio/virtgpu_drv.h   |  9 ++---
 drivers/gpu/drm/virtio/virtgpu_fb.c    | 10 ++++--
 drivers/gpu/drm/virtio/virtgpu_gem.c   | 10 ++++--
 drivers/gpu/drm/virtio/virtgpu_ioctl.c |  5 ++-
 drivers/gpu/drm/virtio/virtgpu_kms.c   | 46 +++++++++-----------------
 drivers/gpu/drm/virtio/virtgpu_vq.c    | 19 ++++-------
 6 files changed, 44 insertions(+), 55 deletions(-)

-- 
2.19.0

[PATCH 1/4] drm/virtio: Replace IDRs with IDAs

[Matthew Wilcox]

These IDRs were only being used to allocate unique numbers, not to look
up pointers, so they can use the more space-efficient IDA instead.

Signed-off-by: Matthew Wilcox <willy@infradead.org>
---
 drivers/gpu/drm/virtio/virtgpu_drv.h |  6 ++----
 drivers/gpu/drm/virtio/virtgpu_kms.c | 18 ++++--------------
 drivers/gpu/drm/virtio/virtgpu_vq.c  | 12 ++----------
 3 files changed, 8 insertions(+), 28 deletions(-)

diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.h b/drivers/gpu/drm/virtio/virtgpu_drv.h
index 65605e207bbe..c4468a4e454e 100644
--- a/drivers/gpu/drm/virtio/virtgpu_drv.h
+++ b/drivers/gpu/drm/virtio/virtgpu_drv.h
@@ -180,8 +180,7 @@ struct virtio_gpu_device {
 	struct kmem_cache *vbufs;
 	bool vqs_ready;
 
-	struct idr	resource_idr;
-	spinlock_t resource_idr_lock;
+	struct ida	resource_ida;
 
 	wait_queue_head_t resp_wq;
 	/* current display info */
@@ -190,8 +189,7 @@ struct virtio_gpu_device {
 
 	struct virtio_gpu_fence_driver fence_drv;
 
-	struct idr	ctx_id_idr;
-	spinlock_t ctx_id_idr_lock;
+	struct ida	ctx_id_ida;
 
 	bool has_virgl_3d;
 
diff --git a/drivers/gpu/drm/virtio/virtgpu_kms.c b/drivers/gpu/drm/virtio/virtgpu_kms.c
index 65060c08522d..e2604fe1b4ae 100644
--- a/drivers/gpu/drm/virtio/virtgpu_kms.c
+++ b/drivers/gpu/drm/virtio/virtgpu_kms.c
@@ -55,21 +55,13 @@ static void virtio_gpu_config_changed_work_func(struct work_struct *work)
 static void virtio_gpu_ctx_id_get(struct virtio_gpu_device *vgdev,
 				  uint32_t *resid)
 {
-	int handle;
-
-	idr_preload(GFP_KERNEL);
-	spin_lock(&vgdev->ctx_id_idr_lock);
-	handle = idr_alloc(&vgdev->ctx_id_idr, NULL, 1, 0, 0);
-	spin_unlock(&vgdev->ctx_id_idr_lock);
-	idr_preload_end();
+	int handle = ida_alloc_min(&vgdev->ctx_id_ida, 1, GFP_KERNEL);
 	*resid = handle;
 }
 
 static void virtio_gpu_ctx_id_put(struct virtio_gpu_device *vgdev, uint32_t id)
 {
-	spin_lock(&vgdev->ctx_id_idr_lock);
-	idr_remove(&vgdev->ctx_id_idr, id);
-	spin_unlock(&vgdev->ctx_id_idr_lock);
+	ida_free(&vgdev->ctx_id_ida, id);
 }
 
 static void virtio_gpu_context_create(struct virtio_gpu_device *vgdev,
@@ -151,10 +143,8 @@ int virtio_gpu_driver_load(struct drm_device *dev, unsigned long flags)
 	vgdev->dev = dev->dev;
 
 	spin_lock_init(&vgdev->display_info_lock);
-	spin_lock_init(&vgdev->ctx_id_idr_lock);
-	idr_init(&vgdev->ctx_id_idr);
-	spin_lock_init(&vgdev->resource_idr_lock);
-	idr_init(&vgdev->resource_idr);
+	ida_init(&vgdev->ctx_id_ida);
+	ida_init(&vgdev->resource_ida);
 	init_waitqueue_head(&vgdev->resp_wq);
 	virtio_gpu_init_vq(&vgdev->ctrlq, virtio_gpu_dequeue_ctrl_func);
 	virtio_gpu_init_vq(&vgdev->cursorq, virtio_gpu_dequeue_cursor_func);
diff --git a/drivers/gpu/drm/virtio/virtgpu_vq.c b/drivers/gpu/drm/virtio/virtgpu_vq.c
index 020070d483d3..58be09d2eed6 100644
--- a/drivers/gpu/drm/virtio/virtgpu_vq.c
+++ b/drivers/gpu/drm/virtio/virtgpu_vq.c
@@ -41,21 +41,13 @@
 void virtio_gpu_resource_id_get(struct virtio_gpu_device *vgdev,
 				uint32_t *resid)
 {
-	int handle;
-
-	idr_preload(GFP_KERNEL);
-	spin_lock(&vgdev->resource_idr_lock);
-	handle = idr_alloc(&vgdev->resource_idr, NULL, 1, 0, GFP_NOWAIT);
-	spin_unlock(&vgdev->resource_idr_lock);
-	idr_preload_end();
+	int handle = ida_alloc_min(&vgdev->resource_ida, 1, GFP_KERNEL);
 	*resid = handle;
 }
 
 void virtio_gpu_resource_id_put(struct virtio_gpu_device *vgdev, uint32_t id)
 {
-	spin_lock(&vgdev->resource_idr_lock);
-	idr_remove(&vgdev->resource_idr, id);
-	spin_unlock(&vgdev->resource_idr_lock);
+	ida_free(&vgdev->resource_ida, id);
 }
 
 void virtio_gpu_ctrl_ack(struct virtqueue *vq)
-- 
2.19.0

[PATCH 2/4] drm/virtio: Handle context ID allocation errors

[Matthew Wilcox]

It is possible to run out of memory while allocating IDs.  The current
code would create a context with an invalid ID; change it to return
-ENOMEM to userspace.

Signed-off-by: Matthew Wilcox <willy@infradead.org>
---
 drivers/gpu/drm/virtio/virtgpu_kms.c | 29 +++++++++++-----------------
 1 file changed, 11 insertions(+), 18 deletions(-)

diff --git a/drivers/gpu/drm/virtio/virtgpu_kms.c b/drivers/gpu/drm/virtio/virtgpu_kms.c
index e2604fe1b4ae..bf609dcae224 100644
--- a/drivers/gpu/drm/virtio/virtgpu_kms.c
+++ b/drivers/gpu/drm/virtio/virtgpu_kms.c
@@ -52,31 +52,22 @@ static void virtio_gpu_config_changed_work_func(struct work_struct *work)
 		      events_clear, &events_clear);
 }
 
-static void virtio_gpu_ctx_id_get(struct virtio_gpu_device *vgdev,
-				  uint32_t *resid)
+static int virtio_gpu_context_create(struct virtio_gpu_device *vgdev,
+				      uint32_t nlen, const char *name)
 {
 	int handle = ida_alloc_min(&vgdev->ctx_id_ida, 1, GFP_KERNEL);
-	*resid = handle;
-}
 
-static void virtio_gpu_ctx_id_put(struct virtio_gpu_device *vgdev, uint32_t id)
-{
-	ida_free(&vgdev->ctx_id_ida, id);
-}
-
-static void virtio_gpu_context_create(struct virtio_gpu_device *vgdev,
-				      uint32_t nlen, const char *name,
-				      uint32_t *ctx_id)
-{
-	virtio_gpu_ctx_id_get(vgdev, ctx_id);
-	virtio_gpu_cmd_context_create(vgdev, *ctx_id, nlen, name);
+	if (handle < 0)
+		return handle;
+	virtio_gpu_cmd_context_create(vgdev, handle, nlen, name);
+	return handle;
 }
 
 static void virtio_gpu_context_destroy(struct virtio_gpu_device *vgdev,
 				      uint32_t ctx_id)
 {
 	virtio_gpu_cmd_context_destroy(vgdev, ctx_id);
-	virtio_gpu_ctx_id_put(vgdev, ctx_id);
+	ida_free(&vgdev->ctx_id_ida, ctx_id);
 }
 
 static void virtio_gpu_init_vq(struct virtio_gpu_queue *vgvq,
@@ -261,7 +252,7 @@ int virtio_gpu_driver_open(struct drm_device *dev, struct drm_file *file)
 {
 	struct virtio_gpu_device *vgdev = dev->dev_private;
 	struct virtio_gpu_fpriv *vfpriv;
-	uint32_t id;
+	int id;
 	char dbgname[TASK_COMM_LEN];
 
 	/* can't create contexts without 3d renderer */
@@ -274,7 +265,9 @@ int virtio_gpu_driver_open(struct drm_device *dev, struct drm_file *file)
 		return -ENOMEM;
 
 	get_task_comm(dbgname, current);
-	virtio_gpu_context_create(vgdev, strlen(dbgname), dbgname, &id);
+	id = virtio_gpu_context_create(vgdev, strlen(dbgname), dbgname);
+	if (id < 0)
+		return id;
 
 	vfpriv->ctx_id = id;
 	file->driver_priv = vfpriv;
-- 
2.19.0

[PATCH 3/4] drm/virtio: Handle object ID allocation errors

[Matthew Wilcox]

It is possible to run out of memory while allocating IDs.  The current
code would create an object with an invalid ID; change it to return
-ENOMEM to the caller.

Signed-off-by: Matthew Wilcox <willy@infradead.org>
---
 drivers/gpu/drm/virtio/virtgpu_drv.h   |  3 +--
 drivers/gpu/drm/virtio/virtgpu_fb.c    | 10 ++++++++--
 drivers/gpu/drm/virtio/virtgpu_gem.c   | 10 ++++++++--
 drivers/gpu/drm/virtio/virtgpu_ioctl.c |  5 ++++-
 drivers/gpu/drm/virtio/virtgpu_vq.c    |  6 ++----
 5 files changed, 23 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.h b/drivers/gpu/drm/virtio/virtgpu_drv.h
index c4468a4e454e..0a3392f2cda3 100644
--- a/drivers/gpu/drm/virtio/virtgpu_drv.h
+++ b/drivers/gpu/drm/virtio/virtgpu_drv.h
@@ -247,8 +247,7 @@ int virtio_gpu_surface_dirty(struct virtio_gpu_framebuffer *qfb,
 /* virtio vg */
 int virtio_gpu_alloc_vbufs(struct virtio_gpu_device *vgdev);
 void virtio_gpu_free_vbufs(struct virtio_gpu_device *vgdev);
-void virtio_gpu_resource_id_get(struct virtio_gpu_device *vgdev,
-			       uint32_t *resid);
+int virtio_gpu_resource_id_get(struct virtio_gpu_device *vgdev);
 void virtio_gpu_resource_id_put(struct virtio_gpu_device *vgdev, uint32_t id);
 void virtio_gpu_cmd_create_resource(struct virtio_gpu_device *vgdev,
 				    uint32_t resource_id,
diff --git a/drivers/gpu/drm/virtio/virtgpu_fb.c b/drivers/gpu/drm/virtio/virtgpu_fb.c
index a121b1c79522..74d815483487 100644
--- a/drivers/gpu/drm/virtio/virtgpu_fb.c
+++ b/drivers/gpu/drm/virtio/virtgpu_fb.c
@@ -244,14 +244,17 @@ static int virtio_gpufb_create(struct drm_fb_helper *helper,
 	if (IS_ERR(obj))
 		return PTR_ERR(obj);
 
-	virtio_gpu_resource_id_get(vgdev, &resid);
+	ret = virtio_gpu_resource_id_get(vgdev);
+	if (ret < 0)
+		goto err_obj_vmap;
+	resid = ret;
 	virtio_gpu_cmd_create_resource(vgdev, resid, format,
 				       mode_cmd.width, mode_cmd.height);
 
 	ret = virtio_gpu_vmap_fb(vgdev, obj);
 	if (ret) {
 		DRM_ERROR("failed to vmap fb %d\n", ret);
-		goto err_obj_vmap;
+		goto err_obj_id;
 	}
 
 	/* attach the object to the resource */
@@ -293,8 +296,11 @@ static int virtio_gpufb_create(struct drm_fb_helper *helper,
 err_fb_alloc:
 	virtio_gpu_cmd_resource_inval_backing(vgdev, resid);
 err_obj_attach:
+err_obj_id:
+	virtio_gpu_resource_id_put(vgdev, resid);
 err_obj_vmap:
 	virtio_gpu_gem_free_object(&obj->gem_base);
+
 	return ret;
 }
 
diff --git a/drivers/gpu/drm/virtio/virtgpu_gem.c b/drivers/gpu/drm/virtio/virtgpu_gem.c
index 0f2768eacaee..9e3af1ec26db 100644
--- a/drivers/gpu/drm/virtio/virtgpu_gem.c
+++ b/drivers/gpu/drm/virtio/virtgpu_gem.c
@@ -100,7 +100,10 @@ int virtio_gpu_mode_dumb_create(struct drm_file *file_priv,
 		goto fail;
 
 	format = virtio_gpu_translate_format(DRM_FORMAT_XRGB8888);
-	virtio_gpu_resource_id_get(vgdev, &resid);
+	ret = virtio_gpu_resource_id_get(vgdev);
+	if (ret < 0)
+		goto fail;
+	resid = ret;
 	virtio_gpu_cmd_create_resource(vgdev, resid, format,
 				       args->width, args->height);
 
@@ -108,13 +111,16 @@ int virtio_gpu_mode_dumb_create(struct drm_file *file_priv,
 	obj = gem_to_virtio_gpu_obj(gobj);
 	ret = virtio_gpu_object_attach(vgdev, obj, resid, NULL);
 	if (ret)
-		goto fail;
+		goto fail_id;
 
 	obj->dumb = true;
 	args->pitch = pitch;
 	return ret;
 
+fail_id:
+	virtio_gpu_resource_id_put(vgdev, resid);
 fail:
+	/* Shouldn't we undo virtio_gpu_gem_create()? */
 	return ret;
 }
 
diff --git a/drivers/gpu/drm/virtio/virtgpu_ioctl.c b/drivers/gpu/drm/virtio/virtgpu_ioctl.c
index 7bdf6f0e58a5..eec9f09f01f0 100644
--- a/drivers/gpu/drm/virtio/virtgpu_ioctl.c
+++ b/drivers/gpu/drm/virtio/virtgpu_ioctl.c
@@ -244,7 +244,10 @@ static int virtio_gpu_resource_create_ioctl(struct drm_device *dev, void *data,
 	INIT_LIST_HEAD(&validate_list);
 	memset(&mainbuf, 0, sizeof(struct ttm_validate_buffer));
 
-	virtio_gpu_resource_id_get(vgdev, &res_id);
+	ret = virtio_gpu_resource_id_get(vgdev);
+	if (ret < 0)
+		return ret;
+	res_id = ret;
 
 	size = rc->size;
 
diff --git a/drivers/gpu/drm/virtio/virtgpu_vq.c b/drivers/gpu/drm/virtio/virtgpu_vq.c
index 58be09d2eed6..387951c971d4 100644
--- a/drivers/gpu/drm/virtio/virtgpu_vq.c
+++ b/drivers/gpu/drm/virtio/virtgpu_vq.c
@@ -38,11 +38,9 @@
 			       + MAX_INLINE_CMD_SIZE		 \
 			       + MAX_INLINE_RESP_SIZE)
 
-void virtio_gpu_resource_id_get(struct virtio_gpu_device *vgdev,
-				uint32_t *resid)
+int virtio_gpu_resource_id_get(struct virtio_gpu_device *vgdev)
 {
-	int handle = ida_alloc_min(&vgdev->resource_ida, 1, GFP_KERNEL);
-	*resid = handle;
+	return ida_alloc_min(&vgdev->resource_ida, 1, GFP_KERNEL);
 }
 
 void virtio_gpu_resource_id_put(struct virtio_gpu_device *vgdev, uint32_t id)
-- 
2.19.0

[PATCH 4/4] drm/virtio: Use IDAs more efficiently

[Matthew Wilcox]

0-based IDAs are more efficient than any other base.  Convert the
1-based IDAs to be 0-based.

Signed-off-by: Matthew Wilcox <willy@infradead.org>
---
 drivers/gpu/drm/virtio/virtgpu_kms.c | 3 ++-
 drivers/gpu/drm/virtio/virtgpu_vq.c  | 7 +++++--
 2 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/virtio/virtgpu_kms.c b/drivers/gpu/drm/virtio/virtgpu_kms.c
index bf609dcae224..b576c9ef6323 100644
--- a/drivers/gpu/drm/virtio/virtgpu_kms.c
+++ b/drivers/gpu/drm/virtio/virtgpu_kms.c
@@ -59,6 +59,7 @@ static int virtio_gpu_context_create(struct virtio_gpu_device *vgdev,
 
 	if (handle < 0)
 		return handle;
+	handle++;
 	virtio_gpu_cmd_context_create(vgdev, handle, nlen, name);
 	return handle;
 }
@@ -67,7 +68,7 @@ static void virtio_gpu_context_destroy(struct virtio_gpu_device *vgdev,
 				      uint32_t ctx_id)
 {
 	virtio_gpu_cmd_context_destroy(vgdev, ctx_id);
-	ida_free(&vgdev->ctx_id_ida, ctx_id);
+	ida_free(&vgdev->ctx_id_ida, ctx_id - 1);
 }
 
 static void virtio_gpu_init_vq(struct virtio_gpu_queue *vgvq,
diff --git a/drivers/gpu/drm/virtio/virtgpu_vq.c b/drivers/gpu/drm/virtio/virtgpu_vq.c
index 387951c971d4..81297fe0147d 100644
--- a/drivers/gpu/drm/virtio/virtgpu_vq.c
+++ b/drivers/gpu/drm/virtio/virtgpu_vq.c
@@ -40,12 +40,15 @@
 
 int virtio_gpu_resource_id_get(struct virtio_gpu_device *vgdev)
 {
-	return ida_alloc_min(&vgdev->resource_ida, 1, GFP_KERNEL);
+	int handle = ida_alloc(&vgdev->resource_ida, GFP_KERNEL);
+	if (handle < 0)
+		return handle;
+	return handle + 1;
 }
 
 void virtio_gpu_resource_id_put(struct virtio_gpu_device *vgdev, uint32_t id)
 {
-	ida_free(&vgdev->resource_ida, id);
+	ida_free(&vgdev->resource_ida, id - 1);
 }
 
 void virtio_gpu_ctrl_ack(struct virtqueue *vq)
-- 
2.19.0

Re: [PATCH 4/4] drm/virtio: Use IDAs more efficiently

[Matthew Wilcox]

On Wed, Sep 26, 2018 at 09:00:31AM -0700, Matthew Wilcox wrote:
> @@ -59,6 +59,7 @@ static int virtio_gpu_context_create(struct virtio_gpu_device *vgdev,
>  
>  	if (handle < 0)
>  		return handle;
> +	handle++;
>  	virtio_gpu_cmd_context_create(vgdev, handle, nlen, name);
>  	return handle;
>  }

Uh.  This line is missing.

-       int handle = ida_alloc_min(&vgdev->ctx_id_ida, 1, GFP_KERNEL);
+       int handle = ida_alloc(&vgdev->ctx_id_ida, GFP_KERNEL);

It'll be there in v2 ;-)

Re: [PATCH 4/4] drm/virtio: Use IDAs more efficiently

[Gerd Hoffmann]

On Wed, Sep 26, 2018 at 09:04:55AM -0700, Matthew Wilcox wrote:
> On Wed, Sep 26, 2018 at 09:00:31AM -0700, Matthew Wilcox wrote:
> > @@ -59,6 +59,7 @@ static int virtio_gpu_context_create(struct virtio_gpu_device *vgdev,
> >  
> >  	if (handle < 0)
> >  		return handle;
> > +	handle++;
> >  	virtio_gpu_cmd_context_create(vgdev, handle, nlen, name);
> >  	return handle;
> >  }
> 
> Uh.  This line is missing.
> 
> -       int handle = ida_alloc_min(&vgdev->ctx_id_ida, 1, GFP_KERNEL);
> +       int handle = ida_alloc(&vgdev->ctx_id_ida, GFP_KERNEL);
> 
> It'll be there in v2 ;-)

I've touched the resource/object id handling too, see my "drm/virtio:
rework ttm resource handling" patch series
(https://patchwork.freedesktop.org/series/50382/).  Which still needs a
review btw.

I think that series obsoletes patch 3/4 (object id fixes) of your
series.  The other patches should rebase without too much trouble, you
could do that as well when preparing v2 ...

cheers,
  Gerd

Re: [PATCH 4/4] drm/virtio: Use IDAs more efficiently

[Matthew Wilcox]

On Tue, Oct 02, 2018 at 01:43:28PM +0200, Gerd Hoffmann wrote:
> On Wed, Sep 26, 2018 at 09:04:55AM -0700, Matthew Wilcox wrote:
> > On Wed, Sep 26, 2018 at 09:00:31AM -0700, Matthew Wilcox wrote:
> > > @@ -59,6 +59,7 @@ static int virtio_gpu_context_create(struct virtio_gpu_device *vgdev,
> > >  
> > >  	if (handle < 0)
> > >  		return handle;
> > > +	handle++;
> > >  	virtio_gpu_cmd_context_create(vgdev, handle, nlen, name);
> > >  	return handle;
> > >  }
> > 
> > Uh.  This line is missing.
> > 
> > -       int handle = ida_alloc_min(&vgdev->ctx_id_ida, 1, GFP_KERNEL);
> > +       int handle = ida_alloc(&vgdev->ctx_id_ida, GFP_KERNEL);
> > 
> > It'll be there in v2 ;-)
> 
> I've touched the resource/object id handling too, see my "drm/virtio:
> rework ttm resource handling" patch series
> (https://patchwork.freedesktop.org/series/50382/).  Which still needs a
> review btw.

Um, according to patchwork, you only posted it yesterday.  Does DRM
normally expect a review within 24 hours?

> I think that series obsoletes patch 3/4 (object id fixes) of your
> series.  The other patches should rebase without too much trouble, you
> could do that as well when preparing v2 ...

It seems a little odd to me to expect a drive-by contributor (ie me) to
rebase their patches on top of a patch series which wasn't even posted
at the time they contributed their original patch.  If it was already
in -next, that'd be a reasonable request.

Re: [PATCH 0/4] Improve virtio ID allocation

[Gerd Hoffmann]

On Wed, Sep 26, 2018 at 09:00:27AM -0700, Matthew Wilcox wrote:
> I noticed you were using IDRs where you could be using the more efficient
> IDAs, then while fixing that I noticed the lack of error handling,
> and I decided to follow that up with an efficiency improvement.
> 
> There's probably a v2 of this to follow because I couldn't figure
> out how to properly handle one of the error cases ... see the comment
> embedded in one of the patches.

#1 + #2 pushed to drm-misc-next now.
#3 should not be needed any more.
waiting for v2 of #4 ...

cheers,
  Gerd

Re: [PATCH 0/4] Improve virtio ID allocation

[Matthew Wilcox]

On Mon, Oct 29, 2018 at 10:53:39PM +0100, Gerd Hoffmann wrote:
> On Wed, Sep 26, 2018 at 09:00:27AM -0700, Matthew Wilcox wrote:
> > I noticed you were using IDRs where you could be using the more efficient
> > IDAs, then while fixing that I noticed the lack of error handling,
> > and I decided to follow that up with an efficiency improvement.
> > 
> > There's probably a v2 of this to follow because I couldn't figure
> > out how to properly handle one of the error cases ... see the comment
> > embedded in one of the patches.
> 
> #1 + #2 pushed to drm-misc-next now.
> #3 should not be needed any more.
> waiting for v2 of #4 ...

Thanks!  I think we do still need a small part of #3.  Patches in
replies to this email.

[PATCH v2 1/2] drm/virtio: Handle error from virtio_gpu_resource_id_get

[Matthew Wilcox]

ida_alloc() can return -ENOMEM in the highly unlikely case we run out
of memory.  The current code creates an object with an invalid ID.

Signed-off-by: Matthew Wilcox <willy@infradead.org>
---
 drivers/gpu/drm/virtio/virtgpu_object.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/virtio/virtgpu_object.c b/drivers/gpu/drm/virtio/virtgpu_object.c
index 77eac4eb06b1..5ac42dded217 100644
--- a/drivers/gpu/drm/virtio/virtgpu_object.c
+++ b/drivers/gpu/drm/virtio/virtgpu_object.c
@@ -25,11 +25,16 @@
 
 #include "virtgpu_drv.h"
 
-static void virtio_gpu_resource_id_get(struct virtio_gpu_device *vgdev,
+static int virtio_gpu_resource_id_get(struct virtio_gpu_device *vgdev,
 				       uint32_t *resid)
 {
 	int handle = ida_alloc_min(&vgdev->resource_ida, 1, GFP_KERNEL);
+
+	if (handle < 0)
+		return handle;
+
 	*resid = handle;
+	return 0;
 }
 
 static void virtio_gpu_resource_id_put(struct virtio_gpu_device *vgdev, uint32_t id)
@@ -94,7 +99,11 @@ int virtio_gpu_object_create(struct virtio_gpu_device *vgdev,
 	bo = kzalloc(sizeof(struct virtio_gpu_object), GFP_KERNEL);
 	if (bo == NULL)
 		return -ENOMEM;
-	virtio_gpu_resource_id_get(vgdev, &bo->hw_res_handle);
+	ret = virtio_gpu_resource_id_get(vgdev, &bo->hw_res_handle);
+	if (ret < 0) {
+		kfree(bo);
+		return ret;
+	}
 	size = roundup(size, PAGE_SIZE);
 	ret = drm_gem_object_init(vgdev->ddev, &bo->gem_base, size);
 	if (ret != 0) {
-- 
2.19.1

[PATCH v2 2/2] drm/virtio: Use IDAs more efficiently

[Matthew Wilcox]

0-based IDAs are more efficient than any other base.  Convert the
1-based IDAs to be 0-based.

Signed-off-by: Matthew Wilcox <willy@infradead.org>
---
 drivers/gpu/drm/virtio/virtgpu_kms.c    | 5 +++--
 drivers/gpu/drm/virtio/virtgpu_object.c | 6 +++---
 2 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/virtio/virtgpu_kms.c b/drivers/gpu/drm/virtio/virtgpu_kms.c
index bf609dcae224..8118f10fde4a 100644
--- a/drivers/gpu/drm/virtio/virtgpu_kms.c
+++ b/drivers/gpu/drm/virtio/virtgpu_kms.c
@@ -55,10 +55,11 @@ static void virtio_gpu_config_changed_work_func(struct work_struct *work)
 static int virtio_gpu_context_create(struct virtio_gpu_device *vgdev,
 				      uint32_t nlen, const char *name)
 {
-	int handle = ida_alloc_min(&vgdev->ctx_id_ida, 1, GFP_KERNEL);
+	int handle = ida_alloc(&vgdev->ctx_id_ida, GFP_KERNEL);
 
 	if (handle < 0)
 		return handle;
+	handle += 1;
 	virtio_gpu_cmd_context_create(vgdev, handle, nlen, name);
 	return handle;
 }
@@ -67,7 +68,7 @@ static void virtio_gpu_context_destroy(struct virtio_gpu_device *vgdev,
 				      uint32_t ctx_id)
 {
 	virtio_gpu_cmd_context_destroy(vgdev, ctx_id);
-	ida_free(&vgdev->ctx_id_ida, ctx_id);
+	ida_free(&vgdev->ctx_id_ida, ctx_id - 1);
 }
 
 static void virtio_gpu_init_vq(struct virtio_gpu_queue *vgvq,
diff --git a/drivers/gpu/drm/virtio/virtgpu_object.c b/drivers/gpu/drm/virtio/virtgpu_object.c
index 5ac42dded217..f39a183d59c2 100644
--- a/drivers/gpu/drm/virtio/virtgpu_object.c
+++ b/drivers/gpu/drm/virtio/virtgpu_object.c
@@ -28,18 +28,18 @@
 static int virtio_gpu_resource_id_get(struct virtio_gpu_device *vgdev,
 				       uint32_t *resid)
 {
-	int handle = ida_alloc_min(&vgdev->resource_ida, 1, GFP_KERNEL);
+	int handle = ida_alloc(&vgdev->resource_ida, GFP_KERNEL);
 
 	if (handle < 0)
 		return handle;
 
-	*resid = handle;
+	*resid = handle + 1;
 	return 0;
 }
 
 static void virtio_gpu_resource_id_put(struct virtio_gpu_device *vgdev, uint32_t id)
 {
-	ida_free(&vgdev->resource_ida, id);
+	ida_free(&vgdev->resource_ida, id - 1);
 }
 
 static void virtio_gpu_ttm_bo_destroy(struct ttm_buffer_object *tbo)
-- 
2.19.1