Based on the previous discussion while I was writing the Rust
abstractions for the DRM scheduler, it looks like we're overdue for some
documentation.
This series first attempts to document what I've learned about the
scheduler and what I believe should be the *intended* lifetime
semantics, and then fixes a few bugs that result from that:
1. The DRM scheduler fences cannot be required to be outlived by the
scheduler. This is non-negotiable. The whole point of these fences is
to decouple the underlying hardware/driver from consumers, such as
dma-bufs with an attached fence. If this requirement were not met,
then we'd have to somehow keep the scheduler and all the driver
components associated with it alive as long as a dma-buf with an
attached drm_sched fence is alive, which could be indefinitely even
after the hardware that produced that dma-buf is long gone. Consider,
for example, using a hot-pluggable GPU to write to a dma-buf in main
memory, which gets presented on an integrated display controller, and
then the GPU is unplugged. That buffer could potentially live
forever, we can't block GPU driver cleanup on that.
2. Make the DRM scheduler properly clean up jobs on shutdown, such that
we can support the use case of tearing down the scheduler with
in-flight jobs. This is important to cleanly support the firmware
scheduling use case, where the DRM scheduler is attached to a file
(which we want to be able to tear down quickly when userspace closes
it) while firmware could continue to (attempt to) run in-flight jobs
after that point. The major missing codepath to make this work is
detaching jobs from their HW fences on scheduler shutdown, so
implement that. This also makes writing a safe Rust abstraction
plausible, since otherwise we'd have to add a huge pile of complexity
to that side in order to enforce the invariant that the scheduler
outlives its jobs (including setting up a workqueue to handle
scheduler teardown and other craziness, which is an unacceptable
level of complexity for what should be a lightweight abstraction).
I believe there *may* still be at least one UAF-type bug related to case
2 above, but it's very hard to trigger and I wasn't able to figure out
what causes it the one time I saw it recently. Other than that, things
look quite robust on the Asahi driver with these patches, even when
trying to break things by killing GPU consumers in a tight loop and
things like that. If we agree this is a good way forward, I think this
is a good start even if there's still a bug lurking somewhere.
Aside (but related to the previous discussion): the can_run_job thing
is gone, I'm using fences returned from prepare() now and that works
well (and actually fixes one corner case related to wait contexts I'd
missed), so hopefully that's OK with everyone ^^
Changes from the previous version of patch #2: explicitly signal
detached job fences with an error. I'd missed that and I think it's what
was causing us some rare lockups due to fences never getting signaled.
Signed-off-by: Asahi Lina <lina@xxxxxxxxxxxxx>
---
Asahi Lina (3):
drm/scheduler: Add more documentation
drm/scheduler: Fix UAF in drm_sched_fence_get_timeline_name
drm/scheduler: Clean up jobs when the scheduler is torn down.
drivers/gpu/drm/scheduler/sched_entity.c | 7 ++-
drivers/gpu/drm/scheduler/sched_fence.c | 4 +-
drivers/gpu/drm/scheduler/sched_main.c | 90 ++++++++++++++++++++++++++++++--
include/drm/gpu_scheduler.h | 5 ++
4 files changed, 99 insertions(+), 7 deletions(-)
---
base-commit: 06c2afb862f9da8dc5efa4b6076a0e48c3fbaaa5
change-id: 20230714-drm-sched-fixes-94bea043bbe7
Thank you,
~~ Lina
