Re: [PATCH] drm/writeback: Delete drm_writeback_cleanup_job
Hi Brian, On Thu, Feb 21, 2019 at 09:34:10AM +, Brian Starkey wrote: > On Thu, Feb 21, 2019 at 10:19:35AM +0200, Laurent Pinchart wrote: > > Hi Daniel, > > > > Thank you for the patch. > > > > On Thu, Feb 21, 2019 at 12:24:01AM +0100, Daniel Vetter wrote: > >> No implementation, no callers. > > > > The issue here isn't that the function is declared, but that it's not > > defined. Jobs are leaked when atomic commit fails (or when using test > > commits). I'm working on a fix, please don't apply this patch in the > > meantime. > > Yeah, looking at the series somehow the call to cleanup the writeback > job on failure looks like it got lost between v9 and v10. I saw a > patch internally, but looks like James didn't send it to the list yet. > > @James, could you send out your patch which fixes the cleanup on > failure? I've just posted [PATCH v5 12/19] drm: writeback: Cleanup job ownership handling when queuing job [PATCH v5 13/19] drm: writeback: Fix leak of writeback job that should address this issue. The series also includes the RFC-like [PATCH v5 14/19] drm: writeback: Add job prepare and cleanup operations that addresses a separate issue with writeback support. > >> Cc: Brian Starkey > >> Cc: Liviu Dudau > >> Cc: Eric Anholt > >> Signed-off-by: Daniel Vetter > >> --- > >> include/drm/drm_writeback.h | 2 -- > >> 1 file changed, 2 deletions(-) > >> > >> diff --git a/include/drm/drm_writeback.h b/include/drm/drm_writeback.h > >> index 23df9d463003..f34895f7fcb1 100644 > >> --- a/include/drm/drm_writeback.h > >> +++ b/include/drm/drm_writeback.h > >> @@ -125,8 +125,6 @@ int drm_writeback_connector_init(struct drm_device > >> *dev, > >> void drm_writeback_queue_job(struct drm_writeback_connector *wb_connector, > >> struct drm_writeback_job *job); > >> > >> -void drm_writeback_cleanup_job(struct drm_writeback_job *job); > >> - > >> void > >> drm_writeback_signal_completion(struct drm_writeback_connector > >> *wb_connector, > >>int status); -- Regards, Laurent Pinchart ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/writeback: Delete drm_writeback_cleanup_job
Hi Daniel, On Thu, Feb 21, 2019 at 10:30:24AM +0100, Daniel Vetter wrote: > On Thu, Feb 21, 2019 at 9:19 AM Laurent Pinchart > wrote: > > Hi Daniel, > > > > Thank you for the patch. > > > > On Thu, Feb 21, 2019 at 12:24:01AM +0100, Daniel Vetter wrote: > >> No implementation, no callers. > > > > The issue here isn't that the function is declared, but that it's not > > defined. Jobs are leaked when atomic commit fails (or when using test > > commits). I'm working on a fix, please don't apply this patch in the > > meantime. > > Hm, can't I merge this one and give you a clean slate? Function in > header but nowhere else kinda upsets my ocd :-) I'm afraid I've already posted the fix patches that depend on this prototype :-) > >> Cc: Brian Starkey > >> Cc: Liviu Dudau > >> Cc: Eric Anholt > >> Signed-off-by: Daniel Vetter > >> --- > >> include/drm/drm_writeback.h | 2 -- > >> 1 file changed, 2 deletions(-) > >> > >> diff --git a/include/drm/drm_writeback.h b/include/drm/drm_writeback.h > >> index 23df9d463003..f34895f7fcb1 100644 > >> --- a/include/drm/drm_writeback.h > >> +++ b/include/drm/drm_writeback.h > >> @@ -125,8 +125,6 @@ int drm_writeback_connector_init(struct drm_device > >> *dev, > >> void drm_writeback_queue_job(struct drm_writeback_connector *wb_connector, > >>struct drm_writeback_job *job); > >> > >> -void drm_writeback_cleanup_job(struct drm_writeback_job *job); > >> - > >> void > >> drm_writeback_signal_completion(struct drm_writeback_connector > >> *wb_connector, > >> int status); -- Regards, Laurent Pinchart ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/writeback: Delete drm_writeback_cleanup_job
On Thu, Feb 21, 2019 at 10:19:35AM +0200, Laurent Pinchart wrote: > Hi Daniel, > > Thank you for the patch. > > On Thu, Feb 21, 2019 at 12:24:01AM +0100, Daniel Vetter wrote: > > No implementation, no callers. > > The issue here isn't that the function is declared, but that it's not > defined. Jobs are leaked when atomic commit fails (or when using test > commits). I'm working on a fix, please don't apply this patch in the > meantime. Yeah, looking at the series somehow the call to cleanup the writeback job on failure looks like it got lost between v9 and v10. I saw a patch internally, but looks like James didn't send it to the list yet. @James, could you send out your patch which fixes the cleanup on failure? Thanks, -Brian > > > Cc: Brian Starkey > > Cc: Liviu Dudau > > Cc: Eric Anholt > > Signed-off-by: Daniel Vetter > > --- > > include/drm/drm_writeback.h | 2 -- > > 1 file changed, 2 deletions(-) > > > > diff --git a/include/drm/drm_writeback.h b/include/drm/drm_writeback.h > > index 23df9d463003..f34895f7fcb1 100644 > > --- a/include/drm/drm_writeback.h > > +++ b/include/drm/drm_writeback.h > > @@ -125,8 +125,6 @@ int drm_writeback_connector_init(struct drm_device *dev, > > void drm_writeback_queue_job(struct drm_writeback_connector *wb_connector, > > struct drm_writeback_job *job); > > > > -void drm_writeback_cleanup_job(struct drm_writeback_job *job); > > - > > void > > drm_writeback_signal_completion(struct drm_writeback_connector > > *wb_connector, > > int status); > > -- > Regards, > > Laurent Pinchart > ___ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/writeback: Delete drm_writeback_cleanup_job
On Thu, Feb 21, 2019 at 9:19 AM Laurent Pinchart wrote: > Hi Daniel, > > Thank you for the patch. > > On Thu, Feb 21, 2019 at 12:24:01AM +0100, Daniel Vetter wrote: > > No implementation, no callers. > > The issue here isn't that the function is declared, but that it's not > defined. Jobs are leaked when atomic commit fails (or when using test > commits). I'm working on a fix, please don't apply this patch in the > meantime. Hm, can't I merge this one and give you a clean slate? Function in header but nowhere else kinda upsets my ocd :-) -Daniel > > Cc: Brian Starkey > > Cc: Liviu Dudau > > Cc: Eric Anholt > > Signed-off-by: Daniel Vetter > > --- > > include/drm/drm_writeback.h | 2 -- > > 1 file changed, 2 deletions(-) > > > > diff --git a/include/drm/drm_writeback.h b/include/drm/drm_writeback.h > > index 23df9d463003..f34895f7fcb1 100644 > > --- a/include/drm/drm_writeback.h > > +++ b/include/drm/drm_writeback.h > > @@ -125,8 +125,6 @@ int drm_writeback_connector_init(struct drm_device *dev, > > void drm_writeback_queue_job(struct drm_writeback_connector *wb_connector, > >struct drm_writeback_job *job); > > > > -void drm_writeback_cleanup_job(struct drm_writeback_job *job); > > - > > void > > drm_writeback_signal_completion(struct drm_writeback_connector > > *wb_connector, > > int status); > > -- > Regards, > > Laurent Pinchart -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/writeback: Delete drm_writeback_cleanup_job
Hi Daniel, Thank you for the patch. On Thu, Feb 21, 2019 at 12:24:01AM +0100, Daniel Vetter wrote: > No implementation, no callers. The issue here isn't that the function is declared, but that it's not defined. Jobs are leaked when atomic commit fails (or when using test commits). I'm working on a fix, please don't apply this patch in the meantime. > Cc: Brian Starkey > Cc: Liviu Dudau > Cc: Eric Anholt > Signed-off-by: Daniel Vetter > --- > include/drm/drm_writeback.h | 2 -- > 1 file changed, 2 deletions(-) > > diff --git a/include/drm/drm_writeback.h b/include/drm/drm_writeback.h > index 23df9d463003..f34895f7fcb1 100644 > --- a/include/drm/drm_writeback.h > +++ b/include/drm/drm_writeback.h > @@ -125,8 +125,6 @@ int drm_writeback_connector_init(struct drm_device *dev, > void drm_writeback_queue_job(struct drm_writeback_connector *wb_connector, >struct drm_writeback_job *job); > > -void drm_writeback_cleanup_job(struct drm_writeback_job *job); > - > void > drm_writeback_signal_completion(struct drm_writeback_connector *wb_connector, > int status); -- Regards, Laurent Pinchart ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/writeback: Delete drm_writeback_cleanup_job
No implementation, no callers. Cc: Brian Starkey Cc: Liviu Dudau Cc: Eric Anholt Signed-off-by: Daniel Vetter --- include/drm/drm_writeback.h | 2 -- 1 file changed, 2 deletions(-) diff --git a/include/drm/drm_writeback.h b/include/drm/drm_writeback.h index 23df9d463003..f34895f7fcb1 100644 --- a/include/drm/drm_writeback.h +++ b/include/drm/drm_writeback.h @@ -125,8 +125,6 @@ int drm_writeback_connector_init(struct drm_device *dev, void drm_writeback_queue_job(struct drm_writeback_connector *wb_connector, struct drm_writeback_job *job); -void drm_writeback_cleanup_job(struct drm_writeback_job *job); - void drm_writeback_signal_completion(struct drm_writeback_connector *wb_connector, int status); -- 2.20.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel