* Thomas Gleixner wrote:
> From: Linus Torvalds
>
> [ tglx: Split it out from the patch which provides the new callback
> and wrapped it into local_irq_save/restore ]
>
> Signed-off-by: Linus Torvalds
> Signed-off-by: Thomas Gleixner
> ---
> arch/x86/kernel/apic/x2apic_cluster.c | 12
> 1 file changed, 12 insertions(+)
>
> Index: linux/arch/x86/kernel/apic/x2apic_cluster.c
> ===
> --- linux.orig/arch/x86/kernel/apic/x2apic_cluster.c
> +++ linux/arch/x86/kernel/apic/x2apic_cluster.c
> @@ -23,6 +23,17 @@ static inline u32 x2apic_cluster(int cpu
> return per_cpu(x86_cpu_to_logical_apicid, cpu) >> 16;
> }
>
> +static void x2apic_send_IPI(int cpu, int vector)
> +{
> + u32 dest = per_cpu(x86_cpu_to_logical_apicid, cpu);
> + unsigned long flags;
> +
> + x2apic_wrmsr_fence();
> + local_irq_save(flags);
> + __x2apic_send_IPI_dest(dest, vector, APIC_DEST_LOGICAL);
> + local_irq_restore(flags);
> +}
So the series looks good to me:
Reviewed-by: Ingo Molnar
but in the above sequence I think we can do even better: we don't need the
local_irq_save()/restore() I think.
The reason, this is how __x2apic_send_IPI_dest() looks like:
unsigned long cfg = __prepare_ICR(0, vector, dest);
native_x2apic_icr_write(cfg, apicid);
__prepare_ICR(), which is a confusing misnomer as it does not prepare anything
about the ICR register, it just pre-calculates some values, is obviously
interrupt-safe:
static inline unsigned int __prepare_ICR(unsigned int shortcut, int vector,
unsigned int dest)
{
unsigned int icr = shortcut | dest;
switch (vector) {
default:
icr |= APIC_DM_FIXED | vector;
break;
case NMI_VECTOR:
icr |= APIC_DM_NMI;
break;
}
return icr;
}
and native_x2apic_icr_write() is a single WRMSR:
static inline void native_x2apic_icr_write(u32 low, u32 id)
{
wrmsrl(APIC_BASE_MSR + (APIC_ICR >> 4), ((__u64) id) << 32 | low);
}
which is interrupt-safe as well.
So we can save another 10-20 cycles of CLI/POPF overhead from this hotpath.
I'd do it as a patch on top, to keep the series simpler - something like the
below. (Completely untested: may the Force be with you.)
Thanks,
Ingo
Signed-off-by: Ingo Molnar
---
arch/x86/kernel/apic/x2apic_cluster.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/arch/x86/kernel/apic/x2apic_cluster.c
b/arch/x86/kernel/apic/x2apic_cluster.c
index 3329dab47efc..aca8b75c1552 100644
--- a/arch/x86/kernel/apic/x2apic_cluster.c
+++ b/arch/x86/kernel/apic/x2apic_cluster.c
@@ -26,12 +26,9 @@ static inline u32 x2apic_cluster(int cpu)
static void x2apic_send_IPI(int cpu, int vector)
{
u32 dest = per_cpu(x86_cpu_to_logical_apicid, cpu);
- unsigned long flags;
x2apic_wrmsr_fence();
- local_irq_save(flags);
__x2apic_send_IPI_dest(dest, vector, APIC_DEST_LOGICAL);
- local_irq_restore(flags);
}
static void
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
