From: Daniel Vetter <daniel@ffwll.ch>
To: Jason Ekstrand <jason@jlekstrand.net>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>,
intel-gfx@lists.freedesktop.org, dri-devel@lists.freedesktop.org
Subject: Re: [Intel-gfx] [PATCH 0/5] drm/i915: Get rid of fence error propagation
Date: Thu, 3 Jun 2021 10:29:17 +0200 [thread overview]
Message-ID: <YLiS3ZaDm/nttAKi@phenom.ffwll.local> (raw)
In-Reply-To: <20210602164149.391653-1-jason@jlekstrand.net>
On Wed, Jun 02, 2021 at 11:41:44AM -0500, Jason Ekstrand wrote:
> Fence error propagation is sketchy at best. Instead of explicitly handling
> fences which might have errors set in the code which is aware of errors, we
> just kick them down the line and hope that userspace knows what to do when
> a wait eventually fails. This is sketchy at best because most userspace
> isn't prepared to handle errors in those places. To make things worse, it
> allows errors to propagate across processes in unpredictable ways. This is
> causing hangs in one client to kill X11.
>
> Unfortunately, there's no quick path from here to there thanks to the fact
> that we're now running the command parser asynchronously and relying on
> fence errors for when it fails. This series first gets rid of asynchronous
> command parsing and then cleans up from there. There was never any real
> use-case for asynchronous parsing and the platforms that rely heavily on
> the command parser are old enough (Gen7) that, when we changed the way the
> command parser works, it wasn't really a change anyone was asking for
> anyway.
>
> I think we probably want this whole mess back-ported. I'm happy to take
> suggestions on the strategy there because the history there is a bit
> annoying and I'm not 100% sure where the Linux release cuts land. In any
> case, I'm happy to make a version of this series per-release if needed for
> Greg to back-port.
I think just the two reversts are enough to be backported, other 3 are
cleanups.
Also I guess this will need to come with an igt patch to adjust the
cmdparser test.
With all the nits addressed, on the series.
Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>
>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Jon Bloomfield <jon.bloomfield@intel.com>
>
> Jason Ekstrand (5):
> drm/i915: Revert "drm/i915/gem: Asynchronous cmdparser"
> drm/i915: Remove allow_alloc from i915_gem_object_get_sg*
> drm/i915: Drop error handling from dma_fence_work
> Revert "drm/i915: Propagate errors on awaiting already signaled
> fences"
> Revert "drm/i915: Skip over MI_NOOP when parsing"
>
> drivers/gpu/drm/i915/gem/i915_gem_clflush.c | 4 +-
> .../gpu/drm/i915/gem/i915_gem_execbuffer.c | 227 +-----------------
> drivers/gpu/drm/i915/gem/i915_gem_object.h | 10 +-
> drivers/gpu/drm/i915/gem/i915_gem_pages.c | 21 +-
> .../i915/gem/selftests/i915_gem_execbuffer.c | 4 +
> drivers/gpu/drm/i915/gt/intel_ggtt.c | 2 +-
> drivers/gpu/drm/i915/i915_cmd_parser.c | 199 ++++++++-------
> drivers/gpu/drm/i915/i915_drv.h | 7 +-
> drivers/gpu/drm/i915/i915_request.c | 8 +-
> drivers/gpu/drm/i915/i915_sw_fence_work.c | 5 +-
> drivers/gpu/drm/i915/i915_sw_fence_work.h | 2 +-
> drivers/gpu/drm/i915/i915_vma.c | 3 +-
> 12 files changed, 141 insertions(+), 351 deletions(-)
>
> --
> 2.31.1
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
WARNING: multiple messages have this Message-ID (diff)
From: Daniel Vetter <daniel@ffwll.ch>
To: Jason Ekstrand <jason@jlekstrand.net>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>,
intel-gfx@lists.freedesktop.org,
Jon Bloomfield <jon.bloomfield@intel.com>,
dri-devel@lists.freedesktop.org
Subject: Re: [PATCH 0/5] drm/i915: Get rid of fence error propagation
Date: Thu, 3 Jun 2021 10:29:17 +0200 [thread overview]
Message-ID: <YLiS3ZaDm/nttAKi@phenom.ffwll.local> (raw)
In-Reply-To: <20210602164149.391653-1-jason@jlekstrand.net>
On Wed, Jun 02, 2021 at 11:41:44AM -0500, Jason Ekstrand wrote:
> Fence error propagation is sketchy at best. Instead of explicitly handling
> fences which might have errors set in the code which is aware of errors, we
> just kick them down the line and hope that userspace knows what to do when
> a wait eventually fails. This is sketchy at best because most userspace
> isn't prepared to handle errors in those places. To make things worse, it
> allows errors to propagate across processes in unpredictable ways. This is
> causing hangs in one client to kill X11.
>
> Unfortunately, there's no quick path from here to there thanks to the fact
> that we're now running the command parser asynchronously and relying on
> fence errors for when it fails. This series first gets rid of asynchronous
> command parsing and then cleans up from there. There was never any real
> use-case for asynchronous parsing and the platforms that rely heavily on
> the command parser are old enough (Gen7) that, when we changed the way the
> command parser works, it wasn't really a change anyone was asking for
> anyway.
>
> I think we probably want this whole mess back-ported. I'm happy to take
> suggestions on the strategy there because the history there is a bit
> annoying and I'm not 100% sure where the Linux release cuts land. In any
> case, I'm happy to make a version of this series per-release if needed for
> Greg to back-port.
I think just the two reversts are enough to be backported, other 3 are
cleanups.
Also I guess this will need to come with an igt patch to adjust the
cmdparser test.
With all the nits addressed, on the series.
Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>
>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Jon Bloomfield <jon.bloomfield@intel.com>
>
> Jason Ekstrand (5):
> drm/i915: Revert "drm/i915/gem: Asynchronous cmdparser"
> drm/i915: Remove allow_alloc from i915_gem_object_get_sg*
> drm/i915: Drop error handling from dma_fence_work
> Revert "drm/i915: Propagate errors on awaiting already signaled
> fences"
> Revert "drm/i915: Skip over MI_NOOP when parsing"
>
> drivers/gpu/drm/i915/gem/i915_gem_clflush.c | 4 +-
> .../gpu/drm/i915/gem/i915_gem_execbuffer.c | 227 +-----------------
> drivers/gpu/drm/i915/gem/i915_gem_object.h | 10 +-
> drivers/gpu/drm/i915/gem/i915_gem_pages.c | 21 +-
> .../i915/gem/selftests/i915_gem_execbuffer.c | 4 +
> drivers/gpu/drm/i915/gt/intel_ggtt.c | 2 +-
> drivers/gpu/drm/i915/i915_cmd_parser.c | 199 ++++++++-------
> drivers/gpu/drm/i915/i915_drv.h | 7 +-
> drivers/gpu/drm/i915/i915_request.c | 8 +-
> drivers/gpu/drm/i915/i915_sw_fence_work.c | 5 +-
> drivers/gpu/drm/i915/i915_sw_fence_work.h | 2 +-
> drivers/gpu/drm/i915/i915_vma.c | 3 +-
> 12 files changed, 141 insertions(+), 351 deletions(-)
>
> --
> 2.31.1
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
next prev parent reply other threads:[~2021-06-03 8:29 UTC|newest]
Thread overview: 38+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-06-02 16:41 [Intel-gfx] [PATCH 0/5] drm/i915: Get rid of fence error propagation Jason Ekstrand
2021-06-02 16:41 ` Jason Ekstrand
2021-06-02 16:41 ` [Intel-gfx] [PATCH 1/5] drm/i915: Revert "drm/i915/gem: Asynchronous cmdparser" Jason Ekstrand
2021-06-02 16:41 ` Jason Ekstrand
2021-06-03 8:22 ` [Intel-gfx] " Daniel Vetter
2021-06-03 8:22 ` Daniel Vetter
2021-06-03 15:20 ` Jason Ekstrand
2021-06-03 15:20 ` Jason Ekstrand
2021-06-02 16:41 ` [Intel-gfx] [PATCH 2/5] drm/i915: Remove allow_alloc from i915_gem_object_get_sg* Jason Ekstrand
2021-06-02 16:41 ` Jason Ekstrand
2021-06-02 16:41 ` [Intel-gfx] [PATCH 3/5] drm/i915: Drop error handling from dma_fence_work Jason Ekstrand
2021-06-02 16:41 ` Jason Ekstrand
2021-06-03 8:21 ` [Intel-gfx] " Daniel Vetter
2021-06-03 8:21 ` Daniel Vetter
2021-06-02 16:41 ` [Intel-gfx] [PATCH 4/5] Revert "drm/i915: Propagate errors on awaiting already signaled fences" Jason Ekstrand
2021-06-02 16:41 ` Jason Ekstrand
2021-06-02 16:41 ` Jason Ekstrand
2021-06-03 8:24 ` [Intel-gfx] " Daniel Vetter
2021-06-03 8:24 ` Daniel Vetter
2021-06-03 8:24 ` Daniel Vetter
2021-06-03 8:25 ` [Intel-gfx] " Daniel Vetter
2021-06-03 8:25 ` Daniel Vetter
2021-06-03 8:25 ` Daniel Vetter
2021-06-03 8:28 ` [Intel-gfx] " Daniel Vetter
2021-06-03 8:28 ` Daniel Vetter
2021-06-03 8:28 ` Daniel Vetter
2021-06-03 15:28 ` [Intel-gfx] " Jason Ekstrand
2021-06-03 15:28 ` Jason Ekstrand
2021-06-03 15:28 ` Jason Ekstrand
2021-06-02 16:41 ` [Intel-gfx] [PATCH 5/5] Revert "drm/i915: Skip over MI_NOOP when parsing" Jason Ekstrand
2021-06-02 16:41 ` Jason Ekstrand
2021-06-02 17:03 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for drm/i915: Get rid of fence error propagation Patchwork
2021-06-02 17:04 ` [Intel-gfx] ✗ Fi.CI.SPARSE: " Patchwork
2021-06-02 17:08 ` [Intel-gfx] ✗ Fi.CI.DOCS: " Patchwork
2021-06-02 17:34 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork
2021-06-02 21:50 ` [Intel-gfx] ✗ Fi.CI.IGT: failure " Patchwork
2021-06-03 8:29 ` Daniel Vetter [this message]
2021-06-03 8:29 ` [PATCH 0/5] " 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=YLiS3ZaDm/nttAKi@phenom.ffwll.local \
--to=daniel@ffwll.ch \
--cc=daniel.vetter@ffwll.ch \
--cc=dri-devel@lists.freedesktop.org \
--cc=intel-gfx@lists.freedesktop.org \
--cc=jason@jlekstrand.net \
/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.