Re: [PATCH 1/2] lockprof: don't leave locks uninitialized upon allocation failure

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 23/07/2020 11:51, Jan Beulich wrote:
> Even if a specific struct lock_profile instance can't be allocated, the
> lock itself should still be functional. As this isn't a production use
> feature, also log a message in the event that the profiling struct can't
> be allocated.
>
> Fixes: d98feda5c756 ("Make lock profiling usable again")
> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
>
> --- a/xen/include/xen/spinlock.h
> +++ b/xen/include/xen/spinlock.h
> @@ -103,10 +103,16 @@ struct lock_profile_qhead {
>      do {                                                                     
>  \
>          struct lock_profile *prof;                                           
>  \
>          prof = xzalloc(struct lock_profile);                                 
>  \
> -        if (!prof) break;                                                    
>  \
> +        (s)->l = (spinlock_t)_SPIN_LOCK_UNLOCKED(prof);                      
>  \
> +        if ( !prof )                                                         
>  \
> +        {                                                                    
>  \
> +            printk(XENLOG_WARNING                                            
>  \
> +                   "lock profiling unavailable for %p(%d)'s " #l "\n",       
>  \
> +                   s, (s)->profile_head.idx);                                
>  \

You'll end up with far less .rodata if you use %s and pass #l in as a
parameter.

Either way, Acked-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>

> +            break;                                                           
>  \
> +        }                                                                    
>  \
>          prof->name = #l;                                                     
>  \
>          prof->lock = &(s)->l;                                                
>  \
> -        (s)->l = (spinlock_t)_SPIN_LOCK_UNLOCKED(prof);                      
>  \
>          prof->next = (s)->profile_head.elem_q;                               
>  \
>          (s)->profile_head.elem_q = prof;                                     
>  \
>      } while(0)
>