On Wed, 29 Jul 2020 at 01:21, Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> 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@xxxxxxxxxxxxxxxxxx> > Cc: Matthew Auld <matthew.auld@xxxxxxxxx> > Cc: <stable@xxxxxxxxxxxxxxx> # v5.4+ > Reviewed-by: Matthew Auld <matthew.auld@xxxxxxxxx> > --- > 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@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
