On Mon, Mar 27, 2017 at 11:45:07AM +0300, Ander Conselvan de Oliveira wrote:
> Currently, the main thread needs to wakeup to run the signal handler
> that ends a spin batch. When testing whether a function call succesfully
> waits for a batch to complete, this behavior is undesired. It actually
> invalidates the test.
>
> Fix this by spawning a new thread to handle the timeout.
Very neat!
> Cc: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
> Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@xxxxxxxxx>
> ---
> lib/igt_dummyload.c | 55 +++++++++++++++++------------------------------------
> lib/igt_dummyload.h | 3 ++-
> 2 files changed, 19 insertions(+), 39 deletions(-)
>
> diff --git a/lib/igt_dummyload.c b/lib/igt_dummyload.c
> index 019c1fb..38b1930 100644
> --- a/lib/igt_dummyload.c
> +++ b/lib/igt_dummyload.c
> @@ -24,6 +24,7 @@
>
> #include "igt.h"
> #include "igt_dummyload.h"
> +#include <pthread.h>
> #include <time.h>
> #include <signal.h>
> #include <sys/syscall.h>
> @@ -166,6 +167,8 @@ igt_spin_batch_new(int fd, int engine, unsigned int dep_handle)
> spin = calloc(1, sizeof(struct igt_spin));
> igt_assert(spin);
>
> + pthread_mutex_init(&spin->mutex, NULL);
> +
> emit_recursive_batch(spin, fd, engine, dep_handle);
> igt_assert(gem_bo_busy(fd, spin->handle));
>
> @@ -174,27 +177,11 @@ igt_spin_batch_new(int fd, int engine, unsigned int dep_handle)
> return spin;
> }
>
> -static void clear_sig_handler(int sig)
> -{
> - struct sigaction act;
> -
> - memset(&act, 0, sizeof(act));
> - act.sa_handler = SIG_DFL;
> - igt_assert(sigaction(sig, &act, NULL) == 0);
> -}
> -
> -static void sig_handler(int sig, siginfo_t *info, void *arg)
> +static void notify(union sigval arg)
> {
> - struct igt_spin *iter;
> + igt_spin_t *spin = arg.sival_ptr;
>
> - igt_list_for_each(iter, &spin_list, link) {
> - if (iter->signo == info->si_signo) {
> - igt_spin_batch_end(iter);
> - return;
> - }
> - }
> -
> - clear_sig_handler(sig);
> + igt_spin_batch_end(spin);
> }
>
> /**
> @@ -208,43 +195,33 @@ static void sig_handler(int sig, siginfo_t *info, void *arg)
> */
> void igt_spin_batch_set_timeout(igt_spin_t *spin, int64_t ns)
> {
> - static int spin_signo = 48; /* Midpoint of SIGRTMIN, SIGRTMAX */
> timer_t timer;
> struct sigevent sev;
> - struct sigaction act, oldact;
> struct itimerspec its;
>
> + pthread_mutex_lock(&spin->mutex);
> +
> igt_assert(ns > 0);
> if (!spin)
> return;
A mutex here is interesting, but only for detecting a race in setting
the timeout twice. Make it assert instead, and we then don't need a
mutex at all.
> igt_assert(!spin->timer);
>
> - /* SIGRTMAX is used by valgrind, SIGRTMAX - 1 by igt_fork_hang_detector */
> - if (spin_signo >= SIGRTMAX - 2)
> - spin_signo = SIGRTMIN;
> - spin->signo = ++spin_signo;
> -
> memset(&sev, 0, sizeof(sev));
> - sev.sigev_notify = SIGEV_SIGNAL | SIGEV_THREAD_ID;
> - sev.sigev_notify_thread_id = gettid();
> - sev.sigev_signo = spin->signo;
> + sev.sigev_notify = SIGEV_THREAD;
> + sev.sigev_value.sival_ptr = spin;
> + sev.sigev_notify_function = notify;
> igt_assert(timer_create(CLOCK_MONOTONIC, &sev, &timer) == 0);
> igt_assert(timer);
>
> - memset(&oldact, 0, sizeof(oldact));
> - memset(&act, 0, sizeof(act));
> - act.sa_sigaction = sig_handler;
> - act.sa_flags = SA_SIGINFO;
> - igt_assert(sigaction(spin->signo, &act, &oldact) == 0);
> - igt_assert(oldact.sa_sigaction == NULL);
> -
> memset(&its, 0, sizeof(its));
> its.it_value.tv_sec = ns / NSEC_PER_SEC;
> its.it_value.tv_nsec = ns % NSEC_PER_SEC;
> igt_assert(timer_settime(timer, 0, &its, NULL) == 0);
>
> spin->timer = timer;
> +
> + pthread_mutex_unlock(&spin->mutex);
> }
>
> /**
> @@ -258,11 +235,12 @@ void igt_spin_batch_end(igt_spin_t *spin)
> if (!spin)
> return;
>
> + pthread_mutex_lock(&spin->mutex);
> +
> *spin->batch = MI_BATCH_BUFFER_END;
> __sync_synchronize();
>
> - if (spin->signo)
> - clear_sig_handler(spin->signo);
> + pthread_mutex_unlock(&spin->mutex);
This doesn't require a mutex.
> }
>
> /**
> @@ -287,6 +265,7 @@ void igt_spin_batch_free(int fd, igt_spin_t *spin)
> gem_munmap(spin->batch, BATCH_SIZE);
>
> gem_close(fd, spin->handle);
> + pthread_mutex_destroy(&spin->mutex);
We need to cancel the timer (before the unmap).
> free(spin);
> }
>
> diff --git a/lib/igt_dummyload.h b/lib/igt_dummyload.h
> index 2adfadf..8fdc3fe 100644
> --- a/lib/igt_dummyload.h
> +++ b/lib/igt_dummyload.h
> @@ -31,11 +31,12 @@
> #include "igt_aux.h"
>
> typedef struct igt_spin {
> + pthread_mutex_t mutex;
> unsigned int handle;
> timer_t timer;
> - int signo;
> struct igt_list link;
> uint32_t *batch;
> +
Oi!
> } igt_spin_t;
--
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
