diff mbox

drm/i915/selftests: Touch the NMI watchdog inside a GTT pass

Message ID 20180703102531.20554-1-chris@chris-wilson.co.uk (mailing list archive)
State New, archived
Headers show

Commit Message

Chris Wilson July 3, 2018, 10:25 a.m. UTC
We want to do a complete pass before checking the timeout, but just in
case the pass is quite slow, touch the NMI watchdog to prevent a
false positive.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 drivers/gpu/drm/i915/selftests/i915_gem_gtt.c | 8 ++++++++
 1 file changed, 8 insertions(+)

Comments

Tvrtko Ursulin July 3, 2018, 11 a.m. UTC | #1
On 03/07/2018 11:25, Chris Wilson wrote:
> We want to do a complete pass before checking the timeout, but just in
> case the pass is quite slow, touch the NMI watchdog to prevent a
> false positive.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> ---
>   drivers/gpu/drm/i915/selftests/i915_gem_gtt.c | 8 ++++++++
>   1 file changed, 8 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c b/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c
> index eefcf7b84054..71c0654b4b4d 100644
> --- a/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c
> @@ -23,6 +23,7 @@
>    */
>   
>   #include <linux/list_sort.h>
> +#include <linux/nmi.h>
>   #include <linux/prime_numbers.h>
>   
>   #include "../i915_selftest.h"
> @@ -686,6 +687,13 @@ static int pot_hole(struct drm_i915_private *i915,
>   			i915_vma_unpin(vma);
>   			err = i915_vma_unbind(vma);
>   			GEM_BUG_ON(err);
> +
> +			/*
> +			 * Do a complete pass before timing out, but just
> +			 * in case we are running too slow, ping the NMI
> +			 * watchdog.
> +			 */
> +			touch_nmi_watchdog();
>   		}
>   
>   		if (igt_timeout(end_time,
> 

Why only in pot_hole, is this the slowest test?

Regards,

Tvrtko
Chris Wilson July 3, 2018, 11:07 a.m. UTC | #2
Quoting Tvrtko Ursulin (2018-07-03 12:00:13)
> 
> On 03/07/2018 11:25, Chris Wilson wrote:
> > We want to do a complete pass before checking the timeout, but just in
> > case the pass is quite slow, touch the NMI watchdog to prevent a
> > false positive.
> > 
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> > ---
> >   drivers/gpu/drm/i915/selftests/i915_gem_gtt.c | 8 ++++++++
> >   1 file changed, 8 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c b/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c
> > index eefcf7b84054..71c0654b4b4d 100644
> > --- a/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c
> > +++ b/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c
> > @@ -23,6 +23,7 @@
> >    */
> >   
> >   #include <linux/list_sort.h>
> > +#include <linux/nmi.h>
> >   #include <linux/prime_numbers.h>
> >   
> >   #include "../i915_selftest.h"
> > @@ -686,6 +687,13 @@ static int pot_hole(struct drm_i915_private *i915,
> >                       i915_vma_unpin(vma);
> >                       err = i915_vma_unbind(vma);
> >                       GEM_BUG_ON(err);
> > +
> > +                     /*
> > +                      * Do a complete pass before timing out, but just
> > +                      * in case we are running too slow, ping the NMI
> > +                      * watchdog.
> > +                      */
> > +                     touch_nmi_watchdog();
> >               }
> >   
> >               if (igt_timeout(end_time,
> > 
> 
> Why only in pot_hole, is this the slowest test?

It's the only test of this style where we don't have the igt_timeout
(cond_resched) inside the innermost loop.
-Chris
Tvrtko Ursulin July 3, 2018, 12:54 p.m. UTC | #3
On 03/07/2018 12:07, Chris Wilson wrote:
> Quoting Tvrtko Ursulin (2018-07-03 12:00:13)
>>
>> On 03/07/2018 11:25, Chris Wilson wrote:
>>> We want to do a complete pass before checking the timeout, but just in
>>> case the pass is quite slow, touch the NMI watchdog to prevent a
>>> false positive.
>>>
>>> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
>>> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>> ---
>>>    drivers/gpu/drm/i915/selftests/i915_gem_gtt.c | 8 ++++++++
>>>    1 file changed, 8 insertions(+)
>>>
>>> diff --git a/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c b/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c
>>> index eefcf7b84054..71c0654b4b4d 100644
>>> --- a/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c
>>> +++ b/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c
>>> @@ -23,6 +23,7 @@
>>>     */
>>>    
>>>    #include <linux/list_sort.h>
>>> +#include <linux/nmi.h>
>>>    #include <linux/prime_numbers.h>
>>>    
>>>    #include "../i915_selftest.h"
>>> @@ -686,6 +687,13 @@ static int pot_hole(struct drm_i915_private *i915,
>>>                        i915_vma_unpin(vma);
>>>                        err = i915_vma_unbind(vma);
>>>                        GEM_BUG_ON(err);
>>> +
>>> +                     /*
>>> +                      * Do a complete pass before timing out, but just
>>> +                      * in case we are running too slow, ping the NMI
>>> +                      * watchdog.
>>> +                      */
>>> +                     touch_nmi_watchdog();
>>>                }
>>>    
>>>                if (igt_timeout(end_time,
>>>
>>
>> Why only in pot_hole, is this the slowest test?
> 
> It's the only test of this style where we don't have the igt_timeout
> (cond_resched) inside the innermost loop.

So.. the obvious question.. :) Why not move igt_timeout to the innermost 
loop and avoid low-level hackery?

Regards,

Tvrtko
Chris Wilson July 3, 2018, 1:16 p.m. UTC | #4
Quoting Tvrtko Ursulin (2018-07-03 13:54:01)
> 
> On 03/07/2018 12:07, Chris Wilson wrote:
> > Quoting Tvrtko Ursulin (2018-07-03 12:00:13)
> >>
> >> On 03/07/2018 11:25, Chris Wilson wrote:
> >>> We want to do a complete pass before checking the timeout, but just in
> >>> case the pass is quite slow, touch the NMI watchdog to prevent a
> >>> false positive.
> >>>
> >>> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> >>> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> >>> ---
> >>>    drivers/gpu/drm/i915/selftests/i915_gem_gtt.c | 8 ++++++++
> >>>    1 file changed, 8 insertions(+)
> >>>
> >>> diff --git a/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c b/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c
> >>> index eefcf7b84054..71c0654b4b4d 100644
> >>> --- a/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c
> >>> +++ b/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c
> >>> @@ -23,6 +23,7 @@
> >>>     */
> >>>    
> >>>    #include <linux/list_sort.h>
> >>> +#include <linux/nmi.h>
> >>>    #include <linux/prime_numbers.h>
> >>>    
> >>>    #include "../i915_selftest.h"
> >>> @@ -686,6 +687,13 @@ static int pot_hole(struct drm_i915_private *i915,
> >>>                        i915_vma_unpin(vma);
> >>>                        err = i915_vma_unbind(vma);
> >>>                        GEM_BUG_ON(err);
> >>> +
> >>> +                     /*
> >>> +                      * Do a complete pass before timing out, but just
> >>> +                      * in case we are running too slow, ping the NMI
> >>> +                      * watchdog.
> >>> +                      */
> >>> +                     touch_nmi_watchdog();
> >>>                }
> >>>    
> >>>                if (igt_timeout(end_time,
> >>>
> >>
> >> Why only in pot_hole, is this the slowest test?
> > 
> > It's the only test of this style where we don't have the igt_timeout
> > (cond_resched) inside the innermost loop.
> 
> So.. the obvious question.. :) Why not move igt_timeout to the innermost 
> loop and avoid low-level hackery?

In looking at it, I felt this case it was more interesting^W important to
test each boundary before checking the timeout. I was concerned about
declaring the pass a success too early.
-Chris
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c b/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c
index eefcf7b84054..71c0654b4b4d 100644
--- a/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c
@@ -23,6 +23,7 @@ 
  */
 
 #include <linux/list_sort.h>
+#include <linux/nmi.h>
 #include <linux/prime_numbers.h>
 
 #include "../i915_selftest.h"
@@ -686,6 +687,13 @@  static int pot_hole(struct drm_i915_private *i915,
 			i915_vma_unpin(vma);
 			err = i915_vma_unbind(vma);
 			GEM_BUG_ON(err);
+
+			/*
+			 * Do a complete pass before timing out, but just
+			 * in case we are running too slow, ping the NMI
+			 * watchdog.
+			 */
+			touch_nmi_watchdog();
 		}
 
 		if (igt_timeout(end_time,