* [PATCH] xen/arm: minor improvement in smp_send_call_function_mask()
@ 2014-08-19 4:48 Anup Patel
2014-08-19 21:06 ` Julien Grall
0 siblings, 1 reply; 7+ messages in thread
From: Anup Patel @ 2014-08-19 4:48 UTC (permalink / raw)
To: xen-devel
Cc: Ian.Campbell, Anup Patel, stefano.stabellini, julien.grall,
patches, stefano.stabellini, Pranavkumar Sawargaonkar
Currently, smp_send_call_function_mask() function implemented
by xen arm/arm64 will use IPI to call function on current CPU.
This means that current smp_send_call_function_mask() will do
the following on current CPU:
Trigger SGI for current CPU => Xen takes interrupt on current CPU
=> IPI interrupt handler will call smp_call_function_interrupt()
This patch improves the above by straight away calling
smp_call_function_interrupt() for current CPU. This is very
similar to smp_send_call_function_mask() implemented by Xen x86.
Signed-off-by: Anup Patel <anup.patel@linaro.org>
Signed-off-by: Pranavkumar Sawargaonkar <pranavkumar@linaro.org>
---
xen/arch/arm/smp.c | 14 +++++++++++++-
1 file changed, 13 insertions(+), 1 deletion(-)
diff --git a/xen/arch/arm/smp.c b/xen/arch/arm/smp.c
index 30203b8..c80c068 100644
--- a/xen/arch/arm/smp.c
+++ b/xen/arch/arm/smp.c
@@ -19,7 +19,19 @@ void smp_send_event_check_mask(const cpumask_t *mask)
void smp_send_call_function_mask(const cpumask_t *mask)
{
- send_SGI_mask(mask, GIC_SGI_CALL_FUNCTION);
+ cpumask_t target_mask;
+
+ cpumask_andnot(&target_mask, mask, cpumask_of(smp_processor_id()));
+
+ if ( cpumask_weight(&target_mask) )
+ send_SGI_mask(&target_mask, GIC_SGI_CALL_FUNCTION);
+
+ if ( cpumask_test_cpu(smp_processor_id(), mask) )
+ {
+ local_irq_disable();
+ smp_call_function_interrupt();
+ local_irq_enable();
+ }
}
/*
--
1.7.9.5
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] xen/arm: minor improvement in smp_send_call_function_mask()
2014-08-19 4:48 [PATCH] xen/arm: minor improvement in smp_send_call_function_mask() Anup Patel
@ 2014-08-19 21:06 ` Julien Grall
2014-08-20 6:14 ` Anup Patel
0 siblings, 1 reply; 7+ messages in thread
From: Julien Grall @ 2014-08-19 21:06 UTC (permalink / raw)
To: Anup Patel, xen-devel
Cc: stefano.stabellini, Pranavkumar Sawargaonkar, patches,
Ian.Campbell, stefano.stabellini
Hi Anup,
On 18/08/14 23:48, Anup Patel wrote:
> xen/arch/arm/smp.c | 14 +++++++++++++-
> 1 file changed, 13 insertions(+), 1 deletion(-)
>
> diff --git a/xen/arch/arm/smp.c b/xen/arch/arm/smp.c
> index 30203b8..c80c068 100644
> --- a/xen/arch/arm/smp.c
> +++ b/xen/arch/arm/smp.c
> @@ -19,7 +19,19 @@ void smp_send_event_check_mask(const cpumask_t *mask)
>
> void smp_send_call_function_mask(const cpumask_t *mask)
> {
> - send_SGI_mask(mask, GIC_SGI_CALL_FUNCTION);
> + cpumask_t target_mask;
> +
> + cpumask_andnot(&target_mask, mask, cpumask_of(smp_processor_id()));
> +
> + if ( cpumask_weight(&target_mask) )
Is it necessary? What happen if Xen tries to send an SGI with an empty mask?
AFAIU, the function cpumask_weight is complex so if we can avoid it, it
would be better.
--
Julien Grall
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] xen/arm: minor improvement in smp_send_call_function_mask()
2014-08-19 21:06 ` Julien Grall
@ 2014-08-20 6:14 ` Anup Patel
2014-08-20 15:07 ` Julien Grall
0 siblings, 1 reply; 7+ messages in thread
From: Anup Patel @ 2014-08-20 6:14 UTC (permalink / raw)
To: Julien Grall
Cc: Ian Campbell, Stefano Stabellini, patches, xen-devel,
stefano.stabellini, Pranavkumar Sawargaonkar
On 20 August 2014 02:36, Julien Grall <julien.grall@linaro.org> wrote:
> Hi Anup,
>
>
> On 18/08/14 23:48, Anup Patel wrote:
>>
>> xen/arch/arm/smp.c | 14 +++++++++++++-
>> 1 file changed, 13 insertions(+), 1 deletion(-)
>>
>> diff --git a/xen/arch/arm/smp.c b/xen/arch/arm/smp.c
>> index 30203b8..c80c068 100644
>> --- a/xen/arch/arm/smp.c
>> +++ b/xen/arch/arm/smp.c
>> @@ -19,7 +19,19 @@ void smp_send_event_check_mask(const cpumask_t *mask)
>>
>> void smp_send_call_function_mask(const cpumask_t *mask)
>> {
>> - send_SGI_mask(mask, GIC_SGI_CALL_FUNCTION);
>> + cpumask_t target_mask;
>> +
>> + cpumask_andnot(&target_mask, mask, cpumask_of(smp_processor_id()));
>> +
>> + if ( cpumask_weight(&target_mask) )
>
>
> Is it necessary? What happen if Xen tries to send an SGI with an empty mask?
If Xen tries to send SGI with empty mask then target_mask will be empty
hence cpumask_weight(&target_mask) will return 0
>
> AFAIU, the function cpumask_weight is complex so if we can avoid it, it
> would be better.
Can you explain more about how cpumask_weight is complex ??
Other alternative is to use "cpumask_first(&target_mask) != NR_CPUS".
>
> --
> Julien Grall
--
Anup
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] xen/arm: minor improvement in smp_send_call_function_mask()
2014-08-20 6:14 ` Anup Patel
@ 2014-08-20 15:07 ` Julien Grall
2014-08-21 11:04 ` Anup Patel
0 siblings, 1 reply; 7+ messages in thread
From: Julien Grall @ 2014-08-20 15:07 UTC (permalink / raw)
To: Anup Patel
Cc: Ian Campbell, Stefano Stabellini, patches, xen-devel,
stefano.stabellini, Pranavkumar Sawargaonkar
On 20/08/14 01:14, Anup Patel wrote:
>> Is it necessary? What happen if Xen tries to send an SGI with an empty mask?
>
> If Xen tries to send SGI with empty mask then target_mask will be empty
> hence cpumask_weight(&target_mask) will return 0
The GIC documentation says:
"If this field is 0x00 when TargetListFilter is 0b00 , the Distributor
does not forward the interrupt to any
CPU interface."
TargetListFilter == 0b00 => Forward interrupt to a specific list of CPUs.
>>
>> AFAIU, the function cpumask_weight is complex so if we can avoid it, it
>> would be better.
>
> Can you explain more about how cpumask_weight is complex ??
cpumask_weight contains a loop and multiple addition. I doubt the case
where the mask only contains the current cpu happens often.
As the GIC won't forward the interrupt if the list of CPUs is empty, I
don't think it's worth to add this check.
> Other alternative is to use "cpumask_first(&target_mask) != NR_CPUS".
The best alternative would be cpumask_empty.
Regards,
--
Julien Grall
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] xen/arm: minor improvement in smp_send_call_function_mask()
2014-08-20 15:07 ` Julien Grall
@ 2014-08-21 11:04 ` Anup Patel
2014-08-21 16:54 ` Julien Grall
0 siblings, 1 reply; 7+ messages in thread
From: Anup Patel @ 2014-08-21 11:04 UTC (permalink / raw)
To: Julien Grall
Cc: Ian Campbell, Stefano Stabellini, patches, xen-devel,
stefano.stabellini, Pranavkumar Sawargaonkar
On 20 August 2014 20:37, Julien Grall <julien.grall@linaro.org> wrote:
>
>
> On 20/08/14 01:14, Anup Patel wrote:
>>>
>>> Is it necessary? What happen if Xen tries to send an SGI with an empty
>>> mask?
>>
>>
>> If Xen tries to send SGI with empty mask then target_mask will be empty
>> hence cpumask_weight(&target_mask) will return 0
>
>
> The GIC documentation says:
> "If this field is 0x00 when TargetListFilter is 0b00 , the Distributor does
> not forward the interrupt to any
> CPU interface."
>
> TargetListFilter == 0b00 => Forward interrupt to a specific list of CPUs.
>
>
>>>
>>> AFAIU, the function cpumask_weight is complex so if we can avoid it, it
>>> would be better.
>>
>>
>> Can you explain more about how cpumask_weight is complex ??
>
>
> cpumask_weight contains a loop and multiple addition. I doubt the case where
> the mask only contains the current cpu happens often.
>
> As the GIC won't forward the interrupt if the list of CPUs is empty, I don't
> think it's worth to add this check.
>
>
>> Other alternative is to use "cpumask_first(&target_mask) != NR_CPUS".
>
>
> The best alternative would be cpumask_empty.
All three cpumask_empty(), cpumask_first(), and cpumask_weight()
are O(N) where N is number of bits in cpumask.
It really does not make much difference which of these operation
is chosen.
Since empty target list is fine with GIC Distributor, I will drop the check.
--
Anup
>
> Regards,
>
> --
> Julien Grall
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] xen/arm: minor improvement in smp_send_call_function_mask()
2014-08-21 11:04 ` Anup Patel
@ 2014-08-21 16:54 ` Julien Grall
2014-08-21 17:22 ` Andrew Cooper
0 siblings, 1 reply; 7+ messages in thread
From: Julien Grall @ 2014-08-21 16:54 UTC (permalink / raw)
To: Anup Patel
Cc: Ian Campbell, Stefano Stabellini, patches, xen-devel,
stefano.stabellini, Pranavkumar Sawargaonkar
Hi Anup,
On 21/08/14 06:04, Anup Patel wrote:
>> The best alternative would be cpumask_empty.
>
> All three cpumask_empty(), cpumask_first(), and cpumask_weight()
> are O(N) where N is number of bits in cpumask.
> It really does not make much difference which of these operation
> is chosen.
Hmmm right. That the worst case for all, and always the case for
cpumask_weight. Anyway...
> Since empty target list is fine with GIC Distributor, I will drop the check.
Assuming you only remove the check in the next version:
Acked-by: Julien Grall <julien.grall@linaro.org>
Regards,
--
Julien Grall
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] xen/arm: minor improvement in smp_send_call_function_mask()
2014-08-21 16:54 ` Julien Grall
@ 2014-08-21 17:22 ` Andrew Cooper
0 siblings, 0 replies; 7+ messages in thread
From: Andrew Cooper @ 2014-08-21 17:22 UTC (permalink / raw)
To: Julien Grall, Anup Patel
Cc: Ian Campbell, Stefano Stabellini, patches, xen-devel,
stefano.stabellini, Pranavkumar Sawargaonkar
On 21/08/14 17:54, Julien Grall wrote:
> Hi Anup,
>
> On 21/08/14 06:04, Anup Patel wrote:
>>> The best alternative would be cpumask_empty.
>>
>> All three cpumask_empty(), cpumask_first(), and cpumask_weight()
>> are O(N) where N is number of bits in cpumask.
>> It really does not make much difference which of these operation
>> is chosen.
They are all O(N), but O() notation hides lesser factors.
cpumask_empty() is slightly cheaper than cpumask_first(), which are both
substantially cheaper than cpumask_weight().
There is no fastpath for calculating the hamming weight of 0, resulting
in a lot of dependent shift/mask operations.
~Andrew
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2014-08-21 17:22 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-08-19 4:48 [PATCH] xen/arm: minor improvement in smp_send_call_function_mask() Anup Patel
2014-08-19 21:06 ` Julien Grall
2014-08-20 6:14 ` Anup Patel
2014-08-20 15:07 ` Julien Grall
2014-08-21 11:04 ` Anup Patel
2014-08-21 16:54 ` Julien Grall
2014-08-21 17:22 ` Andrew Cooper
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.