2016-01-15 John Harrison <John.C.Harrison@xxxxxxxxx>: > On 15/01/2016 14:55, Gustavo Padovan wrote: > >From: Gustavo Padovan <gustavo.padovan@xxxxxxxxxxxxxxx> > > > >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@xxxxxxxxxxxxxxx> > >--- > > 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. > Patches 27 and 28 are attempt to fix that. I assumed that if some code is calling fence_timeline_destroy() it wants to stop everything so I worked on a solution that stops any waiter and allows the timeline to be destroyed. No one is using fence_timeline_destroy() in mainline now, so it is definately a behaviour we can discuss. Gustavo _______________________________________________ devel mailing list devel@xxxxxxxxxxxxxxxxxxxxxx http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel