All of lore.kernel.org
 help / color / mirror / Atom feed
From: Daniel Vetter <daniel@ffwll.ch>
To: John Harrison <John.C.Harrison@Intel.com>
Cc: Intel-GFX@Lists.FreeDesktop.Org
Subject: Re: [PATCH 05/51] drm/i915: Add return code check to i915_gem_execbuffer_retire_commands()
Date: Thu, 5 Mar 2015 17:14:01 +0100	[thread overview]
Message-ID: <20150305161400.GO18775@phenom.ffwll.local> (raw)
In-Reply-To: <54F87102.8020504@Intel.com>

On Thu, Mar 05, 2015 at 03:06:42PM +0000, John Harrison wrote:
> On 05/03/2015 14:44, Daniel Vetter wrote:
> >Imo reserving a bit of ring space for each add_request should be solid.
> >Userspace uses the exact same reservation logic for adding end-of-batch
> >workarounds. The only thing needed to make this solid is to WARN if
> >add_request ends up using more ring space than what we've reserved (not
> >just when it actually runs out, that obviously doesn't happen often
> >enough for testing).
> The problem is that there could be multiple requests being processed in
> parallel. This is especially true with the scheduler. Userland could submit
> a whole stream of batches that all get queued up in the scheduler. Only
> later do they get submitted to the hardware. The request must be allocated
> up front because there is no other means of tracking them. But reserving
> space at that point won't work because you either end up reserving massive
> amounts of space if the reserve is cumulative, or not enough if only one
> slot is reserved.

At least with execlist we don't have that problem really since writing the
ringbuffer is done synchronously and directly.

For the legacy scheduler I expect that we won't do any of the ringbuf
writes directly and instead that's all done by the scheduler
asynchronously.

So this should just be an issue while we are converting to the scheduler
or on platforms that will never have one. And imo the request ringbuf
reservation is the solution with the simplest impact on the design.

> >Everything else just readds olr through the backdoor, which is kinda what
> >we wanted to avoid from an accounting pov. Because then you have again
> >some random request outstanding which scoops up everything it encounters.
> Not quite.  The difference is that with something like an outstanding failed
> request rather than a lazy one, there is still the segregation of work. The
> failed request will be posted and added to the request list in its entirety
> before a new request is allocated and used for the new work.

Well history lesson, but that's exactly how olr started out. Then the
hydra grew heads to no end. That's why I don't want to go down this road
again, since I've been on that trip the past 3 years ;-) And since your
motivation for olr light is exactly the one I provided 3 years ago to get
my patch in I think history repeating is likely.
-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

  reply	other threads:[~2015-03-05 16:12 UTC|newest]

Thread overview: 234+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-02-13 11:48 [PATCH 00/51] Remove the outstanding_lazy_request John.C.Harrison
2015-02-13 11:48 ` [PATCH 01/51] drm/i915: Rename 'flags' to 'dispatch_flags' for better code reading John.C.Harrison
2015-02-25 21:34   ` Daniel Vetter
2015-02-27 12:14     ` John Harrison
2015-02-27 13:15       ` Daniel Vetter
2015-02-13 11:48 ` [PATCH 02/51] drm/i915: Add missing trace point to LRC execbuff code path John.C.Harrison
2015-02-13 11:48 ` [PATCH 03/51] drm/i915: Cache ringbuf pointer in request structure John.C.Harrison
2015-02-25 21:50   ` Daniel Vetter
2015-02-13 11:48 ` [PATCH 04/51] drm/i915: Merged the many do_execbuf() parameters into a structure John.C.Harrison
2015-02-25 21:52   ` Daniel Vetter
2015-02-27 12:22     ` John Harrison
2015-02-27 13:35       ` Daniel Vetter
2015-02-27 18:22         ` John Harrison
2015-02-27 18:43           ` Daniel Vetter
2015-02-13 11:48 ` [PATCH 05/51] drm/i915: Add return code check to i915_gem_execbuffer_retire_commands() John.C.Harrison
2015-02-25 22:17   ` Daniel Vetter
2015-02-26  2:26     ` Daniel Vetter
2015-03-05 13:06       ` John Harrison
2015-03-05 14:44         ` Daniel Vetter
2015-03-05 15:06           ` John Harrison
2015-03-05 16:14             ` Daniel Vetter [this message]
2015-03-06 11:38               ` John Harrison
2015-03-06 15:57                 ` Daniel Vetter
2015-03-06 17:40                   ` Dave Gordon
2015-03-09  8:01                     ` Daniel Vetter
2015-02-13 11:48 ` [PATCH 06/51] drm/i915: Wrap request allocation with a function pointer John.C.Harrison
2015-02-13 11:48 ` [PATCH 07/51] drm/i915: Early alloc request in execbuff John.C.Harrison
2015-02-25 22:22   ` Daniel Vetter
2015-02-27 12:27     ` John Harrison
2015-02-27 13:40       ` Daniel Vetter
2015-02-13 11:48 ` [PATCH 08/51] drm/i915: Update alloc_request to return the allocated request John.C.Harrison
2015-02-13 12:21   ` Chris Wilson
2015-02-25 21:08     ` Daniel Vetter
2015-02-27 12:34       ` John Harrison
2015-02-27 13:43         ` Daniel Vetter
2015-02-13 11:48 ` [PATCH 09/51] drm/i915: Add request to execbuf params and add explicit cleanup John.C.Harrison
2015-02-13 11:48 ` [PATCH 10/51] drm/i915: Update the dispatch tracepoint to use params->request John.C.Harrison
2015-02-13 11:48 ` [PATCH 11/51] drm/i915: Update move_to_gpu() to take a request structure John.C.Harrison
2015-02-13 11:48 ` [PATCH 12/51] drm/i915: Update execbuffer_move_to_active() " John.C.Harrison
2015-02-13 11:48 ` [PATCH 13/51] drm/i915: Add flag to i915_add_request() to skip the cache flush John.C.Harrison
2015-02-13 11:48 ` [PATCH 14/51] drm/i915: Update pin_to_display_plane() to do explicit request management John.C.Harrison
2015-02-25 22:35   ` Daniel Vetter
2015-02-13 11:48 ` [PATCH 15/51] drm/i915: Update i915_gem_object_sync() to take a request structure John.C.Harrison
2015-02-13 11:48 ` [PATCH 16/51] drm/i915: Update i915_gpu_idle() to manage its own request John.C.Harrison
2015-02-13 11:48 ` [PATCH 17/51] drm/i915: Split i915_ppgtt_init_hw() in half - generic and per ring John.C.Harrison
2015-02-13 11:48 ` [PATCH 18/51] drm/i915: Moved the for_each_ring loop outside of i915_gem_context_enable() John.C.Harrison
2015-02-13 11:48 ` [PATCH 19/51] drm/i915: Add explicit request management to i915_gem_init_hw() John.C.Harrison
2015-02-13 11:48 ` [PATCH 20/51] drm/i915: Update ppgtt_init_ring() & context_enable() to take requests John.C.Harrison
2015-02-13 11:48 ` [PATCH 21/51] drm/i915: Set context in request from creation even in legacy mode John.C.Harrison
2015-02-13 11:48 ` [PATCH 22/51] drm/i915: Update i915_switch_context() to take a request structure John.C.Harrison
2015-02-13 11:48 ` [PATCH 23/51] drm/i915: Update do_switch() " John.C.Harrison
2015-02-13 11:48 ` [PATCH 24/51] drm/i915: Update deferred context creation to do explicit request management John.C.Harrison
2015-02-13 12:15   ` Chris Wilson
2015-02-18 15:27     ` John Harrison
2015-02-25 21:15       ` Daniel Vetter
2015-02-27 12:45         ` John Harrison
2015-02-27 13:50           ` Daniel Vetter
2015-02-13 11:48 ` [PATCH 25/51] drm/i915: Update init_context() to take a request structure John.C.Harrison
2015-02-13 11:48 ` [PATCH 26/51] drm/i915: Update render_state_init() " John.C.Harrison
2015-02-13 11:48 ` [PATCH 27/51] drm/i915: Update overlay code to do explicit request management John.C.Harrison
2015-02-13 11:48 ` [PATCH 28/51] drm/i915: Update queue_flip() " John.C.Harrison
2015-02-13 11:48 ` [PATCH 29/51] drm/i915: Update add_request() to take a request structure John.C.Harrison
2015-02-13 11:48 ` [PATCH 30/51] drm/i915: Update [vma|object]_move_to_active() to take request structures John.C.Harrison
2015-02-13 11:48 ` [PATCH 31/51] drm/i915: Update l3_remap to take a request structure John.C.Harrison
2015-02-13 11:48 ` [PATCH 32/51] drm/i915: Update mi_set_context() " John.C.Harrison
2015-02-13 11:48 ` [PATCH 33/51] drm/i915: Update a bunch of execbuffer heplers to take request structures John.C.Harrison
2015-02-13 11:48 ` [PATCH 34/51] drm/i915: Update workarounds_emit() " John.C.Harrison
2015-02-13 11:48 ` [PATCH 35/51] drm/i915: Update flush_all_caches() " John.C.Harrison
2015-02-13 11:48 ` [PATCH 36/51] drm/i915: Update switch_mm() to take a request structure John.C.Harrison
2015-02-13 11:48 ` [PATCH 37/51] drm/i915: Update ring->flush() to take a requests structure John.C.Harrison
2015-02-13 11:48 ` [PATCH 38/51] drm/i915: Update some flush helpers to take request structures John.C.Harrison
2015-02-13 11:48 ` [PATCH 39/51] drm/i915: Update ring->emit_flush() to take a request structure John.C.Harrison
2015-02-13 11:48 ` [PATCH 40/51] drm/i915: Update ring->add_request() " John.C.Harrison
2015-02-13 11:48 ` [PATCH 41/51] drm/i915: Update ring->emit_request() " John.C.Harrison
2015-02-13 11:48 ` [PATCH 42/51] drm/i915: Update ring->dispatch_execbuffer() " John.C.Harrison
2015-02-13 11:48 ` [PATCH 43/51] drm/i915: Update ring->emit_bb_start() " John.C.Harrison
2015-02-13 11:48 ` [PATCH 44/51] drm/i915: Update ring->sync_to() " John.C.Harrison
2015-02-13 11:48 ` [PATCH 45/51] drm/i915: Update ring->signal() " John.C.Harrison
2015-02-13 11:48 ` [PATCH 46/51] drm/i915: Update cacheline_align() " John.C.Harrison
2015-02-13 11:48 ` [PATCH 47/51] drm/i915: Update ironlake_enable_rc6() to do explicit request management John.C.Harrison
2015-02-13 12:19   ` Chris Wilson
2015-02-13 16:58     ` John Harrison
2015-02-13 17:03       ` Chris Wilson
2015-02-18 14:28         ` John Harrison
2015-02-25 21:31           ` Daniel Vetter
2015-02-27 12:49             ` John Harrison
2015-02-27 13:56               ` Daniel Vetter
2015-02-27 13:03             ` Ville Syrjälä
2015-02-27 13:53               ` Daniel Vetter
2015-02-13 11:48 ` [PATCH 48/51] drm/i915: Update intel_ring_begin() to take a request structure John.C.Harrison
2015-02-13 12:20   ` Chris Wilson
2015-02-18 15:37     ` John Harrison
2015-02-13 11:48 ` [PATCH 49/51] drm/i915: Update intel_logical_ring_begin() " John.C.Harrison
2015-02-13 12:17   ` Chris Wilson
2015-02-13 17:04     ` John Harrison
2015-02-13 11:48 ` [PATCH 50/51] drm/i915: Remove the now obsolete intel_ring_get_request() John.C.Harrison
2015-02-13 11:49 ` [PATCH 51/51] drm/i915: Remove the now obsolete 'outstanding_lazy_request' John.C.Harrison
2015-02-19 17:17 ` [PATCH 00/53] Remove the outstanding_lazy_request John.C.Harrison
2015-02-19 17:17   ` [PATCH 01/53] drm/i915: Rename 'flags' to 'dispatch_flags' for better code reading John.C.Harrison
2015-03-05 13:21     ` Tomas Elf
2015-03-06 12:30       ` John Harrison
2015-02-19 17:17   ` [PATCH 02/53] drm/i915: Add missing trace point to LRC execbuff code path John.C.Harrison
2015-03-05 13:26     ` Tomas Elf
2015-02-19 17:17   ` [PATCH 03/53] drm/i915: Cache ringbuf pointer in request structure John.C.Harrison
2015-03-05 13:56     ` Tomas Elf
2015-03-06 12:28       ` John Harrison
2015-02-19 17:17   ` [PATCH 04/53] drm/i915: Merged the many do_execbuf() parameters into a structure John.C.Harrison
2015-03-05 13:37     ` John.C.Harrison
2015-03-05 14:30     ` Tomas Elf
2015-02-19 17:17   ` [PATCH 05/53] drm/i915: Add return code check to i915_gem_execbuffer_retire_commands() John.C.Harrison
2015-03-05 14:45     ` Tomas Elf
2015-03-06 16:15       ` Daniel Vetter
2015-02-19 17:17   ` [PATCH 06/53] drm/i915: Wrap request allocation with a function pointer John.C.Harrison
2015-03-05 15:01     ` Tomas Elf
2015-03-05 16:20       ` Daniel Vetter
2015-02-19 17:17   ` [PATCH 07/53] drm/i915: Early alloc request in execbuff John.C.Harrison
2015-03-05 15:11     ` Tomas Elf
2015-02-19 17:17   ` [PATCH 08/53] drm/i915: Update alloc_request to return the allocated request John.C.Harrison
2015-03-05 15:27     ` Tomas Elf
2015-03-05 15:46       ` John Harrison
2015-03-05 20:13         ` Tomas Elf
2015-03-06 16:18           ` Daniel Vetter
2015-03-06 17:36             ` John Harrison
2015-03-06 20:17               ` Tomas Elf
2015-02-19 17:17   ` [PATCH 09/53] drm/i915: Add request to execbuf params and add explicit cleanup John.C.Harrison
2015-03-05 15:37     ` Tomas Elf
2015-02-19 17:17   ` [PATCH 10/53] drm/i915: Update the dispatch tracepoint to use params->request John.C.Harrison
2015-03-05 15:43     ` Tomas Elf
2015-02-19 17:17   ` [PATCH 11/53] drm/i915: Update move_to_gpu() to take a request structure John.C.Harrison
2015-03-05 15:54     ` Tomas Elf
2015-02-19 17:17   ` [PATCH 12/53] drm/i915: Update execbuffer_move_to_active() " John.C.Harrison
2015-03-05 16:03     ` Tomas Elf
2015-02-19 17:17   ` [PATCH 13/53] drm/i915: Add flag to i915_add_request() to skip the cache flush John.C.Harrison
2015-02-19 17:17   ` [PATCH 14/53] drm/i915: Update pin_to_display_plane() to do explicit request management John.C.Harrison
2015-03-05 16:21     ` Tomas Elf
2015-03-05 16:22     ` Tomas Elf
2015-02-19 17:17   ` [PATCH 15/53] drm/i915: Update i915_gem_object_sync() to take a request structure John.C.Harrison
2015-03-05 16:40     ` Tomas Elf
2015-02-19 17:17   ` [PATCH 16/53] drm/i915: Update i915_gpu_idle() to manage its own request John.C.Harrison
2015-03-05 16:45     ` Tomas Elf
2015-02-19 17:17   ` [PATCH 17/53] drm/i915: Split i915_ppgtt_init_hw() in half - generic and per ring John.C.Harrison
2015-02-24 13:55     ` Daniel, Thomas
2015-03-05 16:53     ` Tomas Elf
2015-02-19 17:17   ` [PATCH 18/53] drm/i915: Moved the for_each_ring loop outside of i915_gem_context_enable() John.C.Harrison
2015-03-05 17:04     ` Tomas Elf
2015-02-19 17:17   ` [PATCH 19/53] drm/i915: Add explicit request management to i915_gem_init_hw() John.C.Harrison
2015-03-05 17:13     ` Tomas Elf
2015-02-19 17:17   ` [PATCH 20/53] drm/i915: Update ppgtt_init_ring() & context_enable() to take requests John.C.Harrison
2015-03-05 17:57     ` Tomas Elf
     [not found]       ` <5502DC35.6020700@Intel.com>
2015-03-13 13:35         ` Tomas Elf
2015-02-19 17:17   ` [PATCH 21/53] drm/i915: Set context in request from creation even in legacy mode John.C.Harrison
2015-03-05 13:42     ` John.C.Harrison
2015-03-05 18:02     ` Tomas Elf
2015-02-19 17:17   ` [PATCH 22/53] drm/i915: Update i915_switch_context() to take a request structure John.C.Harrison
2015-03-05 18:08     ` Tomas Elf
2015-02-19 17:17   ` [PATCH 23/53] drm/i915: Update do_switch() " John.C.Harrison
2015-03-05 18:11     ` Tomas Elf
2015-02-19 17:17   ` [PATCH 24/53] drm/i915: Update deferred context creation to do explicit request management John.C.Harrison
2015-03-05 18:16     ` Tomas Elf
2015-02-19 17:17   ` [PATCH 25/53] drm/i915: Update init_context() to take a request structure John.C.Harrison
2015-03-05 18:38     ` Tomas Elf
2015-02-19 17:17   ` [PATCH 26/53] drm/i915: Update render_state_init() " John.C.Harrison
2015-03-05 18:43     ` Tomas Elf
2015-02-19 17:17   ` [PATCH 27/53] drm/i915: Update overlay code to do explicit request management John.C.Harrison
2015-03-05 18:51     ` Tomas Elf
2015-02-19 17:17   ` [PATCH 28/53] drm/i915: Update queue_flip() " John.C.Harrison
2015-03-05 19:29     ` Tomas Elf
2015-03-06 16:30       ` Daniel Vetter
2015-02-19 17:17   ` [PATCH 29/53] drm/i915: Update add_request() to take a request structure John.C.Harrison
2015-03-05 19:35     ` Tomas Elf
2015-02-19 17:17   ` [PATCH 30/53] drm/i915: Update [vma|object]_move_to_active() to take request structures John.C.Harrison
2015-03-05 19:39     ` Tomas Elf
2015-02-19 17:17   ` [PATCH 31/53] drm/i915: Update l3_remap to take a request structure John.C.Harrison
2015-03-05 19:44     ` Tomas Elf
2015-02-19 17:17   ` [PATCH 32/53] drm/i915: Update mi_set_context() " John.C.Harrison
2015-03-05 19:52     ` Tomas Elf
2015-02-19 17:17   ` [PATCH 33/53] drm/i915: Update a bunch of execbuffer heplers to take request structures John.C.Harrison
2015-03-05 19:58     ` Tomas Elf
2015-02-19 17:17   ` [PATCH 34/53] drm/i915: Update workarounds_emit() " John.C.Harrison
2015-03-09 20:17     ` Tomas Elf
2015-02-19 17:17   ` [PATCH 35/53] drm/i915: Update flush_all_caches() " John.C.Harrison
2015-03-09 20:23     ` Tomas Elf
2015-02-19 17:17   ` [PATCH 36/53] drm/i915: Update switch_mm() to take a request structure John.C.Harrison
2015-03-09 20:33     ` Tomas Elf
2015-02-19 17:17   ` [PATCH 37/53] drm/i915: Update ring->flush() to take a requests structure John.C.Harrison
2015-03-09 20:40     ` Tomas Elf
2015-02-19 17:17   ` [PATCH 38/53] drm/i915: Update some flush helpers to take request structures John.C.Harrison
2015-03-09 20:46     ` Tomas Elf
2015-02-19 17:17   ` [PATCH 39/53] drm/i915: Update ring->emit_flush() to take a request structure John.C.Harrison
2015-03-09 20:51     ` Tomas Elf
2015-02-19 17:17   ` [PATCH 40/53] drm/i915: Update ring->add_request() " John.C.Harrison
2015-03-09 21:02     ` Tomas Elf
2015-02-19 17:17   ` [PATCH 41/53] drm/i915: Update ring->emit_request() " John.C.Harrison
2015-03-09 21:07     ` Tomas Elf
2015-02-19 17:17   ` [PATCH 42/53] drm/i915: Update ring->dispatch_execbuffer() " John.C.Harrison
2015-03-09 21:16     ` Tomas Elf
2015-02-19 17:17   ` [PATCH 43/53] drm/i915: Update ring->emit_bb_start() " John.C.Harrison
2015-03-09 21:29     ` Tomas Elf
2015-02-19 17:17   ` [PATCH 44/53] drm/i915: Update ring->sync_to() " John.C.Harrison
2015-03-09 21:49     ` Tomas Elf
2015-02-19 17:17   ` [PATCH 45/53] drm/i915: Update ring->signal() " John.C.Harrison
2015-03-09 22:04     ` Tomas Elf
2015-02-19 17:17   ` [PATCH 46/53] drm/i915: Update cacheline_align() " John.C.Harrison
2015-03-09 22:16     ` Tomas Elf
2015-02-19 17:17   ` [PATCH 47/53] drm/i915: Update ironlake_enable_rc6() to do explicit request management John.C.Harrison
2015-02-19 17:18   ` [PATCH 48/53] drm/i915: Update intel_ring_begin() to take a request structure John.C.Harrison
2015-03-09 22:39     ` Tomas Elf
2015-02-19 17:18   ` [PATCH 49/53] drm/i915: Make intel_logical_ring_begin() static John.C.Harrison
2015-03-09 23:24     ` Tomas Elf
2015-02-19 17:18   ` [PATCH 50/53] drm/i915: Update intel_logical_ring_begin() to take a request structure John.C.Harrison
2015-03-09 23:33     ` Tomas Elf
2015-02-19 17:18   ` [PATCH 51/53] drm/i915: Remove the now obsolete intel_ring_get_request() John.C.Harrison
2015-03-09 23:35     ` Tomas Elf
2015-02-19 17:18   ` [PATCH 52/53] drm/i915: Remove the now obsolete 'outstanding_lazy_request' John.C.Harrison
2015-03-09 23:51     ` Tomas Elf
2015-03-10 10:18       ` Daniel Vetter
2015-03-13 13:32         ` John Harrison
2015-03-13 17:09           ` Daniel Vetter
2015-02-19 17:18   ` [PATCH 53/53] drm/i915: Move the request/file and request/pid association to creation time John.C.Harrison
2015-03-10  0:12     ` Tomas Elf
2015-03-05 13:57   ` [PATCH 54/56] drm/i915: Rename 'do_execbuf' to 'execbuf_submit' John.C.Harrison
2015-03-05 13:57     ` [PATCH 55/56] drm/i915: Remove 'faked' request from LRC submission John.C.Harrison
2015-03-05 14:49       ` Daniel Vetter
2015-03-11 14:53         ` John Harrison
2015-03-11 16:14           ` Daniel Vetter
2015-03-11 16:44             ` Jesse Barnes
2015-03-11 20:45               ` John Harrison
2015-03-05 13:57     ` [PATCH 56/56] drm/i915: Update a bunch of LRC functions to take requests John.C.Harrison
2015-03-10 23:18     ` [PATCH 54/56] drm/i915: Rename 'do_execbuf' to 'execbuf_submit' Tomas Elf
2015-03-05 14:03   ` [PATCH 01/53] drm/i915: Remove ironlake rc6 support John.C.Harrison
2015-03-05 15:22     ` Daniel Vetter
2015-03-05 15:36       ` John Harrison
2015-03-06 16:32         ` Daniel Vetter
2015-03-10 23:02     ` Tomas Elf

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=20150305161400.GO18775@phenom.ffwll.local \
    --to=daniel@ffwll.ch \
    --cc=Intel-GFX@Lists.FreeDesktop.Org \
    --cc=John.C.Harrison@Intel.com \
    /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.