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 <[email protected]>
Tested-by: Paul E. McKenney <[email protected]>
> ---
>
> 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
>
