Re: [PATCH 2/6] xen/bitops: Rename for_each_set_bit() to bitmap_for_each()

[Jan Beulich <jbeulich@xxxxxxxx>]

On 25.06.2024 21:07, Andrew Cooper wrote:
> The current implementation wants to take an in-memory bitmap.  However, all
> ARM callers and all-but-1 x86 callers spill a scalar to the stack in order to
> use the "generic arbitrary bitmap" helpers under the hood.
> 
> This functions, but it's far from ideal.
> 
> Rename the construct and move it into bitmap.h, because having an iterator for
> an arbitrary bitmap is a useful thing.
> 
> This will allow us to re-implement for_each_set_bit() to be more appropriate
> for scalar values.
> 
> No functional change.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>

Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>
with one cosmetic request: While doing the rename, would you mind sorting
style? Not necessarily uniformly across the entire patch, but at individual
sites presently using "impossible" formatting. IOW ...

> --- a/xen/arch/arm/gic-vgic.c
> +++ b/xen/arch/arm/gic-vgic.c
> @@ -111,7 +111,7 @@ static unsigned int gic_find_unused_lr(struct vcpu *v,
>      {
>          unsigned int used_lr;
>  
> -        for_each_set_bit(used_lr, lr_mask, nr_lrs)
> +        bitmap_for_each(used_lr, lr_mask, nr_lrs)

... while this one's fine (treating bitmap_for_each as ordinary identifier)
and while xstate.c is also fine (treating it as pseudo-keyword), ...

> --- a/xen/arch/arm/vgic.c
> +++ b/xen/arch/arm/vgic.c
> @@ -429,7 +429,7 @@ void vgic_set_irqs_pending(struct vcpu *v, uint32_t r, 
> unsigned int rank)
>      /* LPIs will never be set pending via this function */
>      ASSERT(!is_lpi(32 * rank + 31));
>  
> -    for_each_set_bit( i, &mask, 32 )
> +    bitmap_for_each( i, &mask, 32 )
>      {

... this isn't possible formatting according to our style: Either there are
no blanks immediately inside the parentheses, or there also is one ahead of
the opening one.

Thanks, Jan