Re: [PATCH v1 11/13] tests/sw_sync: Add subtest test_sync_random_merge

[Eric Engestrom <eric@engestrom.ch>]

On Tue, Aug 23, 2016 at 01:56:13PM -0400, robert.foss@collabora.com wrote:
> From: Robert Foss <robert.foss@collabora.com>
> 
> This subtest verifies that creating many timelines and merging random fences
> from each timeline with eachother results in merged fences that are fully
> functional.
> 
> Signed-off-by: Robert Foss <robert.foss@collabora.com>
> ---
>  tests/sw_sync.c | 73 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 73 insertions(+)
> 
> diff --git a/tests/sw_sync.c b/tests/sw_sync.c
> index 0e67ad5..8e5a7c9 100644
> --- a/tests/sw_sync.c
> +++ b/tests/sw_sync.c
> @@ -383,6 +383,76 @@ static void test_sync_multi_consumer_producer(void)
>  	igt_assert_f(thread_ret == 0, "A sync thread reported failure.\n");
>  }
>  
> +static void test_sync_random_merge(void)
> +{
> +	int i, size, ret;
> +	const int nbr_timeline = 32;
> +	const int nbr_merge = 1024;
> +	int fence_map[nbr_timeline];
> +	int timeline_arr[nbr_timeline];
> +	int fence, tmpfence, merged;
> +	int timeline, timeline_offset, sync_pt;
> +
> +	srand(time(NULL));
> +
> +	for (i = 0; i < nbr_timeline; i++)
> +		timeline_arr[i] = sw_sync_timeline_create();
> +
> +	memset(fence_map, -1, sizeof(fence_map));

This sets each byte to -1, which happens to be the same as an int -1,
but I don't really like it: this is only true for two special values, -1
and 0, and would become a bug if it was ever changed.

I'd much prefer if you simply set:
	fence_map[i] = -1;
in the loop you already have just above.

BTW, seeing how many times you set and test for -1 as the "invalid fd
sentinel", how about a:
	#define INVALID_FD (-1)
and use that token throughout the code? I think it would improve
readability as well

> +
> +	sync_pt = rand();
> +	fence = sw_sync_fence_create(timeline_arr[0], sync_pt);
> +
> +	fence_map[0] = sync_pt;
> +
> +	/* Randomly create syncpoints out of a fixed set of timelines,
> +	 * and merge them together.
> +	 */
> +	for (i = 0; i < nbr_merge; i++) {
> +		/* Generate syncpoint. */
> +		timeline_offset = rand() % nbr_timeline;
> +		timeline = timeline_arr[timeline_offset];
> +		sync_pt = rand();
> +
> +		/* Keep track of the latest sync_pt in each timeline. */
> +		if (fence_map[timeline_offset] == -1)
> +			fence_map[timeline_offset] = sync_pt;
> +		else if (fence_map[timeline_offset] < sync_pt)
> +			fence_map[timeline_offset] = sync_pt;
> +
> +		/* Merge. */
> +		tmpfence = sw_sync_fence_create(timeline, sync_pt);
> +		merged = sw_sync_merge(tmpfence, fence);
> +		sw_sync_fence_destroy(tmpfence);
> +		sw_sync_fence_destroy(fence);
> +		fence = merged;
> +	}
> +
> +	size = 0;
> +	for (i = 0; i < nbr_timeline; i++)
> +		if (fence_map[i] != -1)
> +			size++;
> +
> +	/* Trigger the merged fence. */
> +	for (i = 0; i < nbr_timeline; i++) {
> +		if (fence_map[i] != -1) {
> +			ret = sw_sync_wait(fence, 0);
> +			igt_assert_f(ret == 0,
> +				    "Failure waiting on fence until timeout\n");
> +			/* Increment the timeline to the last sync_pt */
> +			sw_sync_timeline_inc(timeline_arr[i], fence_map[i]);
> +		}
> +	}
> +
> +	/* Check that the fence is triggered. */
> +	ret = sw_sync_wait(fence, 0);
> +	igt_assert_f(ret > 0, "Failure triggering fence\n");
> +
> +	sw_sync_fence_destroy(fence);
> +	for (i = 0; i < nbr_timeline; i++)
> +		sw_sync_timeline_destroy(timeline_arr[i]);
> +}
> +
>  igt_main
>  {
>  	igt_subtest("alloc_timeline")
> @@ -411,5 +481,8 @@ igt_main
>  
>  	igt_subtest("sync_multi_consumer_producer")
>  		test_sync_multi_consumer_producer();
> +
> +	igt_subtest("sync_random_merge")
> +		test_sync_random_merge();
>  }
>  
> -- 
> 2.7.4
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev