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 rafael.antognolli@intel.com --- 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),
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 rafael.antognolli@intel.com
Lgtm, Reviewed-by: Chris Wilson chris@chris-wilson.co.uk -Chris
Hi Rafael,
2016-09-14 Rafael Antognolli rafael.antognolli@intel.com:
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 rafael.antognolli@intel.com
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.padovan@collabora.co.uk
Gustavo
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/sync_expired_merge Signed-off-by: Rafael Antognolli rafael.antognolli@intel.com --- 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),
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/sync_expired_merge Signed-off-by: Rafael Antognolli rafael.antognolli@intel.com Reviewed-by: Chris Wilson chris@chris-wilson.co.uk Reviewed-by: Gustavo Padovan gustavo.padovan@collabora.co.uk --- 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),
Hi Rafael,
On 16 September 2016 at 00:44, Rafael Antognolli rafael.antognolli@intel.com 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/sync_expired_merge Signed-off-by: Rafael Antognolli rafael.antognolli@intel.com Reviewed-by: Chris Wilson chris@chris-wilson.co.uk Reviewed-by: Gustavo Padovan gustavo.padovan@collabora.co.uk
Thanks for the patch; applied via drm-misc.
drivers/dma-buf/sync_file.c | 6 ++----
Best, Sumit.
dri-devel@lists.freedesktop.org