mbox series

[v9,00/18] drm/virtio: switch from ttm to gem shmem helpers.

Message ID 20190829103301.3539-1-kraxel@redhat.com (mailing list archive)
Headers show
Series drm/virtio: switch from ttm to gem shmem helpers. | expand

Message

Gerd Hoffmann Aug. 29, 2019, 10:32 a.m. UTC
ttm increasingly gets into the way while hacking on virtio-gpu memory
management.  It also overkill for what virtio-gpu needs.  Lets get rid
of it.

v9:
 - rebase to latest dem-misc-next, adapt to changes.
 - fix issues found by Chia-I Wu.
v8:
 - rebase to latest drm-misc-next, adapt to changes.
v7:
 - rebase to latest drm-misc-next
 - reorder patches: switch all virtio commands to object array helpers
   first. then drop ttm, to make sure we don't release objects still in
   use.
 - misc fixes.
v6:
 - largely rewrite fencing logic, using the virtio_gpu_array_* helpers
 - add more patches to the series.
v5:
 - fence bugfixes.
 - minor optimizations.
v4:
 - make gem array helpers private to virtio.
 - misc minor fixes.
v3:
 - add gem array helpers.
 - rework fencing.

please review.

thanks,
  Gerd

Gerd Hoffmann (18):
  drm/virtio: pass gem reservation object to ttm init
  drm/virtio: switch virtio_gpu_wait_ioctl() to gem helper.
  drm/virtio: simplify cursor updates
  drm/virtio: remove virtio_gpu_object_wait
  drm/virtio: drop no_wait argument from virtio_gpu_object_reserve
  drm/virtio: remove ttm calls from in
    virtio_gpu_object_{reserve,unreserve}
  drm/virtio: add virtio_gpu_object_array & helpers
  drm/virtio: rework virtio_gpu_execbuffer_ioctl fencing
  drm/virtio: rework virtio_gpu_object_create fencing
  drm/virtio: rework virtio_gpu_transfer_from_host_ioctl fencing
  drm/virtio: rework virtio_gpu_transfer_to_host_ioctl fencing
  drm/virtio: rework virtio_gpu_cmd_context_{attach,detach}_resource
  drm/virtio: drop virtio_gpu_object_list_validate/virtio_gpu_unref_list
  drm/virtio: switch from ttm to gem shmem helpers
  drm/virtio: remove virtio_gpu_alloc_object
  drm/virtio: drop virtio_gpu_object_{ref,unref}
  drm/virtio: drop virtio_gpu_object_{reserve,unreserve}
  drm/virtio: add fence sanity check

 drivers/gpu/drm/virtio/virtgpu_drv.h    | 123 +++-------
 drivers/gpu/drm/virtio/virtgpu_drv.c    |  20 +-
 drivers/gpu/drm/virtio/virtgpu_fence.c  |   4 +
 drivers/gpu/drm/virtio/virtgpu_gem.c    | 156 ++++++++----
 drivers/gpu/drm/virtio/virtgpu_ioctl.c  | 224 ++++++-----------
 drivers/gpu/drm/virtio/virtgpu_kms.c    |   9 -
 drivers/gpu/drm/virtio/virtgpu_object.c | 222 ++++++-----------
 drivers/gpu/drm/virtio/virtgpu_plane.c  |  34 +--
 drivers/gpu/drm/virtio/virtgpu_prime.c  |  34 ---
 drivers/gpu/drm/virtio/virtgpu_ttm.c    | 305 ------------------------
 drivers/gpu/drm/virtio/virtgpu_vq.c     |  78 ++++--
 drivers/gpu/drm/virtio/Kconfig          |   2 +-
 drivers/gpu/drm/virtio/Makefile         |   2 +-
 13 files changed, 376 insertions(+), 837 deletions(-)
 delete mode 100644 drivers/gpu/drm/virtio/virtgpu_ttm.c

Comments

Chia-I Wu Aug. 29, 2019, 11:44 p.m. UTC | #1
The series is

  Reviewed-by: Chia-I Wu <olvaffe@gmail.com>

However I ran into a deadlock with one GPU-heavy app.  When I exits
Unigine Valley benchmark with ctrl-c, the entire driver locks up
probably 8 out of 10 times on my machine.  When that happens,
virtio_gpu_dequeue_ctrl_func does not return and is blocked inside
virtio_gpu_array_put_free.

It seems, when the vq becomes full or near full between reclaim_vbufs
and virtio_gpu_array_put_free, virtio_gpu_array_put_free can not free
all of the objects because each call to virtio_gpu_free_object needs
to add several commands to vq.  One of the calls ends up being blocked
at "wait_event(vgdev->ctrlq.ack_queue, ...)".


On Thu, Aug 29, 2019 at 3:33 AM Gerd Hoffmann <kraxel@redhat.com> wrote:
>
> ttm increasingly gets into the way while hacking on virtio-gpu memory
> management.  It also overkill for what virtio-gpu needs.  Lets get rid
> of it.
>
> v9:
>  - rebase to latest dem-misc-next, adapt to changes.
>  - fix issues found by Chia-I Wu.
> v8:
>  - rebase to latest drm-misc-next, adapt to changes.
> v7:
>  - rebase to latest drm-misc-next
>  - reorder patches: switch all virtio commands to object array helpers
>    first. then drop ttm, to make sure we don't release objects still in
>    use.
>  - misc fixes.
> v6:
>  - largely rewrite fencing logic, using the virtio_gpu_array_* helpers
>  - add more patches to the series.
> v5:
>  - fence bugfixes.
>  - minor optimizations.
> v4:
>  - make gem array helpers private to virtio.
>  - misc minor fixes.
> v3:
>  - add gem array helpers.
>  - rework fencing.
>
> please review.
>
> thanks,
>   Gerd
>
> Gerd Hoffmann (18):
>   drm/virtio: pass gem reservation object to ttm init
>   drm/virtio: switch virtio_gpu_wait_ioctl() to gem helper.
>   drm/virtio: simplify cursor updates
>   drm/virtio: remove virtio_gpu_object_wait
>   drm/virtio: drop no_wait argument from virtio_gpu_object_reserve
>   drm/virtio: remove ttm calls from in
>     virtio_gpu_object_{reserve,unreserve}
>   drm/virtio: add virtio_gpu_object_array & helpers
>   drm/virtio: rework virtio_gpu_execbuffer_ioctl fencing
>   drm/virtio: rework virtio_gpu_object_create fencing
>   drm/virtio: rework virtio_gpu_transfer_from_host_ioctl fencing
>   drm/virtio: rework virtio_gpu_transfer_to_host_ioctl fencing
>   drm/virtio: rework virtio_gpu_cmd_context_{attach,detach}_resource
>   drm/virtio: drop virtio_gpu_object_list_validate/virtio_gpu_unref_list
>   drm/virtio: switch from ttm to gem shmem helpers
>   drm/virtio: remove virtio_gpu_alloc_object
>   drm/virtio: drop virtio_gpu_object_{ref,unref}
>   drm/virtio: drop virtio_gpu_object_{reserve,unreserve}
>   drm/virtio: add fence sanity check
>
>  drivers/gpu/drm/virtio/virtgpu_drv.h    | 123 +++-------
>  drivers/gpu/drm/virtio/virtgpu_drv.c    |  20 +-
>  drivers/gpu/drm/virtio/virtgpu_fence.c  |   4 +
>  drivers/gpu/drm/virtio/virtgpu_gem.c    | 156 ++++++++----
>  drivers/gpu/drm/virtio/virtgpu_ioctl.c  | 224 ++++++-----------
>  drivers/gpu/drm/virtio/virtgpu_kms.c    |   9 -
>  drivers/gpu/drm/virtio/virtgpu_object.c | 222 ++++++-----------
>  drivers/gpu/drm/virtio/virtgpu_plane.c  |  34 +--
>  drivers/gpu/drm/virtio/virtgpu_prime.c  |  34 ---
>  drivers/gpu/drm/virtio/virtgpu_ttm.c    | 305 ------------------------
>  drivers/gpu/drm/virtio/virtgpu_vq.c     |  78 ++++--
>  drivers/gpu/drm/virtio/Kconfig          |   2 +-
>  drivers/gpu/drm/virtio/Makefile         |   2 +-
>  13 files changed, 376 insertions(+), 837 deletions(-)
>  delete mode 100644 drivers/gpu/drm/virtio/virtgpu_ttm.c
>
> --
> 2.18.1
>
Gerd Hoffmann Aug. 30, 2019, 6:01 a.m. UTC | #2
On Thu, Aug 29, 2019 at 04:44:49PM -0700, Chia-I Wu wrote:
> The series is
> 
>   Reviewed-by: Chia-I Wu <olvaffe@gmail.com>

Thanks.

> However I ran into a deadlock with one GPU-heavy app.  When I exits
> Unigine Valley benchmark with ctrl-c, the entire driver locks up
> probably 8 out of 10 times on my machine.  When that happens,
> virtio_gpu_dequeue_ctrl_func does not return and is blocked inside
> virtio_gpu_array_put_free.
> 
> It seems, when the vq becomes full or near full between reclaim_vbufs
> and virtio_gpu_array_put_free, virtio_gpu_array_put_free can not free
> all of the objects because each call to virtio_gpu_free_object needs
> to add several commands to vq.  One of the calls ends up being blocked
> at "wait_event(vgdev->ctrlq.ack_queue, ...)".

Probably it is simply the large number of objects released at once when
killing the benchmark which causes the queue to fill up.

cheers,
  Gerd