| Message ID | 20161117150209.14810-15-robert.foss@collabora.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, Nov 17, 2016 at 10:02:02AM -0500, robert.foss@collabora.com wrote: > From: Rafael Antognolli <rafael.antognolli@intel.com> > > This test creates an already expired fence, then creates a merged fence > out of that expired one (passed twice to the merge operation), and > finally closes the merged fence. It shows that if the refcounts are > wrong on the original expired fence, it might get freed while still in > use. Usually a kernel panick will follow. > > Signed-off-by: Rafael Antognolli <rafael.antognolli@intel.com> > Signed-off-by: Robert Foss <robert.foss@collabora.com> > --- > tests/sw_sync.c | 28 ++++++++++++++++++++++++++++ > 1 file changed, 28 insertions(+) > > diff --git a/tests/sw_sync.c b/tests/sw_sync.c > index 4e52f48..65416ad 100644 > --- a/tests/sw_sync.c > +++ b/tests/sw_sync.c > @@ -582,6 +582,31 @@ static void test_sync_multi_producer_single_consumer(void) > pthread_join(threads[i], NULL); > } > > +static void test_sync_expired_merge(void) > +{ > + int iterations = 1 << 20; > + int timeline; > + int i; > + int fence_expired, fence_merged; > + > + timeline = sw_sync_timeline_create(); > + > + sw_sync_timeline_inc(timeline, 100); > + fence_expired = sw_sync_fence_create(timeline, 1); igt_assert(sync_wait(fence_expired, 0) == 1); or igt_assert(fence_signaled(fence_expired)); Both illustrative of the setup, and tests them. > + fence_merged = sync_merge(fence_expired, fence_expired); > + close(fence_merged); > + > + for (i = 0; i < iterations; i++) { > + int fence = sync_merge(fence_expired, fence_expired); > + > + igt_assert_f(sync_wait(fence, -1) == 0, > + "Failure waiting on fence\n"); > + close(fence); Is the millionth iteration more likely to fail than iteration 0? There's no change in code coverage, so is it worth the extra iterations? -Chris
On Thu, 2016-11-17 at 15:39 +0000, Chris Wilson wrote: > On Thu, Nov 17, 2016 at 10:02:02AM -0500, robert.foss@collabora.com > wrote: > > > > From: Rafael Antognolli <rafael.antognolli@intel.com> > > > > This test creates an already expired fence, then creates a merged > > fence > > out of that expired one (passed twice to the merge operation), and > > finally closes the merged fence. It shows that if the refcounts are > > wrong on the original expired fence, it might get freed while still > > in > > use. Usually a kernel panick will follow. > > > > Signed-off-by: Rafael Antognolli <rafael.antognolli@intel.com> > > Signed-off-by: Robert Foss <robert.foss@collabora.com> > > --- > > tests/sw_sync.c | 28 ++++++++++++++++++++++++++++ > > 1 file changed, 28 insertions(+) > > > > diff --git a/tests/sw_sync.c b/tests/sw_sync.c > > index 4e52f48..65416ad 100644 > > --- a/tests/sw_sync.c > > +++ b/tests/sw_sync.c > > @@ -582,6 +582,31 @@ static void > > test_sync_multi_producer_single_consumer(void) > > pthread_join(threads[i], NULL); > > } > > > > +static void test_sync_expired_merge(void) > > +{ > > + int iterations = 1 << 20; > > + int timeline; > > + int i; > > + int fence_expired, fence_merged; > > + > > + timeline = sw_sync_timeline_create(); > > + > > + sw_sync_timeline_inc(timeline, 100); > > + fence_expired = sw_sync_fence_create(timeline, 1); > igt_assert(sync_wait(fence_expired, 0) == 1); > or > igt_assert(fence_signaled(fence_expired)); > > Both illustrative of the setup, and tests them. Agreed, I'll add it for v7. > > > > > + fence_merged = sync_merge(fence_expired, fence_expired); > > + close(fence_merged); > > + > > + for (i = 0; i < iterations; i++) { > > + int fence = sync_merge(fence_expired, > > fence_expired); > > + > > + igt_assert_f(sync_wait(fence, -1) == 0, > > + "Failure waiting on > > fence\n"); > > + close(fence); > Is the millionth iteration more likely to fail than iteration 0? > > There's no change in code coverage, so is it worth the extra > iterations? I don't know if it is more likely to fail or not, which is why multiple iterations are tested. If you can tell me with certainty that multiple iterations always will behave the same way I would be happy to remove the multiple iterations. Granted, there are a few other tests that also do stress testing as a part of what they test. So maybe this is redundant either way?
diff --git a/tests/sw_sync.c b/tests/sw_sync.c index 4e52f48..65416ad 100644 --- a/tests/sw_sync.c +++ b/tests/sw_sync.c @@ -582,6 +582,31 @@ static void test_sync_multi_producer_single_consumer(void) pthread_join(threads[i], NULL); } +static void test_sync_expired_merge(void) +{ + int iterations = 1 << 20; + int timeline; + int i; + int fence_expired, fence_merged; + + timeline = sw_sync_timeline_create(); + + sw_sync_timeline_inc(timeline, 100); + fence_expired = sw_sync_fence_create(timeline, 1); + fence_merged = sync_merge(fence_expired, fence_expired); + close(fence_merged); + + for (i = 0; i < iterations; i++) { + int fence = sync_merge(fence_expired, fence_expired); + + igt_assert_f(sync_wait(fence, -1) == 0, + "Failure waiting on fence\n"); + close(fence); + } + + close(fence_expired); +} + static void test_sync_random_merge(void) { int i, size, ret; @@ -687,6 +712,9 @@ igt_main igt_subtest("sync_multi_producer_single_consumer") test_sync_multi_producer_single_consumer(); + igt_subtest("sync_expired_merge") + test_sync_expired_merge(); + igt_subtest("sync_random_merge") test_sync_random_merge(); }