On Thu, Aug 29, 2019 at 11:01 PM Gerd Hoffmann <kraxel at 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 at gmail.com> > Signed-off-by: Gerd Hoffmann <kraxel at redhat.com>Tested-by: Chia-I Wu <olvaffe at gmail.com> Reviewed-by: Chia-I Wu <olvaffe at 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 >