Re: [PATCH] drm/i915/ring_submission: Fix timeline left held on VMA alloc error

[Sebastian Brzezinka <sebastian.brzezinka@intel.com>]

On Fri Jun 6, 2025 at 1:58 PM UTC, Janusz Krzysztofik wrote:
> The following error has been reported sporadically by CI when a test
> unbinds the i915 driver on a ring submission platform:
>
> <4> [239.330153] ------------[ cut here ]------------
> <4> [239.330166] i915 0000:00:02.0: [drm] drm_WARN_ON(dev_priv->mm.shrink_count)
> <4> [239.330196] WARNING: CPU: 1 PID: 18570 at drivers/gpu/drm/i915/i915_gem.c:1309 i915_gem_cleanup_early+0x13e/0x150 [i915]
> ...
> <4> [239.330640] RIP: 0010:i915_gem_cleanup_early+0x13e/0x150 [i915]
> ...
> <4> [239.330942] Call Trace:
> <4> [239.330944]  <TASK>
> <4> [239.330949]  i915_driver_late_release+0x2b/0xa0 [i915]
> <4> [239.331202]  i915_driver_release+0x86/0xa0 [i915]
> <4> [239.331482]  devm_drm_dev_init_release+0x61/0x90
> <4> [239.331494]  devm_action_release+0x15/0x30
> <4> [239.331504]  release_nodes+0x3d/0x120
> <4> [239.331517]  devres_release_all+0x96/0xd0
> <4> [239.331533]  device_unbind_cleanup+0x12/0x80
> <4> [239.331543]  device_release_driver_internal+0x23a/0x280
> <4> [239.331550]  ? bus_find_device+0xa5/0xe0
> <4> [239.331563]  device_driver_detach+0x14/0x20
> ...
> <4> [357.719679] ---[ end trace 0000000000000000 ]---
>
> If the test also unloads the i915 module then that's followed with:
>
> <3> [357.787478] =============================================================================
> <3> [357.788006] BUG i915_vma (Tainted: G     U  W        N ): Objects remaining on __kmem_cache_shutdown()
> <3> [357.788031] -----------------------------------------------------------------------------
> <3> [357.788204] Object 0xffff888109e7f480 @offset=29824
> <3> [357.788670] Allocated in i915_vma_instance+0xee/0xc10 [i915] age=292729 cpu=4 pid=2244
> <4> [357.788994]  i915_vma_instance+0xee/0xc10 [i915]
> <4> [357.789290]  init_status_page+0x7b/0x420 [i915]
> <4> [357.789532]  intel_engines_init+0x1d8/0x980 [i915]
> <4> [357.789772]  intel_gt_init+0x175/0x450 [i915]
> <4> [357.790014]  i915_gem_init+0x113/0x340 [i915]
> <4> [357.790281]  i915_driver_probe+0x847/0xed0 [i915]
> <4> [357.790504]  i915_pci_probe+0xe6/0x220 [i915]
> ...
>
> Closer analysis of CI results history has revealed a dependency of the
> error on a few IGT tests, namely:
> - igt@api_intel_allocator@fork-simple-stress-signal,
> - igt@api_intel_allocator@two-level-inception-interruptible,
> - igt@gem_linear_blits@interruptible,
> - igt@prime_mmap_coherency@ioctl-errors,
> which invisibly trigger the issue, then exhibited with first driver unbind
> attempt.
>
> All of the above tests perform actions which are actively interrupted with
> signals.  Further debugging has allowed to narrow that scope down to
> DRM_IOCTL_I915_GEM_EXECBUFFER2, and ring_context_alloc(), specific to ring
> submission, in particular.
>
> If successful then that function, or its execlists or GuC submission
> equivalent, is supposed to be called only once per GEM context engine,
> followed by raise of a flag that prevents the function from being called
> again.  The function is expected to unwind its internal errors itself, so
> it may be safely called once more after it returns an error.
>
> In case of ring submission, the function first gets a reference to the
> engine's legacy timeline and then allocates a VMA.  If the VMA allocation
> fails, e.g. when i915_vma_instance() called from inside is interrupted
> with a signal, then ring_context_alloc() fails, leaving the timeline held
> referenced.  On next I915_GEM_EXECBUFFER2 IOCTL, another reference to the
> timeline is got, and only that last one is put on successful completion.
> As a consequence, the legacy timeline, with its underlying engine status
> page's VMA object, is still held and not released on driver unbind.
>
> Get the legacy timeline only after successful allocation of the context
> engine's VMA.
>
> Fixes: 75d0a7f31eec ("drm/i915: Lift timeline into intel_context")
> Closes: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/12061
> Cc: Chris Wilson <chris.p.wilson@linux.intel.com>
> Cc: Matthew Auld <matthew.auld@intel.com>
> Signed-off-by: Janusz Krzysztofik <janusz.krzysztofik@linux.intel.com>
Everything looks good to me overall.

Reviewed-by: Sebastian Brzezinka <sebastian.brzezinka@intel.com>
-- 
Best regards,
Sebastian