All of lore.kernel.org
 help / color / mirror / Atom feed
From: Dave Airlie <airlied@gmail.com>
To: Ben Widawsky <ben@bwidawsk.net>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>,
	Intel Graphics Development <intel-gfx@lists.freedesktop.org>
Subject: Re: [PATCH 2/2] drm/i915: Don't BUG_ON in i915_gem_obj_offset
Date: Wed, 25 Jun 2014 12:00:35 +1000	[thread overview]
Message-ID: <CAPM=9txwenqWwXvNzh-6FP7M9Rj0WiRKH1u_fdx39Kr6UVfMCw@mail.gmail.com> (raw)
In-Reply-To: <20140625013033.GA10886@bwidawsk.net>

On 25 June 2014 11:30, Ben Widawsky <ben@bwidawsk.net> wrote:
> On Wed, Jun 18, 2014 at 12:04:46AM +0200, Daniel Vetter wrote:
>> On Tue, Jun 17, 2014 at 10:34:38PM +0200, Daniel Vetter wrote:
>> > A WARN_ON is perfectly fine.
>> >
>> > The BUG in here seems to be the cause behind hard-hangs when I cat the
>> > i915_gem_pageflip debugfs file (which calls this from an irq
>> > spinlock). But only while running a full igt run after a while. I
>> > still need to root cause the underlying issue.
>> >
>> > I'll also start reject patches which add new BUG_ON but don't come
>> > with a really good justification for it. The general rule really
>> > should be to just WARN and hope the driver survives for long enough.
>> >
>> > Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
>>
>> Both patches merged, this one improved per Chris' suggestions on irc.
>> -Daniel
>>
>
> Hey, here's an idea. How about we root cause bugs instead of making
> blanket statements about the validity of real assertions? If the callers
> of ggtt_offset are calling it on unbound objects, it's a violation of
> the design. And in the other cases, it's a real bug.
>
> I'd NAK this patch if it wasn't already merged, and my NAK meant
> something.
>

Its kinda hard to debug an assert if it takes the whole box down, and you
never see the assert printed anywhere. Any why should the whole
kernel die because the GPU driver stuffed up.

Maybe you confused this with userspace.

Dave.

  reply	other threads:[~2014-06-25  2:00 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-06-17 20:34 [PATCH 1/2] drm/i915: Grab dev->struct_mutex in i915_gem_pageflip_info Daniel Vetter
2014-06-17 20:34 ` [PATCH 2/2] drm/i915: Don't BUG_ON in i915_gem_obj_offset Daniel Vetter
2014-06-17 22:04   ` Daniel Vetter
2014-06-25  1:30     ` Ben Widawsky
2014-06-25  2:00       ` Dave Airlie [this message]
2014-06-25  4:00         ` Ben Widawsky

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CAPM=9txwenqWwXvNzh-6FP7M9Rj0WiRKH1u_fdx39Kr6UVfMCw@mail.gmail.com' \
    --to=airlied@gmail.com \
    --cc=ben@bwidawsk.net \
    --cc=daniel.vetter@ffwll.ch \
    --cc=intel-gfx@lists.freedesktop.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.