* [PATCH v2] arm: clean-up: correct find_*_bit() functions use
@ 2018-05-24 15:24 Artem Mygaiev
2018-05-24 15:37 ` Julien Grall
0 siblings, 1 reply; 3+ messages in thread
From: Artem Mygaiev @ 2018-05-24 15:24 UTC (permalink / raw)
To: Julien Grall, Stefano Stabellini, Andre Przywara, xen-devel
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.
Signed-off-by: Artem Mygaiev <artem_mygaiev@epam.com>
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);
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH v2] arm: clean-up: correct find_*_bit() functions use
2018-05-24 15:24 [PATCH v2] arm: clean-up: correct find_*_bit() functions use Artem Mygaiev
@ 2018-05-24 15:37 ` Julien Grall
2018-05-24 18:56 ` Stefano Stabellini
0 siblings, 1 reply; 3+ messages in thread
From: Julien Grall @ 2018-05-24 15:37 UTC (permalink / raw)
To: Artem Mygaiev, Stefano Stabellini, Andre Przywara, xen-devel
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()."
Cheers,
>
> Signed-off-by: Artem Mygaiev <artem_mygaiev@epam.com>
>
> 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@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH v2] arm: clean-up: correct find_*_bit() functions use
2018-05-24 15:37 ` Julien Grall
@ 2018-05-24 18:56 ` Stefano Stabellini
0 siblings, 0 replies; 3+ messages in thread
From: Stefano Stabellini @ 2018-05-24 18:56 UTC (permalink / raw)
To: Julien Grall; +Cc: Artem Mygaiev, xen-devel, Stefano Stabellini, Andre Przywara
[-- Attachment #1: Type: TEXT/PLAIN, Size: 1792 bytes --]
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@epam.com>
> >
> > 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
>
[-- Attachment #2: Type: text/plain, Size: 157 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2018-05-24 18:56 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-05-24 15:24 [PATCH v2] arm: clean-up: correct find_*_bit() functions use Artem Mygaiev
2018-05-24 15:37 ` Julien Grall
2018-05-24 18:56 ` Stefano Stabellini
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.