Re: [Intel-gfx] [PATCH v4 2/3] drm/i915/guc: Close deregister-context race against CT-loss

["Teres Alexis, Alan Previn" <alan.previn.teres.alexis@intel.com>]

On Wed, 2023-10-04 at 06:34 +0000, Gupta, Anshuman wrote:
> 
> > -----Original Message-----
> > From: Teres Alexis, Alan Previn <alan.previn.teres.alexis@intel.com

alan:snip
> > @@ -289,6 +289,13 @@ int intel_gt_resume(struct intel_gt *gt)
> > 
> >  static void wait_for_suspend(struct intel_gt *gt)  {
> > +	/*
> > +	 * On rare occasions, we've observed the fence completion trigger
> > +	 * free_engines asynchronously via rcu_call. Ensure those are done.
> > +	 * This path is only called on suspend, so it's an acceptable cost.
> > +	 */
> > +	rcu_barrier();
> Let's add the barrier after the end of prepare suspend and at start of late suspend.
> To make sure we don't have any async destroy from any user request or any internal  kmd request during i915 suspend?
> Br,
> Anshuman Gupta.
alan: some thoughts: actuallly wait_fos_suspend is being called at from both intel_gt_suspend_prepare
and intel_gt_suspend_late. so putting the barrier in above location would get hit for both steps.
However, because wait_for_suspend may optionally wedge the system if outstanding requests were stuck
for more than I915_GT_SUSPEND_IDLE_TIMEOUT, wouldnt it be better to add the barrier before that
check (i.e. in above location) as opposed to after the return from wait_for_suspend at the end
of suspend_prepare - which would defeat the purpose of even checking that intel_gt_wait_for_idle
from within wait_for_suspend? (which is existing code). Perhaps we need to get one last test on this?