[PATCH] dma-buf/sync_file: Increment refcount of fence when all are signaled.

[Chris Wilson]

On Wed, Sep 14, 2016 at 04:46:18PM -0700, Rafael Antognolli wrote:
> When we merge several fences, if all of them are signaled already, we
> still keep one of them. So instead of using add_fence(), which will not
> increase the refcount of signaled fences, we should explicitly call
> fence_get() for the fence we are keeping.
> 
> This patch fixes a kernel panic that can be triggered by creating a fence that
> is expired (or increasing the timeline until it expires), then creating a
> merged fence out of it, and deleting the merged fence. This will make the
> original expired fence's refcount go to zero.
> 

Testcase: igt/sw_sync/...?
> Signed-off-by: Rafael Antognolli 

Lgtm,
Reviewed-by: Chris Wilson 
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

[PATCH] dma-buf/sync_file: Increment refcount of fence when all are signaled.

[Gustavo Padovan]

Hi Rafael,

2016-09-14 Rafael Antognolli :

> When we merge several fences, if all of them are signaled already, we
> still keep one of them. So instead of using add_fence(), which will not
> increase the refcount of signaled fences, we should explicitly call
> fence_get() for the fence we are keeping.
> 
> This patch fixes a kernel panic that can be triggered by creating a fence that
> is expired (or increasing the timeline until it expires), then creating a
> merged fence out of it, and deleting the merged fence. This will make the
> original expired fence's refcount go to zero.
> 
> Signed-off-by: Rafael Antognolli 
> ---
>  drivers/dma-buf/sync_file.c | 6 ++
>  1 file changed, 2 insertions(+), 4 deletions(-)

Yes, mention the igt test case would be good.

Otherwise:

Reviewed-by: Gustavo Padovan 

Gustavo

[PATCH] dma-buf/sync_file: Increment refcount of fence when all are signaled.

[Rafael Antognolli]

When we merge several fences, if all of them are signaled already, we
still keep one of them. So instead of using add_fence(), which will not
increase the refcount of signaled fences, we should explicitly call
fence_get() for the fence we are keeping.

This patch fixes a kernel panic that can be triggered by creating a fence that
is expired (or increasing the timeline until it expires), then creating a
merged fence out of it, and deleting the merged fence. This will make the
original expired fence's refcount go to zero.

Signed-off-by: Rafael Antognolli 
---
 drivers/dma-buf/sync_file.c | 6 ++
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/dma-buf/sync_file.c b/drivers/dma-buf/sync_file.c
index abb5fda..0fe7ec2 100644
--- a/drivers/dma-buf/sync_file.c
+++ b/drivers/dma-buf/sync_file.c
@@ -253,10 +253,8 @@ static struct sync_file *sync_file_merge(const char *name, 
struct sync_file *a,
for (; i_b < b_num_fences; i_b++)
add_fence(fences, &i, b_fences[i_b]);

-   if (i == 0) {
-   add_fence(fences, &i, a_fences[0]);
-   i++;
-   }
+   if (i == 0)
+   fences[i++] = fence_get(a_fences[0]);

if (num_fences > i) {
nfences = krealloc(fences, i * sizeof(*fences),
-- 
2.7.4