Re: [PATCH] drm/i915: hangcheck timeout for debugfs

[Ben Widawsky <ben@bwidawsk.net>]

On Sat, Jun 25, 2011 at 10:23:28AM +0100, Chris Wilson wrote:
> On Fri, 24 Jun 2011 08:46:53 -0700, Ben Widawsky <ben@bwidawsk.net> wrote:
> > On Fri, Jun 24, 2011 at 12:48:22AM +0100, Chris Wilson wrote:
> > > On Thu, 23 Jun 2011 15:49:14 -0700, Ben Widawsky <ben@bwidawsk.net> wrote:
> > > > Provide a user accessible way to change the hangcheck timer. This is
> > > > useful mostly for disabling the timer completely (value <= 0).
> > > 
> > > Having i915.hangcheck_interval as a read/write module parameter was
> > > better. :-p
> > > -Chris
> > 
> > I considered this, but I wasn't sure how to manage the sysfs parameters,
> > and prevent users from doing stupid things. Furthermore, I think to be
> > correct we must delete sync the timer if the user requests an interval
> > of 0, and we can only do that if we have struct mutex (again the sysfs
> > problem).
> 
> You can either register a callback for when the parameter changes, but in
> this case it is as easy as deleting the timer in the next hangcheck before
> touching any GPU state. In that scenario the timer will only be enabled
> again after the next execbuffers, that's a restriction I can live with for
> simple code and keeping parameters out of debugfs.
> 
> On the other hand, a debugfs would allow for a per-device parameter. For
> that day in the far far future with multiprocessor igfx. Surreal isn't it?
> -Chris

So what's the verdict? In term of LOC, your suggestion would probably be
smaller, but in terms of complexity I actually think the current patch
would be easier to understand, although to be fair, I didn't actually
try coding it to see.

If you feel strongly about a module parameter being the better solution,
I will code it up.

Ben