drm/drm_exec: avoid indirect goto

The drm_exec component uses a variable with scope limited to the for() and
an indirect goto to allow instantiating multiple macros in the same
function.

This unfortunately doesn't work well with certain compilers when the
indirect goto can't be lowered to a direct jump.

Switch the indirect goto to a direct goto, the drawback is that we now
can't use the dma_exec_until_all_locked() macro in the same function
multiple times.

The is currently only one user of this and only as a hacky workaround
which is about to be removed.

So document that the __label__ statement should be used when the macro is
used multiple times and fix the tests and the only use case where that is
necessary.

Suggested-by: Peter Zijlstra <peterz@infradead.org>
Signed-off-by: Christian König <christian.koenig@amd.com>
Fixes: 9920249a5288 ("drm/amdgpu: convert amdgpu_vm_lock_by_pasid() to drm_exec")
Reported-by: kernel test robot <lkp@intel.com>
Closes: https://lore.kernel.org/oe-kbuild-all/202606231854.7LeCtlLe-lkp@intel.com/
Closes: https://lore.kernel.org/oe-kbuild-all/202606232356.gwHMAJAW-lkp@intel.com/
Closes: https://lore.kernel.org/oe-kbuild-all/202606240753.kYjobJVl-lkp@intel.com/
Closes: https://lore.kernel.org/oe-kbuild-all/202606241110.iUga5vVw-lkp@intel.com/
Closes: https://lore.kernel.org/oe-kbuild-all/202607031446.1PWG18mN-lkp@intel.com/
Closes: https://lore.kernel.org/oe-kbuild-all/202607031837.HSmBj8pr-lkp@intel.com/
Closes: https://lore.kernel.org/oe-kbuild-all/202607040159.GopyEswS-lkp@intel.com/
Tested-by: Mikhail Gavrilov <mikhail.v.gavrilov@gmail.com>
Reviewed-by: Dave Airlie <airlied@redhat.com>
Link: https://lore.kernel.org/r/20260704084133.122053-1-christian.koenig@amd.com

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index fee4c94c25858e0ed1838cc78797d2a2b86e8a1a..fc28d0fdad3700a42449088a97a9b14b0b5df1ab 100644 (file)
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -3011,6 +3011,8 @@ bool amdgpu_vm_handle_fault(struct amdgpu_device *adev, u32 pasid,
        is_compute_context = vm->is_compute_context;
 
        if (is_compute_context) {
+               __label__ drm_exec_retry;
+
                /* Release the root PD lock since svm_range_restore_pages
                 * might try to take it.
                 * TODO: rework svm_range_restore_pages so that this isn't

diff --git a/drivers/gpu/drm/tests/drm_exec_test.c b/drivers/gpu/drm/tests/drm_exec_test.c
index 2fc47f3b463b7e99b2ac5773683d5ffc333f03b6..7a374e462348bc032c5e75a7a1dabb4dbc1a6a89 100644 (file)
--- a/drivers/gpu/drm/tests/drm_exec_test.c
+++ b/drivers/gpu/drm/tests/drm_exec_test.c
@@ -180,19 +180,27 @@ static void test_multiple_loops(struct kunit *test)
 {
        struct drm_exec exec;
 
-       drm_exec_init(&exec, DRM_EXEC_INTERRUPTIBLE_WAIT, 0);
-       drm_exec_until_all_locked(&exec)
        {
-               break;
+               __label__ drm_exec_retry;
+
+               drm_exec_init(&exec, DRM_EXEC_INTERRUPTIBLE_WAIT, 0);
+               drm_exec_until_all_locked(&exec)
+               {
+                       break;
+               }
+               drm_exec_fini(&exec);
        }
-       drm_exec_fini(&exec);
 
-       drm_exec_init(&exec, DRM_EXEC_INTERRUPTIBLE_WAIT, 0);
-       drm_exec_until_all_locked(&exec)
        {
-               break;
+               __label__ drm_exec_retry;
+
+               drm_exec_init(&exec, DRM_EXEC_INTERRUPTIBLE_WAIT, 0);
+               drm_exec_until_all_locked(&exec)
+               {
+                       break;
+               }
+               drm_exec_fini(&exec);
        }
-       drm_exec_fini(&exec);
        KUNIT_SUCCEED(test);
 }
 

diff --git a/include/drm/drm_exec.h b/include/drm/drm_exec.h
index 8725ba92ff9163a070085df7bd2e1dab0ff31a9b..cc2937185a9f769bb63b62fb84682284f85290eb 100644 (file)
--- a/include/drm/drm_exec.h
+++ b/include/drm/drm_exec.h
@@ -101,17 +101,6 @@ drm_exec_obj(struct drm_exec *exec, unsigned long index)
 #define drm_exec_for_each_locked_object_reverse(exec, obj)             \
        __drm_exec_for_each_locked_object_reverse(exec, obj, __UNIQUE_ID(drm_exec))
 
-/*
- * Helper to drm_exec_until_all_locked(). Don't use directly.
- *
- * Since labels can't be defined local to the loop's body we use a jump pointer
- * to make sure that the retry is only used from within the loop's body.
- */
-#define __drm_exec_until_all_locked(exec, _label)                       \
-_label:                                                                         \
-       for (void *const __maybe_unused __drm_exec_retry_ptr = &&_label; \
-            drm_exec_cleanup(exec);)
-
 /**
  * drm_exec_until_all_locked - loop until all GEM objects are locked
  * @exec: drm_exec object
@@ -119,9 +108,18 @@ _label:                                                                     \
  * Core functionality of the drm_exec object. Loops until all GEM objects are
  * locked and no more contention exists. At the beginning of the loop it is
  * guaranteed that no GEM object is locked.
+ *
+ * A global label name drm_exec_retry is used, if you need to use more than one
+ * instance of this macro in the same function the label needs to be made local
+ * to the block with the __label__ keyword.
  */
 #define drm_exec_until_all_locked(exec)                                        \
-       __drm_exec_until_all_locked(exec, __UNIQUE_ID(drm_exec))
+       for (bool const __maybe_unused __drm_exec_loop = false;         \
+            drm_exec_cleanup(exec);)                                   \
+               if (false) {                                            \
+drm_exec_retry: __maybe_unused;                                                \
+                       continue;                                       \
+               } else
 
 /**
  * drm_exec_retry_on_contention - restart the loop to grap all locks
@@ -129,12 +127,14 @@ _label:                                                                    \
  *
  * Control flow helper to continue when a contention was detected and we need to
  * clean up and re-start the loop to prepare all GEM objects.
+ * The __drm_exec_loop check exists to prevent usage outside of an
+ * drm_exec_until_all_locked() loop.
  */
 #define drm_exec_retry_on_contention(exec)                     \
        do {                                                    \
                if (unlikely(drm_exec_is_contended(exec)))      \
-                       goto *__drm_exec_retry_ptr;             \
-       } while (0)
+                       goto drm_exec_retry;                    \
+       } while (__drm_exec_loop)
 
 /**
  * drm_exec_is_contended - check for contention
@@ -154,12 +154,14 @@ static inline bool drm_exec_is_contended(struct drm_exec *exec)
  *
  * Unconditionally retry the loop to lock all objects. For consistency,
  * the exec object needs to be newly initialized.
+ * The __drm_exec_loop check exists to prevent usage outside of an
+ * drm_exec_until_all_locked() loop.
  */
 #define drm_exec_retry(_exec)                                  \
        do {                                                    \
                WARN_ON((_exec)->contended != DRM_EXEC_DUMMY);  \
-               goto *__drm_exec_retry_ptr;                     \
-       } while (0)
+               goto drm_exec_retry;                            \
+       } while (__drm_exec_loop)
 
 /**
  * drm_exec_ticket - return the ww_acquire_ctx for this exec context