From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ben Widawsky Subject: Re: [PATCH] drm/i915: hangcheck timeout for debugfs Date: Sat, 25 Jun 2011 17:20:11 -0700 Message-ID: <20110626002011.GD9413@snipes.kumite> References: <1308869354-3145-1-git-send-email-ben@bwidawsk.net> <20110624154653.GD21246@snipes.kumite> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from cloud01.chad-versace.us (184-106-247-128.static.cloud-ips.com [184.106.247.128]) by gabe.freedesktop.org (Postfix) with ESMTP id 3A49A9E77C for ; Sat, 25 Jun 2011 17:20:43 -0700 (PDT) Content-Disposition: inline In-Reply-To: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org Errors-To: intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org To: Chris Wilson Cc: intel-gfx@lists.freedesktop.org List-Id: intel-gfx@lists.freedesktop.org On Sat, Jun 25, 2011 at 10:23:28AM +0100, Chris Wilson wrote: > On Fri, 24 Jun 2011 08:46:53 -0700, Ben Widawsky 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 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