On Tue, Oct 21, 2014 at 01:52:46PM -0200, Paulo Zanoni wrote: > 2014-10-21 13:18 GMT-02:00 Daniel Vetter <daniel@xxxxxxxx>: > > On Tue, Oct 14, 2014 at 04:12:22PM -0300, Paulo Zanoni wrote: > >> From: Paulo Zanoni <paulo.r.zanoni@xxxxxxxxx> > >> > >> If the signal helper is active, the usleep() calls return earlier, and > >> we may end up returning false way before the 10s timeout, failing the > >> subtests. This currently happens on the kms_flip RPM interruptible > >> subtests. > >> > >> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=78893 > >> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@xxxxxxxxx> > >> --- > >> lib/igt_aux.c | 17 ++++++++++++----- > >> 1 file changed, 12 insertions(+), 5 deletions(-) > >> > >> diff --git a/lib/igt_aux.c b/lib/igt_aux.c > >> index b32297e..01654f0 100644 > >> --- a/lib/igt_aux.c > >> +++ b/lib/igt_aux.c > >> @@ -42,6 +42,7 @@ > >> #include <stdlib.h> > >> #include <unistd.h> > >> #include <sys/wait.h> > >> +#include <sys/time.h> > >> #include <sys/types.h> > >> #include <sys/syscall.h> > >> #include <sys/utsname.h> > >> @@ -502,21 +503,27 @@ enum igt_runtime_pm_status igt_get_runtime_pm_status(void) > >> * Waits until for the driver to switch to into the desired runtime PM status, > >> * with a 10 second timeout. > >> * > >> + * Some subtests call this function while the signal helper is active, so we > >> + * can't assume each usleep() call will sleep for 100ms. > >> + * > >> * Returns: > >> * True if the desired runtime PM status was attained, false if the operation > >> * timed out. > >> */ > >> bool igt_wait_for_pm_status(enum igt_runtime_pm_status status) > >> { > >> - int i; > >> - int hundred_ms = 100 * 1000, ten_s = 10 * 1000 * 1000; > >> + struct timeval start, end, diff; > >> > >> - for (i = 0; i < ten_s; i += hundred_ms) { > >> + igt_assert(gettimeofday(&start, NULL) == 0); > >> + do { > >> if (igt_get_runtime_pm_status() == status) > >> return true; > >> > >> - usleep(hundred_ms); > >> - } > >> + usleep(100 * 1000); > >> + > >> + igt_assert(gettimeofday(&end, NULL) == 0); > >> + timersub(&end, &start, &diff); > > > > Should we have a generic igt_usleep helper which is signal-robust? Just a > > quick idea since there's probably more like these. > > If we think we could use it in many places, it makes sense. I'm not > aware yet of where we could use it. Do a cocci patch to replace all the existing usleep with igt_usleep extracted from here. Run it on tests/*.c. We already have a cocci spatch in lib/igt.cocci with lots of default refactorings like that to replace C functions with igt helpers where it makes sense. Signed up? And even if it's just to learn a bit how coccinelle works, that alone is awesome ;-) -Daniel > > > -Daniel > > > >> + } while (diff.tv_sec < 10); > >> > >> return false; > >> } > >> -- > >> 1.9.1 > >> > >> _______________________________________________ > >> Intel-gfx mailing list > >> Intel-gfx@xxxxxxxxxxxxxxxxxxxxx > >> http://lists.freedesktop.org/mailman/listinfo/intel-gfx > > > > -- > > Daniel Vetter > > Software Engineer, Intel Corporation > > +41 (0) 79 365 57 48 - http://blog.ffwll.ch > > > > -- > Paulo Zanoni -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx
