diff mbox

[i-g-t,4/8] tests/gem_close_race: Tune down for BAT ~1s.

Message ID 1464349838-9959-4-git-send-email-marius.c.vlad@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Marius Vlad May 27, 2016, 11:50 a.m. UTC
Signed-off-by: Marius Vlad <marius.c.vlad@intel.com>
---
 tests/gem_close_race.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Chris Wilson May 27, 2016, 11:58 a.m. UTC | #1
On Fri, May 27, 2016 at 02:50:34PM +0300, Marius Vlad wrote:
> Signed-off-by: Marius Vlad <marius.c.vlad@intel.com>

Nak. It's a race detector. Please suggest how to increase detection
rates.
-Chris
Tvrtko Ursulin May 27, 2016, 12:10 p.m. UTC | #2
On 27/05/16 12:58, Chris Wilson wrote:
> On Fri, May 27, 2016 at 02:50:34PM +0300, Marius Vlad wrote:
>> Signed-off-by: Marius Vlad <marius.c.vlad@intel.com>
>
> Nak. It's a race detector. Please suggest how to increase detection
> rates.

As a more or less well know TV personality would say - "it's better than 
nothing"! :))

Seriously, under the new rules this is all we can do. Full 
gem-close-race will (will it?) run in the nightly run so a little bit 
more pain while bisecting breakages but those are the rules. We can give 
them a go and see how it works out.

Regards,

Tvrtko
Chris Wilson May 27, 2016, 12:16 p.m. UTC | #3
On Fri, May 27, 2016 at 01:10:07PM +0100, Tvrtko Ursulin wrote:
> 
> On 27/05/16 12:58, Chris Wilson wrote:
> >On Fri, May 27, 2016 at 02:50:34PM +0300, Marius Vlad wrote:
> >>Signed-off-by: Marius Vlad <marius.c.vlad@intel.com>
> >
> >Nak. It's a race detector. Please suggest how to increase detection
> >rates.
> 
> As a more or less well know TV personality would say - "it's better
> than nothing"! :))
> 
> Seriously, under the new rules this is all we can do. Full
> gem-close-race will (will it?) run in the nightly run so a little
> bit more pain while bisecting breakages but those are the rules. We
> can give them a go and see how it works out.

Wrong approach. Right approach would be to add a new test that reliably
detected a checklist of the most common races in under 1s. Nerfing a
test to make it useless makes BAT equally useless.

So we are giving up on BAT?
-Chris
Tvrtko Ursulin May 27, 2016, 12:31 p.m. UTC | #4
On 27/05/16 13:16, Chris Wilson wrote:
> On Fri, May 27, 2016 at 01:10:07PM +0100, Tvrtko Ursulin wrote:
>>
>> On 27/05/16 12:58, Chris Wilson wrote:
>>> On Fri, May 27, 2016 at 02:50:34PM +0300, Marius Vlad wrote:
>>>> Signed-off-by: Marius Vlad <marius.c.vlad@intel.com>
>>>
>>> Nak. It's a race detector. Please suggest how to increase detection
>>> rates.
>>
>> As a more or less well know TV personality would say - "it's better
>> than nothing"! :))
>>
>> Seriously, under the new rules this is all we can do. Full
>> gem-close-race will (will it?) run in the nightly run so a little
>> bit more pain while bisecting breakages but those are the rules. We
>> can give them a go and see how it works out.
>
> Wrong approach. Right approach would be to add a new test that reliably
> detected a checklist of the most common races in under 1s. Nerfing a
> test to make it useless makes BAT equally useless.
>
> So we are giving up on BAT?

If it is not possible to stuff everything into the allocated budget, and 
in cases where it may not be possible to come up with a 1s race 
detector, it makes sense to move the test out of BAT and into the 
nightly runs.

Agreement was that the time limits are hard limits so thats pretty much 
it. We can do this now and hit the targets and them add more tests if 
and when someone manages to implement them.

I don't have a problem with that, B in BAT stands for basic anyway.

We just adjust the model that when nightly test run fails someone from 
QA gets tasked with bisection etc as a top priority.

Regards,

Tvrtko
Chris Wilson May 27, 2016, 12:44 p.m. UTC | #5
On Fri, May 27, 2016 at 01:31:10PM +0100, Tvrtko Ursulin wrote:
> 
> On 27/05/16 13:16, Chris Wilson wrote:
> >On Fri, May 27, 2016 at 01:10:07PM +0100, Tvrtko Ursulin wrote:
> >>
> >>On 27/05/16 12:58, Chris Wilson wrote:
> >>>On Fri, May 27, 2016 at 02:50:34PM +0300, Marius Vlad wrote:
> >>>>Signed-off-by: Marius Vlad <marius.c.vlad@intel.com>
> >>>
> >>>Nak. It's a race detector. Please suggest how to increase detection
> >>>rates.
> >>
> >>As a more or less well know TV personality would say - "it's better
> >>than nothing"! :))
> >>
> >>Seriously, under the new rules this is all we can do. Full
> >>gem-close-race will (will it?) run in the nightly run so a little
> >>bit more pain while bisecting breakages but those are the rules. We
> >>can give them a go and see how it works out.
> >
> >Wrong approach. Right approach would be to add a new test that reliably
> >detected a checklist of the most common races in under 1s. Nerfing a
> >test to make it useless makes BAT equally useless.
> >
> >So we are giving up on BAT?
> 
> If it is not possible to stuff everything into the allocated budget,
> and in cases where it may not be possible to come up with a 1s race
> detector, it makes sense to move the test out of BAT and into the
> nightly runs.
> 
> Agreement was that the time limits are hard limits so thats pretty
> much it. We can do this now and hit the targets and them add more
> tests if and when someone manages to implement them.

1s is totally arbitrary, setting it as a hard limit is ridiculous.

> I don't have a problem with that, B in BAT stands for basic anyway.

The most basic userspace appears far more complex than igt. Quite often
BAT passes, but a couple of seconds of dogfooding reveals an unusable
mess. Basic Acceptance Testing is failing in its task, and we seem to
heading down the path of making it fail harder.
-Chris
Mika Kuoppala May 27, 2016, 12:44 p.m. UTC | #6
Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com> writes:

> [ text/plain ]
>
> On 27/05/16 13:16, Chris Wilson wrote:
>> On Fri, May 27, 2016 at 01:10:07PM +0100, Tvrtko Ursulin wrote:
>>>
>>> On 27/05/16 12:58, Chris Wilson wrote:
>>>> On Fri, May 27, 2016 at 02:50:34PM +0300, Marius Vlad wrote:
>>>>> Signed-off-by: Marius Vlad <marius.c.vlad@intel.com>
>>>>
>>>> Nak. It's a race detector. Please suggest how to increase detection
>>>> rates.
>>>
>>> As a more or less well know TV personality would say - "it's better
>>> than nothing"! :))
>>>
>>> Seriously, under the new rules this is all we can do. Full
>>> gem-close-race will (will it?) run in the nightly run so a little
>>> bit more pain while bisecting breakages but those are the rules. We
>>> can give them a go and see how it works out.
>>
>> Wrong approach. Right approach would be to add a new test that reliably
>> detected a checklist of the most common races in under 1s. Nerfing a
>> test to make it useless makes BAT equally useless.
>>
>> So we are giving up on BAT?
>
> If it is not possible to stuff everything into the allocated budget, and 
> in cases where it may not be possible to come up with a 1s race 
> detector, it makes sense to move the test out of BAT and into the 
> nightly runs.
>

With tests to poke out races, I think the best way is to take
them out from BAT and move to nightly.

As A is for acceptance, it is not much value as some poor soul
will hit the race 1 month after we regressed it.

- Mika


> Agreement was that the time limits are hard limits so thats pretty much 
> it. We can do this now and hit the targets and them add more tests if 
> and when someone manages to implement them.
>
> I don't have a problem with that, B in BAT stands for basic anyway.
>
> We just adjust the model that when nightly test run fails someone from 
> QA gets tasked with bisection etc as a top priority.
>
> Regards,
>
> Tvrtko
Marius Vlad May 27, 2016, 2:25 p.m. UTC | #7
On Fri, May 27, 2016 at 01:10:07PM +0100, Tvrtko Ursulin wrote:
> 
> On 27/05/16 12:58, Chris Wilson wrote:
> >On Fri, May 27, 2016 at 02:50:34PM +0300, Marius Vlad wrote:
> >>Signed-off-by: Marius Vlad <marius.c.vlad@intel.com>
> >
> >Nak. It's a race detector. Please suggest how to increase detection
> >rates.
> 
> As a more or less well know TV personality would say - "it's better than
> nothing"! :))
> 
> Seriously, under the new rules this is all we can do. Full gem-close-race
> will (will it?) run in the nightly run so a little bit more pain while
> bisecting breakages but those are the rules. We can give them a go and see
> how it works out.

Combinatorial tests (gem_concurrent_all/blit) and gem_evict_everything
are the only exceptions. We've been running nightly for over a week now.

You can have a look under /archive/results/Nightly/.

I'm not saying its perfect, but at least we have somewhere to start.

> 
> Regards,
> 
> Tvrtko
>
Marius Vlad May 27, 2016, 2:26 p.m. UTC | #8
On Fri, May 27, 2016 at 01:16:06PM +0100, Chris Wilson wrote:
> On Fri, May 27, 2016 at 01:10:07PM +0100, Tvrtko Ursulin wrote:
> > 
> > On 27/05/16 12:58, Chris Wilson wrote:
> > >On Fri, May 27, 2016 at 02:50:34PM +0300, Marius Vlad wrote:
> > >>Signed-off-by: Marius Vlad <marius.c.vlad@intel.com>
> > >
> > >Nak. It's a race detector. Please suggest how to increase detection
> > >rates.
> > 
> > As a more or less well know TV personality would say - "it's better
> > than nothing"! :))
> > 
> > Seriously, under the new rules this is all we can do. Full
> > gem-close-race will (will it?) run in the nightly run so a little
> > bit more pain while bisecting breakages but those are the rules. We
> > can give them a go and see how it works out.
> 
> Wrong approach. Right approach would be to add a new test that reliably
> detected a checklist of the most common races in under 1s. Nerfing a
> test to make it useless makes BAT equally useless.
> 
> So we are giving up on BAT?
It is a compromise. Either this or nigthly.

> -Chris
> 
> -- 
> Chris Wilson, Intel Open Source Technology Centre
Marius Vlad May 27, 2016, 2:28 p.m. UTC | #9
On Fri, May 27, 2016 at 03:44:20PM +0300, Mika Kuoppala wrote:
> Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com> writes:
> 
> > [ text/plain ]
> >
> > On 27/05/16 13:16, Chris Wilson wrote:
> >> On Fri, May 27, 2016 at 01:10:07PM +0100, Tvrtko Ursulin wrote:
> >>>
> >>> On 27/05/16 12:58, Chris Wilson wrote:
> >>>> On Fri, May 27, 2016 at 02:50:34PM +0300, Marius Vlad wrote:
> >>>>> Signed-off-by: Marius Vlad <marius.c.vlad@intel.com>
> >>>>
> >>>> Nak. It's a race detector. Please suggest how to increase detection
> >>>> rates.
> >>>
> >>> As a more or less well know TV personality would say - "it's better
> >>> than nothing"! :))
> >>>
> >>> Seriously, under the new rules this is all we can do. Full
> >>> gem-close-race will (will it?) run in the nightly run so a little
> >>> bit more pain while bisecting breakages but those are the rules. We
> >>> can give them a go and see how it works out.
> >>
> >> Wrong approach. Right approach would be to add a new test that reliably
> >> detected a checklist of the most common races in under 1s. Nerfing a
> >> test to make it useless makes BAT equally useless.
> >>
> >> So we are giving up on BAT?
> >
> > If it is not possible to stuff everything into the allocated budget, and 
> > in cases where it may not be possible to come up with a 1s race 
> > detector, it makes sense to move the test out of BAT and into the 
> > nightly runs.
> >
> 
> With tests to poke out races, I think the best way is to take
> them out from BAT and move to nightly.

Right, going to send another series with them removed if everyone
agrees on this.

> 
> As A is for acceptance, it is not much value as some poor soul
> will hit the race 1 month after we regressed it.
> 
> - Mika
> 
> 
> > Agreement was that the time limits are hard limits so thats pretty much 
> > it. We can do this now and hit the targets and them add more tests if 
> > and when someone manages to implement them.
> >
> > I don't have a problem with that, B in BAT stands for basic anyway.
> >
> > We just adjust the model that when nightly test run fails someone from 
> > QA gets tasked with bisection etc as a top priority.
> >
> > Regards,
> >
> > Tvrtko
diff mbox

Patch

diff --git a/tests/gem_close_race.c b/tests/gem_close_race.c
index 94fb905..45aa2cc 100644
--- a/tests/gem_close_race.c
+++ b/tests/gem_close_race.c
@@ -232,7 +232,7 @@  igt_main
 	}
 
 	igt_subtest("basic-threads")
-		threads(10);
+		threads(1);
 
 	igt_subtest("process-exit") {
 		igt_fork(child, 768)