* [Intel-gfx] [CI] drm/i915: Filter wake_flags passed to default_wake_function
@ 2020-07-28 15:21 Chris Wilson
2020-07-28 18:26 ` [Intel-gfx] ✗ Fi.CI.BAT: failure for drm/i915: Filter wake_flags passed to default_wake_function (rev2) Patchwork
2020-08-03 7:16 ` [Intel-gfx] [CI] drm/i915: Filter wake_flags passed to default_wake_function Dave Airlie
0 siblings, 2 replies; 3+ messages in thread
From: Chris Wilson @ 2020-07-28 15:21 UTC (permalink / raw)
To: intel-gfx
The flags passed to the wait_entry.func are passed onwards to
try_to_wake_up(), which has a very particular interpretation for its
wake_flags. In particular, beyond the published WF_SYNC, it has a few
internal flags as well. Since we passed the fence->error down the chain
via the flags argument, these ended up in the default_wake_function
confusing the kernel/sched.
Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/2110
Fixes: ef4688497512 ("drm/i915: Propagate fence errors")
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Matthew Auld <matthew.auld@intel.com>
Cc: <stable@vger.kernel.org> # v5.4+
Reviewed-by: Matthew Auld <matthew.auld@intel.com>
---
drivers/gpu/drm/i915/i915_sw_fence.c | 10 +++++++---
1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_sw_fence.c b/drivers/gpu/drm/i915/i915_sw_fence.c
index 295b9829e2da..4cd2038cbe35 100644
--- a/drivers/gpu/drm/i915/i915_sw_fence.c
+++ b/drivers/gpu/drm/i915/i915_sw_fence.c
@@ -164,9 +164,13 @@ static void __i915_sw_fence_wake_up_all(struct i915_sw_fence *fence,
do {
list_for_each_entry_safe(pos, next, &x->head, entry) {
- pos->func(pos,
- TASK_NORMAL, fence->error,
- &extra);
+ int wake_flags;
+
+ wake_flags = fence->error;
+ if (pos->func == autoremove_wake_function)
+ wake_flags = 0;
+
+ pos->func(pos, TASK_NORMAL, wake_flags, &extra);
}
if (list_empty(&extra))
--
2.20.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 3+ messages in thread
* [Intel-gfx] ✗ Fi.CI.BAT: failure for drm/i915: Filter wake_flags passed to default_wake_function (rev2)
2020-07-28 15:21 [Intel-gfx] [CI] drm/i915: Filter wake_flags passed to default_wake_function Chris Wilson
@ 2020-07-28 18:26 ` Patchwork
2020-08-03 7:16 ` [Intel-gfx] [CI] drm/i915: Filter wake_flags passed to default_wake_function Dave Airlie
1 sibling, 0 replies; 3+ messages in thread
From: Patchwork @ 2020-07-28 18:26 UTC (permalink / raw)
To: Chris Wilson; +Cc: intel-gfx
[-- Attachment #1.1: Type: text/plain, Size: 429 bytes --]
== Series Details ==
Series: drm/i915: Filter wake_flags passed to default_wake_function (rev2)
URL : https://patchwork.freedesktop.org/series/79824/
State : failure
== Summary ==
Applying: drm/i915: Filter wake_flags passed to default_wake_function
Using index info to reconstruct a base tree...
M drivers/gpu/drm/i915/i915_sw_fence.c
Falling back to patching base and 3-way merge...
No changes -- Patch already applied.
[-- Attachment #1.2: Type: text/html, Size: 923 bytes --]
[-- Attachment #2: Type: text/plain, Size: 160 bytes --]
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [Intel-gfx] [CI] drm/i915: Filter wake_flags passed to default_wake_function
2020-07-28 15:21 [Intel-gfx] [CI] drm/i915: Filter wake_flags passed to default_wake_function Chris Wilson
2020-07-28 18:26 ` [Intel-gfx] ✗ Fi.CI.BAT: failure for drm/i915: Filter wake_flags passed to default_wake_function (rev2) Patchwork
@ 2020-08-03 7:16 ` Dave Airlie
1 sibling, 0 replies; 3+ messages in thread
From: Dave Airlie @ 2020-08-03 7:16 UTC (permalink / raw)
To: Chris Wilson; +Cc: Intel Graphics Development
On Wed, 29 Jul 2020 at 01:21, Chris Wilson <chris@chris-wilson.co.uk> wrote:
>
> The flags passed to the wait_entry.func are passed onwards to
> try_to_wake_up(), which has a very particular interpretation for its
> wake_flags. In particular, beyond the published WF_SYNC, it has a few
> internal flags as well. Since we passed the fence->error down the chain
> via the flags argument, these ended up in the default_wake_function
> confusing the kernel/sched.
>
> Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/2110
> Fixes: ef4688497512 ("drm/i915: Propagate fence errors")
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Matthew Auld <matthew.auld@intel.com>
> Cc: <stable@vger.kernel.org> # v5.4+
> Reviewed-by: Matthew Auld <matthew.auld@intel.com>
> ---
> drivers/gpu/drm/i915/i915_sw_fence.c | 10 +++++++---
> 1 file changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_sw_fence.c b/drivers/gpu/drm/i915/i915_sw_fence.c
> index 295b9829e2da..4cd2038cbe35 100644
> --- a/drivers/gpu/drm/i915/i915_sw_fence.c
> +++ b/drivers/gpu/drm/i915/i915_sw_fence.c
> @@ -164,9 +164,13 @@ static void __i915_sw_fence_wake_up_all(struct i915_sw_fence *fence,
>
> do {
> list_for_each_entry_safe(pos, next, &x->head, entry) {
> - pos->func(pos,
> - TASK_NORMAL, fence->error,
> - &extra);
> + int wake_flags;
> +
> + wake_flags = fence->error;
> + if (pos->func == autoremove_wake_function)
> + wake_flags = 0;
> +
> + pos->func(pos, TASK_NORMAL, wake_flags, &extra);
> }
>
> if (list_empty(&extra))
This seems to be heading for my tree at the moment, there is only one
place in the kernel where someone compares pos->func with
autoremove_wake_function, and it's in this file.
This seems horribly brittle, can we at least make this better in -next
even if we have to have this fix in fixes?
I also have to question the whole raison d'etre for i915_sw_fence,
it's initial commit says it was meant to be a core kernel struct, but
I haven't seen any effort on behalf of i915 team to make that happen,
I expect when that is attempted the whole thing will get shredded for
layering violations like the above.
Dave.
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2020-08-03 7:16 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-28 15:21 [Intel-gfx] [CI] drm/i915: Filter wake_flags passed to default_wake_function Chris Wilson
2020-07-28 18:26 ` [Intel-gfx] ✗ Fi.CI.BAT: failure for drm/i915: Filter wake_flags passed to default_wake_function (rev2) Patchwork
2020-08-03 7:16 ` [Intel-gfx] [CI] drm/i915: Filter wake_flags passed to default_wake_function Dave Airlie
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).