[src/trunk]: src/sys/external/bsd/drm2/dist/drm/i915 Revert "i915: Defer fina...

[riastradh <riastradh%NetBSD.org@localhost>]

details:   https://anonhg.NetBSD.org/src/rev/96a621d66af5
branches:  trunk
changeset: 361577:96a621d66af5
user:      riastradh <riastradh%NetBSD.org@localhost>
date:      Tue Feb 15 18:14:18 2022 +0000

description:
Revert "i915: Defer final wakeup on active until after retirement."

This reverts i915_active.c 1.11.  ref->retire might free the object,
so touching it is not allowed -- that would use-after-free.  Linux
uses the object only for its address with wake_up_var.

The reason I made this change was that I guessed i915_active_wait has
to wait until after ref->retire finishes -- after all, Linux seems to
defer the wakeup until then.  However, even the Linux code doesn't
guarantee this, because i915_active_wait could be called _during_
ref->retire, and would witness ref->count == 0, and would not wait
until it has completed in that case.  So maybe my guess was wrong,
and it is OK for i915_active_wait to return while ref->retire is
still in flight -- I don't see any logic that obviously requires it
to wait for ref->retire, in any case.

Or maybe something does rely on i915_active_wait to wait for
ref->retire to finish, in which case we need a different mechanism
for i915_active_release itself to wait until i915_active_retire has
woken up, without dereferencing ref since it might be dead after
ref->retire.

diffstat:

 sys/external/bsd/drm2/dist/drm/i915/i915_active.c |  9 ++++-----
 1 files changed, 4 insertions(+), 5 deletions(-)

diffs (37 lines):

diff -r d2285e9679ec -r 96a621d66af5 sys/external/bsd/drm2/dist/drm/i915/i915_active.c
--- a/sys/external/bsd/drm2/dist/drm/i915/i915_active.c Tue Feb 15 16:21:35 2022 +0000
+++ b/sys/external/bsd/drm2/dist/drm/i915/i915_active.c Tue Feb 15 18:14:18 2022 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: i915_active.c,v 1.11 2022/02/14 20:37:51 riastradh Exp $       */
+/*     $NetBSD: i915_active.c,v 1.12 2022/02/15 18:14:18 riastradh Exp $       */
 
 /*
  * SPDX-License-Identifier: MIT
@@ -7,7 +7,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: i915_active.c,v 1.11 2022/02/14 20:37:51 riastradh Exp $");
+__KERNEL_RCSID(0, "$NetBSD: i915_active.c,v 1.12 2022/02/15 18:14:18 riastradh Exp $");
 
 #include <linux/debugobjects.h>
 
@@ -195,6 +195,8 @@
 #endif
        ref->cache = NULL;
 
+       DRM_SPIN_WAKEUP_ALL(&ref->tree_wq, &ref->tree_lock);
+
        spin_unlock_irqrestore(&ref->tree_lock, flags);
 
        /* After the final retire, the entire struct may be freed */
@@ -202,9 +204,6 @@
                ref->retire(ref);
 
        /* ... except if you wait on it, you must manage your own references! */
-       spin_lock(&ref->tree_lock);
-       DRM_SPIN_WAKEUP_ALL(&ref->tree_wq, &ref->tree_lock);
-       spin_unlock(&ref->tree_lock);
 
        rbtree_postorder_for_each_entry_safe(it, n, &root, node) {
                GEM_BUG_ON(i915_active_fence_isset(&it->base));