On 15/01/2016 14:55, Gustavo Padovan wrote:
> From: Gustavo Padovan <gustavo.padovan at 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 at 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);
