[PATCH] drm/atomic: swap_state should stall on cleanup_done

From: Rob Clark <robdclark@gmail.com>
To: dri-devel@lists.freedesktop.org
Cc: Rob Clark <robdclark@chromium.org>,
	Sean Paul <seanpaul@chromium.org>,
	Maarten Lankhorst <maarten.lankhorst@linux.intel.com>,
	Maxime Ripard <mripard@kernel.org>, Sean Paul <sean@poorly.run>,
	David Airlie <airlied@linux.ie>, Daniel Vetter <daniel@ffwll.ch>,
	linux-kernel@vger.kernel.org (open list)
Subject: [PATCH] drm/atomic: swap_state should stall on cleanup_done
Date: Thu, 31 Oct 2019 15:36:41 -0700	[thread overview]
Message-ID: <20191031223641.19208-1-robdclark@gmail.com> (raw)

From: Rob Clark <robdclark@chromium.org>

Stalling on cleanup_done ensures that any atomic state related to a
nonblock commit no longer has dangling references to per-object state
that can be freed.

Otherwise, if a !nonblock commit completes after a nonblock commit has
swapped state (ie. the synchronous part of the nonblock commit comes
before the !nonblock commit), but before the asynchronous part of the
nonblock commit completes, what was the new per-object state in the
nonblock commit can be freed.

This shows up with the new self-refresh helper, as _update_avg_times()
dereferences the original old and new crtc_state.

Fixes: d4da4e33341c ("drm: Measure Self Refresh Entry/Exit times to avoid thrashing")
Cc: Sean Paul <seanpaul@chromium.org>
Signed-off-by: Rob Clark <robdclark@chromium.org>
---
Other possibilities:
1) maybe block later before freeing atomic state?
2) refcount individual per-object state

 drivers/gpu/drm/drm_atomic_helper.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
index 3ef2ac52ce94..a5d95429f91b 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -2711,7 +2711,7 @@ int drm_atomic_helper_swap_state(struct drm_atomic_state *state,
 			if (!commit)
 				continue;
 
-			ret = wait_for_completion_interruptible(&commit->hw_done);
+			ret = wait_for_completion_interruptible(&commit->cleanup_done);
 			if (ret)
 				return ret;
 		}
@@ -2722,7 +2722,7 @@ int drm_atomic_helper_swap_state(struct drm_atomic_state *state,
 			if (!commit)
 				continue;
 
-			ret = wait_for_completion_interruptible(&commit->hw_done);
+			ret = wait_for_completion_interruptible(&commit->cleanup_done);
 			if (ret)
 				return ret;
 		}
@@ -2733,7 +2733,7 @@ int drm_atomic_helper_swap_state(struct drm_atomic_state *state,
 			if (!commit)
 				continue;
 
-			ret = wait_for_completion_interruptible(&commit->hw_done);
+			ret = wait_for_completion_interruptible(&commit->cleanup_done);
 			if (ret)
 				return ret;
 		}
-- 
2.21.0

