Hi, On Thu, Feb 21, 2019 at 12:32:06PM +0200, Laurent Pinchart wrote: > Writeback jobs are allocated when the WRITEBACK_FB_ID is set, and > deleted when the jobs complete. This results in both a memory leak of > the job and a leak of the framebuffer if the atomic commit returns > before the job is queued for processing, for instance if the atomic > check fails or if the commit runs in test-only mode. > > Fix this by implementing the drm_writeback_cleanup_job() function and > calling it from __drm_atomic_helper_connector_destroy_state(). As > writeback jobs are removed from the state when they're queued for > processing, any job left in the state when the state gets destroyed > needs to be cleaned up. > > The existing declaration of the drm_writeback_cleanup_job() function > without an implementation hints that this problem was considered, but > never addressed. > > Signed-off-by: Laurent Pinchart <laurent.pinchart+rene...@ideasonboard.com>
Thanks for fixing this, it looks like it got dropped in one of the rework/rebases. Reviewed-by: Brian Starkey <brian.star...@arm.com> > --- > drivers/gpu/drm/drm_atomic_state_helper.c | 4 ++++ > drivers/gpu/drm/drm_writeback.c | 13 ++++++++++--- > 2 files changed, 14 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/drm_atomic_state_helper.c > b/drivers/gpu/drm/drm_atomic_state_helper.c > index 4985384e51f6..59ffb6b9c745 100644 > --- a/drivers/gpu/drm/drm_atomic_state_helper.c > +++ b/drivers/gpu/drm/drm_atomic_state_helper.c > @@ -30,6 +30,7 @@ > #include <drm/drm_connector.h> > #include <drm/drm_atomic.h> > #include <drm/drm_device.h> > +#include <drm/drm_writeback.h> > > #include <linux/slab.h> > #include <linux/dma-fence.h> > @@ -412,6 +413,9 @@ __drm_atomic_helper_connector_destroy_state(struct > drm_connector_state *state) > > if (state->commit) > drm_crtc_commit_put(state->commit); > + > + if (state->writeback_job) > + drm_writeback_cleanup_job(state->writeback_job); > } > EXPORT_SYMBOL(__drm_atomic_helper_connector_destroy_state); > > diff --git a/drivers/gpu/drm/drm_writeback.c b/drivers/gpu/drm/drm_writeback.c > index 338b993d7c9f..afb1ae6e0ecb 100644 > --- a/drivers/gpu/drm/drm_writeback.c > +++ b/drivers/gpu/drm/drm_writeback.c > @@ -273,6 +273,14 @@ void drm_writeback_queue_job(struct > drm_writeback_connector *wb_connector, > } > EXPORT_SYMBOL(drm_writeback_queue_job); > > +void drm_writeback_cleanup_job(struct drm_writeback_job *job) > +{ > + if (job->fb) > + drm_framebuffer_put(job->fb); > + > + kfree(job); > +} > + > /* > * @cleanup_work: deferred cleanup of a writeback job > * > @@ -285,10 +293,9 @@ static void cleanup_work(struct work_struct *work) > struct drm_writeback_job *job = container_of(work, > struct drm_writeback_job, > cleanup_work); > - drm_framebuffer_put(job->fb); > - kfree(job); > -} > > + drm_writeback_cleanup_job(job); > +} > > /** > * drm_writeback_signal_completion - Signal the completion of a writeback job > -- > Regards, > > Laurent Pinchart > _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel