Re: [PATCH 2/5] drm/msm: Remove abuse of drm_exec internals
On Tue, Mar 31, 2026 at 2:21 AM Thomas Hellström
wrote:
>
> The code was reading drm_exec internal state to determine whether
> the drm_exec structure had been initialized or not, and therefore
> needed cleaning up, relying on undocumented behaviour.
>
> Instead add a bool to struct msm_gem_submit to indicate whether
> drm_exec cleaning up is needed.
>
> Signed-off-by: Thomas Hellström
Reviewed-by: Rob Clark
This is pretty stand-alone, so I can pick it up for v7.1. Or ack for
landing it via drm-misc with the rest of the series if that is easier
for you. It shouldn't conflict with anything in flight.
BR,
-R
> ---
> drivers/gpu/drm/msm/msm_gem.h| 1 +
> drivers/gpu/drm/msm/msm_gem_submit.c | 4 +++-
> 2 files changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/msm/msm_gem.h b/drivers/gpu/drm/msm/msm_gem.h
> index cb32093fda47..762e546d25ef 100644
> --- a/drivers/gpu/drm/msm/msm_gem.h
> +++ b/drivers/gpu/drm/msm/msm_gem.h
> @@ -452,6 +452,7 @@ struct msm_gem_submit {
> bool bos_pinned : 1;
> bool fault_dumped:1;/* Limit devcoredump dumping to one per submit */
> bool in_rb : 1; /* "sudo" mode, copy cmds into RB */
> + bool has_exec : 1; /* @exec is initialized. */
> struct msm_ringbuffer *ring;
> unsigned int nr_cmds;
> unsigned int nr_bos;
> diff --git a/drivers/gpu/drm/msm/msm_gem_submit.c
> b/drivers/gpu/drm/msm/msm_gem_submit.c
> index 75d9f3574370..26ea8a28be47 100644
> --- a/drivers/gpu/drm/msm/msm_gem_submit.c
> +++ b/drivers/gpu/drm/msm/msm_gem_submit.c
> @@ -278,6 +278,7 @@ static int submit_lock_objects_vmbind(struct
> msm_gem_submit *submit)
> int ret = 0;
>
> drm_exec_init(&submit->exec, flags, submit->nr_bos);
> + submit->has_exec = true;
>
> drm_exec_until_all_locked (&submit->exec) {
> ret = drm_gpuvm_prepare_vm(submit->vm, exec, 1);
> @@ -304,6 +305,7 @@ static int submit_lock_objects(struct msm_gem_submit
> *submit)
> return submit_lock_objects_vmbind(submit);
>
> drm_exec_init(&submit->exec, flags, submit->nr_bos);
> + submit->has_exec = true;
>
> drm_exec_until_all_locked (&submit->exec) {
> ret = drm_exec_lock_obj(&submit->exec,
> @@ -523,7 +525,7 @@ static void submit_cleanup(struct msm_gem_submit *submit,
> bool error)
> if (error)
> submit_unpin_objects(submit);
>
> - if (submit->exec.objects)
> + if (submit->has_exec)
> drm_exec_fini(&submit->exec);
>
> /* if job wasn't enqueued to scheduler, early retirement: */
> --
> 2.53.0
>
Re: [PATCH 2/5] drm/msm: Remove abuse of drm_exec internals
On Tue, Mar 31, 2026 at 12:52 PM Thomas Hellström
wrote:
>
> On Tue, 2026-03-31 at 12:08 -0700, Rob Clark wrote:
> > On Tue, Mar 31, 2026 at 2:21 AM Thomas Hellström
> > wrote:
> > >
> > > The code was reading drm_exec internal state to determine whether
> > > the drm_exec structure had been initialized or not, and therefore
> > > needed cleaning up, relying on undocumented behaviour.
> > >
> > > Instead add a bool to struct msm_gem_submit to indicate whether
> > > drm_exec cleaning up is needed.
> > >
> > > Signed-off-by: Thomas Hellström
> >
> > Reviewed-by: Rob Clark
> >
> > This is pretty stand-alone, so I can pick it up for v7.1. Or ack for
> > landing it via drm-misc with the rest of the series if that is easier
> > for you. It shouldn't conflict with anything in flight.
>
> Thanks Rob. Please pick it up and I'll exclude it from the next
> iteration of the series.
Will do, I have it queued up:
https://gitlab.freedesktop.org/drm/msm/-/merge_requests/227
BR,
-R
> Thanks,
> Thomas
>
> >
> > BR,
> > -R
> >
> > > ---
> > > drivers/gpu/drm/msm/msm_gem.h| 1 +
> > > drivers/gpu/drm/msm/msm_gem_submit.c | 4 +++-
> > > 2 files changed, 4 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/gpu/drm/msm/msm_gem.h
> > > b/drivers/gpu/drm/msm/msm_gem.h
> > > index cb32093fda47..762e546d25ef 100644
> > > --- a/drivers/gpu/drm/msm/msm_gem.h
> > > +++ b/drivers/gpu/drm/msm/msm_gem.h
> > > @@ -452,6 +452,7 @@ struct msm_gem_submit {
> > > bool bos_pinned : 1;
> > > bool fault_dumped:1;/* Limit devcoredump dumping to one per
> > > submit */
> > > bool in_rb : 1; /* "sudo" mode, copy cmds into RB */
> > > + bool has_exec : 1; /* @exec is initialized. */
> > > struct msm_ringbuffer *ring;
> > > unsigned int nr_cmds;
> > > unsigned int nr_bos;
> > > diff --git a/drivers/gpu/drm/msm/msm_gem_submit.c
> > > b/drivers/gpu/drm/msm/msm_gem_submit.c
> > > index 75d9f3574370..26ea8a28be47 100644
> > > --- a/drivers/gpu/drm/msm/msm_gem_submit.c
> > > +++ b/drivers/gpu/drm/msm/msm_gem_submit.c
> > > @@ -278,6 +278,7 @@ static int submit_lock_objects_vmbind(struct
> > > msm_gem_submit *submit)
> > > int ret = 0;
> > >
> > > drm_exec_init(&submit->exec, flags, submit->nr_bos);
> > > + submit->has_exec = true;
> > >
> > > drm_exec_until_all_locked (&submit->exec) {
> > > ret = drm_gpuvm_prepare_vm(submit->vm, exec, 1);
> > > @@ -304,6 +305,7 @@ static int submit_lock_objects(struct
> > > msm_gem_submit *submit)
> > > return submit_lock_objects_vmbind(submit);
> > >
> > > drm_exec_init(&submit->exec, flags, submit->nr_bos);
> > > + submit->has_exec = true;
> > >
> > > drm_exec_until_all_locked (&submit->exec) {
> > > ret = drm_exec_lock_obj(&submit->exec,
> > > @@ -523,7 +525,7 @@ static void submit_cleanup(struct
> > > msm_gem_submit *submit, bool error)
> > > if (error)
> > > submit_unpin_objects(submit);
> > >
> > > - if (submit->exec.objects)
> > > + if (submit->has_exec)
> > > drm_exec_fini(&submit->exec);
> > >
> > > /* if job wasn't enqueued to scheduler, early retirement:
> > > */
> > > --
> > > 2.53.0
> > >
Re: [PATCH 2/5] drm/msm: Remove abuse of drm_exec internals
On Tue, 2026-03-31 at 12:08 -0700, Rob Clark wrote:
> On Tue, Mar 31, 2026 at 2:21 AM Thomas Hellström
> wrote:
> >
> > The code was reading drm_exec internal state to determine whether
> > the drm_exec structure had been initialized or not, and therefore
> > needed cleaning up, relying on undocumented behaviour.
> >
> > Instead add a bool to struct msm_gem_submit to indicate whether
> > drm_exec cleaning up is needed.
> >
> > Signed-off-by: Thomas Hellström
>
> Reviewed-by: Rob Clark
>
> This is pretty stand-alone, so I can pick it up for v7.1. Or ack for
> landing it via drm-misc with the rest of the series if that is easier
> for you. It shouldn't conflict with anything in flight.
Thanks Rob. Please pick it up and I'll exclude it from the next
iteration of the series.
Thanks,
Thomas
>
> BR,
> -R
>
> > ---
> > drivers/gpu/drm/msm/msm_gem.h | 1 +
> > drivers/gpu/drm/msm/msm_gem_submit.c | 4 +++-
> > 2 files changed, 4 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/msm/msm_gem.h
> > b/drivers/gpu/drm/msm/msm_gem.h
> > index cb32093fda47..762e546d25ef 100644
> > --- a/drivers/gpu/drm/msm/msm_gem.h
> > +++ b/drivers/gpu/drm/msm/msm_gem.h
> > @@ -452,6 +452,7 @@ struct msm_gem_submit {
> > bool bos_pinned : 1;
> > bool fault_dumped:1;/* Limit devcoredump dumping to one per
> > submit */
> > bool in_rb : 1; /* "sudo" mode, copy cmds into RB */
> > + bool has_exec : 1; /* @exec is initialized. */
> > struct msm_ringbuffer *ring;
> > unsigned int nr_cmds;
> > unsigned int nr_bos;
> > diff --git a/drivers/gpu/drm/msm/msm_gem_submit.c
> > b/drivers/gpu/drm/msm/msm_gem_submit.c
> > index 75d9f3574370..26ea8a28be47 100644
> > --- a/drivers/gpu/drm/msm/msm_gem_submit.c
> > +++ b/drivers/gpu/drm/msm/msm_gem_submit.c
> > @@ -278,6 +278,7 @@ static int submit_lock_objects_vmbind(struct
> > msm_gem_submit *submit)
> > int ret = 0;
> >
> > drm_exec_init(&submit->exec, flags, submit->nr_bos);
> > + submit->has_exec = true;
> >
> > drm_exec_until_all_locked (&submit->exec) {
> > ret = drm_gpuvm_prepare_vm(submit->vm, exec, 1);
> > @@ -304,6 +305,7 @@ static int submit_lock_objects(struct
> > msm_gem_submit *submit)
> > return submit_lock_objects_vmbind(submit);
> >
> > drm_exec_init(&submit->exec, flags, submit->nr_bos);
> > + submit->has_exec = true;
> >
> > drm_exec_until_all_locked (&submit->exec) {
> > ret = drm_exec_lock_obj(&submit->exec,
> > @@ -523,7 +525,7 @@ static void submit_cleanup(struct
> > msm_gem_submit *submit, bool error)
> > if (error)
> > submit_unpin_objects(submit);
> >
> > - if (submit->exec.objects)
> > + if (submit->has_exec)
> > drm_exec_fini(&submit->exec);
> >
> > /* if job wasn't enqueued to scheduler, early retirement:
> > */
> > --
> > 2.53.0
> >
Re: [PATCH 2/5] drm/msm: Remove abuse of drm_exec internals
On 3/31/26 11:20, Thomas Hellström wrote:
> The code was reading drm_exec internal state to determine whether
> the drm_exec structure had been initialized or not, and therefore
> needed cleaning up, relying on undocumented behaviour.
>
> Instead add a bool to struct msm_gem_submit to indicate whether
> drm_exec cleaning up is needed.
>
> Signed-off-by: Thomas Hellström
Acked-by: Christian König
> ---
> drivers/gpu/drm/msm/msm_gem.h| 1 +
> drivers/gpu/drm/msm/msm_gem_submit.c | 4 +++-
> 2 files changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/msm/msm_gem.h b/drivers/gpu/drm/msm/msm_gem.h
> index cb32093fda47..762e546d25ef 100644
> --- a/drivers/gpu/drm/msm/msm_gem.h
> +++ b/drivers/gpu/drm/msm/msm_gem.h
> @@ -452,6 +452,7 @@ struct msm_gem_submit {
> bool bos_pinned : 1;
> bool fault_dumped:1;/* Limit devcoredump dumping to one per submit */
> bool in_rb : 1; /* "sudo" mode, copy cmds into RB */
> + bool has_exec : 1; /* @exec is initialized. */
> struct msm_ringbuffer *ring;
> unsigned int nr_cmds;
> unsigned int nr_bos;
> diff --git a/drivers/gpu/drm/msm/msm_gem_submit.c
> b/drivers/gpu/drm/msm/msm_gem_submit.c
> index 75d9f3574370..26ea8a28be47 100644
> --- a/drivers/gpu/drm/msm/msm_gem_submit.c
> +++ b/drivers/gpu/drm/msm/msm_gem_submit.c
> @@ -278,6 +278,7 @@ static int submit_lock_objects_vmbind(struct
> msm_gem_submit *submit)
> int ret = 0;
>
> drm_exec_init(&submit->exec, flags, submit->nr_bos);
> + submit->has_exec = true;
>
> drm_exec_until_all_locked (&submit->exec) {
> ret = drm_gpuvm_prepare_vm(submit->vm, exec, 1);
> @@ -304,6 +305,7 @@ static int submit_lock_objects(struct msm_gem_submit
> *submit)
> return submit_lock_objects_vmbind(submit);
>
> drm_exec_init(&submit->exec, flags, submit->nr_bos);
> + submit->has_exec = true;
>
> drm_exec_until_all_locked (&submit->exec) {
> ret = drm_exec_lock_obj(&submit->exec,
> @@ -523,7 +525,7 @@ static void submit_cleanup(struct msm_gem_submit *submit,
> bool error)
> if (error)
> submit_unpin_objects(submit);
>
> - if (submit->exec.objects)
> + if (submit->has_exec)
> drm_exec_fini(&submit->exec);
>
> /* if job wasn't enqueued to scheduler, early retirement: */
Re: [PATCH 2/5] drm/msm: Remove abuse of drm_exec internals
On 3/31/26 11:20, Thomas Hellström wrote:
> The code was reading drm_exec internal state to determine whether
> the drm_exec structure had been initialized or not, and therefore
> needed cleaning up, relying on undocumented behaviour.
>
> Instead add a bool to struct msm_gem_submit to indicate whether
> drm_exec cleaning up is needed.
>
> Signed-off-by: Thomas Hellström
Acked-by: Christian König
> ---
> drivers/gpu/drm/msm/msm_gem.h| 1 +
> drivers/gpu/drm/msm/msm_gem_submit.c | 4 +++-
> 2 files changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/msm/msm_gem.h b/drivers/gpu/drm/msm/msm_gem.h
> index cb32093fda47..762e546d25ef 100644
> --- a/drivers/gpu/drm/msm/msm_gem.h
> +++ b/drivers/gpu/drm/msm/msm_gem.h
> @@ -452,6 +452,7 @@ struct msm_gem_submit {
> bool bos_pinned : 1;
> bool fault_dumped:1;/* Limit devcoredump dumping to one per submit */
> bool in_rb : 1; /* "sudo" mode, copy cmds into RB */
> + bool has_exec : 1; /* @exec is initialized. */
> struct msm_ringbuffer *ring;
> unsigned int nr_cmds;
> unsigned int nr_bos;
> diff --git a/drivers/gpu/drm/msm/msm_gem_submit.c
> b/drivers/gpu/drm/msm/msm_gem_submit.c
> index 75d9f3574370..26ea8a28be47 100644
> --- a/drivers/gpu/drm/msm/msm_gem_submit.c
> +++ b/drivers/gpu/drm/msm/msm_gem_submit.c
> @@ -278,6 +278,7 @@ static int submit_lock_objects_vmbind(struct
> msm_gem_submit *submit)
> int ret = 0;
>
> drm_exec_init(&submit->exec, flags, submit->nr_bos);
> + submit->has_exec = true;
>
> drm_exec_until_all_locked (&submit->exec) {
> ret = drm_gpuvm_prepare_vm(submit->vm, exec, 1);
> @@ -304,6 +305,7 @@ static int submit_lock_objects(struct msm_gem_submit
> *submit)
> return submit_lock_objects_vmbind(submit);
>
> drm_exec_init(&submit->exec, flags, submit->nr_bos);
> + submit->has_exec = true;
>
> drm_exec_until_all_locked (&submit->exec) {
> ret = drm_exec_lock_obj(&submit->exec,
> @@ -523,7 +525,7 @@ static void submit_cleanup(struct msm_gem_submit *submit,
> bool error)
> if (error)
> submit_unpin_objects(submit);
>
> - if (submit->exec.objects)
> + if (submit->has_exec)
> drm_exec_fini(&submit->exec);
>
> /* if job wasn't enqueued to scheduler, early retirement: */
