* [PATCH 1/4] drm/virtio: Replace IDRs with IDAs
2018-09-26 16:00 [PATCH 0/4] Improve virtio ID allocation Matthew Wilcox
@ 2018-09-26 16:00 ` Matthew Wilcox
2018-09-26 16:00 ` [PATCH 2/4] drm/virtio: Handle context ID allocation errors Matthew Wilcox
` (3 subsequent siblings)
4 siblings, 0 replies; 12+ messages in thread
From: Matthew Wilcox @ 2018-09-26 16:00 UTC (permalink / raw)
To: David Airlie, Gerd Hoffmann
Cc: Matthew Wilcox, dri-devel, virtualization, linux-kernel
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
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH 2/4] drm/virtio: Handle context ID allocation errors
2018-09-26 16:00 [PATCH 0/4] Improve virtio ID allocation Matthew Wilcox
2018-09-26 16:00 ` [PATCH 1/4] drm/virtio: Replace IDRs with IDAs Matthew Wilcox
@ 2018-09-26 16:00 ` Matthew Wilcox
2018-09-26 16:00 ` [PATCH 3/4] drm/virtio: Handle object " Matthew Wilcox
` (2 subsequent siblings)
4 siblings, 0 replies; 12+ messages in thread
From: Matthew Wilcox @ 2018-09-26 16:00 UTC (permalink / raw)
To: David Airlie, Gerd Hoffmann
Cc: Matthew Wilcox, dri-devel, virtualization, linux-kernel
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
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH 3/4] drm/virtio: Handle object ID allocation errors
2018-09-26 16:00 [PATCH 0/4] Improve virtio ID allocation Matthew Wilcox
2018-09-26 16:00 ` [PATCH 1/4] drm/virtio: Replace IDRs with IDAs Matthew Wilcox
2018-09-26 16:00 ` [PATCH 2/4] drm/virtio: Handle context ID allocation errors Matthew Wilcox
@ 2018-09-26 16:00 ` Matthew Wilcox
2018-09-26 16:00 ` [PATCH 4/4] drm/virtio: Use IDAs more efficiently Matthew Wilcox
2018-10-29 21:53 ` [PATCH 0/4] Improve virtio ID allocation Gerd Hoffmann
4 siblings, 0 replies; 12+ messages in thread
From: Matthew Wilcox @ 2018-09-26 16:00 UTC (permalink / raw)
To: David Airlie, Gerd Hoffmann
Cc: Matthew Wilcox, dri-devel, virtualization, linux-kernel
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
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH 4/4] drm/virtio: Use IDAs more efficiently
2018-09-26 16:00 [PATCH 0/4] Improve virtio ID allocation Matthew Wilcox
` (2 preceding siblings ...)
2018-09-26 16:00 ` [PATCH 3/4] drm/virtio: Handle object " Matthew Wilcox
@ 2018-09-26 16:00 ` Matthew Wilcox
2018-09-26 16:04 ` Matthew Wilcox
2018-10-29 21:53 ` [PATCH 0/4] Improve virtio ID allocation Gerd Hoffmann
4 siblings, 1 reply; 12+ messages in thread
From: Matthew Wilcox @ 2018-09-26 16:00 UTC (permalink / raw)
To: David Airlie, Gerd Hoffmann
Cc: Matthew Wilcox, dri-devel, virtualization, linux-kernel
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
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH 4/4] drm/virtio: Use IDAs more efficiently
2018-09-26 16:00 ` [PATCH 4/4] drm/virtio: Use IDAs more efficiently Matthew Wilcox
@ 2018-09-26 16:04 ` Matthew Wilcox
2018-10-02 11:43 ` Gerd Hoffmann
0 siblings, 1 reply; 12+ messages in thread
From: Matthew Wilcox @ 2018-09-26 16:04 UTC (permalink / raw)
To: David Airlie, Gerd Hoffmann; +Cc: dri-devel, virtualization, linux-kernel
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 ;-)
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 4/4] drm/virtio: Use IDAs more efficiently
2018-09-26 16:04 ` Matthew Wilcox
@ 2018-10-02 11:43 ` Gerd Hoffmann
2018-10-02 12:55 ` Matthew Wilcox
0 siblings, 1 reply; 12+ messages in thread
From: Gerd Hoffmann @ 2018-10-02 11:43 UTC (permalink / raw)
To: Matthew Wilcox; +Cc: David Airlie, dri-devel, virtualization, linux-kernel
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
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 4/4] drm/virtio: Use IDAs more efficiently
2018-10-02 11:43 ` Gerd Hoffmann
@ 2018-10-02 12:55 ` Matthew Wilcox
0 siblings, 0 replies; 12+ messages in thread
From: Matthew Wilcox @ 2018-10-02 12:55 UTC (permalink / raw)
To: Gerd Hoffmann; +Cc: David Airlie, dri-devel, virtualization, linux-kernel
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.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 0/4] Improve virtio ID allocation
2018-09-26 16:00 [PATCH 0/4] Improve virtio ID allocation Matthew Wilcox
` (3 preceding siblings ...)
2018-09-26 16:00 ` [PATCH 4/4] drm/virtio: Use IDAs more efficiently Matthew Wilcox
@ 2018-10-29 21:53 ` Gerd Hoffmann
2018-10-30 16:52 ` Matthew Wilcox
2018-10-30 16:53 ` [PATCH v2 1/2] drm/virtio: Handle error from virtio_gpu_resource_id_get Matthew Wilcox
4 siblings, 2 replies; 12+ messages in thread
From: Gerd Hoffmann @ 2018-10-29 21:53 UTC (permalink / raw)
To: Matthew Wilcox; +Cc: David Airlie, dri-devel, virtualization, linux-kernel
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
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH 0/4] Improve virtio ID allocation
2018-10-29 21:53 ` [PATCH 0/4] Improve virtio ID allocation Gerd Hoffmann
@ 2018-10-30 16:52 ` Matthew Wilcox
2018-10-30 16:53 ` [PATCH v2 1/2] drm/virtio: Handle error from virtio_gpu_resource_id_get Matthew Wilcox
1 sibling, 0 replies; 12+ messages in thread
From: Matthew Wilcox @ 2018-10-30 16:52 UTC (permalink / raw)
To: Gerd Hoffmann; +Cc: David Airlie, dri-devel, virtualization, linux-kernel
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.
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v2 1/2] drm/virtio: Handle error from virtio_gpu_resource_id_get
2018-10-29 21:53 ` [PATCH 0/4] Improve virtio ID allocation Gerd Hoffmann
2018-10-30 16:52 ` Matthew Wilcox
@ 2018-10-30 16:53 ` Matthew Wilcox
2018-10-30 16:53 ` [PATCH v2 2/2] drm/virtio: Use IDAs more efficiently Matthew Wilcox
1 sibling, 1 reply; 12+ messages in thread
From: Matthew Wilcox @ 2018-10-30 16:53 UTC (permalink / raw)
To: David Airlie, Gerd Hoffmann, dri-devel, virtualization, linux-kernel
Cc: 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
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH v2 2/2] drm/virtio: Use IDAs more efficiently
2018-10-30 16:53 ` [PATCH v2 1/2] drm/virtio: Handle error from virtio_gpu_resource_id_get Matthew Wilcox
@ 2018-10-30 16:53 ` Matthew Wilcox
0 siblings, 0 replies; 12+ messages in thread
From: Matthew Wilcox @ 2018-10-30 16:53 UTC (permalink / raw)
To: David Airlie, Gerd Hoffmann, dri-devel, virtualization, linux-kernel
Cc: 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
^ permalink raw reply related [flat|nested] 12+ messages in thread