On Wed, 2023-10-04 at 06:34 +0000, Gupta, Anshuman wrote: > > > -----Original Message----- > > From: Teres Alexis, Alan Previn <alan.previn.teres.alexis@xxxxxxxxx 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?