Re: [linux-next:master 14191/14955] vmlinux.o: error: objtool: amdgpu_vm_handle_fault+0x186: sibling call from callable instruction with modified stack frame

[Christian König]

On 6/24/26 12:56, Peter Zijlstra wrote:
> On Wed, Jun 24, 2026 at 11:48:09AM +0200, Christian König wrote:
> 
>>> So, if my heat-addled brain isn't completely gone, then this all boils
>>> down to something like:
>>>
>>>   drm_exec_init(&exec);
>>> label:
>>>   for (;drm_exec_cleanup(&exec);) {
>>>      ...
>>>      if (unlikely(drm_exec_is_contended(&exec)))
>>>        goto label;
>>>      ...
>>>   }
>>>   ...
>>>   drm_exec_fini(&exec);
>>>
>>> Except, you've laundered that label through a computed goto to allow
>>> multiple such constructs in a single function -- because can't have
>>> multiple identical labels etc.
>>
>> Just FYI: That's actually not the reason for this, the label name is unique now.
> 
> Yeah, I know, but I thought that the only way to transfer the name to
> the drm_exec_retry() sites was to create that computed-goto alias.

Yes, that was also a problem.

> 
>> The computed goto is used to limit the scope you can use the retry
>> macros because we initially had problems with people using that
>> outside of the loop.
> 
> Ah, also a very good reason.
> 
>> Exactly that, yes. The nested loops are unfortunately a very common use case here.
> 
> Yep, saw that.
> 
>>> That is, I think this drm_exec_until_all_locked() thing has some
>>> fundamental problems the moment clang fails to optimize it away.
>>> Correctness really depends on the compiler not actually ever doing a
>>> computed goto.
>>
>> Mhm, that's not good at all but I also of hand don't see a way to
>> avoid the computed goto.
> 
> Best I can come up with is something like this. Not exactly nice, but
> should be fairly trivial to do.

Yeah, definitely not nice. The macro was created exactly because people messed this approach up more than once.

I could live with some other hack/workaround, but clearly not with changing all users.

Maybe we can goto the fixed named label but have an extra variable to check that we are in the right scope or something like that.

Regards,
Christian.

> 
> ---
> diff --git a/drivers/gpu/drm/tests/drm_exec_test.c b/drivers/gpu/drm/tests/drm_exec_test.c
> index 2fc47f3b463b..7d69c06b2dd3 100644
> --- a/drivers/gpu/drm/tests/drm_exec_test.c
> +++ b/drivers/gpu/drm/tests/drm_exec_test.c
> @@ -60,6 +60,7 @@ static void test_lock(struct kunit *test)
>  
>  	drm_exec_init(&exec, DRM_EXEC_INTERRUPTIBLE_WAIT, 0);
>  	drm_exec_until_all_locked(&exec) {
> +drm_exec_label:
>  		ret = drm_exec_lock_obj(&exec, &gobj);
>  		drm_exec_retry_on_contention(&exec);
>  		KUNIT_EXPECT_EQ(test, ret, 0);
> @@ -80,6 +81,7 @@ static void test_lock_unlock(struct kunit *test)
>  
>  	drm_exec_init(&exec, DRM_EXEC_INTERRUPTIBLE_WAIT, 0);
>  	drm_exec_until_all_locked(&exec) {
> +drm_exec_label:
>  		ret = drm_exec_lock_obj(&exec, &gobj);
>  		drm_exec_retry_on_contention(&exec);
>  		KUNIT_EXPECT_EQ(test, ret, 0);
> @@ -107,6 +109,7 @@ static void test_duplicates(struct kunit *test)
>  
>  	drm_exec_init(&exec, DRM_EXEC_IGNORE_DUPLICATES, 0);
>  	drm_exec_until_all_locked(&exec) {
> +drm_exec_label:
>  		ret = drm_exec_lock_obj(&exec, &gobj);
>  		drm_exec_retry_on_contention(&exec);
>  		KUNIT_EXPECT_EQ(test, ret, 0);
> @@ -134,6 +137,7 @@ static void test_prepare(struct kunit *test)
>  
>  	drm_exec_init(&exec, DRM_EXEC_INTERRUPTIBLE_WAIT, 0);
>  	drm_exec_until_all_locked(&exec) {
> +drm_exec_label:
>  		ret = drm_exec_prepare_obj(&exec, &gobj, 1);
>  		drm_exec_retry_on_contention(&exec);
>  		KUNIT_EXPECT_EQ(test, ret, 0);
> @@ -166,9 +170,10 @@ static void test_prepare_array(struct kunit *test)
>  	drm_gem_private_object_init(priv->drm, gobj2, PAGE_SIZE);
>  
>  	drm_exec_init(&exec, DRM_EXEC_INTERRUPTIBLE_WAIT, 0);
> -	drm_exec_until_all_locked(&exec)
> -		ret = drm_exec_prepare_array(&exec, array, ARRAY_SIZE(array),
> -					     1);
> +	drm_exec_until_all_locked(&exec) {
> +drm_exec_label:
> +		ret = drm_exec_prepare_array(&exec, array, ARRAY_SIZE(array), 1);
> +	}
>  	KUNIT_EXPECT_EQ(test, ret, 0);
>  	drm_exec_fini(&exec);
>  
> @@ -181,15 +186,15 @@ 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)
> -	{
> +	drm_exec_until_all_locked(&exec) {
> +drm_exec_label:
>  		break;
>  	}
>  	drm_exec_fini(&exec);
>  
>  	drm_exec_init(&exec, DRM_EXEC_INTERRUPTIBLE_WAIT, 0);
> -	drm_exec_until_all_locked(&exec)
> -	{
> +	drm_exec_until_all_locked(&exec) {
> +drm_exec_label:
>  		break;
>  	}
>  	drm_exec_fini(&exec);
> diff --git a/include/drm/drm_exec.h b/include/drm/drm_exec.h
> index 8725ba92ff91..f2bfa6aa7b7b 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
> @@ -121,7 +110,18 @@ _label:									 \
>   * guaranteed that no GEM object is locked.
>   */
>  #define drm_exec_until_all_locked(exec)					\
> -	__drm_exec_until_all_locked(exec, __UNIQUE_ID(drm_exec))
> +	while (drm_exec_cleanup(exec))
> +
> +#define drm_exec_label							\
> +	__label__ __drm_exec_retry;					\
> +	__label__ __drm_foo;						\
> +	if (0) {							\
> +	__drm_exec_retry: __maybe_unused;				\
> +		continue;						\
> +		goto __drm_foo;						\
> +	}								\
> +	__drm_foo
> +
>  
>  /**
>   * drm_exec_retry_on_contention - restart the loop to grap all locks
> @@ -133,7 +133,7 @@ _label:									 \
>  #define drm_exec_retry_on_contention(exec)			\
>  	do {							\
>  		if (unlikely(drm_exec_is_contended(exec)))	\
> -			goto *__drm_exec_retry_ptr;		\
> +			goto __drm_exec_retry;			\
>  	} while (0)
>  
>  /**
> @@ -158,7 +158,7 @@ static inline bool drm_exec_is_contended(struct drm_exec *exec)
>  #define drm_exec_retry(_exec)					\
>  	do {							\
>  		WARN_ON((_exec)->contended != DRM_EXEC_DUMMY);	\
> -		goto *__drm_exec_retry_ptr;			\
> +		goto __drm_exec_retry;				\
>  	} while (0)
>  
>  /**