ACK/Cmnt: [PATCH 1/2] drm/i915: Fix ref->mutex deadlock in i915_active_wait()

[Sultan Alsawaf]

On Thu, Apr 09, 2020 at 06:19:23PM +0200, Andrea Righi wrote:
> BTW, is 750bde2fd4ff really needed? Looking at those commits it seems
> that only 274cbf20fd10 and 093b92287363 should be enough to prevent the
> deadlock.

I think it is needed. In the commit message for 750bde2fd4ff, Chris says that
there is a synchronization issue when retirement occurs in a worker on another
CPU. The very first commit in my list, 274cbf20fd10, does exactly that: it moves
retirement into a worker for some cases, so it appears that 750bde2fd4ff is a
necessary fix for that.

> I've tried to backport these two and it seems a doable backport (I
> have the patches against focal, but I haven't tested them, I just
> build-tested).

Doable, but I think it carries a lot more risk. Backporting i915 fixes is a
dangerous game because its code changes so much in between releases. I've tried
to backport other i915 stuff before, which wasn't difficult to backport at all,
but then the backport caused panics because extra mutex locks were needed. Scary
stuff...

> However, if you confirm that your patch has been tested I think it can
> be an easier fix and a reasonable compromise. Under this assumption:

Yep, and you can test on your own too! Just boot with `mem=2G` or something, and
then add a printk inside of i915_active_request_retire() to notice when it gets
called. If you don't deadlock when it gets called then it's a success.

Thanks for the review.

Sultan