On Sat, Jul 4, 2026 at 1:41 PM Christian König
<[email protected]> 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 <[email protected]>
> Signed-off-by: Christian König <[email protected]>
> ---
> 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 <[email protected]>
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 <[email protected]>
Closes:
https://lore.kernel.org/oe-kbuild-all/[email protected]/
Closes:
https://lore.kernel.org/oe-kbuild-all/[email protected]/
Closes:
https://lore.kernel.org/oe-kbuild-all/[email protected]/
Closes:
https://lore.kernel.org/oe-kbuild-all/[email protected]/
Closes:
https://lore.kernel.org/oe-kbuild-all/[email protected]/
Closes:
https://lore.kernel.org/oe-kbuild-all/[email protected]/
Closes:
https://lore.kernel.org/oe-kbuild-all/[email protected]/
--
Best Regards,
Mike Gavrilov.
