Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1310319

Re: [RFC 26/29] dma-buf/fence: remove pointless fence_timeline_signal at destroy phase

Show all headers | View raw

On 15/01/2016 14:55, Gustavo Padovan wrote:
> From: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
>
> All changes to timeline value come through the user via
> fence_timeline_signal() calls. When fence_timeline_destroy() is called no
> changes on timeline->value happens hence call fence_timeline_signal() with
> no increment is pointless.
>
> Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
> ---
>   drivers/dma-buf/fence.c | 6 +-----
>   1 file changed, 1 insertion(+), 5 deletions(-)
>
> diff --git a/drivers/dma-buf/fence.c b/drivers/dma-buf/fence.c
> index 7a5fc9b..26f5f0f 100644
> --- a/drivers/dma-buf/fence.c
> +++ b/drivers/dma-buf/fence.c
> @@ -136,7 +136,7 @@ EXPORT_SYMBOL(fence_timeline_put);
>    * fence_timeline_destroy - destroy a fence_timeline
>    * @timeline	[in]	the fence_timeline to destroy
>    *
> - * This function destroys a timeline. It signals any active fence first.
> + * This function destroys a timeline.

The implementation for this was certainly broken but I would say it 
should be fixed to match the comment rather than just abandoned 
completely. That is, what happens if a timeline owner destroys their 
timeline while there are outstanding fences which other drivers are 
waiting on? That is presumably a bug in the code that called destroy 
prematurely, but bugs happen.

The old implementation simply leaked the fences. Doing a debugfs dump 
would show the timeline with all its outstanding fences still floating 
around forever after. Worse, anything waiting on them would never be 
signalled and is therefore potentially deadlocked.

Note that I haven't had chance to look through the entire patch series 
yet so maybe this has been fixed up elsewhere. If not, then I think it 
definitely needs looking into.


>    */
>   void fence_timeline_destroy(struct fence_timeline *timeline)
>   {
> @@ -147,10 +147,6 @@ void fence_timeline_destroy(struct fence_timeline *timeline)
>   	 */
>   	smp_wmb();
>   
> -	/*
> -	 * signal any children that their parent is going away.
> -	 */
> -	fence_timeline_signal(timeline, 0);
>   	fence_timeline_put(timeline);
>   }
>   EXPORT_SYMBOL(fence_timeline_destroy);

Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread

Thread

[RFC 00/29] De-stage android's sync framework Gustavo Padovan <gustavo@padovan.org> - 2016-01-15 16:00 +0100
  [RFC 19/29] dma-buf/fence: create fence_default_fill_driver_data() Gustavo Padovan <gustavo@padovan.org> - 2016-01-15 16:00 +0100
  [RFC 15/29] dma-buf/fence: create fence_default_get_driver_name() Gustavo Padovan <gustavo@padovan.org> - 2016-01-15 16:00 +0100
  [RFC 29/29] dma-buf/fence: de-stage sync framework Gustavo Padovan <gustavo@padovan.org> - 2016-01-15 16:00 +0100
  [RFC 26/29] dma-buf/fence: remove pointless fence_timeline_signal at destroy phase Gustavo Padovan <gustavo@padovan.org> - 2016-01-15 16:00 +0100
    Re: [RFC 26/29] dma-buf/fence: remove pointless fence_timeline_signal  at destroy phase John Harrison <John.C.Harrison@Intel.com> - 2016-01-15 18:50 +0100
      Re: [RFC 26/29] dma-buf/fence: remove pointless  fence_timeline_signal at destroy phase Gustavo Padovan <gustavo.padovan@collabora.co.uk> - 2016-01-15 19:10 +0100
        Re: [RFC 26/29] dma-buf/fence: remove pointless fence_timeline_signal  at destroy phase Greg Hackmann <ghackmann@google.com> - 2016-01-16 00:50 +0100
  [RFC 08/29] staging/android: Remove WARN_ON_ONCE when releasing sync_fence Gustavo Padovan <gustavo@padovan.org> - 2016-01-15 16:10 +0100
  [RFC 02/29] staging/android: fix checkpatch warning Gustavo Padovan <gustavo@padovan.org> - 2016-01-15 16:10 +0100
  [RFC 11/29] dma-buf/fence: move sync_timeline to fence_timeline Gustavo Padovan <gustavo@padovan.org> - 2016-01-15 16:10 +0100
    Re: [RFC 11/29] dma-buf/fence: move sync_timeline to fence_timeline Greg Hackmann <ghackmann@google.com> - 2016-01-20 02:00 +0100
  [RFC 13/29] dma-buf/fence: create fence_default_enable_signaling() Gustavo Padovan <gustavo@padovan.org> - 2016-01-15 16:10 +0100
  [RFC 05/29] staging/android: remove not used sync_timeline ops Gustavo Padovan <gustavo@padovan.org> - 2016-01-15 16:10 +0100
  [RFC 14/29] dma-buf/fence: create fence_default_release() Gustavo Padovan <gustavo@padovan.org> - 2016-01-15 16:10 +0100
  Re: [RFC 00/29] De-stage android's sync framework Joe Perches <joe@perches.com> - 2016-01-15 20:20 +0100
  Re: [RFC 00/29] De-stage android's sync framework Daniel Vetter <daniel@ffwll.ch> - 2016-01-19 12:10 +0100
    Re: [RFC 00/29] De-stage android's sync framework Gustavo Padovan <gustavo@padovan.org> - 2016-01-19 16:30 +0100
      Re: [RFC 00/29] De-stage android's sync framework John Harrison <John.C.Harrison@Intel.com> - 2016-01-19 17:20 +0100
        Re: [RFC 00/29] De-stage android's sync framework Gustavo Padovan <gustavo@padovan.org> - 2016-01-19 19:00 +0100
          Re: [RFC 00/29] De-stage android's sync framework Daniel Vetter <daniel@ffwll.ch> - 2016-01-19 19:10 +0100
            Re: [RFC 00/29] De-stage android's sync framework Gustavo Padovan <gustavo@padovan.org> - 2016-01-19 19:20 +0100
    Re: [RFC 00/29] De-stage android's sync framework Gustavo Padovan <gustavo@padovan.org> - 2016-01-19 21:20 +0100
      Re: [RFC 00/29] De-stage android's sync framework Daniel Vetter <daniel@ffwll.ch> - 2016-01-19 21:40 +0100
  Re: [RFC 00/29] De-stage android's sync framework Maarten Lankhorst <maarten.lankhorst@linux.intel.com> - 2016-01-20 11:30 +0100
    Re: [RFC 00/29] De-stage android's sync framework Gustavo Padovan <gustavo@padovan.org> - 2016-01-20 15:40 +0100
      Re: [RFC 00/29] De-stage android's sync framework Maarten Lankhorst <maarten.lankhorst@linux.intel.com> - 2016-01-20 16:10 +0100
        Re: [RFC 00/29] De-stage android's sync framework Daniel Vetter <daniel@ffwll.ch> - 2016-01-20 17:30 +0100
        Re: [RFC 00/29] De-stage android's sync framework Gustavo Padovan <gustavo@padovan.org> - 2016-01-20 19:30 +0100

csiph-web