This series was originally motivated by a deadlock, introduced in commit 1d7c71a3e2f77336df536855b0efd2dc5bdeb41b 'drm/nouveau/disp: port vblank handling to event interface', due to inverted lock order between nouveau_drm_vblank_enable() and nouveau_drm_vblank_handler() (the complete lockdep report is included in the patch 4/5 changelog).
Because this series fixes the vblank event deadlock, it is a competing solution to Maarten Lankhorst's 'drm/nouveau: fix vblank deadlock'.
This series fixes the vblank event deadlock by converting the event handler list to RCU. This solution allows the event trigger to be lockless, and thus avoiding the lock inversion.
Typical hurdles to RCU conversion include: 1) ensuring the object lifetime exceeds the lockless access; 2) preventing premature object reuse; and 3) verifying that stale object use not present logical errors.
Because object reuse is not safe in RCU-based operations, the nouveau_event_get/_put interface is migrated from add/remove semantics to enable/disable semantics with a separate install/remove step (which also serves to document the handler lifetime). This also corrects an unsafe interface design where handlers can mistakenly be reused while in use.
The nouveau driver currently supports 4 events -- gpio, uevent, cevent, and vblank. Every event is created by and stored within its respective subdev/engine object -- gpio in the GPIO subdev, uevent and cevent in the FIFO engine, and vblank in the DISP engine. Thus event lifetime extends until the subdev is destructed during devobj teardown.
Event handler lifetime varies and is detailed in the table below (apologies for the wide-format):
Event Handler function Container Until ----- ---------------- --------------- ------------------ gpio nouveau_connector_hotplug struct nouveau_connector nouveau_connector_destroy uevent nouveau_fence_wait_uevent_handler local stack object nouveau_fence_wait_uevent returns cevent none n/a n/a vblank nouveau_drm_vblank_handler struct nouveau_drm nouveau_drm_remove vblank nv50_software_vblsem_release struct nouveau_software_chan _nouveau_engctx_dtor (call stack originates with nouveau_abi16_chan_free ioctl) vblank nvc0_software_vblsem_release struct nouveau_software_chan same as above
RCU lifetime considerations for handlers:
Event Handler Lifetime ----- ---------------- --------------------------------- gpio nouveau_connector_hotplug kfree_rcu(nv_connector) uevent nouveau_fence_wait_uevent_handler explicit use of nouveau_event_hander_create/_destroy cevent none n/a vblank nouveau_drm_vblank_handler synchronize_rcu() in nouveau_drm_unload vblank nv50_software_vblsem_release synchronize_rcu() in container dtor vblank nvc0_software_vblsem_release synchronize_rcu() in container dtor
synchronize_rcu() is used for nouveau_object-based containers for which kfree_rcu() is not suitable/possible.
Stale event handler execution is not a concern for the existing handlers because either: 1) the handler is responsible for disabling itself (via returning NVKM_EVENT_DROP), or 2) the existing handler can already be stale, as is the case with nouveau_connector_hotplug, which only schedules a work item, and nouveau_drm_vblank_handler, which the drm core expects may be stale.
Peter Hurley (9): drm/nouveau: Add priv field for event handlers drm/nouveau: Move event index check from critical section drm/nouveau: Allocate local event handlers drm/nouveau: Allow asymmetric nouveau_event_get/_put drm/nouveau: Add install/remove semantics for event handlers drm/nouveau: Convert event handler list to RCU drm/nouveau: Fold nouveau_event_put_locked into caller drm/nouveau: Simplify event interface drm/nouveau: Simplify event handler interface
drivers/gpu/drm/nouveau/core/core/event.c | 121 +++++++++++++++++---- .../gpu/drm/nouveau/core/engine/software/nv50.c | 32 ++++-- .../gpu/drm/nouveau/core/engine/software/nvc0.c | 32 ++++-- drivers/gpu/drm/nouveau/core/include/core/event.h | 27 ++++- .../gpu/drm/nouveau/core/include/engine/software.h | 2 +- drivers/gpu/drm/nouveau/nouveau_connector.c | 16 ++- drivers/gpu/drm/nouveau/nouveau_display.c | 16 +-- drivers/gpu/drm/nouveau/nouveau_drm.c | 35 +++--- drivers/gpu/drm/nouveau/nouveau_fence.c | 27 ++--- 9 files changed, 220 insertions(+), 88 deletions(-)