Re: [Xen-devel] [PATCH v2] arm: clean-up: correct find_*_bit() functions use

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Thu, 24 May 2018, Julien Grall wrote:
> Hi Artem,
> 
> Title: It would be good to specify the subsystem you modify.
> 
> arm: vgic: ...
> 
> On 24/05/18 16:24, Artem Mygaiev wrote:
> > vgic_vcpu_pending_irq() uses find_next_bit() library function with single
> > 'unsigned long' variable, while it is designed to work with memory regions
> > and offsets. It would be more correct to use the find_first_bit() function
> > instead.
> 
> The commit message sounds like it is wrong to use find_next_bit(). However, as
> I mentioned earlier, find_first_bit() is just a wrapper of find_next_bit() on
> Arm64.
> 
> So I would reword the commit message as:
> 
> "arm: vgic: Use find_first_bit instead of find_next_bit when possible
> 
> find_next_bit(foo, sz, 0) is equivalent to find_first_bit(foo, sz). The latter
> is easier to understand. Some architecture may also have a optimized version
> of find_first_bit(...). So replace the occurrence of find_next_bit in
> vgic_vcpu_pending_irq()."

I was going to fix the commit message while committing the patch, using
Julien's wording, but we have a commit moratorium at the moment. I'll
commit when the tree reopens.


> 
> > 
> > Signed-off-by: Artem Mygaiev <artem_mygaiev@xxxxxxxx>
> > 
> > diff --git a/xen/arch/arm/gic-vgic.c b/xen/arch/arm/gic-vgic.c
> > index d831b35525..fd63906e9b 100644
> > --- a/xen/arch/arm/gic-vgic.c
> > +++ b/xen/arch/arm/gic-vgic.c
> > @@ -362,7 +362,7 @@ int vgic_vcpu_pending_irq(struct vcpu *v)
> >       ASSERT(v == current);
> > 
> >       mask_priority = gic_hw_ops->read_vmcr_priority();
> > -    active_priority = find_next_bit(&apr, 32, 0);
> > +    active_priority = find_first_bit(&apr, 32);
> > 
> >       spin_lock_irqsave(&v->arch.vgic.lock, flags);
> > 
> 
> -- 
> Julien Grall
>

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel