* [PATCH] drm/virtio: add worker for object release
@ 2019-08-30 6:01 Gerd Hoffmann
2019-09-03 16:39 ` Chia-I Wu
0 siblings, 1 reply; 2+ messages in thread
From: Gerd Hoffmann @ 2019-08-30 6:01 UTC (permalink / raw)
To: dri-devel
Cc: olvaffe, Gerd Hoffmann, David Airlie, Daniel Vetter,
open list:VIRTIO GPU DRIVER, open list
Move object release into a separate worker. Releasing objects requires
sending commands to the host. Doing that in the dequeue worker will
cause deadlocks in case the command queue gets filled up, because the
dequeue worker is also the one which will free up slots in the command
queue.
Reported-by: Chia-I Wu <olvaffe@gmail.com>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
drivers/gpu/drm/virtio/virtgpu_drv.h | 8 ++++++++
drivers/gpu/drm/virtio/virtgpu_gem.c | 27 +++++++++++++++++++++++++++
drivers/gpu/drm/virtio/virtgpu_kms.c | 6 ++++++
drivers/gpu/drm/virtio/virtgpu_vq.c | 2 +-
4 files changed, 42 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.h b/drivers/gpu/drm/virtio/virtgpu_drv.h
index fb35831ed351..314e02f94d9c 100644
--- a/drivers/gpu/drm/virtio/virtgpu_drv.h
+++ b/drivers/gpu/drm/virtio/virtgpu_drv.h
@@ -78,6 +78,7 @@ struct virtio_gpu_object {
struct virtio_gpu_object_array {
struct ww_acquire_ctx ticket;
+ struct list_head next;
u32 nents, total;
struct drm_gem_object *objs[];
};
@@ -197,6 +198,10 @@ struct virtio_gpu_device {
struct work_struct config_changed_work;
+ struct work_struct obj_free_work;
+ spinlock_t obj_free_lock;
+ struct list_head obj_free_list;
+
struct virtio_gpu_drv_capset *capsets;
uint32_t num_capsets;
struct list_head cap_cache;
@@ -246,6 +251,9 @@ void virtio_gpu_array_unlock_resv(struct virtio_gpu_object_array *objs);
void virtio_gpu_array_add_fence(struct virtio_gpu_object_array *objs,
struct dma_fence *fence);
void virtio_gpu_array_put_free(struct virtio_gpu_object_array *objs);
+void virtio_gpu_array_put_free_delayed(struct virtio_gpu_device *vgdev,
+ struct virtio_gpu_object_array *objs);
+void virtio_gpu_array_put_free_work(struct work_struct *work);
/* virtio vg */
int virtio_gpu_alloc_vbufs(struct virtio_gpu_device *vgdev);
diff --git a/drivers/gpu/drm/virtio/virtgpu_gem.c b/drivers/gpu/drm/virtio/virtgpu_gem.c
index b812094ae916..4c1f579edfb3 100644
--- a/drivers/gpu/drm/virtio/virtgpu_gem.c
+++ b/drivers/gpu/drm/virtio/virtgpu_gem.c
@@ -239,3 +239,30 @@ void virtio_gpu_array_put_free(struct virtio_gpu_object_array *objs)
drm_gem_object_put_unlocked(objs->objs[i]);
virtio_gpu_array_free(objs);
}
+
+void virtio_gpu_array_put_free_delayed(struct virtio_gpu_device *vgdev,
+ struct virtio_gpu_object_array *objs)
+{
+ spin_lock(&vgdev->obj_free_lock);
+ list_add_tail(&objs->next, &vgdev->obj_free_list);
+ spin_unlock(&vgdev->obj_free_lock);
+ schedule_work(&vgdev->obj_free_work);
+}
+
+void virtio_gpu_array_put_free_work(struct work_struct *work)
+{
+ struct virtio_gpu_device *vgdev =
+ container_of(work, struct virtio_gpu_device, obj_free_work);
+ struct virtio_gpu_object_array *objs;
+
+ spin_lock(&vgdev->obj_free_lock);
+ while (!list_empty(&vgdev->obj_free_list)) {
+ objs = list_first_entry(&vgdev->obj_free_list,
+ struct virtio_gpu_object_array, next);
+ list_del(&objs->next);
+ spin_unlock(&vgdev->obj_free_lock);
+ virtio_gpu_array_put_free(objs);
+ spin_lock(&vgdev->obj_free_lock);
+ }
+ spin_unlock(&vgdev->obj_free_lock);
+}
diff --git a/drivers/gpu/drm/virtio/virtgpu_kms.c b/drivers/gpu/drm/virtio/virtgpu_kms.c
index 231c4e27b3b3..0b3cdb0d83b0 100644
--- a/drivers/gpu/drm/virtio/virtgpu_kms.c
+++ b/drivers/gpu/drm/virtio/virtgpu_kms.c
@@ -147,6 +147,11 @@ int virtio_gpu_init(struct drm_device *dev)
INIT_WORK(&vgdev->config_changed_work,
virtio_gpu_config_changed_work_func);
+ INIT_WORK(&vgdev->obj_free_work,
+ virtio_gpu_array_put_free_work);
+ INIT_LIST_HEAD(&vgdev->obj_free_list);
+ spin_lock_init(&vgdev->obj_free_lock);
+
#ifdef __LITTLE_ENDIAN
if (virtio_has_feature(vgdev->vdev, VIRTIO_GPU_F_VIRGL))
vgdev->has_virgl_3d = true;
@@ -226,6 +231,7 @@ void virtio_gpu_deinit(struct drm_device *dev)
{
struct virtio_gpu_device *vgdev = dev->dev_private;
+ flush_work(&vgdev->obj_free_work);
vgdev->vqs_ready = false;
flush_work(&vgdev->ctrlq.dequeue_work);
flush_work(&vgdev->cursorq.dequeue_work);
diff --git a/drivers/gpu/drm/virtio/virtgpu_vq.c b/drivers/gpu/drm/virtio/virtgpu_vq.c
index ecf57df965b0..595fa6ec2d58 100644
--- a/drivers/gpu/drm/virtio/virtgpu_vq.c
+++ b/drivers/gpu/drm/virtio/virtgpu_vq.c
@@ -227,7 +227,7 @@ void virtio_gpu_dequeue_ctrl_func(struct work_struct *work)
list_for_each_entry_safe(entry, tmp, &reclaim_list, list) {
if (entry->objs)
- virtio_gpu_array_put_free(entry->objs);
+ virtio_gpu_array_put_free_delayed(vgdev, entry->objs);
list_del(&entry->list);
free_vbuf(vgdev, entry);
}
--
2.18.1
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH] drm/virtio: add worker for object release
2019-08-30 6:01 [PATCH] drm/virtio: add worker for object release Gerd Hoffmann
@ 2019-09-03 16:39 ` Chia-I Wu
0 siblings, 0 replies; 2+ messages in thread
From: Chia-I Wu @ 2019-09-03 16:39 UTC (permalink / raw)
To: Gerd Hoffmann
Cc: ML dri-devel, David Airlie, Daniel Vetter,
open list:VIRTIO GPU DRIVER, open list
On Thu, Aug 29, 2019 at 11:01 PM Gerd Hoffmann <kraxel@redhat.com> wrote:
>
> Move object release into a separate worker. Releasing objects requires
> sending commands to the host. Doing that in the dequeue worker will
> cause deadlocks in case the command queue gets filled up, because the
> dequeue worker is also the one which will free up slots in the command
> queue.
>
> Reported-by: Chia-I Wu <olvaffe@gmail.com>
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Tested-by: Chia-I Wu <olvaffe@gmail.com>
Reviewed-by: Chia-I Wu <olvaffe@gmail.com>
> ---
> drivers/gpu/drm/virtio/virtgpu_drv.h | 8 ++++++++
> drivers/gpu/drm/virtio/virtgpu_gem.c | 27 +++++++++++++++++++++++++++
> drivers/gpu/drm/virtio/virtgpu_kms.c | 6 ++++++
> drivers/gpu/drm/virtio/virtgpu_vq.c | 2 +-
> 4 files changed, 42 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.h b/drivers/gpu/drm/virtio/virtgpu_drv.h
> index fb35831ed351..314e02f94d9c 100644
> --- a/drivers/gpu/drm/virtio/virtgpu_drv.h
> +++ b/drivers/gpu/drm/virtio/virtgpu_drv.h
> @@ -78,6 +78,7 @@ struct virtio_gpu_object {
>
> struct virtio_gpu_object_array {
> struct ww_acquire_ctx ticket;
> + struct list_head next;
> u32 nents, total;
> struct drm_gem_object *objs[];
> };
> @@ -197,6 +198,10 @@ struct virtio_gpu_device {
>
> struct work_struct config_changed_work;
>
> + struct work_struct obj_free_work;
> + spinlock_t obj_free_lock;
> + struct list_head obj_free_list;
> +
> struct virtio_gpu_drv_capset *capsets;
> uint32_t num_capsets;
> struct list_head cap_cache;
> @@ -246,6 +251,9 @@ void virtio_gpu_array_unlock_resv(struct virtio_gpu_object_array *objs);
> void virtio_gpu_array_add_fence(struct virtio_gpu_object_array *objs,
> struct dma_fence *fence);
> void virtio_gpu_array_put_free(struct virtio_gpu_object_array *objs);
> +void virtio_gpu_array_put_free_delayed(struct virtio_gpu_device *vgdev,
> + struct virtio_gpu_object_array *objs);
> +void virtio_gpu_array_put_free_work(struct work_struct *work);
>
> /* virtio vg */
> int virtio_gpu_alloc_vbufs(struct virtio_gpu_device *vgdev);
> diff --git a/drivers/gpu/drm/virtio/virtgpu_gem.c b/drivers/gpu/drm/virtio/virtgpu_gem.c
> index b812094ae916..4c1f579edfb3 100644
> --- a/drivers/gpu/drm/virtio/virtgpu_gem.c
> +++ b/drivers/gpu/drm/virtio/virtgpu_gem.c
> @@ -239,3 +239,30 @@ void virtio_gpu_array_put_free(struct virtio_gpu_object_array *objs)
> drm_gem_object_put_unlocked(objs->objs[i]);
> virtio_gpu_array_free(objs);
> }
> +
> +void virtio_gpu_array_put_free_delayed(struct virtio_gpu_device *vgdev,
> + struct virtio_gpu_object_array *objs)
> +{
> + spin_lock(&vgdev->obj_free_lock);
> + list_add_tail(&objs->next, &vgdev->obj_free_list);
> + spin_unlock(&vgdev->obj_free_lock);
> + schedule_work(&vgdev->obj_free_work);
> +}
> +
> +void virtio_gpu_array_put_free_work(struct work_struct *work)
> +{
> + struct virtio_gpu_device *vgdev =
> + container_of(work, struct virtio_gpu_device, obj_free_work);
> + struct virtio_gpu_object_array *objs;
> +
> + spin_lock(&vgdev->obj_free_lock);
> + while (!list_empty(&vgdev->obj_free_list)) {
> + objs = list_first_entry(&vgdev->obj_free_list,
> + struct virtio_gpu_object_array, next);
> + list_del(&objs->next);
> + spin_unlock(&vgdev->obj_free_lock);
> + virtio_gpu_array_put_free(objs);
> + spin_lock(&vgdev->obj_free_lock);
> + }
> + spin_unlock(&vgdev->obj_free_lock);
> +}
> diff --git a/drivers/gpu/drm/virtio/virtgpu_kms.c b/drivers/gpu/drm/virtio/virtgpu_kms.c
> index 231c4e27b3b3..0b3cdb0d83b0 100644
> --- a/drivers/gpu/drm/virtio/virtgpu_kms.c
> +++ b/drivers/gpu/drm/virtio/virtgpu_kms.c
> @@ -147,6 +147,11 @@ int virtio_gpu_init(struct drm_device *dev)
> INIT_WORK(&vgdev->config_changed_work,
> virtio_gpu_config_changed_work_func);
>
> + INIT_WORK(&vgdev->obj_free_work,
> + virtio_gpu_array_put_free_work);
> + INIT_LIST_HEAD(&vgdev->obj_free_list);
> + spin_lock_init(&vgdev->obj_free_lock);
> +
> #ifdef __LITTLE_ENDIAN
> if (virtio_has_feature(vgdev->vdev, VIRTIO_GPU_F_VIRGL))
> vgdev->has_virgl_3d = true;
> @@ -226,6 +231,7 @@ void virtio_gpu_deinit(struct drm_device *dev)
> {
> struct virtio_gpu_device *vgdev = dev->dev_private;
>
> + flush_work(&vgdev->obj_free_work);
> vgdev->vqs_ready = false;
> flush_work(&vgdev->ctrlq.dequeue_work);
> flush_work(&vgdev->cursorq.dequeue_work);
> diff --git a/drivers/gpu/drm/virtio/virtgpu_vq.c b/drivers/gpu/drm/virtio/virtgpu_vq.c
> index ecf57df965b0..595fa6ec2d58 100644
> --- a/drivers/gpu/drm/virtio/virtgpu_vq.c
> +++ b/drivers/gpu/drm/virtio/virtgpu_vq.c
> @@ -227,7 +227,7 @@ void virtio_gpu_dequeue_ctrl_func(struct work_struct *work)
>
> list_for_each_entry_safe(entry, tmp, &reclaim_list, list) {
> if (entry->objs)
> - virtio_gpu_array_put_free(entry->objs);
> + virtio_gpu_array_put_free_delayed(vgdev, entry->objs);
> list_del(&entry->list);
> free_vbuf(vgdev, entry);
> }
> --
> 2.18.1
>
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2019-09-03 16:39 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-08-30 6:01 [PATCH] drm/virtio: add worker for object release Gerd Hoffmann
2019-09-03 16:39 ` Chia-I Wu
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).