diff mbox series

[v2,1/4] drm: Clear state->acquire_ctx before leaving drm_atomic_helper_commit_duplicated_state()

Message ID 20181129150423.239081-1-sean@poorly.run (mailing list archive)
State New, archived
Headers show
Series [v2,1/4] drm: Clear state->acquire_ctx before leaving drm_atomic_helper_commit_duplicated_state() | expand

Commit Message

Sean Paul Nov. 29, 2018, 3:04 p.m. UTC
From: Sean Paul <seanpaul@chromium.org>

drm_atomic_helper_commit_duplicated_state() sets state->acquire_ctx to
the context given in the argument and leaves it in state after it
quits. The lifetime of state and context are not guaranteed to be the
same, so we shouldn't leave that pointer hanging around. This patch
resets the context to NULL to avoid any oopses.

Changes in v2:
- Added to the set

Suggested-by: Daniel Vetter <daniel@ffwll.ch>
Signed-off-by: Sean Paul <seanpaul@chromium.org>
---
 drivers/gpu/drm/drm_atomic_helper.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

Comments

Daniel Vetter Nov. 29, 2018, 3:27 p.m. UTC | #1
On Thu, Nov 29, 2018 at 10:04:14AM -0500, Sean Paul wrote:
> From: Sean Paul <seanpaul@chromium.org>
> 
> drm_atomic_helper_commit_duplicated_state() sets state->acquire_ctx to
> the context given in the argument and leaves it in state after it
> quits. The lifetime of state and context are not guaranteed to be the
> same, so we shouldn't leave that pointer hanging around. This patch
> resets the context to NULL to avoid any oopses.
> 
> Changes in v2:
> - Added to the set
> 
> Suggested-by: Daniel Vetter <daniel@ffwll.ch>
> Signed-off-by: Sean Paul <seanpaul@chromium.org>

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

> ---
>  drivers/gpu/drm/drm_atomic_helper.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
> index fe8dd8aa4ae40..5e03bebc7f9e6 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -3225,7 +3225,7 @@ EXPORT_SYMBOL(drm_atomic_helper_suspend);
>  int drm_atomic_helper_commit_duplicated_state(struct drm_atomic_state *state,
>  					      struct drm_modeset_acquire_ctx *ctx)
>  {
> -	int i;
> +	int i, ret;
>  	struct drm_plane *plane;
>  	struct drm_plane_state *new_plane_state;
>  	struct drm_connector *connector;
> @@ -3244,7 +3244,11 @@ int drm_atomic_helper_commit_duplicated_state(struct drm_atomic_state *state,
>  	for_each_new_connector_in_state(state, connector, new_conn_state, i)
>  		state->connectors[i].old_state = connector->state;
>  
> -	return drm_atomic_commit(state);
> +	ret = drm_atomic_commit(state);
> +
> +	state->acquire_ctx = NULL;
> +
> +	return ret;
>  }
>  EXPORT_SYMBOL(drm_atomic_helper_commit_duplicated_state);
>  
> -- 
> Sean Paul, Software Engineer, Google / Chromium OS
>
Sean Paul Nov. 29, 2018, 3:50 p.m. UTC | #2
On Thu, Nov 29, 2018 at 04:27:57PM +0100, Daniel Vetter wrote:
> On Thu, Nov 29, 2018 at 10:04:14AM -0500, Sean Paul wrote:
> > From: Sean Paul <seanpaul@chromium.org>
> > 
> > drm_atomic_helper_commit_duplicated_state() sets state->acquire_ctx to
> > the context given in the argument and leaves it in state after it
> > quits. The lifetime of state and context are not guaranteed to be the
> > same, so we shouldn't leave that pointer hanging around. This patch
> > resets the context to NULL to avoid any oopses.
> > 
> > Changes in v2:
> > - Added to the set
> > 
> > Suggested-by: Daniel Vetter <daniel@ffwll.ch>
> > Signed-off-by: Sean Paul <seanpaul@chromium.org>
> 
> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

Thanks for your reviews! The series is pushed to drm-misc-next.

Sean

> 
> > ---
> >  drivers/gpu/drm/drm_atomic_helper.c | 8 ++++++--
> >  1 file changed, 6 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
> > index fe8dd8aa4ae40..5e03bebc7f9e6 100644
> > --- a/drivers/gpu/drm/drm_atomic_helper.c
> > +++ b/drivers/gpu/drm/drm_atomic_helper.c
> > @@ -3225,7 +3225,7 @@ EXPORT_SYMBOL(drm_atomic_helper_suspend);
> >  int drm_atomic_helper_commit_duplicated_state(struct drm_atomic_state *state,
> >  					      struct drm_modeset_acquire_ctx *ctx)
> >  {
> > -	int i;
> > +	int i, ret;
> >  	struct drm_plane *plane;
> >  	struct drm_plane_state *new_plane_state;
> >  	struct drm_connector *connector;
> > @@ -3244,7 +3244,11 @@ int drm_atomic_helper_commit_duplicated_state(struct drm_atomic_state *state,
> >  	for_each_new_connector_in_state(state, connector, new_conn_state, i)
> >  		state->connectors[i].old_state = connector->state;
> >  
> > -	return drm_atomic_commit(state);
> > +	ret = drm_atomic_commit(state);
> > +
> > +	state->acquire_ctx = NULL;
> > +
> > +	return ret;
> >  }
> >  EXPORT_SYMBOL(drm_atomic_helper_commit_duplicated_state);
> >  
> > -- 
> > Sean Paul, Software Engineer, Google / Chromium OS
> > 
> 
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
diff mbox series

Patch

diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
index fe8dd8aa4ae40..5e03bebc7f9e6 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -3225,7 +3225,7 @@  EXPORT_SYMBOL(drm_atomic_helper_suspend);
 int drm_atomic_helper_commit_duplicated_state(struct drm_atomic_state *state,
 					      struct drm_modeset_acquire_ctx *ctx)
 {
-	int i;
+	int i, ret;
 	struct drm_plane *plane;
 	struct drm_plane_state *new_plane_state;
 	struct drm_connector *connector;
@@ -3244,7 +3244,11 @@  int drm_atomic_helper_commit_duplicated_state(struct drm_atomic_state *state,
 	for_each_new_connector_in_state(state, connector, new_conn_state, i)
 		state->connectors[i].old_state = connector->state;
 
-	return drm_atomic_commit(state);
+	ret = drm_atomic_commit(state);
+
+	state->acquire_ctx = NULL;
+
+	return ret;
 }
 EXPORT_SYMBOL(drm_atomic_helper_commit_duplicated_state);