Re: [PATCH] drm/drm_exec: avoid indirect goto

[Mikhail Gavrilov]

On Sat, Jul 4, 2026 at 1:41 PM Christian König
<ckoenig.leichtzumerken@xxxxxxxxx> wrote:
>
> 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@xxxxxxxxxxxxx>
> Signed-off-by: Christian König <christian.koenig@xxxxxxx>
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c |  2 ++
>  drivers/gpu/drm/tests/drm_exec_test.c  | 24 ++++++++++++------
>  include/drm/drm_exec.h                 | 34 ++++++++++++++------------
>  3 files changed, 36 insertions(+), 24 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> index fee4c94c2585..fc28d0fdad37 100644
> --- 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 2fc47f3b463b..7a374e462348 100644
> --- 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 8725ba92ff91..cc2937185a9f 100644
> --- 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
> --
> 2.43.0
>

Tested on the configs that originally tripped objtool, the error is gone
with this patch:

  - gcc, non-LTO: amdgpu_vm.o builds clean
    (was amdgpu_vm_handle_fault+0x169 / +0x97)
  - clang, non-LTO: amdgpu_vm.o builds clean
    (was amdgpu_vm_handle_fault+0x12d)
  - clang + ThinLTO + KASAN, OBJTOOL_WERROR=y: full vmlinux.o link
    passes objtool (was amdgpu_vm_handle_fault+0x186 / +0x16b)

x86_64, clang 22.1.8.

Tested-by: Mikhail Gavrilov <mikhail.v.gavrilov@xxxxxxxxx>

FWIW the kernel test robot reported this many times across trees (clang
LTO, clang non-LTO, and gcc builds); it's all the same drm_exec
computed-goto pattern. Full list if you want to add Reported-by/Closes:

Fixes: 9920249a5288 ("drm/amdgpu: convert amdgpu_vm_lock_by_pasid() to
drm_exec")
Reported-by: kernel test robot <lkp@xxxxxxxxx>
Closes: https://lore.kernel.org/oe-kbuild-all/202606231854.7LeCtlLe-lkp@xxxxxxxxx/
Closes: https://lore.kernel.org/oe-kbuild-all/202606232356.gwHMAJAW-lkp@xxxxxxxxx/
Closes: https://lore.kernel.org/oe-kbuild-all/202606240753.kYjobJVl-lkp@xxxxxxxxx/
Closes: https://lore.kernel.org/oe-kbuild-all/202606241110.iUga5vVw-lkp@xxxxxxxxx/
Closes: https://lore.kernel.org/oe-kbuild-all/202607031446.1PWG18mN-lkp@xxxxxxxxx/
Closes: https://lore.kernel.org/oe-kbuild-all/202607031837.HSmBj8pr-lkp@xxxxxxxxx/
Closes: https://lore.kernel.org/oe-kbuild-all/202607040159.GopyEswS-lkp@xxxxxxxxx/

-- 
Best Regards,
Mike Gavrilov.