Re: [PATCH] drm/i915: Add module param to test the load detect code

[Daniel Vetter <daniel@ffwll.ch>]

On Tue, Mar 03, 2015 at 04:27:47PM -0800, Jesse Barnes wrote:
> On 03/03/2015 04:06 PM, Daniel Vetter wrote:
> > On Tue, Mar 3, 2015 at 10:15 PM, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> >> On Tue, Mar 03, 2015 at 11:23:53AM -0800, Jesse Barnes wrote:
> >>> On 03/03/2015 09:03 AM, Daniel Vetter wrote:
> >>>> This is useful for writing igts to make sure we don't break this,
> >>>> without being forced to own a one of these dinosaurs.
> >>>>
> >>>> Suggested-by: Jesse Barnes <jbarnes@virtuousgeek.org>
> >>>> Cc: Matt Roper <matthew.d.roper@intel.com>
> >>>> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> >>>> ---
> >>>>  drivers/gpu/drm/i915/i915_drv.h    | 1 +
> >>>>  drivers/gpu/drm/i915/i915_params.c | 8 +++++++-
> >>>>  drivers/gpu/drm/i915/intel_crt.c   | 6 ++++--
> >>>>  3 files changed, 12 insertions(+), 3 deletions(-)
> >>>
> >>> See below for comments.
> >>>
> >>> I think there's probably even more room for testing like this.  E.g. the
> >>> tiled swapping test could be done this way rather than trying to force
> >>> swapping.  Some of the races we try to induce could probably also be
> >>> done this way with code in the kernel to trigger the case we're worried
> >>> about...
> >>
> >> I am not wholly convinced. The primary purpose of the test suite is
> >> prevent bugs of tomorrow, not to chase bugs of yesterday. If we focus too
> >> much on bugs we have fixed, I worry we won't serendipitously detect bugs
> >> early. Yes, bugs cluster and a mistake once made is likely to be made
> >> again (so regression testing is vital) but I think we cross a line if
> >> igt only exercises code written for conformance testing.
> > 
> > Also swapping tests are a solved problem really, we've had an "mlock
> > most of mem" todo since years and Thomas Wood is implementing it. Well
> > it's committed for gem_tiled_swapping already
> > 
> > commit 42b02c284ed24871528df8f1b3eaad7fe1554fd9
> > Author: Thomas Wood <thomas.wood@intel.com>
> > Date:   Mon Dec 8 11:12:51 2014 +0000
> > 
> >     lib: add a function to lock memory into RAM
> > 
> > what's missing is rolling this out for other tests.
> > 
> > Now I love igt bashing as much as the next person, but maybe check
> > occasionally whether your rant-du-jour is still relevant ...
> 
> My "rant du jour" still is.  mlock() is a good solution for some things,
> but for the simple task of testing kernel swap out code, just running
> that code is the most straightforward thing to do, rather than trying to
> contort into it from userspace.

swap-out/in isn't really simple. We could extract the bit 17 swizzling
logic into a separate bit with everything else mock-ups, but that's just
testing our sw implementation against our sw model. Not that useful imo,
and like Chris I'm not convinced of the value of lots of such special-case
code.

On top of that bit17 swizzling is but a facet of swap-in/out support.
Active tracking, page dirtying and all that also need to work correctly. I
don't see how you can test that in a simple fashion.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx