summaryrefslogtreecommitdiff
path: root/tests
diff options
context:
space:
mode:
authorChris Wilson <chris@chris-wilson.co.uk>2018-05-02 10:24:26 +0100
committerChris Wilson <chris@chris-wilson.co.uk>2018-06-15 12:04:42 +0100
commit91f5d4665b07f073c78abd3cd4b8e0e347dbf638 (patch)
tree4d1e2f6030a4c188d692dab88965152c09c92f26 /tests
parentb7a2e4e35b8017fbf043fcbefae318813feee462 (diff)
igt/sw_sync: Wrap threaded counter manipulation with mb
sw_sync/sync_multi_consumer_producer was communicating between threads using the sw_sync ioctl and manipulating a shared volatile counter. However, the ioctl itself does not imply a memory barrier, and so different CPUs may see different states of the counter (the volatile making GCC perform the operation in stages making the race even more likely). Instead of using volatile, use locked operations to make the counter manipulation thread-safe. References: https://bugs.freedesktop.org/show_bug.cgi?id=106344 Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Diffstat (limited to 'tests')
-rw-r--r--tests/sw_sync.c24
1 files changed, 11 insertions, 13 deletions
diff --git a/tests/sw_sync.c b/tests/sw_sync.c
index 20dfbbb9..c6867e09 100644
--- a/tests/sw_sync.c
+++ b/tests/sw_sync.c
@@ -43,7 +43,7 @@ IGT_TEST_DESCRIPTION("Test SW Sync Framework");
typedef struct {
int timeline;
uint32_t thread_id;
- volatile uint32_t * volatile counter;
+ uint32_t *counter;
sem_t *sem;
} data_t;
@@ -508,7 +508,7 @@ static void * test_sync_multi_consumer_thread(void *arg)
if (sync_fence_wait(fence, 1000) < 0)
return (void *) 1;
- if (*(data->counter) != next_point)
+ if (READ_ONCE(*data->counter) != next_point)
return (void *) 1;
sem_post(data->sem);
@@ -524,7 +524,7 @@ static void test_sync_multi_consumer(void)
pthread_t thread_arr[MULTI_CONSUMER_THREADS];
sem_t sem;
int timeline;
- volatile uint32_t counter = 0;
+ uint32_t counter = 0;
uintptr_t thread_ret = 0;
data_t data;
int i, ret;
@@ -552,7 +552,7 @@ static void test_sync_multi_consumer(void)
{
sem_wait(&sem);
- counter++;
+ __sync_fetch_and_add(&counter, 1);
sw_sync_timeline_inc(timeline, 1);
}
@@ -567,8 +567,8 @@ static void test_sync_multi_consumer(void)
close(timeline);
sem_destroy(&sem);
- igt_assert_f(counter == MULTI_CONSUMER_THREADS * MULTI_CONSUMER_ITERATIONS,
- "Counter has unexpected value.\n");
+ igt_assert_eq(counter,
+ MULTI_CONSUMER_THREADS * MULTI_CONSUMER_ITERATIONS);
igt_assert_f(thread_ret == 0, "A sync thread reported failure.\n");
}
@@ -589,11 +589,9 @@ static void * test_sync_multi_consumer_producer_thread(void *arg)
if (sync_fence_wait(fence, 1000) < 0)
return (void *) 1;
- if (*(data->counter) != next_point)
+ if (__sync_fetch_and_add(data->counter, 1) != next_point)
return (void *) 1;
- (*data->counter)++;
-
/* Kick off the next thread. */
sw_sync_timeline_inc(timeline, 1);
@@ -607,7 +605,7 @@ static void test_sync_multi_consumer_producer(void)
data_t data_arr[MULTI_CONSUMER_PRODUCER_THREADS];
pthread_t thread_arr[MULTI_CONSUMER_PRODUCER_THREADS];
int timeline;
- volatile uint32_t counter = 0;
+ uint32_t counter = 0;
uintptr_t thread_ret = 0;
data_t data;
int i, ret;
@@ -638,9 +636,9 @@ static void test_sync_multi_consumer_producer(void)
close(timeline);
- igt_assert_f(counter == MULTI_CONSUMER_PRODUCER_THREADS *
- MULTI_CONSUMER_PRODUCER_ITERATIONS,
- "Counter has unexpected value.\n");
+ igt_assert_eq(counter,
+ MULTI_CONSUMER_PRODUCER_THREADS *
+ MULTI_CONSUMER_PRODUCER_ITERATIONS);
igt_assert_f(thread_ret == 0, "A sync thread reported failure.\n");
}