Hi Shuah,
On Wed, 2017-08-02 at 13:45 -0600, Shuah Khan wrote:
On 07/31/2017 01:43 PM, Gustavo Padovan wrote:
2017-07-30 Chris Wilson chris@chris-wilson.co.uk:
Quoting Gustavo Padovan (2017-07-29 16:22:17)
From: Gustavo Padovan gustavo.padovan@collabora.com
We found this bug in the sw_sync so adding a test case to prevent it to happen in the future.
Cc: Shuah Khan shuahkh@osg.samsung.com Cc: linux-kselftest@vger.kernel.org Signed-off-by: Gustavo Padovan gustavo.padovan@collabora.com
To be applied after the TAP13 convertion patches.
tools/testing/selftests/sync/sync_fence.c | 23 +++++++++++++++++++++++ tools/testing/selftests/sync/sync_test.c | 1 + tools/testing/selftests/sync/synctest.h | 1 + 3 files changed, 25 insertions(+)
diff --git a/tools/testing/selftests/sync/sync_fence.c b/tools/testing/selftests/sync/sync_fence.c index 13f1752..70cfa61 100644 --- a/tools/testing/selftests/sync/sync_fence.c +++ b/tools/testing/selftests/sync/sync_fence.c @@ -29,6 +29,29 @@ #include "sw_sync.h" #include "synctest.h" +int test_close_fence_fd_before_inc(void) +{ + int fence, valid, ret; + int timeline = sw_sync_timeline_create();
+ valid = sw_sync_timeline_is_valid(timeline); + ASSERT(valid, "Failure allocating timeline\n");
+ fence = sw_sync_fence_create(timeline, "allocFence", 1); + valid = sw_sync_fence_is_valid(fence); + ASSERT(valid, "Failure allocating fence\n");
/* * We want the destroy + inc to run within the same RCU grace period so * that the zombie fence still exists on the timeline. */
+ sw_sync_fence_destroy(fence);
I think this doesn't exercise the bug you found as we should be entering the timeline_inc loop with fence.refcount==0 rather than the refcount going to zero within the loop.
To achieve that we need to find a callback that does unreference a dma-fence and chain those together so that it frees a sw_sync from the same timeline.
Indeed. Without the internal callback this test is a bit useless. We could test this under drm atomic tests on IGT. Particulary, I hit it playing with tests for v4l2 fences.
Hi Gustavo,
Would you like this pulled into 4.14-rc1? Sounds like this test is for a feature that doesn't exist yet?
It is for the current code, but I need to find a better way to trigger what I want. I'll rethink this test and resend.
Gustavo