Re: [PATCH v2 02/13] xen/spinlock: reduce lock profile ifdefs

[Jan Beulich <jbeulich@xxxxxxxx>]

On 13.10.2023 11:42, Juergen Gross wrote:
> With some small adjustments to the LOCK_PROFILE_* macros some #ifdefs
> can be dropped from spinlock.c.
> 
> Signed-off-by: Juergen Gross <jgross@xxxxxxxx>
> ---
> V2:
> - new patch
> ---
>  xen/common/spinlock.c | 45 ++++++++++++++++++-------------------------
>  1 file changed, 19 insertions(+), 26 deletions(-)
> 
> diff --git a/xen/common/spinlock.c b/xen/common/spinlock.c
> index 202c707540..4878a01302 100644
> --- a/xen/common/spinlock.c
> +++ b/xen/common/spinlock.c
> @@ -267,25 +267,28 @@ void spin_debug_disable(void)
>          lock->profile->time_hold += NOW() - lock->profile->time_locked;      
> \
>          lock->profile->lock_cnt++;                                           
> \
>      }
> -#define LOCK_PROFILE_VAR    s_time_t block = 0
> -#define LOCK_PROFILE_BLOCK  block = block ? : NOW();
> -#define LOCK_PROFILE_GOT                                                     
> \
> +#define LOCK_PROFILE_VAR(val)    s_time_t block = (val)

This macro, and then at least for consistency also ...

> +#define LOCK_PROFILE_BLOCK  block = block ? : NOW()

... this one should imo take the variable name as an argument. Otherwise
situations like ...

>  void _spin_barrier(spinlock_t *lock)
>  {
>      spinlock_tickets_t sample;
> -#ifdef CONFIG_DEBUG_LOCK_PROFILE
> -    s_time_t block = NOW();
> -#endif
> +    LOCK_PROFILE_VAR(NOW());
>  
>      check_barrier(&lock->debug);
>      smp_mb();
> @@ -432,13 +431,7 @@ void _spin_barrier(spinlock_t *lock)
>      {
>          while ( observe_head(&lock->tickets) == sample.head )
>              arch_lock_relax();
> -#ifdef CONFIG_DEBUG_LOCK_PROFILE
> -        if ( lock->profile )
> -        {
> -            lock->profile->time_block += NOW() - block;
> -            lock->profile->block_cnt++;
> -        }
> -#endif
> +        LOCK_PROFILE_BLKACC(lock->profile, block);
>      }
>      smp_mb();
>  }

... this arise where there's no visible declaration of "block", but a
use. (Originally I was meaning to ask how this function would build,
when the declaration is dropped.)

Jan