* [PATCH] drm/msm: Fix objtool warning in submit_lock_objects()
@ 2025-08-05 22:55 Sasha Levin
2025-08-06 23:38 ` Rob Clark
0 siblings, 1 reply; 3+ messages in thread
From: Sasha Levin @ 2025-08-05 22:55 UTC (permalink / raw)
To: robin.clark, lumag
Cc: abhinav.kumar, jessica.zhang, sean, marijn.suijten, airlied,
simona, antomani103, linux-arm-msm, dri-devel, freedreno,
linux-kernel, Sasha Levin
Restructure submit_lock_objects() to use a single loop with break
statements to fix objtool warning:
drivers/gpu/drm/msm/msm.o: warning: objtool: submit_lock_objects+0x451:
sibling call from callable instruction with modified stack frame
The drm_exec_until_all_locked() macro uses computed gotos internally
for its retry loop. Having return statements inside this macro, or
immediately after it in certain code paths, confuses objtool's static
analysis of stack frames, causing it to incorrectly flag tail call
optimizations.
Fixes: 92395af63a99 ("drm/msm: Add VM_BIND submitqueue")
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
drivers/gpu/drm/msm/msm_gem_submit.c | 43 ++++++++++++----------------
1 file changed, 19 insertions(+), 24 deletions(-)
diff --git a/drivers/gpu/drm/msm/msm_gem_submit.c b/drivers/gpu/drm/msm/msm_gem_submit.c
index 5f8e939a5906..253347b6e328 100644
--- a/drivers/gpu/drm/msm/msm_gem_submit.c
+++ b/drivers/gpu/drm/msm/msm_gem_submit.c
@@ -276,46 +276,41 @@ static int submit_lock_objects(struct msm_gem_submit *submit)
{
unsigned flags = DRM_EXEC_INTERRUPTIBLE_WAIT;
struct drm_exec *exec = &submit->exec;
- int ret;
+ int ret = 0;
- if (msm_context_is_vmbind(submit->queue->ctx)) {
+ if (msm_context_is_vmbind(submit->queue->ctx))
flags |= DRM_EXEC_IGNORE_DUPLICATES;
- drm_exec_init(&submit->exec, flags, submit->nr_bos);
+ drm_exec_init(&submit->exec, flags, submit->nr_bos);
- drm_exec_until_all_locked (&submit->exec) {
+ drm_exec_until_all_locked (&submit->exec) {
+ if (msm_context_is_vmbind(submit->queue->ctx)) {
ret = drm_gpuvm_prepare_vm(submit->vm, exec, 1);
drm_exec_retry_on_contention(exec);
if (ret)
- return ret;
+ break;
ret = drm_gpuvm_prepare_objects(submit->vm, exec, 1);
drm_exec_retry_on_contention(exec);
if (ret)
- return ret;
- }
-
- return 0;
- }
-
- drm_exec_init(&submit->exec, flags, submit->nr_bos);
-
- drm_exec_until_all_locked (&submit->exec) {
- ret = drm_exec_lock_obj(&submit->exec,
- drm_gpuvm_resv_obj(submit->vm));
- drm_exec_retry_on_contention(&submit->exec);
- if (ret)
- return ret;
- for (unsigned i = 0; i < submit->nr_bos; i++) {
- struct drm_gem_object *obj = submit->bos[i].obj;
- ret = drm_exec_prepare_obj(&submit->exec, obj, 1);
+ break;
+ } else {
+ ret = drm_exec_lock_obj(&submit->exec,
+ drm_gpuvm_resv_obj(submit->vm));
drm_exec_retry_on_contention(&submit->exec);
if (ret)
- return ret;
+ break;
+ for (unsigned i = 0; i < submit->nr_bos; i++) {
+ struct drm_gem_object *obj = submit->bos[i].obj;
+ ret = drm_exec_prepare_obj(&submit->exec, obj, 1);
+ drm_exec_retry_on_contention(&submit->exec);
+ if (ret)
+ break;
+ }
}
}
- return 0;
+ return ret;
}
static int submit_fence_sync(struct msm_gem_submit *submit)
--
2.39.5
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [PATCH] drm/msm: Fix objtool warning in submit_lock_objects()
2025-08-05 22:55 [PATCH] drm/msm: Fix objtool warning in submit_lock_objects() Sasha Levin
@ 2025-08-06 23:38 ` Rob Clark
2025-08-07 13:03 ` Sasha Levin
0 siblings, 1 reply; 3+ messages in thread
From: Rob Clark @ 2025-08-06 23:38 UTC (permalink / raw)
To: Sasha Levin
Cc: lumag, abhinav.kumar, jessica.zhang, sean, marijn.suijten,
airlied, simona, antomani103, linux-arm-msm, dri-devel, freedreno,
linux-kernel
On Tue, Aug 5, 2025 at 3:56 PM Sasha Levin <sashal@kernel.org> wrote:
>
> Restructure submit_lock_objects() to use a single loop with break
> statements to fix objtool warning:
>
> drivers/gpu/drm/msm/msm.o: warning: objtool: submit_lock_objects+0x451:
> sibling call from callable instruction with modified stack frame
>
> The drm_exec_until_all_locked() macro uses computed gotos internally
> for its retry loop. Having return statements inside this macro, or
> immediately after it in certain code paths, confuses objtool's static
> analysis of stack frames, causing it to incorrectly flag tail call
> optimizations.
Maybe we should instead just split out a separate
submit_lock_objects_vmbind() and restore the error path 'goto error'
instead of returning from within the loop? Ie. basically revert
submit_lock_objects to the way it was before commit 92395af63a99
("drm/msm: Add VM_BIND submitqueue"), and then move the rest into a
new fxn (with 'goto error' instead of 'return ret'? In retrospect the
vmbind case is kinda just shoehorned into the existing fxn.
I can type up this version if you have better things to do.
BR,
-R
> Fixes: 92395af63a99 ("drm/msm: Add VM_BIND submitqueue")
> Signed-off-by: Sasha Levin <sashal@kernel.org>
> ---
> drivers/gpu/drm/msm/msm_gem_submit.c | 43 ++++++++++++----------------
> 1 file changed, 19 insertions(+), 24 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/msm_gem_submit.c b/drivers/gpu/drm/msm/msm_gem_submit.c
> index 5f8e939a5906..253347b6e328 100644
> --- a/drivers/gpu/drm/msm/msm_gem_submit.c
> +++ b/drivers/gpu/drm/msm/msm_gem_submit.c
> @@ -276,46 +276,41 @@ static int submit_lock_objects(struct msm_gem_submit *submit)
> {
> unsigned flags = DRM_EXEC_INTERRUPTIBLE_WAIT;
> struct drm_exec *exec = &submit->exec;
> - int ret;
> + int ret = 0;
>
> - if (msm_context_is_vmbind(submit->queue->ctx)) {
> + if (msm_context_is_vmbind(submit->queue->ctx))
> flags |= DRM_EXEC_IGNORE_DUPLICATES;
>
> - drm_exec_init(&submit->exec, flags, submit->nr_bos);
> + drm_exec_init(&submit->exec, flags, submit->nr_bos);
>
> - drm_exec_until_all_locked (&submit->exec) {
> + drm_exec_until_all_locked (&submit->exec) {
> + if (msm_context_is_vmbind(submit->queue->ctx)) {
> ret = drm_gpuvm_prepare_vm(submit->vm, exec, 1);
> drm_exec_retry_on_contention(exec);
> if (ret)
> - return ret;
> + break;
>
> ret = drm_gpuvm_prepare_objects(submit->vm, exec, 1);
> drm_exec_retry_on_contention(exec);
> if (ret)
> - return ret;
> - }
> -
> - return 0;
> - }
> -
> - drm_exec_init(&submit->exec, flags, submit->nr_bos);
> -
> - drm_exec_until_all_locked (&submit->exec) {
> - ret = drm_exec_lock_obj(&submit->exec,
> - drm_gpuvm_resv_obj(submit->vm));
> - drm_exec_retry_on_contention(&submit->exec);
> - if (ret)
> - return ret;
> - for (unsigned i = 0; i < submit->nr_bos; i++) {
> - struct drm_gem_object *obj = submit->bos[i].obj;
> - ret = drm_exec_prepare_obj(&submit->exec, obj, 1);
> + break;
> + } else {
> + ret = drm_exec_lock_obj(&submit->exec,
> + drm_gpuvm_resv_obj(submit->vm));
> drm_exec_retry_on_contention(&submit->exec);
> if (ret)
> - return ret;
> + break;
> + for (unsigned i = 0; i < submit->nr_bos; i++) {
> + struct drm_gem_object *obj = submit->bos[i].obj;
> + ret = drm_exec_prepare_obj(&submit->exec, obj, 1);
> + drm_exec_retry_on_contention(&submit->exec);
> + if (ret)
> + break;
> + }
> }
> }
>
> - return 0;
> + return ret;
> }
>
> static int submit_fence_sync(struct msm_gem_submit *submit)
> --
> 2.39.5
>
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH] drm/msm: Fix objtool warning in submit_lock_objects()
2025-08-06 23:38 ` Rob Clark
@ 2025-08-07 13:03 ` Sasha Levin
0 siblings, 0 replies; 3+ messages in thread
From: Sasha Levin @ 2025-08-07 13:03 UTC (permalink / raw)
To: Rob Clark
Cc: lumag, abhinav.kumar, jessica.zhang, sean, marijn.suijten,
airlied, simona, antomani103, linux-arm-msm, dri-devel, freedreno,
linux-kernel
On Wed, Aug 06, 2025 at 04:38:19PM -0700, Rob Clark wrote:
>On Tue, Aug 5, 2025 at 3:56 PM Sasha Levin <sashal@kernel.org> wrote:
>>
>> Restructure submit_lock_objects() to use a single loop with break
>> statements to fix objtool warning:
>>
>> drivers/gpu/drm/msm/msm.o: warning: objtool: submit_lock_objects+0x451:
>> sibling call from callable instruction with modified stack frame
>>
>> The drm_exec_until_all_locked() macro uses computed gotos internally
>> for its retry loop. Having return statements inside this macro, or
>> immediately after it in certain code paths, confuses objtool's static
>> analysis of stack frames, causing it to incorrectly flag tail call
>> optimizations.
>
>Maybe we should instead just split out a separate
>submit_lock_objects_vmbind() and restore the error path 'goto error'
>instead of returning from within the loop? Ie. basically revert
>submit_lock_objects to the way it was before commit 92395af63a99
>("drm/msm: Add VM_BIND submitqueue"), and then move the rest into a
>new fxn (with 'goto error' instead of 'return ret'? In retrospect the
>vmbind case is kinda just shoehorned into the existing fxn.
>
>I can type up this version if you have better things to do.
I'll send it out :)
--
Thanks,
Sasha
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2025-08-07 13:03 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-08-05 22:55 [PATCH] drm/msm: Fix objtool warning in submit_lock_objects() Sasha Levin
2025-08-06 23:38 ` Rob Clark
2025-08-07 13:03 ` Sasha Levin
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).