* [PATCH v2] drm/msm: Fix objtool warning in submit_lock_objects()
@ 2025-08-07 13:10 Sasha Levin
2025-08-07 16:34 ` Rob Clark
2025-08-14 23:48 ` Paul E. McKenney
0 siblings, 2 replies; 3+ messages in thread
From: Sasha Levin @ 2025-08-07 13:10 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
Split the vmbind case into a separate helper function
submit_lock_objects_vmbind() 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>
---
Changes since v1:
- Extract helper submit_lock_objects_vmbind() instead of refactoring
single loop
drivers/gpu/drm/msm/msm_gem_submit.c | 49 +++++++++++++++-------------
1 file changed, 27 insertions(+), 22 deletions(-)
diff --git a/drivers/gpu/drm/msm/msm_gem_submit.c b/drivers/gpu/drm/msm/msm_gem_submit.c
index 5f8e939a5906..1ce90e351b7a 100644
--- a/drivers/gpu/drm/msm/msm_gem_submit.c
+++ b/drivers/gpu/drm/msm/msm_gem_submit.c
@@ -271,32 +271,37 @@ static int submit_lookup_cmds(struct msm_gem_submit *submit,
return ret;
}
-/* This is where we make sure all the bo's are reserved and pin'd: */
-static int submit_lock_objects(struct msm_gem_submit *submit)
+static int submit_lock_objects_vmbind(struct msm_gem_submit *submit)
{
- unsigned flags = DRM_EXEC_INTERRUPTIBLE_WAIT;
+ unsigned flags = DRM_EXEC_INTERRUPTIBLE_WAIT | DRM_EXEC_IGNORE_DUPLICATES;
struct drm_exec *exec = &submit->exec;
- int ret;
+ int ret = 0;
- 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) {
+ ret = drm_gpuvm_prepare_vm(submit->vm, exec, 1);
+ drm_exec_retry_on_contention(exec);
+ if (ret)
+ break;
- drm_exec_until_all_locked (&submit->exec) {
- ret = drm_gpuvm_prepare_vm(submit->vm, exec, 1);
- drm_exec_retry_on_contention(exec);
- if (ret)
- return ret;
+ ret = drm_gpuvm_prepare_objects(submit->vm, exec, 1);
+ drm_exec_retry_on_contention(exec);
+ if (ret)
+ break;
+ }
- ret = drm_gpuvm_prepare_objects(submit->vm, exec, 1);
- drm_exec_retry_on_contention(exec);
- if (ret)
- return ret;
- }
+ return ret;
+}
- return 0;
- }
+/* This is where we make sure all the bo's are reserved and pin'd: */
+static int submit_lock_objects(struct msm_gem_submit *submit)
+{
+ unsigned flags = DRM_EXEC_INTERRUPTIBLE_WAIT;
+ int ret = 0;
+
+ if (msm_context_is_vmbind(submit->queue->ctx))
+ return submit_lock_objects_vmbind(submit);
drm_exec_init(&submit->exec, flags, submit->nr_bos);
@@ -305,17 +310,17 @@ static int submit_lock_objects(struct msm_gem_submit *submit)
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)
- return 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 v2] drm/msm: Fix objtool warning in submit_lock_objects()
2025-08-07 13:10 [PATCH v2] drm/msm: Fix objtool warning in submit_lock_objects() Sasha Levin
@ 2025-08-07 16:34 ` Rob Clark
2025-08-14 23:48 ` Paul E. McKenney
1 sibling, 0 replies; 3+ messages in thread
From: Rob Clark @ 2025-08-07 16:34 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 Thu, Aug 7, 2025 at 6:11 AM Sasha Levin <sashal@kernel.org> wrote:
>
> Split the vmbind case into a separate helper function
> submit_lock_objects_vmbind() 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>
Thanks, I've pushed this to my staging branch for msm-fixes
BR,
-R
> ---
>
> Changes since v1:
> - Extract helper submit_lock_objects_vmbind() instead of refactoring
> single loop
>
> drivers/gpu/drm/msm/msm_gem_submit.c | 49 +++++++++++++++-------------
> 1 file changed, 27 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/msm_gem_submit.c b/drivers/gpu/drm/msm/msm_gem_submit.c
> index 5f8e939a5906..1ce90e351b7a 100644
> --- a/drivers/gpu/drm/msm/msm_gem_submit.c
> +++ b/drivers/gpu/drm/msm/msm_gem_submit.c
> @@ -271,32 +271,37 @@ static int submit_lookup_cmds(struct msm_gem_submit *submit,
> return ret;
> }
>
> -/* This is where we make sure all the bo's are reserved and pin'd: */
> -static int submit_lock_objects(struct msm_gem_submit *submit)
> +static int submit_lock_objects_vmbind(struct msm_gem_submit *submit)
> {
> - unsigned flags = DRM_EXEC_INTERRUPTIBLE_WAIT;
> + unsigned flags = DRM_EXEC_INTERRUPTIBLE_WAIT | DRM_EXEC_IGNORE_DUPLICATES;
> struct drm_exec *exec = &submit->exec;
> - int ret;
> + int ret = 0;
>
> - 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) {
> + ret = drm_gpuvm_prepare_vm(submit->vm, exec, 1);
> + drm_exec_retry_on_contention(exec);
> + if (ret)
> + break;
>
> - drm_exec_until_all_locked (&submit->exec) {
> - ret = drm_gpuvm_prepare_vm(submit->vm, exec, 1);
> - drm_exec_retry_on_contention(exec);
> - if (ret)
> - return ret;
> + ret = drm_gpuvm_prepare_objects(submit->vm, exec, 1);
> + drm_exec_retry_on_contention(exec);
> + if (ret)
> + break;
> + }
>
> - ret = drm_gpuvm_prepare_objects(submit->vm, exec, 1);
> - drm_exec_retry_on_contention(exec);
> - if (ret)
> - return ret;
> - }
> + return ret;
> +}
>
> - return 0;
> - }
> +/* This is where we make sure all the bo's are reserved and pin'd: */
> +static int submit_lock_objects(struct msm_gem_submit *submit)
> +{
> + unsigned flags = DRM_EXEC_INTERRUPTIBLE_WAIT;
> + int ret = 0;
> +
> + if (msm_context_is_vmbind(submit->queue->ctx))
> + return submit_lock_objects_vmbind(submit);
>
> drm_exec_init(&submit->exec, flags, submit->nr_bos);
>
> @@ -305,17 +310,17 @@ static int submit_lock_objects(struct msm_gem_submit *submit)
> 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)
> - return 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 v2] drm/msm: Fix objtool warning in submit_lock_objects()
2025-08-07 13:10 [PATCH v2] drm/msm: Fix objtool warning in submit_lock_objects() Sasha Levin
2025-08-07 16:34 ` Rob Clark
@ 2025-08-14 23:48 ` Paul E. McKenney
1 sibling, 0 replies; 3+ messages in thread
From: Paul E. McKenney @ 2025-08-14 23:48 UTC (permalink / raw)
To: Sasha Levin
Cc: robin.clark, lumag, abhinav.kumar, jessica.zhang, sean,
marijn.suijten, airlied, simona, antomani103, linux-arm-msm,
dri-devel, freedreno, linux-kernel
On Thu, Aug 07, 2025 at 09:10:58AM -0400, Sasha Levin wrote:
> Split the vmbind case into a separate helper function
> submit_lock_objects_vmbind() 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>
Tested-by: Paul E. McKenney <paulmck@kernel.org>
> ---
>
> Changes since v1:
> - Extract helper submit_lock_objects_vmbind() instead of refactoring
> single loop
>
> drivers/gpu/drm/msm/msm_gem_submit.c | 49 +++++++++++++++-------------
> 1 file changed, 27 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/msm_gem_submit.c b/drivers/gpu/drm/msm/msm_gem_submit.c
> index 5f8e939a5906..1ce90e351b7a 100644
> --- a/drivers/gpu/drm/msm/msm_gem_submit.c
> +++ b/drivers/gpu/drm/msm/msm_gem_submit.c
> @@ -271,32 +271,37 @@ static int submit_lookup_cmds(struct msm_gem_submit *submit,
> return ret;
> }
>
> -/* This is where we make sure all the bo's are reserved and pin'd: */
> -static int submit_lock_objects(struct msm_gem_submit *submit)
> +static int submit_lock_objects_vmbind(struct msm_gem_submit *submit)
> {
> - unsigned flags = DRM_EXEC_INTERRUPTIBLE_WAIT;
> + unsigned flags = DRM_EXEC_INTERRUPTIBLE_WAIT | DRM_EXEC_IGNORE_DUPLICATES;
> struct drm_exec *exec = &submit->exec;
> - int ret;
> + int ret = 0;
>
> - 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) {
> + ret = drm_gpuvm_prepare_vm(submit->vm, exec, 1);
> + drm_exec_retry_on_contention(exec);
> + if (ret)
> + break;
>
> - drm_exec_until_all_locked (&submit->exec) {
> - ret = drm_gpuvm_prepare_vm(submit->vm, exec, 1);
> - drm_exec_retry_on_contention(exec);
> - if (ret)
> - return ret;
> + ret = drm_gpuvm_prepare_objects(submit->vm, exec, 1);
> + drm_exec_retry_on_contention(exec);
> + if (ret)
> + break;
> + }
>
> - ret = drm_gpuvm_prepare_objects(submit->vm, exec, 1);
> - drm_exec_retry_on_contention(exec);
> - if (ret)
> - return ret;
> - }
> + return ret;
> +}
>
> - return 0;
> - }
> +/* This is where we make sure all the bo's are reserved and pin'd: */
> +static int submit_lock_objects(struct msm_gem_submit *submit)
> +{
> + unsigned flags = DRM_EXEC_INTERRUPTIBLE_WAIT;
> + int ret = 0;
> +
> + if (msm_context_is_vmbind(submit->queue->ctx))
> + return submit_lock_objects_vmbind(submit);
>
> drm_exec_init(&submit->exec, flags, submit->nr_bos);
>
> @@ -305,17 +310,17 @@ static int submit_lock_objects(struct msm_gem_submit *submit)
> 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)
> - return 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
end of thread, other threads:[~2025-08-14 23:48 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-08-07 13:10 [PATCH v2] drm/msm: Fix objtool warning in submit_lock_objects() Sasha Levin
2025-08-07 16:34 ` Rob Clark
2025-08-14 23:48 ` Paul E. McKenney
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).