Re: [PATCH 3/3] selftests: sync: add test that closes the fd before fence signal

[Gustavo Padovan <gustavo@padovan.org>]

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.

	Gustavo
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel