On Wed, Mar 24, 2021 at 07:15:36PM +0200, Ville Syrjälä wrote:
On Wed, Mar 24, 2021 at 06:00:12PM +0100, Daniel Vetter wrote:
On Tue, Mar 23, 2021 at 04:50:52PM +0100, Maarten Lankhorst wrote:
We get a lockdep splat when the reset mutex is held, because it can be taken from fence_wait. This conflicts with the mmu notifier we have, because we recurse between reset mutex and mmap lock -> mmu notifier.
Remove this recursion by calling revoke_mmaps before taking the lock.
The reset code still needs fixing, as taking mmap locks during reset is not allowed.
Signed-off-by: Maarten Lankhorst maarten.lankhorst@linux.intel.com Reviewed-by: Thomas Hellström thomas.hellstrom@linux.intel.com
drivers/gpu/drm/i915/gt/intel_reset.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/intel_reset.c b/drivers/gpu/drm/i915/gt/intel_reset.c index 990cb4adbb9a..447f589750c2 100644 --- a/drivers/gpu/drm/i915/gt/intel_reset.c +++ b/drivers/gpu/drm/i915/gt/intel_reset.c @@ -970,8 +970,6 @@ static int do_reset(struct intel_gt *gt, intel_engine_mask_t stalled_mask) { int err, i;
- gt_revoke(gt);
- err = __intel_gt_reset(gt, ALL_ENGINES); for (i = 0; err && i < RESET_MAX_RETRIES; i++) { msleep(10 * (i + 1));
@@ -1026,6 +1024,9 @@ void intel_gt_reset(struct intel_gt *gt,
might_sleep(); GEM_BUG_ON(!test_bit(I915_RESET_BACKOFF, >->reset.flags));
I've added a FIXME comment here just so we don't totally forget. This will also blow up again when we wrap the entire reset path into a dma_fence critical section annotation (at least going forward, we can't do that on hw that needs display reset with the current code unfortunately).
But I did look at the code which originally added this in
commit 2caffbf1176256cc4f8d4e5c3c524fc689cb9876 Author: Chris Wilson chris@chris-wilson.co.uk Date: Fri Feb 8 15:37:03 2019 +0000
drm/i915: Revoke mmaps and prevent access to fence registers across resetand noped right out.
I think this complexity needs to go entirely, and instead we just protect the fence register state to make sure that after reset they are all good again:
- add a new mutex for low level fence register state
- hold that mutex around fence register writes (really just the low level fence writes)
- hold it in the reset path when we restore fence registers
This means that a global reset also thrashes mmaps, but it's a global reset we're talking about here, everything is thrash anyway. Plus/minus fenced gtt mmaps really doesn't change the tally.
My recollection is that GPU reset doesn't actually clobber the fence registers. Though not 100% sure I can trust my brain on this. Also dunno if it actually matter here or not, but figured I'd point it out.
I think on gen2/3 it does, because there everything goes over. But yeah maybe on gen4+ it's all fine, would be worth to check that. -Daniel