All of lore.kernel.org
 help / color / mirror / Atom feed
From: Dave Gordon <david.s.gordon@intel.com>
To: Chris Wilson <chris@chris-wilson.co.uk>
Cc: "intel-gfx@lists.freedesktop.org" <intel-gfx@lists.freedesktop.org>
Subject: Re: [PATCH v2] drm/i915: recheck ringspace after wait+retire
Date: Thu, 18 Dec 2014 18:09:18 +0000	[thread overview]
Message-ID: <5493184E.3090203@intel.com> (raw)
In-Reply-To: <20141218171831.GL32100@nuc-i3427.alporthouse.com>

On 18/12/14 17:18, Chris Wilson wrote:
> On Thu, Dec 18, 2014 at 05:03:41PM +0000, Dave Gordon wrote:
>> In {intel,logical}_ring_wait_request(), we try to find a request
>> whose completion will release the amount of ring space required.
>> If we find such a request, we wait for it, and then retire it, in
>> the expectation that we will now have at least the required amount
>> of free space in the ring. But it's good to check that this has
>> actually happened, so we can back out with a meaningful error
>> code if something unexpected has happened, such as wait_request
>> returning early.
> 
> It's pretty pointless. It's a programming bug, if anything it should
> WARN if it happens.
> -Chris

I don't regard preventing the propagation of errors as pointless,
whether or not it's a programming bug. If this case arose, then without
this check we would go ahead and trample over whatever was next in the
ring. Quite likely TAIL would overtake HEAD, and then all sorts of
bizarre things would happen some time later.

In particular, this may be able to catch bugs introduced by /future
features/, such as TDR, scheduling, and preemption, all of which are in
preparation now and all of which have the potential to disrupt the
predictable flow of requests. So it's good to be resilient to upcoming
changes and make it as easy as possible to catch bugs at the earliest
opportunity.

This is also a step towards the deduplication of the ringbuffer and LRC
paths by making sure that they are as similar as possible, and thus
simple to merge at some later opportunity.

.Dave.
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

  reply	other threads:[~2014-12-18 18:13 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-12-18 17:03 [PATCH v2] drm/i915: recheck ringspace after wait+retire Dave Gordon
2014-12-18 17:18 ` Chris Wilson
2014-12-18 18:09   ` Dave Gordon [this message]
2014-12-18 20:19     ` Daniel Vetter

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=5493184E.3090203@intel.com \
    --to=david.s.gordon@intel.com \
    --cc=chris@chris-wilson.co.uk \
    --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.