| Message ID | 1394789268-28703-1-git-send-email-daniel.vetter@ffwll.ch (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, Mar 14, 2014 at 10:27:46AM +0100, Daniel Vetter wrote: > We don't call cleanup handlers when exiting a subtest currently, only > when exiting the entire binary. Which means pm_rps falls over when it > fails more than one subtest. > > Cc: Jeff McGee <jeff.mcgee@intel.com> > Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch> > --- > tests/pm_rps.c | 32 +++++++++++++++++++------------- > 1 file changed, 19 insertions(+), 13 deletions(-) > > diff --git a/tests/pm_rps.c b/tests/pm_rps.c > index a652cf580dc7..b1cd13fc33a7 100644 > --- a/tests/pm_rps.c > +++ b/tests/pm_rps.c > @@ -196,9 +196,27 @@ static void emit_store_dword_imm(uint32_t val) > } > > #define LOAD_HELPER_PAUSE_USEC 500 > +static void load_helper_set_load(enum load load) > +{ > + assert(lh.igt_proc.running); > + > + if (lh.load == load) > + return; > + > + lh.load = load; > + kill(lh.igt_proc.pid, SIGUSR2); > +} > + > static void load_helper_run(enum load load) > { > - assert(!lh.igt_proc.running); > + /* > + * FIXME fork helpers won't get cleaned up when started from within a > + * subtest, so handle the case where it sticks around a bit too long. > + */ > + if (lh.igt_proc.running) { > + load_helper_set_load(load); > + return; > + } > > igt_require(lh.ready == true); > > @@ -229,20 +247,8 @@ static void load_helper_run(enum load load) > } > } > > -static void load_helper_set_load(enum load load) > -{ > - assert(lh.igt_proc.running); > - > - if (lh.load == load) > - return; > - > - lh.load = load; > - kill(lh.igt_proc.pid, SIGUSR2); > -} > - > static void load_helper_stop(void) > { > - assert(lh.igt_proc.running); > kill(lh.igt_proc.pid, SIGUSR1); > igt_wait_helper(&lh.igt_proc); > } > -- > 1.8.4.rc3 > Unfortunately there are probably several ways in which a failed subtest will contaminate subsequent subtests if all are run in the same process instance. I asked about that earlier and you said that we don't concern too much about it because the preferred way to run is with a test runner and each subtest executed in a separate instance. If we do in fact care about supporting all subtests in a single instance, can we put in place a subtest exit handler? That would solve all issues similar to this. -Jeff
On Fri, Mar 14, 2014 at 3:29 PM, Jeff McGee <jeff.mcgee@intel.com> wrote: > Unfortunately there are probably several ways in which a failed subtest will > contaminate subsequent subtests if all are run in the same process instance. > I asked about that earlier and you said that we don't concern too much about > it because the preferred way to run is with a test runner and each subtest > executed in a separate instance. If we do in fact care about supporting all > subtests in a single instance, can we put in place a subtest exit handler? > That would solve all issues similar to this. Yeah, that's my long-term idea, hence the FIXME comment. But historically I've fumbled way too many special-cases wrt forked children, subtests and getting the exit handler logic right. So I want to do this carefully and have it all covered in a pile of igt testcases to make sure it actually does what I want it to do. Since I'm terribly busy writing igt library api docs atm I've postponed this. The quick hack in this patch here just fixed the annoying assert while I've hacked around on the pm_rps testcase. -Daniel
diff --git a/tests/pm_rps.c b/tests/pm_rps.c index a652cf580dc7..b1cd13fc33a7 100644 --- a/tests/pm_rps.c +++ b/tests/pm_rps.c @@ -196,9 +196,27 @@ static void emit_store_dword_imm(uint32_t val) } #define LOAD_HELPER_PAUSE_USEC 500 +static void load_helper_set_load(enum load load) +{ + assert(lh.igt_proc.running); + + if (lh.load == load) + return; + + lh.load = load; + kill(lh.igt_proc.pid, SIGUSR2); +} + static void load_helper_run(enum load load) { - assert(!lh.igt_proc.running); + /* + * FIXME fork helpers won't get cleaned up when started from within a + * subtest, so handle the case where it sticks around a bit too long. + */ + if (lh.igt_proc.running) { + load_helper_set_load(load); + return; + } igt_require(lh.ready == true); @@ -229,20 +247,8 @@ static void load_helper_run(enum load load) } } -static void load_helper_set_load(enum load load) -{ - assert(lh.igt_proc.running); - - if (lh.load == load) - return; - - lh.load = load; - kill(lh.igt_proc.pid, SIGUSR2); -} - static void load_helper_stop(void) { - assert(lh.igt_proc.running); kill(lh.igt_proc.pid, SIGUSR1); igt_wait_helper(&lh.igt_proc); }
We don't call cleanup handlers when exiting a subtest currently, only when exiting the entire binary. Which means pm_rps falls over when it fails more than one subtest. Cc: Jeff McGee <jeff.mcgee@intel.com> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch> --- tests/pm_rps.c | 32 +++++++++++++++++++------------- 1 file changed, 19 insertions(+), 13 deletions(-)