Re: [PATCH 3/5] drm/exec: Make the drm_exec_until_all_locked() macro more readable

2026-04-01 Thread Thomas Hellström
On Tue, 2026-03-31 at 11:39 +0200, Christian König wrote:
> 
> 
> On 3/31/26 11:20, Thomas Hellström wrote:
> > Use __UNIQUE_ID as done elsewhere in the kernel rather than a
> > hand-rolled __PASTE to craft a unique id.
> > 
> > Also use __maybe_unused rather than (void) to signify that a
> > variable, althrough written to, may not actually be used.
> > 
> > Signed-off-by: Thomas Hellström 
> > ---
> >  include/drm/drm_exec.h | 23 ++-
> >  1 file changed, 14 insertions(+), 9 deletions(-)
> > 
> > diff --git a/include/drm/drm_exec.h b/include/drm/drm_exec.h
> > index 25db52dd2af0..fc95a979e253 100644
> > --- a/include/drm/drm_exec.h
> > +++ b/include/drm/drm_exec.h
> > @@ -89,6 +89,19 @@ drm_exec_obj(struct drm_exec *exec, unsigned
> > long index)
> >     for (unsigned long _index = (exec)->num_objects -
> > 1;  \
> >      ((obj) = drm_exec_obj(exec, _index)); --_index)
> >  
> > +/*
> > + * 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 * __maybe_unused __drm_exec_retry_ptr;
> > ({  \
> > +   __drm_exec_retry_ptr =
> > &&_label;   \
> 
> I think when using __maybe_unused we could also move assigning the
> variable to the deceleration and drop the extra ({}).

Sure. Looks even better.

Thanks,
Thomas



> 
> Apart from that looks good to me.
> 
> Regards,
> Christian.
> 
> > +   drm_exec_cleanup(exec); 
> > \
> > +   });)
> > +
> >  /**
> >   * drm_exec_until_all_locked - loop until all GEM objects are
> > locked
> >   * @exec: drm_exec object
> > @@ -96,17 +109,9 @@ drm_exec_obj(struct drm_exec *exec, unsigned
> > long index)
> >   * Core functionality of the drm_exec object. Loops until all GEM
> > objects are
> >   * locked and no more contention exists. At the beginning of the
> > loop it is
> >   * guaranteed that no GEM object is locked.
> > - *
> > - * Since labels can't be defined local to the loops body we use a
> > jump pointer
> > - * to make sure that the retry is only used from within the loops
> > body.
> >   */
> >  #define
> > drm_exec_until_all_locked(exec) \
> > -__PASTE(__drm_exec_,
> > __LINE__):  \
> > -   for (void *__drm_exec_retry_ptr;
> > ({  \
> > -   __drm_exec_retry_ptr = &&__PASTE(__drm_exec_,
> > __LINE__);\
> > -
> > (void)__drm_exec_retry_ptr; \
> > -
> > drm_exec_cleanup(exec); \
> > -   });)
> > +   __drm_exec_until_all_locked(exec, __UNIQUE_ID(drm_exec))
> >  
> >  /**
> >   * drm_exec_retry_on_contention - restart the loop to grap all
> > locks


Re: [PATCH 3/5] drm/exec: Make the drm_exec_until_all_locked() macro more readable

2026-03-31 Thread Christian König



On 3/31/26 11:20, Thomas Hellström wrote:
> Use __UNIQUE_ID as done elsewhere in the kernel rather than a
> hand-rolled __PASTE to craft a unique id.
> 
> Also use __maybe_unused rather than (void) to signify that a
> variable, althrough written to, may not actually be used.
> 
> Signed-off-by: Thomas Hellström 
> ---
>  include/drm/drm_exec.h | 23 ++-
>  1 file changed, 14 insertions(+), 9 deletions(-)
> 
> diff --git a/include/drm/drm_exec.h b/include/drm/drm_exec.h
> index 25db52dd2af0..fc95a979e253 100644
> --- a/include/drm/drm_exec.h
> +++ b/include/drm/drm_exec.h
> @@ -89,6 +89,19 @@ drm_exec_obj(struct drm_exec *exec, unsigned long index)
>   for (unsigned long _index = (exec)->num_objects - 1;
> \
>((obj) = drm_exec_obj(exec, _index)); --_index)
>  
> +/*
> + * 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 * __maybe_unused __drm_exec_retry_ptr; ({ \
> + __drm_exec_retry_ptr = &&_label;\

I think when using __maybe_unused we could also move assigning the variable to 
the deceleration and drop the extra ({}).

Apart from that looks good to me.

Regards,
Christian.

> + drm_exec_cleanup(exec); \
> + });)
> +
>  /**
>   * drm_exec_until_all_locked - loop until all GEM objects are locked
>   * @exec: drm_exec object
> @@ -96,17 +109,9 @@ drm_exec_obj(struct drm_exec *exec, unsigned long index)
>   * Core functionality of the drm_exec object. Loops until all GEM objects are
>   * locked and no more contention exists. At the beginning of the loop it is
>   * guaranteed that no GEM object is locked.
> - *
> - * Since labels can't be defined local to the loops body we use a jump 
> pointer
> - * to make sure that the retry is only used from within the loops body.
>   */
>  #define drm_exec_until_all_locked(exec)  
> \
> -__PASTE(__drm_exec_, __LINE__):  
> \
> - for (void *__drm_exec_retry_ptr; ({ \
> - __drm_exec_retry_ptr = &&__PASTE(__drm_exec_, __LINE__);\
> - (void)__drm_exec_retry_ptr; \
> - drm_exec_cleanup(exec); \
> - });)
> + __drm_exec_until_all_locked(exec, __UNIQUE_ID(drm_exec))
>  
>  /**
>   * drm_exec_retry_on_contention - restart the loop to grap all locks