* Re: [PATCH 4/8] x86: apic: Factor out default vector_allocation_domain() operations
2012-06-06 8:22 ` Ingo Molnar
@ 2012-06-06 8:42 ` Alexander Gordeev
2012-06-07 13:14 ` [PATCH 4/8] x86/apic: Factor out default vector_allocation_domain() operation Alexander Gordeev
` (4 subsequent siblings)
5 siblings, 0 replies; 26+ messages in thread
From: Alexander Gordeev @ 2012-06-06 8:42 UTC (permalink / raw)
To: Ingo Molnar; +Cc: linux-kernel, x86, Suresh Siddha, Yinghai Lu
On Wed, Jun 06, 2012 at 10:22:31AM +0200, Ingo Molnar wrote:
> This one didn't apply due to the following fresh patch in -tip:
>
> 0b8255e660a0 x86/x2apic/cluster: Use all the members of one cluster specified in the smp_affinity
>
> So I applied the first three to tip:x86/apic, mind sending the
> remaining 4 patches on top of -tip?
Sure.
5/8 should apply now, too.
And sorry for the absent In-Reply-To.
--
Regards,
Alexander Gordeev
agordeev@redhat.com
^ permalink raw reply [flat|nested] 26+ messages in thread* [PATCH 4/8] x86/apic: Factor out default vector_allocation_domain() operation
2012-06-06 8:22 ` Ingo Molnar
2012-06-06 8:42 ` Alexander Gordeev
@ 2012-06-07 13:14 ` Alexander Gordeev
2012-06-07 22:24 ` Suresh Siddha
2012-06-08 14:49 ` [tip:x86/apic] " tip-bot for Alexander Gordeev
2012-06-07 13:15 ` [PATCH 5/8] x86/apic: Try to spread IRQ vectors to different priority levels Alexander Gordeev
` (3 subsequent siblings)
5 siblings, 2 replies; 26+ messages in thread
From: Alexander Gordeev @ 2012-06-07 13:14 UTC (permalink / raw)
To: Ingo Molnar; +Cc: linux-kernel, x86, Suresh Siddha, Yinghai Lu
Signed-off-by: Alexander Gordeev <agordeev@redhat.com>
---
arch/x86/include/asm/apic.h | 21 +++++++++++++++++++++
arch/x86/kernel/apic/apic_flat_64.c | 22 +---------------------
arch/x86/kernel/apic/apic_noop.c | 3 +--
arch/x86/kernel/apic/apic_numachip.c | 8 +-------
arch/x86/kernel/apic/bigsmp_32.c | 8 +-------
arch/x86/kernel/apic/es7000_32.c | 19 ++-----------------
arch/x86/kernel/apic/numaq_32.c | 16 +---------------
arch/x86/kernel/apic/probe_32.c | 17 +----------------
arch/x86/kernel/apic/summit_32.c | 16 +---------------
arch/x86/kernel/apic/x2apic_phys.c | 11 +----------
arch/x86/kernel/apic/x2apic_uv_x.c | 8 +-------
11 files changed, 32 insertions(+), 117 deletions(-)
diff --git a/arch/x86/include/asm/apic.h b/arch/x86/include/asm/apic.h
index bef5717..feb2dbd 100644
--- a/arch/x86/include/asm/apic.h
+++ b/arch/x86/include/asm/apic.h
@@ -615,6 +615,27 @@ extern unsigned int
default_cpu_mask_to_apicid_and(const struct cpumask *cpumask,
const struct cpumask *andmask);
+static inline void
+flat_vector_allocation_domain(int cpu, struct cpumask *retmask)
+{
+ /* Careful. Some cpus do not strictly honor the set of cpus
+ * specified in the interrupt destination when using lowest
+ * priority interrupt delivery mode.
+ *
+ * In particular there was a hyperthreading cpu observed to
+ * deliver interrupts to the wrong hyperthread when only one
+ * hyperthread was specified in the interrupt desitination.
+ */
+ cpumask_clear(retmask);
+ cpumask_bits(retmask)[0] = APIC_ALL_CPUS;
+}
+
+static inline void
+default_vector_allocation_domain(int cpu, struct cpumask *retmask)
+{
+ cpumask_copy(retmask, cpumask_of(cpu));
+}
+
static inline unsigned long default_check_apicid_used(physid_mask_t *map, int apicid)
{
return physid_isset(apicid, *map);
diff --git a/arch/x86/kernel/apic/apic_flat_64.c b/arch/x86/kernel/apic/apic_flat_64.c
index 55b97ce..bddc925 100644
--- a/arch/x86/kernel/apic/apic_flat_64.c
+++ b/arch/x86/kernel/apic/apic_flat_64.c
@@ -36,20 +36,6 @@ static int flat_acpi_madt_oem_check(char *oem_id, char *oem_table_id)
return 1;
}
-static void flat_vector_allocation_domain(int cpu, struct cpumask *retmask)
-{
- /* Careful. Some cpus do not strictly honor the set of cpus
- * specified in the interrupt destination when using lowest
- * priority interrupt delivery mode.
- *
- * In particular there was a hyperthreading cpu observed to
- * deliver interrupts to the wrong hyperthread when only one
- * hyperthread was specified in the interrupt desitination.
- */
- cpumask_clear(retmask);
- cpumask_bits(retmask)[0] = APIC_ALL_CPUS;
-}
-
/*
* Set up the logical destination ID.
*
@@ -257,12 +243,6 @@ static int physflat_acpi_madt_oem_check(char *oem_id, char *oem_table_id)
return 0;
}
-static void physflat_vector_allocation_domain(int cpu, struct cpumask *retmask)
-{
- cpumask_clear(retmask);
- cpumask_set_cpu(cpu, retmask);
-}
-
static void physflat_send_IPI_mask(const struct cpumask *cpumask, int vector)
{
default_send_IPI_mask_sequence_phys(cpumask, vector);
@@ -309,7 +289,7 @@ static struct apic apic_physflat = {
.check_apicid_used = NULL,
.check_apicid_present = NULL,
- .vector_allocation_domain = physflat_vector_allocation_domain,
+ .vector_allocation_domain = default_vector_allocation_domain,
/* not needed, but shouldn't hurt: */
.init_apic_ldr = flat_init_apic_ldr,
diff --git a/arch/x86/kernel/apic/apic_noop.c b/arch/x86/kernel/apic/apic_noop.c
index 7c3dd4f..3e43cf5 100644
--- a/arch/x86/kernel/apic/apic_noop.c
+++ b/arch/x86/kernel/apic/apic_noop.c
@@ -104,8 +104,7 @@ static void noop_vector_allocation_domain(int cpu, struct cpumask *retmask)
{
if (cpu != 0)
pr_warning("APIC: Vector allocated for non-BSP cpu\n");
- cpumask_clear(retmask);
- cpumask_set_cpu(cpu, retmask);
+ cpumask_copy(retmask, cpumask_of(cpu));
}
static u32 noop_apic_read(u32 reg)
diff --git a/arch/x86/kernel/apic/apic_numachip.c b/arch/x86/kernel/apic/apic_numachip.c
index dba4bf2..c028132 100644
--- a/arch/x86/kernel/apic/apic_numachip.c
+++ b/arch/x86/kernel/apic/apic_numachip.c
@@ -72,12 +72,6 @@ static int numachip_phys_pkg_id(int initial_apic_id, int index_msb)
return initial_apic_id >> index_msb;
}
-static void numachip_vector_allocation_domain(int cpu, struct cpumask *retmask)
-{
- cpumask_clear(retmask);
- cpumask_set_cpu(cpu, retmask);
-}
-
static int __cpuinit numachip_wakeup_secondary(int phys_apicid, unsigned long start_rip)
{
union numachip_csr_g3_ext_irq_gen int_gen;
@@ -222,7 +216,7 @@ static struct apic apic_numachip __refconst = {
.check_apicid_used = NULL,
.check_apicid_present = NULL,
- .vector_allocation_domain = numachip_vector_allocation_domain,
+ .vector_allocation_domain = default_vector_allocation_domain,
.init_apic_ldr = flat_init_apic_ldr,
.ioapic_phys_id_map = NULL,
diff --git a/arch/x86/kernel/apic/bigsmp_32.c b/arch/x86/kernel/apic/bigsmp_32.c
index 907aa3d..df342fe 100644
--- a/arch/x86/kernel/apic/bigsmp_32.c
+++ b/arch/x86/kernel/apic/bigsmp_32.c
@@ -142,12 +142,6 @@ static const struct dmi_system_id bigsmp_dmi_table[] = {
{ } /* NULL entry stops DMI scanning */
};
-static void bigsmp_vector_allocation_domain(int cpu, struct cpumask *retmask)
-{
- cpumask_clear(retmask);
- cpumask_set_cpu(cpu, retmask);
-}
-
static int probe_bigsmp(void)
{
if (def_to_bigsmp)
@@ -176,7 +170,7 @@ static struct apic apic_bigsmp = {
.check_apicid_used = bigsmp_check_apicid_used,
.check_apicid_present = bigsmp_check_apicid_present,
- .vector_allocation_domain = bigsmp_vector_allocation_domain,
+ .vector_allocation_domain = default_vector_allocation_domain,
.init_apic_ldr = bigsmp_init_apic_ldr,
.ioapic_phys_id_map = bigsmp_ioapic_phys_id_map,
diff --git a/arch/x86/kernel/apic/es7000_32.c b/arch/x86/kernel/apic/es7000_32.c
index db4ab1b..3c42865 100644
--- a/arch/x86/kernel/apic/es7000_32.c
+++ b/arch/x86/kernel/apic/es7000_32.c
@@ -394,21 +394,6 @@ static void es7000_enable_apic_mode(void)
WARN(1, "Command failed, status = %x\n", mip_status);
}
-static void es7000_vector_allocation_domain(int cpu, struct cpumask *retmask)
-{
- /* Careful. Some cpus do not strictly honor the set of cpus
- * specified in the interrupt destination when using lowest
- * priority interrupt delivery mode.
- *
- * In particular there was a hyperthreading cpu observed to
- * deliver interrupts to the wrong hyperthread when only one
- * hyperthread was specified in the interrupt desitination.
- */
- cpumask_clear(retmask);
- cpumask_bits(retmask)[0] = APIC_ALL_CPUS;
-}
-
-
static void es7000_wait_for_init_deassert(atomic_t *deassert)
{
while (!atomic_read(deassert))
@@ -638,7 +623,7 @@ static struct apic __refdata apic_es7000_cluster = {
.check_apicid_used = es7000_check_apicid_used,
.check_apicid_present = es7000_check_apicid_present,
- .vector_allocation_domain = es7000_vector_allocation_domain,
+ .vector_allocation_domain = flat_vector_allocation_domain,
.init_apic_ldr = es7000_init_apic_ldr_cluster,
.ioapic_phys_id_map = es7000_ioapic_phys_id_map,
@@ -705,7 +690,7 @@ static struct apic __refdata apic_es7000 = {
.check_apicid_used = es7000_check_apicid_used,
.check_apicid_present = es7000_check_apicid_present,
- .vector_allocation_domain = es7000_vector_allocation_domain,
+ .vector_allocation_domain = flat_vector_allocation_domain,
.init_apic_ldr = es7000_init_apic_ldr,
.ioapic_phys_id_map = es7000_ioapic_phys_id_map,
diff --git a/arch/x86/kernel/apic/numaq_32.c b/arch/x86/kernel/apic/numaq_32.c
index f00a68c..eb2d466 100644
--- a/arch/x86/kernel/apic/numaq_32.c
+++ b/arch/x86/kernel/apic/numaq_32.c
@@ -441,20 +441,6 @@ static int probe_numaq(void)
return found_numaq;
}
-static void numaq_vector_allocation_domain(int cpu, struct cpumask *retmask)
-{
- /* Careful. Some cpus do not strictly honor the set of cpus
- * specified in the interrupt destination when using lowest
- * priority interrupt delivery mode.
- *
- * In particular there was a hyperthreading cpu observed to
- * deliver interrupts to the wrong hyperthread when only one
- * hyperthread was specified in the interrupt desitination.
- */
- cpumask_clear(retmask);
- cpumask_bits(retmask)[0] = APIC_ALL_CPUS;
-}
-
static void numaq_setup_portio_remap(void)
{
int num_quads = num_online_nodes();
@@ -491,7 +477,7 @@ static struct apic __refdata apic_numaq = {
.check_apicid_used = numaq_check_apicid_used,
.check_apicid_present = numaq_check_apicid_present,
- .vector_allocation_domain = numaq_vector_allocation_domain,
+ .vector_allocation_domain = flat_vector_allocation_domain,
.init_apic_ldr = numaq_init_apic_ldr,
.ioapic_phys_id_map = numaq_ioapic_phys_id_map,
diff --git a/arch/x86/kernel/apic/probe_32.c b/arch/x86/kernel/apic/probe_32.c
index 71b6ac4..2c6f003 100644
--- a/arch/x86/kernel/apic/probe_32.c
+++ b/arch/x86/kernel/apic/probe_32.c
@@ -66,21 +66,6 @@ static void setup_apic_flat_routing(void)
#endif
}
-static void default_vector_allocation_domain(int cpu, struct cpumask *retmask)
-{
- /*
- * Careful. Some cpus do not strictly honor the set of cpus
- * specified in the interrupt destination when using lowest
- * priority interrupt delivery mode.
- *
- * In particular there was a hyperthreading cpu observed to
- * deliver interrupts to the wrong hyperthread when only one
- * hyperthread was specified in the interrupt desitination.
- */
- cpumask_clear(retmask);
- cpumask_bits(retmask)[0] = APIC_ALL_CPUS;
-}
-
/* should be called last. */
static int probe_default(void)
{
@@ -105,7 +90,7 @@ static struct apic apic_default = {
.check_apicid_used = default_check_apicid_used,
.check_apicid_present = default_check_apicid_present,
- .vector_allocation_domain = default_vector_allocation_domain,
+ .vector_allocation_domain = flat_vector_allocation_domain,
.init_apic_ldr = default_init_apic_ldr,
.ioapic_phys_id_map = default_ioapic_phys_id_map,
diff --git a/arch/x86/kernel/apic/summit_32.c b/arch/x86/kernel/apic/summit_32.c
index 659897c..35d254c 100644
--- a/arch/x86/kernel/apic/summit_32.c
+++ b/arch/x86/kernel/apic/summit_32.c
@@ -320,20 +320,6 @@ static int probe_summit(void)
return 0;
}
-static void summit_vector_allocation_domain(int cpu, struct cpumask *retmask)
-{
- /* Careful. Some cpus do not strictly honor the set of cpus
- * specified in the interrupt destination when using lowest
- * priority interrupt delivery mode.
- *
- * In particular there was a hyperthreading cpu observed to
- * deliver interrupts to the wrong hyperthread when only one
- * hyperthread was specified in the interrupt desitination.
- */
- cpumask_clear(retmask);
- cpumask_bits(retmask)[0] = APIC_ALL_CPUS;
-}
-
#ifdef CONFIG_X86_SUMMIT_NUMA
static struct rio_table_hdr *rio_table_hdr;
static struct scal_detail *scal_devs[MAX_NUMNODES];
@@ -509,7 +495,7 @@ static struct apic apic_summit = {
.check_apicid_used = summit_check_apicid_used,
.check_apicid_present = summit_check_apicid_present,
- .vector_allocation_domain = summit_vector_allocation_domain,
+ .vector_allocation_domain = flat_vector_allocation_domain,
.init_apic_ldr = summit_init_apic_ldr,
.ioapic_phys_id_map = summit_ioapic_phys_id_map,
diff --git a/arch/x86/kernel/apic/x2apic_phys.c b/arch/x86/kernel/apic/x2apic_phys.c
index f730269..f109388 100644
--- a/arch/x86/kernel/apic/x2apic_phys.c
+++ b/arch/x86/kernel/apic/x2apic_phys.c
@@ -88,15 +88,6 @@ static int x2apic_phys_probe(void)
return apic == &apic_x2apic_phys;
}
-/*
- * Each logical cpu is in its own vector allocation domain.
- */
-static void x2apic_vector_allocation_domain(int cpu, struct cpumask *retmask)
-{
- cpumask_clear(retmask);
- cpumask_set_cpu(cpu, retmask);
-}
-
static struct apic apic_x2apic_phys = {
.name = "physical x2apic",
@@ -114,7 +105,7 @@ static struct apic apic_x2apic_phys = {
.check_apicid_used = NULL,
.check_apicid_present = NULL,
- .vector_allocation_domain = x2apic_vector_allocation_domain,
+ .vector_allocation_domain = default_vector_allocation_domain,
.init_apic_ldr = init_x2apic_ldr,
.ioapic_phys_id_map = NULL,
diff --git a/arch/x86/kernel/apic/x2apic_uv_x.c b/arch/x86/kernel/apic/x2apic_uv_x.c
index 16efb92..df89a7d 100644
--- a/arch/x86/kernel/apic/x2apic_uv_x.c
+++ b/arch/x86/kernel/apic/x2apic_uv_x.c
@@ -185,12 +185,6 @@ EXPORT_SYMBOL_GPL(uv_possible_blades);
unsigned long sn_rtc_cycles_per_second;
EXPORT_SYMBOL(sn_rtc_cycles_per_second);
-static void uv_vector_allocation_domain(int cpu, struct cpumask *retmask)
-{
- cpumask_clear(retmask);
- cpumask_set_cpu(cpu, retmask);
-}
-
static int __cpuinit uv_wakeup_secondary(int phys_apicid, unsigned long start_rip)
{
#ifdef CONFIG_SMP
@@ -363,7 +357,7 @@ static struct apic __refdata apic_x2apic_uv_x = {
.check_apicid_used = NULL,
.check_apicid_present = NULL,
- .vector_allocation_domain = uv_vector_allocation_domain,
+ .vector_allocation_domain = default_vector_allocation_domain,
.init_apic_ldr = uv_init_apic_ldr,
.ioapic_phys_id_map = NULL,
--
1.7.7.6
--
Regards,
Alexander Gordeev
agordeev@redhat.com
^ permalink raw reply related [flat|nested] 26+ messages in thread* Re: [PATCH 4/8] x86/apic: Factor out default vector_allocation_domain() operation
2012-06-07 13:14 ` [PATCH 4/8] x86/apic: Factor out default vector_allocation_domain() operation Alexander Gordeev
@ 2012-06-07 22:24 ` Suresh Siddha
2012-06-08 14:49 ` [tip:x86/apic] " tip-bot for Alexander Gordeev
1 sibling, 0 replies; 26+ messages in thread
From: Suresh Siddha @ 2012-06-07 22:24 UTC (permalink / raw)
To: Alexander Gordeev; +Cc: Ingo Molnar, linux-kernel, x86, Yinghai Lu
On Thu, 2012-06-07 at 15:14 +0200, Alexander Gordeev wrote:
> Signed-off-by: Alexander Gordeev <agordeev@redhat.com>
> ---
> arch/x86/include/asm/apic.h | 21 +++++++++++++++++++++
> arch/x86/kernel/apic/apic_flat_64.c | 22 +---------------------
> arch/x86/kernel/apic/apic_noop.c | 3 +--
> arch/x86/kernel/apic/apic_numachip.c | 8 +-------
> arch/x86/kernel/apic/bigsmp_32.c | 8 +-------
> arch/x86/kernel/apic/es7000_32.c | 19 ++-----------------
> arch/x86/kernel/apic/numaq_32.c | 16 +---------------
> arch/x86/kernel/apic/probe_32.c | 17 +----------------
> arch/x86/kernel/apic/summit_32.c | 16 +---------------
> arch/x86/kernel/apic/x2apic_phys.c | 11 +----------
> arch/x86/kernel/apic/x2apic_uv_x.c | 8 +-------
> 11 files changed, 32 insertions(+), 117 deletions(-)
>
Acked-by: Suresh Siddha <suresh.b.siddha@intel.com>
^ permalink raw reply [flat|nested] 26+ messages in thread
* [tip:x86/apic] x86/apic: Factor out default vector_allocation_domain() operation
2012-06-07 13:14 ` [PATCH 4/8] x86/apic: Factor out default vector_allocation_domain() operation Alexander Gordeev
2012-06-07 22:24 ` Suresh Siddha
@ 2012-06-08 14:49 ` tip-bot for Alexander Gordeev
1 sibling, 0 replies; 26+ messages in thread
From: tip-bot for Alexander Gordeev @ 2012-06-08 14:49 UTC (permalink / raw)
To: linux-tip-commits
Cc: linux-kernel, agordeev, hpa, mingo, yinghai, suresh.b.siddha,
tglx
Commit-ID: 9d8e10667624ea6411f04495aef1fa4a8a778ee8
Gitweb: http://git.kernel.org/tip/9d8e10667624ea6411f04495aef1fa4a8a778ee8
Author: Alexander Gordeev <agordeev@redhat.com>
AuthorDate: Thu, 7 Jun 2012 15:14:49 +0200
Committer: Ingo Molnar <mingo@kernel.org>
CommitDate: Fri, 8 Jun 2012 11:44:27 +0200
x86/apic: Factor out default vector_allocation_domain() operation
Signed-off-by: Alexander Gordeev <agordeev@redhat.com>
Acked-by: Suresh Siddha <suresh.b.siddha@intel.com>
Cc: Yinghai Lu <yinghai@kernel.org>
Link: http://lkml.kernel.org/r/20120607131449.GC4759@dhcp-26-207.brq.redhat.com
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
arch/x86/include/asm/apic.h | 21 +++++++++++++++++++++
arch/x86/kernel/apic/apic_flat_64.c | 22 +---------------------
arch/x86/kernel/apic/apic_noop.c | 3 +--
arch/x86/kernel/apic/apic_numachip.c | 8 +-------
arch/x86/kernel/apic/bigsmp_32.c | 8 +-------
arch/x86/kernel/apic/es7000_32.c | 19 ++-----------------
arch/x86/kernel/apic/numaq_32.c | 16 +---------------
arch/x86/kernel/apic/probe_32.c | 17 +----------------
arch/x86/kernel/apic/summit_32.c | 16 +---------------
arch/x86/kernel/apic/x2apic_phys.c | 11 +----------
arch/x86/kernel/apic/x2apic_uv_x.c | 8 +-------
11 files changed, 32 insertions(+), 117 deletions(-)
diff --git a/arch/x86/include/asm/apic.h b/arch/x86/include/asm/apic.h
index bef5717..feb2dbd 100644
--- a/arch/x86/include/asm/apic.h
+++ b/arch/x86/include/asm/apic.h
@@ -615,6 +615,27 @@ extern unsigned int
default_cpu_mask_to_apicid_and(const struct cpumask *cpumask,
const struct cpumask *andmask);
+static inline void
+flat_vector_allocation_domain(int cpu, struct cpumask *retmask)
+{
+ /* Careful. Some cpus do not strictly honor the set of cpus
+ * specified in the interrupt destination when using lowest
+ * priority interrupt delivery mode.
+ *
+ * In particular there was a hyperthreading cpu observed to
+ * deliver interrupts to the wrong hyperthread when only one
+ * hyperthread was specified in the interrupt desitination.
+ */
+ cpumask_clear(retmask);
+ cpumask_bits(retmask)[0] = APIC_ALL_CPUS;
+}
+
+static inline void
+default_vector_allocation_domain(int cpu, struct cpumask *retmask)
+{
+ cpumask_copy(retmask, cpumask_of(cpu));
+}
+
static inline unsigned long default_check_apicid_used(physid_mask_t *map, int apicid)
{
return physid_isset(apicid, *map);
diff --git a/arch/x86/kernel/apic/apic_flat_64.c b/arch/x86/kernel/apic/apic_flat_64.c
index 55b97ce..bddc925 100644
--- a/arch/x86/kernel/apic/apic_flat_64.c
+++ b/arch/x86/kernel/apic/apic_flat_64.c
@@ -36,20 +36,6 @@ static int flat_acpi_madt_oem_check(char *oem_id, char *oem_table_id)
return 1;
}
-static void flat_vector_allocation_domain(int cpu, struct cpumask *retmask)
-{
- /* Careful. Some cpus do not strictly honor the set of cpus
- * specified in the interrupt destination when using lowest
- * priority interrupt delivery mode.
- *
- * In particular there was a hyperthreading cpu observed to
- * deliver interrupts to the wrong hyperthread when only one
- * hyperthread was specified in the interrupt desitination.
- */
- cpumask_clear(retmask);
- cpumask_bits(retmask)[0] = APIC_ALL_CPUS;
-}
-
/*
* Set up the logical destination ID.
*
@@ -257,12 +243,6 @@ static int physflat_acpi_madt_oem_check(char *oem_id, char *oem_table_id)
return 0;
}
-static void physflat_vector_allocation_domain(int cpu, struct cpumask *retmask)
-{
- cpumask_clear(retmask);
- cpumask_set_cpu(cpu, retmask);
-}
-
static void physflat_send_IPI_mask(const struct cpumask *cpumask, int vector)
{
default_send_IPI_mask_sequence_phys(cpumask, vector);
@@ -309,7 +289,7 @@ static struct apic apic_physflat = {
.check_apicid_used = NULL,
.check_apicid_present = NULL,
- .vector_allocation_domain = physflat_vector_allocation_domain,
+ .vector_allocation_domain = default_vector_allocation_domain,
/* not needed, but shouldn't hurt: */
.init_apic_ldr = flat_init_apic_ldr,
diff --git a/arch/x86/kernel/apic/apic_noop.c b/arch/x86/kernel/apic/apic_noop.c
index 7c3dd4f..3e43cf5 100644
--- a/arch/x86/kernel/apic/apic_noop.c
+++ b/arch/x86/kernel/apic/apic_noop.c
@@ -104,8 +104,7 @@ static void noop_vector_allocation_domain(int cpu, struct cpumask *retmask)
{
if (cpu != 0)
pr_warning("APIC: Vector allocated for non-BSP cpu\n");
- cpumask_clear(retmask);
- cpumask_set_cpu(cpu, retmask);
+ cpumask_copy(retmask, cpumask_of(cpu));
}
static u32 noop_apic_read(u32 reg)
diff --git a/arch/x86/kernel/apic/apic_numachip.c b/arch/x86/kernel/apic/apic_numachip.c
index dba4bf2..c028132 100644
--- a/arch/x86/kernel/apic/apic_numachip.c
+++ b/arch/x86/kernel/apic/apic_numachip.c
@@ -72,12 +72,6 @@ static int numachip_phys_pkg_id(int initial_apic_id, int index_msb)
return initial_apic_id >> index_msb;
}
-static void numachip_vector_allocation_domain(int cpu, struct cpumask *retmask)
-{
- cpumask_clear(retmask);
- cpumask_set_cpu(cpu, retmask);
-}
-
static int __cpuinit numachip_wakeup_secondary(int phys_apicid, unsigned long start_rip)
{
union numachip_csr_g3_ext_irq_gen int_gen;
@@ -222,7 +216,7 @@ static struct apic apic_numachip __refconst = {
.check_apicid_used = NULL,
.check_apicid_present = NULL,
- .vector_allocation_domain = numachip_vector_allocation_domain,
+ .vector_allocation_domain = default_vector_allocation_domain,
.init_apic_ldr = flat_init_apic_ldr,
.ioapic_phys_id_map = NULL,
diff --git a/arch/x86/kernel/apic/bigsmp_32.c b/arch/x86/kernel/apic/bigsmp_32.c
index 907aa3d..df342fe 100644
--- a/arch/x86/kernel/apic/bigsmp_32.c
+++ b/arch/x86/kernel/apic/bigsmp_32.c
@@ -142,12 +142,6 @@ static const struct dmi_system_id bigsmp_dmi_table[] = {
{ } /* NULL entry stops DMI scanning */
};
-static void bigsmp_vector_allocation_domain(int cpu, struct cpumask *retmask)
-{
- cpumask_clear(retmask);
- cpumask_set_cpu(cpu, retmask);
-}
-
static int probe_bigsmp(void)
{
if (def_to_bigsmp)
@@ -176,7 +170,7 @@ static struct apic apic_bigsmp = {
.check_apicid_used = bigsmp_check_apicid_used,
.check_apicid_present = bigsmp_check_apicid_present,
- .vector_allocation_domain = bigsmp_vector_allocation_domain,
+ .vector_allocation_domain = default_vector_allocation_domain,
.init_apic_ldr = bigsmp_init_apic_ldr,
.ioapic_phys_id_map = bigsmp_ioapic_phys_id_map,
diff --git a/arch/x86/kernel/apic/es7000_32.c b/arch/x86/kernel/apic/es7000_32.c
index db4ab1b..3c42865 100644
--- a/arch/x86/kernel/apic/es7000_32.c
+++ b/arch/x86/kernel/apic/es7000_32.c
@@ -394,21 +394,6 @@ static void es7000_enable_apic_mode(void)
WARN(1, "Command failed, status = %x\n", mip_status);
}
-static void es7000_vector_allocation_domain(int cpu, struct cpumask *retmask)
-{
- /* Careful. Some cpus do not strictly honor the set of cpus
- * specified in the interrupt destination when using lowest
- * priority interrupt delivery mode.
- *
- * In particular there was a hyperthreading cpu observed to
- * deliver interrupts to the wrong hyperthread when only one
- * hyperthread was specified in the interrupt desitination.
- */
- cpumask_clear(retmask);
- cpumask_bits(retmask)[0] = APIC_ALL_CPUS;
-}
-
-
static void es7000_wait_for_init_deassert(atomic_t *deassert)
{
while (!atomic_read(deassert))
@@ -638,7 +623,7 @@ static struct apic __refdata apic_es7000_cluster = {
.check_apicid_used = es7000_check_apicid_used,
.check_apicid_present = es7000_check_apicid_present,
- .vector_allocation_domain = es7000_vector_allocation_domain,
+ .vector_allocation_domain = flat_vector_allocation_domain,
.init_apic_ldr = es7000_init_apic_ldr_cluster,
.ioapic_phys_id_map = es7000_ioapic_phys_id_map,
@@ -705,7 +690,7 @@ static struct apic __refdata apic_es7000 = {
.check_apicid_used = es7000_check_apicid_used,
.check_apicid_present = es7000_check_apicid_present,
- .vector_allocation_domain = es7000_vector_allocation_domain,
+ .vector_allocation_domain = flat_vector_allocation_domain,
.init_apic_ldr = es7000_init_apic_ldr,
.ioapic_phys_id_map = es7000_ioapic_phys_id_map,
diff --git a/arch/x86/kernel/apic/numaq_32.c b/arch/x86/kernel/apic/numaq_32.c
index f00a68c..eb2d466 100644
--- a/arch/x86/kernel/apic/numaq_32.c
+++ b/arch/x86/kernel/apic/numaq_32.c
@@ -441,20 +441,6 @@ static int probe_numaq(void)
return found_numaq;
}
-static void numaq_vector_allocation_domain(int cpu, struct cpumask *retmask)
-{
- /* Careful. Some cpus do not strictly honor the set of cpus
- * specified in the interrupt destination when using lowest
- * priority interrupt delivery mode.
- *
- * In particular there was a hyperthreading cpu observed to
- * deliver interrupts to the wrong hyperthread when only one
- * hyperthread was specified in the interrupt desitination.
- */
- cpumask_clear(retmask);
- cpumask_bits(retmask)[0] = APIC_ALL_CPUS;
-}
-
static void numaq_setup_portio_remap(void)
{
int num_quads = num_online_nodes();
@@ -491,7 +477,7 @@ static struct apic __refdata apic_numaq = {
.check_apicid_used = numaq_check_apicid_used,
.check_apicid_present = numaq_check_apicid_present,
- .vector_allocation_domain = numaq_vector_allocation_domain,
+ .vector_allocation_domain = flat_vector_allocation_domain,
.init_apic_ldr = numaq_init_apic_ldr,
.ioapic_phys_id_map = numaq_ioapic_phys_id_map,
diff --git a/arch/x86/kernel/apic/probe_32.c b/arch/x86/kernel/apic/probe_32.c
index 71b6ac4..2c6f003 100644
--- a/arch/x86/kernel/apic/probe_32.c
+++ b/arch/x86/kernel/apic/probe_32.c
@@ -66,21 +66,6 @@ static void setup_apic_flat_routing(void)
#endif
}
-static void default_vector_allocation_domain(int cpu, struct cpumask *retmask)
-{
- /*
- * Careful. Some cpus do not strictly honor the set of cpus
- * specified in the interrupt destination when using lowest
- * priority interrupt delivery mode.
- *
- * In particular there was a hyperthreading cpu observed to
- * deliver interrupts to the wrong hyperthread when only one
- * hyperthread was specified in the interrupt desitination.
- */
- cpumask_clear(retmask);
- cpumask_bits(retmask)[0] = APIC_ALL_CPUS;
-}
-
/* should be called last. */
static int probe_default(void)
{
@@ -105,7 +90,7 @@ static struct apic apic_default = {
.check_apicid_used = default_check_apicid_used,
.check_apicid_present = default_check_apicid_present,
- .vector_allocation_domain = default_vector_allocation_domain,
+ .vector_allocation_domain = flat_vector_allocation_domain,
.init_apic_ldr = default_init_apic_ldr,
.ioapic_phys_id_map = default_ioapic_phys_id_map,
diff --git a/arch/x86/kernel/apic/summit_32.c b/arch/x86/kernel/apic/summit_32.c
index 659897c..35d254c 100644
--- a/arch/x86/kernel/apic/summit_32.c
+++ b/arch/x86/kernel/apic/summit_32.c
@@ -320,20 +320,6 @@ static int probe_summit(void)
return 0;
}
-static void summit_vector_allocation_domain(int cpu, struct cpumask *retmask)
-{
- /* Careful. Some cpus do not strictly honor the set of cpus
- * specified in the interrupt destination when using lowest
- * priority interrupt delivery mode.
- *
- * In particular there was a hyperthreading cpu observed to
- * deliver interrupts to the wrong hyperthread when only one
- * hyperthread was specified in the interrupt desitination.
- */
- cpumask_clear(retmask);
- cpumask_bits(retmask)[0] = APIC_ALL_CPUS;
-}
-
#ifdef CONFIG_X86_SUMMIT_NUMA
static struct rio_table_hdr *rio_table_hdr;
static struct scal_detail *scal_devs[MAX_NUMNODES];
@@ -509,7 +495,7 @@ static struct apic apic_summit = {
.check_apicid_used = summit_check_apicid_used,
.check_apicid_present = summit_check_apicid_present,
- .vector_allocation_domain = summit_vector_allocation_domain,
+ .vector_allocation_domain = flat_vector_allocation_domain,
.init_apic_ldr = summit_init_apic_ldr,
.ioapic_phys_id_map = summit_ioapic_phys_id_map,
diff --git a/arch/x86/kernel/apic/x2apic_phys.c b/arch/x86/kernel/apic/x2apic_phys.c
index f730269..f109388 100644
--- a/arch/x86/kernel/apic/x2apic_phys.c
+++ b/arch/x86/kernel/apic/x2apic_phys.c
@@ -88,15 +88,6 @@ static int x2apic_phys_probe(void)
return apic == &apic_x2apic_phys;
}
-/*
- * Each logical cpu is in its own vector allocation domain.
- */
-static void x2apic_vector_allocation_domain(int cpu, struct cpumask *retmask)
-{
- cpumask_clear(retmask);
- cpumask_set_cpu(cpu, retmask);
-}
-
static struct apic apic_x2apic_phys = {
.name = "physical x2apic",
@@ -114,7 +105,7 @@ static struct apic apic_x2apic_phys = {
.check_apicid_used = NULL,
.check_apicid_present = NULL,
- .vector_allocation_domain = x2apic_vector_allocation_domain,
+ .vector_allocation_domain = default_vector_allocation_domain,
.init_apic_ldr = init_x2apic_ldr,
.ioapic_phys_id_map = NULL,
diff --git a/arch/x86/kernel/apic/x2apic_uv_x.c b/arch/x86/kernel/apic/x2apic_uv_x.c
index 16efb92..df89a7d 100644
--- a/arch/x86/kernel/apic/x2apic_uv_x.c
+++ b/arch/x86/kernel/apic/x2apic_uv_x.c
@@ -185,12 +185,6 @@ EXPORT_SYMBOL_GPL(uv_possible_blades);
unsigned long sn_rtc_cycles_per_second;
EXPORT_SYMBOL(sn_rtc_cycles_per_second);
-static void uv_vector_allocation_domain(int cpu, struct cpumask *retmask)
-{
- cpumask_clear(retmask);
- cpumask_set_cpu(cpu, retmask);
-}
-
static int __cpuinit uv_wakeup_secondary(int phys_apicid, unsigned long start_rip)
{
#ifdef CONFIG_SMP
@@ -363,7 +357,7 @@ static struct apic __refdata apic_x2apic_uv_x = {
.check_apicid_used = NULL,
.check_apicid_present = NULL,
- .vector_allocation_domain = uv_vector_allocation_domain,
+ .vector_allocation_domain = default_vector_allocation_domain,
.init_apic_ldr = uv_init_apic_ldr,
.ioapic_phys_id_map = NULL,
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH 5/8] x86/apic: Try to spread IRQ vectors to different priority levels
2012-06-06 8:22 ` Ingo Molnar
2012-06-06 8:42 ` Alexander Gordeev
2012-06-07 13:14 ` [PATCH 4/8] x86/apic: Factor out default vector_allocation_domain() operation Alexander Gordeev
@ 2012-06-07 13:15 ` Alexander Gordeev
2012-06-07 22:26 ` Suresh Siddha
2012-06-08 14:50 ` [tip:x86/apic] " tip-bot for Alexander Gordeev
2012-06-07 13:15 ` [PATCH 6/8] x86/apic: Avoid useless scanning thru a cpumask in assign_irq_vector() Alexander Gordeev
` (2 subsequent siblings)
5 siblings, 2 replies; 26+ messages in thread
From: Alexander Gordeev @ 2012-06-07 13:15 UTC (permalink / raw)
To: Ingo Molnar; +Cc: linux-kernel, x86, Suresh Siddha, Yinghai Lu
When assigning a new vector it is primarially done by adding 8 to the
previously given out vector number. Hence, two consequently allocated
vector numbers would likely fall into the same priority level. Try to
spread vector numbers to different priority levels better by changing
the step from 8 to 16.
Signed-off-by: Alexander Gordeev <agordeev@redhat.com>
---
arch/x86/kernel/apic/io_apic.c | 7 +++----
1 files changed, 3 insertions(+), 4 deletions(-)
diff --git a/arch/x86/kernel/apic/io_apic.c b/arch/x86/kernel/apic/io_apic.c
index 74c5697..05af3d3 100644
--- a/arch/x86/kernel/apic/io_apic.c
+++ b/arch/x86/kernel/apic/io_apic.c
@@ -1112,7 +1112,7 @@ __assign_irq_vector(int irq, struct irq_cfg *cfg, const struct cpumask *mask)
* 0x80, because int 0x80 is hm, kind of importantish. ;)
*/
static int current_vector = FIRST_EXTERNAL_VECTOR + VECTOR_OFFSET_START;
- static int current_offset = VECTOR_OFFSET_START % 8;
+ static int current_offset = VECTOR_OFFSET_START % 16;
unsigned int old_vector;
int cpu, err;
cpumask_var_t tmp_mask;
@@ -1148,10 +1148,9 @@ __assign_irq_vector(int irq, struct irq_cfg *cfg, const struct cpumask *mask)
vector = current_vector;
offset = current_offset;
next:
- vector += 8;
+ vector += 16;
if (vector >= first_system_vector) {
- /* If out of vectors on large boxen, must share them. */
- offset = (offset + 1) % 8;
+ offset = (offset + 1) % 16;
vector = FIRST_EXTERNAL_VECTOR + offset;
}
if (unlikely(current_vector == vector))
--
1.7.7.6
--
Regards,
Alexander Gordeev
agordeev@redhat.com
^ permalink raw reply related [flat|nested] 26+ messages in thread* Re: [PATCH 5/8] x86/apic: Try to spread IRQ vectors to different priority levels
2012-06-07 13:15 ` [PATCH 5/8] x86/apic: Try to spread IRQ vectors to different priority levels Alexander Gordeev
@ 2012-06-07 22:26 ` Suresh Siddha
2012-06-08 14:50 ` [tip:x86/apic] " tip-bot for Alexander Gordeev
1 sibling, 0 replies; 26+ messages in thread
From: Suresh Siddha @ 2012-06-07 22:26 UTC (permalink / raw)
To: Alexander Gordeev; +Cc: Ingo Molnar, linux-kernel, x86, Yinghai Lu
On Thu, 2012-06-07 at 15:15 +0200, Alexander Gordeev wrote:
> When assigning a new vector it is primarially done by adding 8 to the
> previously given out vector number. Hence, two consequently allocated
> vector numbers would likely fall into the same priority level. Try to
> spread vector numbers to different priority levels better by changing
> the step from 8 to 16.
>
Acked-by: Suresh Siddha <suresh.b.siddha@intel.com>
> Signed-off-by: Alexander Gordeev <agordeev@redhat.com>
> ---
> arch/x86/kernel/apic/io_apic.c | 7 +++----
> 1 files changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/arch/x86/kernel/apic/io_apic.c b/arch/x86/kernel/apic/io_apic.c
> index 74c5697..05af3d3 100644
> --- a/arch/x86/kernel/apic/io_apic.c
> +++ b/arch/x86/kernel/apic/io_apic.c
> @@ -1112,7 +1112,7 @@ __assign_irq_vector(int irq, struct irq_cfg *cfg, const struct cpumask *mask)
> * 0x80, because int 0x80 is hm, kind of importantish. ;)
> */
> static int current_vector = FIRST_EXTERNAL_VECTOR + VECTOR_OFFSET_START;
> - static int current_offset = VECTOR_OFFSET_START % 8;
> + static int current_offset = VECTOR_OFFSET_START % 16;
> unsigned int old_vector;
> int cpu, err;
> cpumask_var_t tmp_mask;
> @@ -1148,10 +1148,9 @@ __assign_irq_vector(int irq, struct irq_cfg *cfg, const struct cpumask *mask)
> vector = current_vector;
> offset = current_offset;
> next:
> - vector += 8;
> + vector += 16;
> if (vector >= first_system_vector) {
> - /* If out of vectors on large boxen, must share them. */
> - offset = (offset + 1) % 8;
> + offset = (offset + 1) % 16;
> vector = FIRST_EXTERNAL_VECTOR + offset;
> }
> if (unlikely(current_vector == vector))
> --
> 1.7.7.6
>
^ permalink raw reply [flat|nested] 26+ messages in thread* [tip:x86/apic] x86/apic: Try to spread IRQ vectors to different priority levels
2012-06-07 13:15 ` [PATCH 5/8] x86/apic: Try to spread IRQ vectors to different priority levels Alexander Gordeev
2012-06-07 22:26 ` Suresh Siddha
@ 2012-06-08 14:50 ` tip-bot for Alexander Gordeev
2012-06-20 17:23 ` H. Peter Anvin
1 sibling, 1 reply; 26+ messages in thread
From: tip-bot for Alexander Gordeev @ 2012-06-08 14:50 UTC (permalink / raw)
To: linux-tip-commits
Cc: linux-kernel, agordeev, hpa, mingo, yinghai, suresh.b.siddha,
tglx
Commit-ID: 1bccd58bfffc5a677051937b332b71f0686187c1
Gitweb: http://git.kernel.org/tip/1bccd58bfffc5a677051937b332b71f0686187c1
Author: Alexander Gordeev <agordeev@redhat.com>
AuthorDate: Thu, 7 Jun 2012 15:15:15 +0200
Committer: Ingo Molnar <mingo@kernel.org>
CommitDate: Fri, 8 Jun 2012 11:44:28 +0200
x86/apic: Try to spread IRQ vectors to different priority levels
When assigning a new vector it is primarially done by adding 8
to the previously given out vector number. Hence, two
consequently allocated vector numbers would likely fall into the
same priority level. Try to spread vector numbers to different
priority levels better by changing the step from 8 to 16.
Signed-off-by: Alexander Gordeev <agordeev@redhat.com>
Acked-by: Suresh Siddha <suresh.b.siddha@intel.com>
Cc: Yinghai Lu <yinghai@kernel.org>
Link: http://lkml.kernel.org/r/20120607131514.GD4759@dhcp-26-207.brq.redhat.com
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
arch/x86/kernel/apic/io_apic.c | 7 +++----
1 files changed, 3 insertions(+), 4 deletions(-)
diff --git a/arch/x86/kernel/apic/io_apic.c b/arch/x86/kernel/apic/io_apic.c
index 74c5697..05af3d3 100644
--- a/arch/x86/kernel/apic/io_apic.c
+++ b/arch/x86/kernel/apic/io_apic.c
@@ -1112,7 +1112,7 @@ __assign_irq_vector(int irq, struct irq_cfg *cfg, const struct cpumask *mask)
* 0x80, because int 0x80 is hm, kind of importantish. ;)
*/
static int current_vector = FIRST_EXTERNAL_VECTOR + VECTOR_OFFSET_START;
- static int current_offset = VECTOR_OFFSET_START % 8;
+ static int current_offset = VECTOR_OFFSET_START % 16;
unsigned int old_vector;
int cpu, err;
cpumask_var_t tmp_mask;
@@ -1148,10 +1148,9 @@ __assign_irq_vector(int irq, struct irq_cfg *cfg, const struct cpumask *mask)
vector = current_vector;
offset = current_offset;
next:
- vector += 8;
+ vector += 16;
if (vector >= first_system_vector) {
- /* If out of vectors on large boxen, must share them. */
- offset = (offset + 1) % 8;
+ offset = (offset + 1) % 16;
vector = FIRST_EXTERNAL_VECTOR + offset;
}
if (unlikely(current_vector == vector))
^ permalink raw reply related [flat|nested] 26+ messages in thread* Re: [tip:x86/apic] x86/apic: Try to spread IRQ vectors to different priority levels
2012-06-08 14:50 ` [tip:x86/apic] " tip-bot for Alexander Gordeev
@ 2012-06-20 17:23 ` H. Peter Anvin
2012-06-20 21:41 ` Suresh Siddha
0 siblings, 1 reply; 26+ messages in thread
From: H. Peter Anvin @ 2012-06-20 17:23 UTC (permalink / raw)
To: mingo, hpa, agordeev, linux-kernel, yinghai, suresh.b.siddha,
tglx
Cc: linux-tip-commits
On 06/08/2012 07:50 AM, tip-bot for Alexander Gordeev wrote:
> Commit-ID: 1bccd58bfffc5a677051937b332b71f0686187c1
> Gitweb: http://git.kernel.org/tip/1bccd58bfffc5a677051937b332b71f0686187c1
> Author: Alexander Gordeev <agordeev@redhat.com>
> AuthorDate: Thu, 7 Jun 2012 15:15:15 +0200
> Committer: Ingo Molnar <mingo@kernel.org>
> CommitDate: Fri, 8 Jun 2012 11:44:28 +0200
>
> x86/apic: Try to spread IRQ vectors to different priority levels
>
> When assigning a new vector it is primarially done by adding 8
> to the previously given out vector number. Hence, two
> consequently allocated vector numbers would likely fall into the
> same priority level. Try to spread vector numbers to different
> priority levels better by changing the step from 8 to 16.
>
OK, stupid question: WHY?
In general, in Linux the random prioritization is actually a negative.
The only reason for the spreading by 8 is because of bugs/misfeatures in
old APIC implementations which made them handle more than two interrupts
per priority level rather inefficiently.
-hpa
--
H. Peter Anvin, Intel Open Source Technology Center
I work for Intel. I don't speak on their behalf.
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [tip:x86/apic] x86/apic: Try to spread IRQ vectors to different priority levels
2012-06-20 17:23 ` H. Peter Anvin
@ 2012-06-20 21:41 ` Suresh Siddha
2012-06-20 21:46 ` H. Peter Anvin
0 siblings, 1 reply; 26+ messages in thread
From: Suresh Siddha @ 2012-06-20 21:41 UTC (permalink / raw)
To: H. Peter Anvin
Cc: mingo, agordeev, linux-kernel, yinghai, tglx, linux-tip-commits
On Wed, 2012-06-20 at 10:23 -0700, H. Peter Anvin wrote:
> On 06/08/2012 07:50 AM, tip-bot for Alexander Gordeev wrote:
> > Commit-ID: 1bccd58bfffc5a677051937b332b71f0686187c1
> > Gitweb: http://git.kernel.org/tip/1bccd58bfffc5a677051937b332b71f0686187c1
> > Author: Alexander Gordeev <agordeev@redhat.com>
> > AuthorDate: Thu, 7 Jun 2012 15:15:15 +0200
> > Committer: Ingo Molnar <mingo@kernel.org>
> > CommitDate: Fri, 8 Jun 2012 11:44:28 +0200
> >
> > x86/apic: Try to spread IRQ vectors to different priority levels
> >
> > When assigning a new vector it is primarially done by adding 8
> > to the previously given out vector number. Hence, two
> > consequently allocated vector numbers would likely fall into the
> > same priority level. Try to spread vector numbers to different
> > priority levels better by changing the step from 8 to 16.
> >
>
> OK, stupid question: WHY?
>
> In general, in Linux the random prioritization is actually a negative.
Thinking loud in the context of your e-mail. With the relatively recent
changes like the commit mentioned below, window of higher priority class
preempting the lower priority class is minimized to the point at which
the cpu decides which interrupt to be serviced next. And in this case,
it doesn't matter if the two vectors are in two different priority
classes or the same class. Higher the vector number higher the priority
for the cpu to service next.
commit e58aa3d2d0cc01ad8d6f7f640a0670433f794922
Author: Ingo Molnar <mingo@elte.hu>
Date: Fri Mar 26 00:06:51 2010 +0000
genirq: Run irq handlers with interrupts disabled
> The only reason for the spreading by 8 is because of bugs/misfeatures in
> old APIC implementations which made them handle more than two interrupts
> per priority level rather inefficiently.
Peter, Is it just inefficiency or a functional bug in those old apic's?
Just wondering if it is just inefficiency and given the above linux
behavior does the inefficiency matter?
Anyways, these are old platforms that we probably don't want to mess
with. Perhaps we should go back to '8' and add a comment with all this
info, that the real intention is not to spread them across different
priority class but to avoid running into some old apic bugs.
thanks,
suresh
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [tip:x86/apic] x86/apic: Try to spread IRQ vectors to different priority levels
2012-06-20 21:41 ` Suresh Siddha
@ 2012-06-20 21:46 ` H. Peter Anvin
0 siblings, 0 replies; 26+ messages in thread
From: H. Peter Anvin @ 2012-06-20 21:46 UTC (permalink / raw)
To: Suresh Siddha
Cc: mingo, agordeev, linux-kernel, yinghai, tglx, linux-tip-commits
On 06/20/2012 02:41 PM, Suresh Siddha wrote:
>>
>> OK, stupid question: WHY?
>>
>> In general, in Linux the random prioritization is actually a negative.
>
> Thinking loud in the context of your e-mail. With the relatively recent
> changes like the commit mentioned below, window of higher priority class
> preempting the lower priority class is minimized to the point at which
> the cpu decides which interrupt to be serviced next. And in this case,
> it doesn't matter if the two vectors are in two different priority
> classes or the same class. Higher the vector number higher the priority
> for the cpu to service next.
>
> commit e58aa3d2d0cc01ad8d6f7f640a0670433f794922
> Author: Ingo Molnar <mingo@elte.hu>
> Date: Fri Mar 26 00:06:51 2010 +0000
>
> genirq: Run irq handlers with interrupts disabled
>
>
>> The only reason for the spreading by 8 is because of bugs/misfeatures in
>> old APIC implementations which made them handle more than two interrupts
>> per priority level rather inefficiently.
>
> Peter, Is it just inefficiency or a functional bug in those old apic's?
> Just wondering if it is just inefficiency and given the above linux
> behavior does the inefficiency matter?
>
> Anyways, these are old platforms that we probably don't want to mess
> with. Perhaps we should go back to '8' and add a comment with all this
> info, that the real intention is not to spread them across different
> priority class but to avoid running into some old apic bugs.
>
I think it's just an inefficiency, in the sense that the interrupt will
be held at the IOAPIC until the LAPIC frees up a slot, but I could be
wrong. xAPIC implementations can queue an interrupt per vector, and so
are unaffected; arguably we might not even want to do the "spread by 8"
at all on those implementations.
Overall, I think there is no real upside or downside, but the poster
seemed to assume that there would be an automatic upside, and I don't
think there is.
-hpa
--
H. Peter Anvin, Intel Open Source Technology Center
I work for Intel. I don't speak on their behalf.
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH 6/8] x86/apic: Avoid useless scanning thru a cpumask in assign_irq_vector()
2012-06-06 8:22 ` Ingo Molnar
` (2 preceding siblings ...)
2012-06-07 13:15 ` [PATCH 5/8] x86/apic: Try to spread IRQ vectors to different priority levels Alexander Gordeev
@ 2012-06-07 13:15 ` Alexander Gordeev
2012-06-07 22:28 ` Suresh Siddha
2012-06-08 14:51 ` [tip:x86/apic] " tip-bot for Alexander Gordeev
2012-06-07 13:15 ` [PATCH 7/8] x86/apic: Make cpu_mask_to_apicid() operations return error code Alexander Gordeev
2012-06-07 13:16 ` [PATCH 8/8] x86/apic: Make cpu_mask_to_apicid() operations check cpu_online_mask Alexander Gordeev
5 siblings, 2 replies; 26+ messages in thread
From: Alexander Gordeev @ 2012-06-07 13:15 UTC (permalink / raw)
To: Ingo Molnar; +Cc: linux-kernel, x86, Suresh Siddha, Yinghai Lu
In case of static vector allocation domains (i.e. flat) if all vector
numbers are exhausted, an attempt to assign a new vector will lead to
useless scans through all CPUs in the cpumask, even though it is known
that each new pass would fail. Make this corner case less painful by
letting report whether the vector allocation domain depends on passed
arguments or not and stop scanning early.
The same could have been achived by introducing a static flag to the
apic operations. But let's allow vector_allocation_domain() have more
intelligence here and decide dynamically, in case we would need it in
the future.
Signed-off-by: Alexander Gordeev <agordeev@redhat.com>
---
arch/x86/include/asm/apic.h | 8 +++++---
arch/x86/kernel/apic/apic_noop.c | 3 ++-
arch/x86/kernel/apic/io_apic.c | 12 +++++++++---
3 files changed, 16 insertions(+), 7 deletions(-)
diff --git a/arch/x86/include/asm/apic.h b/arch/x86/include/asm/apic.h
index feb2dbd..e3fecd5 100644
--- a/arch/x86/include/asm/apic.h
+++ b/arch/x86/include/asm/apic.h
@@ -306,7 +306,7 @@ struct apic {
unsigned long (*check_apicid_used)(physid_mask_t *map, int apicid);
unsigned long (*check_apicid_present)(int apicid);
- void (*vector_allocation_domain)(int cpu, struct cpumask *retmask);
+ bool (*vector_allocation_domain)(int cpu, struct cpumask *retmask);
void (*init_apic_ldr)(void);
void (*ioapic_phys_id_map)(physid_mask_t *phys_map, physid_mask_t *retmap);
@@ -615,7 +615,7 @@ extern unsigned int
default_cpu_mask_to_apicid_and(const struct cpumask *cpumask,
const struct cpumask *andmask);
-static inline void
+static inline bool
flat_vector_allocation_domain(int cpu, struct cpumask *retmask)
{
/* Careful. Some cpus do not strictly honor the set of cpus
@@ -628,12 +628,14 @@ flat_vector_allocation_domain(int cpu, struct cpumask *retmask)
*/
cpumask_clear(retmask);
cpumask_bits(retmask)[0] = APIC_ALL_CPUS;
+ return false;
}
-static inline void
+static inline bool
default_vector_allocation_domain(int cpu, struct cpumask *retmask)
{
cpumask_copy(retmask, cpumask_of(cpu));
+ return true;
}
static inline unsigned long default_check_apicid_used(physid_mask_t *map, int apicid)
diff --git a/arch/x86/kernel/apic/apic_noop.c b/arch/x86/kernel/apic/apic_noop.c
index 3e43cf5..ac9edf2 100644
--- a/arch/x86/kernel/apic/apic_noop.c
+++ b/arch/x86/kernel/apic/apic_noop.c
@@ -100,11 +100,12 @@ static unsigned long noop_check_apicid_present(int bit)
return physid_isset(bit, phys_cpu_present_map);
}
-static void noop_vector_allocation_domain(int cpu, struct cpumask *retmask)
+static bool noop_vector_allocation_domain(int cpu, struct cpumask *retmask)
{
if (cpu != 0)
pr_warning("APIC: Vector allocated for non-BSP cpu\n");
cpumask_copy(retmask, cpumask_of(cpu));
+ return true;
}
static u32 noop_apic_read(u32 reg)
diff --git a/arch/x86/kernel/apic/io_apic.c b/arch/x86/kernel/apic/io_apic.c
index 05af3d3..4061a7d 100644
--- a/arch/x86/kernel/apic/io_apic.c
+++ b/arch/x86/kernel/apic/io_apic.c
@@ -1137,8 +1137,9 @@ __assign_irq_vector(int irq, struct irq_cfg *cfg, const struct cpumask *mask)
for_each_cpu_and(cpu, mask, cpu_online_mask) {
int new_cpu;
int vector, offset;
+ bool more_domains;
- apic->vector_allocation_domain(cpu, tmp_mask);
+ more_domains = apic->vector_allocation_domain(cpu, tmp_mask);
if (cpumask_subset(tmp_mask, cfg->domain)) {
free_cpumask_var(tmp_mask);
@@ -1153,8 +1154,13 @@ next:
offset = (offset + 1) % 16;
vector = FIRST_EXTERNAL_VECTOR + offset;
}
- if (unlikely(current_vector == vector))
- continue;
+
+ if (unlikely(current_vector == vector)) {
+ if (more_domains)
+ continue;
+ else
+ break;
+ }
if (test_bit(vector, used_vectors))
goto next;
--
1.7.7.6
--
Regards,
Alexander Gordeev
agordeev@redhat.com
^ permalink raw reply related [flat|nested] 26+ messages in thread* Re: [PATCH 6/8] x86/apic: Avoid useless scanning thru a cpumask in assign_irq_vector()
2012-06-07 13:15 ` [PATCH 6/8] x86/apic: Avoid useless scanning thru a cpumask in assign_irq_vector() Alexander Gordeev
@ 2012-06-07 22:28 ` Suresh Siddha
2012-06-08 14:51 ` [tip:x86/apic] " tip-bot for Alexander Gordeev
1 sibling, 0 replies; 26+ messages in thread
From: Suresh Siddha @ 2012-06-07 22:28 UTC (permalink / raw)
To: Alexander Gordeev; +Cc: Ingo Molnar, linux-kernel, x86, Yinghai Lu
On Thu, 2012-06-07 at 15:15 +0200, Alexander Gordeev wrote:
> In case of static vector allocation domains (i.e. flat) if all vector
> numbers are exhausted, an attempt to assign a new vector will lead to
> useless scans through all CPUs in the cpumask, even though it is known
> that each new pass would fail. Make this corner case less painful by
> letting report whether the vector allocation domain depends on passed
> arguments or not and stop scanning early.
>
> The same could have been achived by introducing a static flag to the
> apic operations. But let's allow vector_allocation_domain() have more
> intelligence here and decide dynamically, in case we would need it in
> the future.
Acked-by: Suresh Siddha <suresh.b.siddha@intel.com>
>
> Signed-off-by: Alexander Gordeev <agordeev@redhat.com>
> ---
> arch/x86/include/asm/apic.h | 8 +++++---
> arch/x86/kernel/apic/apic_noop.c | 3 ++-
> arch/x86/kernel/apic/io_apic.c | 12 +++++++++---
> 3 files changed, 16 insertions(+), 7 deletions(-)
>
> diff --git a/arch/x86/include/asm/apic.h b/arch/x86/include/asm/apic.h
> index feb2dbd..e3fecd5 100644
> --- a/arch/x86/include/asm/apic.h
> +++ b/arch/x86/include/asm/apic.h
> @@ -306,7 +306,7 @@ struct apic {
> unsigned long (*check_apicid_used)(physid_mask_t *map, int apicid);
> unsigned long (*check_apicid_present)(int apicid);
>
> - void (*vector_allocation_domain)(int cpu, struct cpumask *retmask);
> + bool (*vector_allocation_domain)(int cpu, struct cpumask *retmask);
> void (*init_apic_ldr)(void);
>
> void (*ioapic_phys_id_map)(physid_mask_t *phys_map, physid_mask_t *retmap);
> @@ -615,7 +615,7 @@ extern unsigned int
> default_cpu_mask_to_apicid_and(const struct cpumask *cpumask,
> const struct cpumask *andmask);
>
> -static inline void
> +static inline bool
> flat_vector_allocation_domain(int cpu, struct cpumask *retmask)
> {
> /* Careful. Some cpus do not strictly honor the set of cpus
> @@ -628,12 +628,14 @@ flat_vector_allocation_domain(int cpu, struct cpumask *retmask)
> */
> cpumask_clear(retmask);
> cpumask_bits(retmask)[0] = APIC_ALL_CPUS;
> + return false;
> }
>
> -static inline void
> +static inline bool
> default_vector_allocation_domain(int cpu, struct cpumask *retmask)
> {
> cpumask_copy(retmask, cpumask_of(cpu));
> + return true;
> }
>
> static inline unsigned long default_check_apicid_used(physid_mask_t *map, int apicid)
> diff --git a/arch/x86/kernel/apic/apic_noop.c b/arch/x86/kernel/apic/apic_noop.c
> index 3e43cf5..ac9edf2 100644
> --- a/arch/x86/kernel/apic/apic_noop.c
> +++ b/arch/x86/kernel/apic/apic_noop.c
> @@ -100,11 +100,12 @@ static unsigned long noop_check_apicid_present(int bit)
> return physid_isset(bit, phys_cpu_present_map);
> }
>
> -static void noop_vector_allocation_domain(int cpu, struct cpumask *retmask)
> +static bool noop_vector_allocation_domain(int cpu, struct cpumask *retmask)
> {
> if (cpu != 0)
> pr_warning("APIC: Vector allocated for non-BSP cpu\n");
> cpumask_copy(retmask, cpumask_of(cpu));
> + return true;
> }
>
> static u32 noop_apic_read(u32 reg)
> diff --git a/arch/x86/kernel/apic/io_apic.c b/arch/x86/kernel/apic/io_apic.c
> index 05af3d3..4061a7d 100644
> --- a/arch/x86/kernel/apic/io_apic.c
> +++ b/arch/x86/kernel/apic/io_apic.c
> @@ -1137,8 +1137,9 @@ __assign_irq_vector(int irq, struct irq_cfg *cfg, const struct cpumask *mask)
> for_each_cpu_and(cpu, mask, cpu_online_mask) {
> int new_cpu;
> int vector, offset;
> + bool more_domains;
>
> - apic->vector_allocation_domain(cpu, tmp_mask);
> + more_domains = apic->vector_allocation_domain(cpu, tmp_mask);
>
> if (cpumask_subset(tmp_mask, cfg->domain)) {
> free_cpumask_var(tmp_mask);
> @@ -1153,8 +1154,13 @@ next:
> offset = (offset + 1) % 16;
> vector = FIRST_EXTERNAL_VECTOR + offset;
> }
> - if (unlikely(current_vector == vector))
> - continue;
> +
> + if (unlikely(current_vector == vector)) {
> + if (more_domains)
> + continue;
> + else
> + break;
> + }
>
> if (test_bit(vector, used_vectors))
> goto next;
> --
> 1.7.7.6
>
^ permalink raw reply [flat|nested] 26+ messages in thread* [tip:x86/apic] x86/apic: Avoid useless scanning thru a cpumask in assign_irq_vector()
2012-06-07 13:15 ` [PATCH 6/8] x86/apic: Avoid useless scanning thru a cpumask in assign_irq_vector() Alexander Gordeev
2012-06-07 22:28 ` Suresh Siddha
@ 2012-06-08 14:51 ` tip-bot for Alexander Gordeev
1 sibling, 0 replies; 26+ messages in thread
From: tip-bot for Alexander Gordeev @ 2012-06-08 14:51 UTC (permalink / raw)
To: linux-tip-commits
Cc: linux-kernel, agordeev, hpa, mingo, yinghai, suresh.b.siddha,
tglx
Commit-ID: 8637e38aff14d048b649075114023023a2e80fba
Gitweb: http://git.kernel.org/tip/8637e38aff14d048b649075114023023a2e80fba
Author: Alexander Gordeev <agordeev@redhat.com>
AuthorDate: Thu, 7 Jun 2012 15:15:44 +0200
Committer: Ingo Molnar <mingo@kernel.org>
CommitDate: Fri, 8 Jun 2012 11:44:29 +0200
x86/apic: Avoid useless scanning thru a cpumask in assign_irq_vector()
In case of static vector allocation domains (i.e. flat) if all
vector numbers are exhausted, an attempt to assign a new vector
will lead to useless scans through all CPUs in the cpumask, even
though it is known that each new pass would fail. Make this
corner case less painful by letting report whether the vector
allocation domain depends on passed arguments or not and stop
scanning early.
The same could have been achived by introducing a static flag to
the apic operations. But let's allow vector_allocation_domain()
have more intelligence here and decide dynamically, in case we
would need it in the future.
Signed-off-by: Alexander Gordeev <agordeev@redhat.com>
Acked-by: Suresh Siddha <suresh.b.siddha@intel.com>
Cc: Yinghai Lu <yinghai@kernel.org>
Link: http://lkml.kernel.org/r/20120607131542.GE4759@dhcp-26-207.brq.redhat.com
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
arch/x86/include/asm/apic.h | 8 +++++---
arch/x86/kernel/apic/apic_noop.c | 3 ++-
arch/x86/kernel/apic/io_apic.c | 12 +++++++++---
3 files changed, 16 insertions(+), 7 deletions(-)
diff --git a/arch/x86/include/asm/apic.h b/arch/x86/include/asm/apic.h
index feb2dbd..e3fecd5 100644
--- a/arch/x86/include/asm/apic.h
+++ b/arch/x86/include/asm/apic.h
@@ -306,7 +306,7 @@ struct apic {
unsigned long (*check_apicid_used)(physid_mask_t *map, int apicid);
unsigned long (*check_apicid_present)(int apicid);
- void (*vector_allocation_domain)(int cpu, struct cpumask *retmask);
+ bool (*vector_allocation_domain)(int cpu, struct cpumask *retmask);
void (*init_apic_ldr)(void);
void (*ioapic_phys_id_map)(physid_mask_t *phys_map, physid_mask_t *retmap);
@@ -615,7 +615,7 @@ extern unsigned int
default_cpu_mask_to_apicid_and(const struct cpumask *cpumask,
const struct cpumask *andmask);
-static inline void
+static inline bool
flat_vector_allocation_domain(int cpu, struct cpumask *retmask)
{
/* Careful. Some cpus do not strictly honor the set of cpus
@@ -628,12 +628,14 @@ flat_vector_allocation_domain(int cpu, struct cpumask *retmask)
*/
cpumask_clear(retmask);
cpumask_bits(retmask)[0] = APIC_ALL_CPUS;
+ return false;
}
-static inline void
+static inline bool
default_vector_allocation_domain(int cpu, struct cpumask *retmask)
{
cpumask_copy(retmask, cpumask_of(cpu));
+ return true;
}
static inline unsigned long default_check_apicid_used(physid_mask_t *map, int apicid)
diff --git a/arch/x86/kernel/apic/apic_noop.c b/arch/x86/kernel/apic/apic_noop.c
index 3e43cf5..ac9edf2 100644
--- a/arch/x86/kernel/apic/apic_noop.c
+++ b/arch/x86/kernel/apic/apic_noop.c
@@ -100,11 +100,12 @@ static unsigned long noop_check_apicid_present(int bit)
return physid_isset(bit, phys_cpu_present_map);
}
-static void noop_vector_allocation_domain(int cpu, struct cpumask *retmask)
+static bool noop_vector_allocation_domain(int cpu, struct cpumask *retmask)
{
if (cpu != 0)
pr_warning("APIC: Vector allocated for non-BSP cpu\n");
cpumask_copy(retmask, cpumask_of(cpu));
+ return true;
}
static u32 noop_apic_read(u32 reg)
diff --git a/arch/x86/kernel/apic/io_apic.c b/arch/x86/kernel/apic/io_apic.c
index 05af3d3..4061a7d 100644
--- a/arch/x86/kernel/apic/io_apic.c
+++ b/arch/x86/kernel/apic/io_apic.c
@@ -1137,8 +1137,9 @@ __assign_irq_vector(int irq, struct irq_cfg *cfg, const struct cpumask *mask)
for_each_cpu_and(cpu, mask, cpu_online_mask) {
int new_cpu;
int vector, offset;
+ bool more_domains;
- apic->vector_allocation_domain(cpu, tmp_mask);
+ more_domains = apic->vector_allocation_domain(cpu, tmp_mask);
if (cpumask_subset(tmp_mask, cfg->domain)) {
free_cpumask_var(tmp_mask);
@@ -1153,8 +1154,13 @@ next:
offset = (offset + 1) % 16;
vector = FIRST_EXTERNAL_VECTOR + offset;
}
- if (unlikely(current_vector == vector))
- continue;
+
+ if (unlikely(current_vector == vector)) {
+ if (more_domains)
+ continue;
+ else
+ break;
+ }
if (test_bit(vector, used_vectors))
goto next;
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH 7/8] x86/apic: Make cpu_mask_to_apicid() operations return error code
2012-06-06 8:22 ` Ingo Molnar
` (3 preceding siblings ...)
2012-06-07 13:15 ` [PATCH 6/8] x86/apic: Avoid useless scanning thru a cpumask in assign_irq_vector() Alexander Gordeev
@ 2012-06-07 13:15 ` Alexander Gordeev
2012-06-07 22:24 ` Suresh Siddha
2012-06-08 14:51 ` [tip:x86/apic] x86/apic: Make cpu_mask_to_apicid() operations return error code tip-bot for Alexander Gordeev
2012-06-07 13:16 ` [PATCH 8/8] x86/apic: Make cpu_mask_to_apicid() operations check cpu_online_mask Alexander Gordeev
5 siblings, 2 replies; 26+ messages in thread
From: Alexander Gordeev @ 2012-06-07 13:15 UTC (permalink / raw)
To: Ingo Molnar; +Cc: linux-kernel, x86, Suresh Siddha, Yinghai Lu
Current cpu_mask_to_apicid() and cpu_mask_to_apicid_and()
implementations have few shortcomings:
1. A value returned by cpu_mask_to_apicid() is written to hardware
registers unconditionally. Should BAD_APICID get ever returned it will
be written to a hardware too. But the value of BAD_APICID is not
universal across all hardware in all modes and might cause unexpected
results, i.e. interrupts might get routed to CPUs that are not
configured to receive it.
2. Because the value of BAD_APICID is not universal it is counter-
intuitive to return it for a hardware where it does not make sense
(i.e. x2apic).
3. cpu_mask_to_apicid_and() operation is thought as an complement to
cpu_mask_to_apicid() that only applies a AND mask on top of a cpumask
being passed. Yet, as consequence of 18374d8 commit the two operations
are inconsistent in that of:
cpu_mask_to_apicid() should not get a offline CPU with the cpumask
cpu_mask_to_apicid_and() should not fail and return BAD_APICID
These limitations are impossible to realize just from looking at the
operations prototypes.
Most of these shortcomings are resolved by returning a error code
instead of BAD_APICID. As the result, faults are reported back early
rather than possibilities to cause a unexpected behaviour exist (in case
of [1]).
The only exception is setup_timer_IRQ0_pin() routine. Although obviously
controversial to this fix, its existing behaviour is preserved to not
break the fragile check_timer() and would better addressed in a separate
fix.
Signed-off-by: Alexander Gordeev <agordeev@redhat.com>
---
arch/x86/include/asm/apic.h | 44 ++++++++++++-----
arch/x86/kernel/apic/apic.c | 33 +++++++------
arch/x86/kernel/apic/es7000_32.c | 21 +++++---
arch/x86/kernel/apic/io_apic.c | 88 +++++++++++++++++++++------------
arch/x86/kernel/apic/numaq_32.c | 14 +++--
arch/x86/kernel/apic/summit_32.c | 22 +++++---
arch/x86/kernel/apic/x2apic_cluster.c | 24 +++++----
arch/x86/kernel/apic/x2apic_uv_x.c | 27 +++++++---
arch/x86/platform/uv/uv_irq.c | 7 ++-
drivers/iommu/intel_irq_remapping.c | 13 ++++-
10 files changed, 188 insertions(+), 105 deletions(-)
diff --git a/arch/x86/include/asm/apic.h b/arch/x86/include/asm/apic.h
index e3fecd5..ae91f9c 100644
--- a/arch/x86/include/asm/apic.h
+++ b/arch/x86/include/asm/apic.h
@@ -331,9 +331,11 @@ struct apic {
unsigned long (*set_apic_id)(unsigned int id);
unsigned long apic_id_mask;
- unsigned int (*cpu_mask_to_apicid)(const struct cpumask *cpumask);
- unsigned int (*cpu_mask_to_apicid_and)(const struct cpumask *cpumask,
- const struct cpumask *andmask);
+ int (*cpu_mask_to_apicid)(const struct cpumask *cpumask,
+ unsigned int *apicid);
+ int (*cpu_mask_to_apicid_and)(const struct cpumask *cpumask,
+ const struct cpumask *andmask,
+ unsigned int *apicid);
/* ipi */
void (*send_IPI_mask)(const struct cpumask *mask, int vector);
@@ -591,29 +593,45 @@ static inline int default_phys_pkg_id(int cpuid_apic, int index_msb)
#endif
-static inline unsigned int
-flat_cpu_mask_to_apicid(const struct cpumask *cpumask)
+static inline int
+__flat_cpu_mask_to_apicid(unsigned long cpu_mask, unsigned int *apicid)
{
- return cpumask_bits(cpumask)[0] & APIC_ALL_CPUS;
+ cpu_mask &= APIC_ALL_CPUS;
+ if (likely(cpu_mask)) {
+ *apicid = (unsigned int)cpu_mask;
+ return 0;
+ } else {
+ return -EINVAL;
+ }
}
-static inline unsigned int
+static inline int
+flat_cpu_mask_to_apicid(const struct cpumask *cpumask,
+ unsigned int *apicid)
+{
+ return __flat_cpu_mask_to_apicid(cpumask_bits(cpumask)[0], apicid);
+}
+
+static inline int
flat_cpu_mask_to_apicid_and(const struct cpumask *cpumask,
- const struct cpumask *andmask)
+ const struct cpumask *andmask,
+ unsigned int *apicid)
{
unsigned long mask1 = cpumask_bits(cpumask)[0];
unsigned long mask2 = cpumask_bits(andmask)[0];
unsigned long mask3 = cpumask_bits(cpu_online_mask)[0];
- return (unsigned int)(mask1 & mask2 & mask3);
+ return __flat_cpu_mask_to_apicid(mask1 & mask2 & mask3, apicid);
}
-extern unsigned int
-default_cpu_mask_to_apicid(const struct cpumask *cpumask);
+extern int
+default_cpu_mask_to_apicid(const struct cpumask *cpumask,
+ unsigned int *apicid);
-extern unsigned int
+extern int
default_cpu_mask_to_apicid_and(const struct cpumask *cpumask,
- const struct cpumask *andmask);
+ const struct cpumask *andmask,
+ unsigned int *apicid);
static inline bool
flat_vector_allocation_domain(int cpu, struct cpumask *retmask)
diff --git a/arch/x86/kernel/apic/apic.c b/arch/x86/kernel/apic/apic.c
index 96a2608..b8d9260 100644
--- a/arch/x86/kernel/apic/apic.c
+++ b/arch/x86/kernel/apic/apic.c
@@ -2123,24 +2123,26 @@ void default_init_apic_ldr(void)
apic_write(APIC_LDR, val);
}
-unsigned int default_cpu_mask_to_apicid(const struct cpumask *cpumask)
+static inline int __default_cpu_to_apicid(int cpu, unsigned int *apicid)
{
- int cpu;
-
- /*
- * We're using fixed IRQ delivery, can only return one phys APIC ID.
- * May as well be the first.
- */
- cpu = cpumask_first(cpumask);
- if (likely((unsigned)cpu < nr_cpu_ids))
- return per_cpu(x86_cpu_to_apicid, cpu);
+ if (likely((unsigned int)cpu < nr_cpu_ids)) {
+ *apicid = per_cpu(x86_cpu_to_apicid, cpu);
+ return 0;
+ } else {
+ return -EINVAL;
+ }
+}
- return BAD_APICID;
+int default_cpu_mask_to_apicid(const struct cpumask *cpumask,
+ unsigned int *apicid)
+{
+ int cpu = cpumask_first(cpumask);
+ return __default_cpu_to_apicid(cpu, apicid);
}
-unsigned int
-default_cpu_mask_to_apicid_and(const struct cpumask *cpumask,
- const struct cpumask *andmask)
+int default_cpu_mask_to_apicid_and(const struct cpumask *cpumask,
+ const struct cpumask *andmask,
+ unsigned int *apicid)
{
int cpu;
@@ -2148,7 +2150,8 @@ default_cpu_mask_to_apicid_and(const struct cpumask *cpumask,
if (cpumask_test_cpu(cpu, cpu_online_mask))
break;
}
- return per_cpu(x86_cpu_to_apicid, cpu);
+
+ return __default_cpu_to_apicid(cpu, apicid);
}
/*
diff --git a/arch/x86/kernel/apic/es7000_32.c b/arch/x86/kernel/apic/es7000_32.c
index 3c42865..515ebb0 100644
--- a/arch/x86/kernel/apic/es7000_32.c
+++ b/arch/x86/kernel/apic/es7000_32.c
@@ -525,7 +525,8 @@ static int es7000_check_phys_apicid_present(int cpu_physical_apicid)
return 1;
}
-static unsigned int es7000_cpu_mask_to_apicid(const struct cpumask *cpumask)
+static int
+es7000_cpu_mask_to_apicid(const struct cpumask *cpumask, unsigned int *dest_id)
{
unsigned int round = 0;
int cpu, uninitialized_var(apicid);
@@ -539,31 +540,33 @@ static unsigned int es7000_cpu_mask_to_apicid(const struct cpumask *cpumask)
if (round && APIC_CLUSTER(apicid) != APIC_CLUSTER(new_apicid)) {
WARN(1, "Not a valid mask!");
- return BAD_APICID;
+ return -EINVAL;
}
apicid = new_apicid;
round++;
}
- return apicid;
+ *dest_id = apicid;
+ return 0;
}
-static unsigned int
+static int
es7000_cpu_mask_to_apicid_and(const struct cpumask *inmask,
- const struct cpumask *andmask)
+ const struct cpumask *andmask,
+ unsigned int *apicid)
{
- int apicid = early_per_cpu(x86_cpu_to_logical_apicid, 0);
+ *apicid = early_per_cpu(x86_cpu_to_logical_apicid, 0);
cpumask_var_t cpumask;
if (!alloc_cpumask_var(&cpumask, GFP_ATOMIC))
- return apicid;
+ return 0;
cpumask_and(cpumask, inmask, andmask);
cpumask_and(cpumask, cpumask, cpu_online_mask);
- apicid = es7000_cpu_mask_to_apicid(cpumask);
+ es7000_cpu_mask_to_apicid(cpumask, apicid);
free_cpumask_var(cpumask);
- return apicid;
+ return 0;
}
static int es7000_phys_pkg_id(int cpuid_apic, int index_msb)
diff --git a/arch/x86/kernel/apic/io_apic.c b/arch/x86/kernel/apic/io_apic.c
index 4061a7d..0deb773 100644
--- a/arch/x86/kernel/apic/io_apic.c
+++ b/arch/x86/kernel/apic/io_apic.c
@@ -1359,7 +1359,14 @@ static void setup_ioapic_irq(unsigned int irq, struct irq_cfg *cfg,
if (assign_irq_vector(irq, cfg, apic->target_cpus()))
return;
- dest = apic->cpu_mask_to_apicid_and(cfg->domain, apic->target_cpus());
+ if (apic->cpu_mask_to_apicid_and(cfg->domain, apic->target_cpus(),
+ &dest)) {
+ pr_warn("Failed to obtain apicid for ioapic %d, pin %d\n",
+ mpc_ioapic_id(attr->ioapic), attr->ioapic_pin);
+ __clear_irq_vector(irq, cfg);
+
+ return;
+ }
apic_printk(APIC_VERBOSE,KERN_DEBUG
"IOAPIC[%d]: Set routing entry (%d-%d -> 0x%x -> "
@@ -1474,6 +1481,7 @@ static void __init setup_timer_IRQ0_pin(unsigned int ioapic_idx,
unsigned int pin, int vector)
{
struct IO_APIC_route_entry entry;
+ unsigned int dest;
if (irq_remapping_enabled)
return;
@@ -1484,9 +1492,12 @@ static void __init setup_timer_IRQ0_pin(unsigned int ioapic_idx,
* We use logical delivery to get the timer IRQ
* to the first CPU.
*/
+ if (unlikely(apic->cpu_mask_to_apicid(apic->target_cpus(), &dest)))
+ dest = BAD_APICID;
+
entry.dest_mode = apic->irq_dest_mode;
entry.mask = 0; /* don't mask IRQ for edge */
- entry.dest = apic->cpu_mask_to_apicid(apic->target_cpus());
+ entry.dest = dest;
entry.delivery_mode = apic->irq_delivery_mode;
entry.polarity = 0;
entry.trigger = 0;
@@ -2245,16 +2256,25 @@ int __ioapic_set_affinity(struct irq_data *data, const struct cpumask *mask,
unsigned int *dest_id)
{
struct irq_cfg *cfg = data->chip_data;
+ unsigned int irq = data->irq;
+ int err;
if (!cpumask_intersects(mask, cpu_online_mask))
- return -1;
+ return -EINVAL;
- if (assign_irq_vector(data->irq, data->chip_data, mask))
- return -1;
+ err = assign_irq_vector(irq, cfg, mask);
+ if (err)
+ return err;
+
+ err = apic->cpu_mask_to_apicid_and(mask, cfg->domain, dest_id);
+ if (err) {
+ if (assign_irq_vector(irq, cfg, data->affinity))
+ pr_err("Failed to recover vector for irq %d\n", irq);
+ return err;
+ }
cpumask_copy(data->affinity, mask);
- *dest_id = apic->cpu_mask_to_apicid_and(mask, cfg->domain);
return 0;
}
@@ -3040,7 +3060,10 @@ static int msi_compose_msg(struct pci_dev *pdev, unsigned int irq,
if (err)
return err;
- dest = apic->cpu_mask_to_apicid_and(cfg->domain, apic->target_cpus());
+ err = apic->cpu_mask_to_apicid_and(cfg->domain,
+ apic->target_cpus(), &dest);
+ if (err)
+ return err;
if (irq_remapped(cfg)) {
compose_remapped_msi_msg(pdev, irq, dest, msg, hpet_id);
@@ -3361,6 +3384,8 @@ static struct irq_chip ht_irq_chip = {
int arch_setup_ht_irq(unsigned int irq, struct pci_dev *dev)
{
struct irq_cfg *cfg;
+ struct ht_irq_msg msg;
+ unsigned dest;
int err;
if (disable_apic)
@@ -3368,36 +3393,37 @@ int arch_setup_ht_irq(unsigned int irq, struct pci_dev *dev)
cfg = irq_cfg(irq);
err = assign_irq_vector(irq, cfg, apic->target_cpus());
- if (!err) {
- struct ht_irq_msg msg;
- unsigned dest;
+ if (err)
+ return err;
- dest = apic->cpu_mask_to_apicid_and(cfg->domain,
- apic->target_cpus());
+ err = apic->cpu_mask_to_apicid_and(cfg->domain,
+ apic->target_cpus(), &dest);
+ if (err)
+ return err;
- msg.address_hi = HT_IRQ_HIGH_DEST_ID(dest);
+ msg.address_hi = HT_IRQ_HIGH_DEST_ID(dest);
- msg.address_lo =
- HT_IRQ_LOW_BASE |
- HT_IRQ_LOW_DEST_ID(dest) |
- HT_IRQ_LOW_VECTOR(cfg->vector) |
- ((apic->irq_dest_mode == 0) ?
- HT_IRQ_LOW_DM_PHYSICAL :
- HT_IRQ_LOW_DM_LOGICAL) |
- HT_IRQ_LOW_RQEOI_EDGE |
- ((apic->irq_delivery_mode != dest_LowestPrio) ?
- HT_IRQ_LOW_MT_FIXED :
- HT_IRQ_LOW_MT_ARBITRATED) |
- HT_IRQ_LOW_IRQ_MASKED;
+ msg.address_lo =
+ HT_IRQ_LOW_BASE |
+ HT_IRQ_LOW_DEST_ID(dest) |
+ HT_IRQ_LOW_VECTOR(cfg->vector) |
+ ((apic->irq_dest_mode == 0) ?
+ HT_IRQ_LOW_DM_PHYSICAL :
+ HT_IRQ_LOW_DM_LOGICAL) |
+ HT_IRQ_LOW_RQEOI_EDGE |
+ ((apic->irq_delivery_mode != dest_LowestPrio) ?
+ HT_IRQ_LOW_MT_FIXED :
+ HT_IRQ_LOW_MT_ARBITRATED) |
+ HT_IRQ_LOW_IRQ_MASKED;
- write_ht_irq_msg(irq, &msg);
+ write_ht_irq_msg(irq, &msg);
- irq_set_chip_and_handler_name(irq, &ht_irq_chip,
- handle_edge_irq, "edge");
+ irq_set_chip_and_handler_name(irq, &ht_irq_chip,
+ handle_edge_irq, "edge");
- dev_printk(KERN_DEBUG, &dev->dev, "irq %d for HT\n", irq);
- }
- return err;
+ dev_printk(KERN_DEBUG, &dev->dev, "irq %d for HT\n", irq);
+
+ return 0;
}
#endif /* CONFIG_HT_IRQ */
diff --git a/arch/x86/kernel/apic/numaq_32.c b/arch/x86/kernel/apic/numaq_32.c
index eb2d466..2b55514 100644
--- a/arch/x86/kernel/apic/numaq_32.c
+++ b/arch/x86/kernel/apic/numaq_32.c
@@ -406,16 +406,20 @@ static inline int numaq_check_phys_apicid_present(int phys_apicid)
* We use physical apicids here, not logical, so just return the default
* physical broadcast to stop people from breaking us
*/
-static unsigned int numaq_cpu_mask_to_apicid(const struct cpumask *cpumask)
+static int
+numaq_cpu_mask_to_apicid(const struct cpumask *cpumask, unsigned int *apicid)
{
- return 0x0F;
+ *apicid = 0x0F;
+ return 0;
}
-static inline unsigned int
+static int
numaq_cpu_mask_to_apicid_and(const struct cpumask *cpumask,
- const struct cpumask *andmask)
+ const struct cpumask *andmask,
+ unsigned int *apicid)
{
- return 0x0F;
+ *apicid = 0x0F;
+ return 0;
}
/* No NUMA-Q box has a HT CPU, but it can't hurt to use the default code. */
diff --git a/arch/x86/kernel/apic/summit_32.c b/arch/x86/kernel/apic/summit_32.c
index 35d254c..5766d84 100644
--- a/arch/x86/kernel/apic/summit_32.c
+++ b/arch/x86/kernel/apic/summit_32.c
@@ -263,7 +263,8 @@ static int summit_check_phys_apicid_present(int physical_apicid)
return 1;
}
-static unsigned int summit_cpu_mask_to_apicid(const struct cpumask *cpumask)
+static int
+summit_cpu_mask_to_apicid(const struct cpumask *cpumask, unsigned int *dest_id)
{
unsigned int round = 0;
int cpu, apicid = 0;
@@ -276,30 +277,33 @@ static unsigned int summit_cpu_mask_to_apicid(const struct cpumask *cpumask)
if (round && APIC_CLUSTER(apicid) != APIC_CLUSTER(new_apicid)) {
printk("%s: Not a valid mask!\n", __func__);
- return BAD_APICID;
+ return -EINVAL;
}
apicid |= new_apicid;
round++;
}
- return apicid;
+ *dest_id = apicid;
+ return 0;
}
-static unsigned int summit_cpu_mask_to_apicid_and(const struct cpumask *inmask,
- const struct cpumask *andmask)
+static int
+summit_cpu_mask_to_apicid_and(const struct cpumask *inmask,
+ const struct cpumask *andmask,
+ unsigned int *apicid)
{
- int apicid = early_per_cpu(x86_cpu_to_logical_apicid, 0);
+ *apicid = early_per_cpu(x86_cpu_to_logical_apicid, 0);
cpumask_var_t cpumask;
if (!alloc_cpumask_var(&cpumask, GFP_ATOMIC))
- return apicid;
+ return 0;
cpumask_and(cpumask, inmask, andmask);
cpumask_and(cpumask, cpumask, cpu_online_mask);
- apicid = summit_cpu_mask_to_apicid(cpumask);
+ summit_cpu_mask_to_apicid(cpumask, apicid);
free_cpumask_var(cpumask);
- return apicid;
+ return 0;
}
/*
diff --git a/arch/x86/kernel/apic/x2apic_cluster.c b/arch/x86/kernel/apic/x2apic_cluster.c
index 612622c..5f86f79 100644
--- a/arch/x86/kernel/apic/x2apic_cluster.c
+++ b/arch/x86/kernel/apic/x2apic_cluster.c
@@ -96,24 +96,26 @@ static void x2apic_send_IPI_all(int vector)
__x2apic_send_IPI_mask(cpu_online_mask, vector, APIC_DEST_ALLINC);
}
-static unsigned int x2apic_cpu_mask_to_apicid(const struct cpumask *cpumask)
+static int
+x2apic_cpu_mask_to_apicid(const struct cpumask *cpumask, unsigned int *apicid)
{
int cpu = cpumask_first(cpumask);
- u32 dest = 0;
int i;
- if (cpu > nr_cpu_ids)
- return BAD_APICID;
+ if (cpu >= nr_cpu_ids)
+ return -EINVAL;
+ *apicid = 0;
for_each_cpu_and(i, cpumask, per_cpu(cpus_in_cluster, cpu))
- dest |= per_cpu(x86_cpu_to_logical_apicid, i);
+ *apicid |= per_cpu(x86_cpu_to_logical_apicid, i);
- return dest;
+ return 0;
}
-static unsigned int
+static int
x2apic_cpu_mask_to_apicid_and(const struct cpumask *cpumask,
- const struct cpumask *andmask)
+ const struct cpumask *andmask,
+ unsigned int *apicid)
{
u32 dest = 0;
u16 cluster;
@@ -128,7 +130,7 @@ x2apic_cpu_mask_to_apicid_and(const struct cpumask *cpumask,
}
if (!dest)
- return BAD_APICID;
+ return -EINVAL;
for_each_cpu_and(i, cpumask, andmask) {
if (!cpumask_test_cpu(i, cpu_online_mask))
@@ -138,7 +140,9 @@ x2apic_cpu_mask_to_apicid_and(const struct cpumask *cpumask,
dest |= per_cpu(x86_cpu_to_logical_apicid, i);
}
- return dest;
+ *apicid = dest;
+
+ return 0;
}
static void init_x2apic_ldr(void)
diff --git a/arch/x86/kernel/apic/x2apic_uv_x.c b/arch/x86/kernel/apic/x2apic_uv_x.c
index df89a7d..2f3030f 100644
--- a/arch/x86/kernel/apic/x2apic_uv_x.c
+++ b/arch/x86/kernel/apic/x2apic_uv_x.c
@@ -269,23 +269,31 @@ static void uv_init_apic_ldr(void)
{
}
-static unsigned int uv_cpu_mask_to_apicid(const struct cpumask *cpumask)
+static inline int __uv_cpu_to_apicid(int cpu, unsigned int *apicid)
+{
+ if (likely((unsigned int)cpu < nr_cpu_ids)) {
+ *apicid = per_cpu(x86_cpu_to_apicid, cpu) | uv_apicid_hibits;
+ return 0;
+ } else {
+ return -EINVAL;
+ }
+}
+
+static int
+uv_cpu_mask_to_apicid(const struct cpumask *cpumask, unsigned int *apicid)
{
/*
* We're using fixed IRQ delivery, can only return one phys APIC ID.
* May as well be the first.
*/
int cpu = cpumask_first(cpumask);
-
- if ((unsigned)cpu < nr_cpu_ids)
- return per_cpu(x86_cpu_to_apicid, cpu) | uv_apicid_hibits;
- else
- return BAD_APICID;
+ return __uv_cpu_to_apicid(cpu, apicid);
}
-static unsigned int
+static int
uv_cpu_mask_to_apicid_and(const struct cpumask *cpumask,
- const struct cpumask *andmask)
+ const struct cpumask *andmask,
+ unsigned int *apicid)
{
int cpu;
@@ -297,7 +305,8 @@ uv_cpu_mask_to_apicid_and(const struct cpumask *cpumask,
if (cpumask_test_cpu(cpu, cpu_online_mask))
break;
}
- return per_cpu(x86_cpu_to_apicid, cpu) | uv_apicid_hibits;
+
+ return __uv_cpu_to_apicid(cpu, apicid);
}
static unsigned int x2apic_get_apic_id(unsigned long x)
diff --git a/arch/x86/platform/uv/uv_irq.c b/arch/x86/platform/uv/uv_irq.c
index f25c276..dd1ff39 100644
--- a/arch/x86/platform/uv/uv_irq.c
+++ b/arch/x86/platform/uv/uv_irq.c
@@ -135,6 +135,7 @@ arch_enable_uv_irq(char *irq_name, unsigned int irq, int cpu, int mmr_blade,
unsigned long mmr_value;
struct uv_IO_APIC_route_entry *entry;
int mmr_pnode, err;
+ unsigned int dest;
BUILD_BUG_ON(sizeof(struct uv_IO_APIC_route_entry) !=
sizeof(unsigned long));
@@ -143,6 +144,10 @@ arch_enable_uv_irq(char *irq_name, unsigned int irq, int cpu, int mmr_blade,
if (err != 0)
return err;
+ err = apic->cpu_mask_to_apicid(eligible_cpu, &dest);
+ if (err != 0)
+ return err;
+
if (limit == UV_AFFINITY_CPU)
irq_set_status_flags(irq, IRQ_NO_BALANCING);
else
@@ -159,7 +164,7 @@ arch_enable_uv_irq(char *irq_name, unsigned int irq, int cpu, int mmr_blade,
entry->polarity = 0;
entry->trigger = 0;
entry->mask = 0;
- entry->dest = apic->cpu_mask_to_apicid(eligible_cpu);
+ entry->dest = dest;
mmr_pnode = uv_blade_to_pnode(mmr_blade);
uv_write_global_mmr64(mmr_pnode, mmr_offset, mmr_value);
diff --git a/drivers/iommu/intel_irq_remapping.c b/drivers/iommu/intel_irq_remapping.c
index 6d34706..dafbad0 100644
--- a/drivers/iommu/intel_irq_remapping.c
+++ b/drivers/iommu/intel_irq_remapping.c
@@ -924,6 +924,7 @@ intel_ioapic_set_affinity(struct irq_data *data, const struct cpumask *mask,
struct irq_cfg *cfg = data->chip_data;
unsigned int dest, irq = data->irq;
struct irte irte;
+ int err;
if (!cpumask_intersects(mask, cpu_online_mask))
return -EINVAL;
@@ -931,10 +932,16 @@ intel_ioapic_set_affinity(struct irq_data *data, const struct cpumask *mask,
if (get_irte(irq, &irte))
return -EBUSY;
- if (assign_irq_vector(irq, cfg, mask))
- return -EBUSY;
+ err = assign_irq_vector(irq, cfg, mask);
+ if (err)
+ return err;
- dest = apic->cpu_mask_to_apicid_and(cfg->domain, mask);
+ err = apic->cpu_mask_to_apicid_and(cfg->domain, mask, &dest);
+ if (err) {
+ if (assign_irq_vector(irq, cfg, data->affinity));
+ pr_err("Failed to recover vector for irq %d\n", irq);
+ return err;
+ }
irte.vector = cfg->vector;
irte.dest_id = IRTE_DEST(dest);
--
1.7.7.6
--
Regards,
Alexander Gordeev
agordeev@redhat.com
^ permalink raw reply related [flat|nested] 26+ messages in thread* Re: [PATCH 7/8] x86/apic: Make cpu_mask_to_apicid() operations return error code
2012-06-07 13:15 ` [PATCH 7/8] x86/apic: Make cpu_mask_to_apicid() operations return error code Alexander Gordeev
@ 2012-06-07 22:24 ` Suresh Siddha
2012-06-08 15:15 ` Alexander Gordeev
2012-06-08 16:53 ` [PATCH] x86/apic: Eliminate cpu_mask_to_apicid() operation Alexander Gordeev
2012-06-08 14:51 ` [tip:x86/apic] x86/apic: Make cpu_mask_to_apicid() operations return error code tip-bot for Alexander Gordeev
1 sibling, 2 replies; 26+ messages in thread
From: Suresh Siddha @ 2012-06-07 22:24 UTC (permalink / raw)
To: Alexander Gordeev; +Cc: Ingo Molnar, linux-kernel, x86, Yinghai Lu
On Thu, 2012-06-07 at 15:15 +0200, Alexander Gordeev wrote:
> Current cpu_mask_to_apicid() and cpu_mask_to_apicid_and()
> implementations have few shortcomings:
>
> 1. A value returned by cpu_mask_to_apicid() is written to hardware
> registers unconditionally. Should BAD_APICID get ever returned it will
> be written to a hardware too. But the value of BAD_APICID is not
> universal across all hardware in all modes and might cause unexpected
> results, i.e. interrupts might get routed to CPUs that are not
> configured to receive it.
>
> 2. Because the value of BAD_APICID is not universal it is counter-
> intuitive to return it for a hardware where it does not make sense
> (i.e. x2apic).
>
> 3. cpu_mask_to_apicid_and() operation is thought as an complement to
> cpu_mask_to_apicid() that only applies a AND mask on top of a cpumask
> being passed. Yet, as consequence of 18374d8 commit the two operations
> are inconsistent in that of:
> cpu_mask_to_apicid() should not get a offline CPU with the cpumask
> cpu_mask_to_apicid_and() should not fail and return BAD_APICID
> These limitations are impossible to realize just from looking at the
> operations prototypes.
>
> Most of these shortcomings are resolved by returning a error code
> instead of BAD_APICID. As the result, faults are reported back early
> rather than possibilities to cause a unexpected behaviour exist (in case
> of [1]).
>
> The only exception is setup_timer_IRQ0_pin() routine. Although obviously
> controversial to this fix, its existing behaviour is preserved to not
> break the fragile check_timer() and would better addressed in a separate
> fix.
>
I am ok with these changes. But even better would be to remove the
cpu_mask_to_apicid() and just use cpu_mask_to_apicid_and() instead.
Looks like there are only two places cpu_mask_to_apicid() being used
anyways. So instead of patches 7 and 8, can you remove
cpu_mask_to_apicid() in patch-7 and fixup the return value of
cpu_mask_to_apicid_and() in patch-8
thanks,
suresh
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 7/8] x86/apic: Make cpu_mask_to_apicid() operations return error code
2012-06-07 22:24 ` Suresh Siddha
@ 2012-06-08 15:15 ` Alexander Gordeev
2012-06-08 16:53 ` [PATCH] x86/apic: Eliminate cpu_mask_to_apicid() operation Alexander Gordeev
1 sibling, 0 replies; 26+ messages in thread
From: Alexander Gordeev @ 2012-06-08 15:15 UTC (permalink / raw)
To: Suresh Siddha; +Cc: Ingo Molnar, linux-kernel, x86, Yinghai Lu
On Thu, Jun 07, 2012 at 03:24:05PM -0700, Suresh Siddha wrote:
> On Thu, 2012-06-07 at 15:15 +0200, Alexander Gordeev wrote:
> I am ok with these changes. But even better would be to remove the
> cpu_mask_to_apicid() and just use cpu_mask_to_apicid_and() instead.
>
> Looks like there are only two places cpu_mask_to_apicid() being used
> anyways. So instead of patches 7 and 8, can you remove
> cpu_mask_to_apicid() in patch-7 and fixup the return value of
> cpu_mask_to_apicid_and() in patch-8
oops, I was not fast enough - Ingo has pulled 7 and 8
I will post the update on top of that
--
Regards,
Alexander Gordeev
agordeev@redhat.com
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH] x86/apic: Eliminate cpu_mask_to_apicid() operation
2012-06-07 22:24 ` Suresh Siddha
2012-06-08 15:15 ` Alexander Gordeev
@ 2012-06-08 16:53 ` Alexander Gordeev
2012-06-08 18:24 ` Suresh Siddha
` (2 more replies)
1 sibling, 3 replies; 26+ messages in thread
From: Alexander Gordeev @ 2012-06-08 16:53 UTC (permalink / raw)
To: Suresh Siddha; +Cc: Ingo Molnar, linux-kernel, x86, Yinghai Lu
Since there are only two locations where cpu_mask_to_apicid() is called
from, remove the operation and use only cpu_mask_to_apicid_and() instead.
Signed-off-by: Alexander Gordeev <agordeev@redhat.com>
---
arch/x86/include/asm/apic.h | 33 ++++++++-------------------------
arch/x86/kernel/apic/apic.c | 24 ++++++------------------
arch/x86/kernel/apic/apic_flat_64.c | 2 --
arch/x86/kernel/apic/apic_noop.c | 1 -
arch/x86/kernel/apic/apic_numachip.c | 1 -
arch/x86/kernel/apic/bigsmp_32.c | 1 -
arch/x86/kernel/apic/es7000_32.c | 4 +---
arch/x86/kernel/apic/io_apic.c | 3 ++-
arch/x86/kernel/apic/numaq_32.c | 8 --------
arch/x86/kernel/apic/probe_32.c | 1 -
arch/x86/kernel/apic/summit_32.c | 3 +--
arch/x86/kernel/apic/x2apic_cluster.c | 17 -----------------
arch/x86/kernel/apic/x2apic_phys.c | 1 -
arch/x86/kernel/apic/x2apic_uv_x.c | 29 ++++++-----------------------
arch/x86/platform/uv/uv_irq.c | 2 +-
15 files changed, 25 insertions(+), 105 deletions(-)
diff --git a/arch/x86/include/asm/apic.h b/arch/x86/include/asm/apic.h
index 1ed3eea..eec240e 100644
--- a/arch/x86/include/asm/apic.h
+++ b/arch/x86/include/asm/apic.h
@@ -331,8 +331,6 @@ struct apic {
unsigned long (*set_apic_id)(unsigned int id);
unsigned long apic_id_mask;
- int (*cpu_mask_to_apicid)(const struct cpumask *cpumask,
- unsigned int *apicid);
int (*cpu_mask_to_apicid_and)(const struct cpumask *cpumask,
const struct cpumask *andmask,
unsigned int *apicid);
@@ -594,9 +592,15 @@ static inline int default_phys_pkg_id(int cpuid_apic, int index_msb)
#endif
static inline int
-__flat_cpu_mask_to_apicid(unsigned long cpu_mask, unsigned int *apicid)
+flat_cpu_mask_to_apicid_and(const struct cpumask *cpumask,
+ const struct cpumask *andmask,
+ unsigned int *apicid)
{
- cpu_mask = cpu_mask & APIC_ALL_CPUS & cpumask_bits(cpu_online_mask)[0];
+ unsigned long cpu_mask = cpumask_bits(cpumask)[0] &
+ cpumask_bits(andmask)[0] &
+ cpumask_bits(cpu_online_mask)[0] &
+ APIC_ALL_CPUS;
+
if (likely(cpu_mask)) {
*apicid = (unsigned int)cpu_mask;
return 0;
@@ -605,27 +609,6 @@ __flat_cpu_mask_to_apicid(unsigned long cpu_mask, unsigned int *apicid)
}
}
-static inline int
-flat_cpu_mask_to_apicid(const struct cpumask *cpumask,
- unsigned int *apicid)
-{
- return __flat_cpu_mask_to_apicid(cpumask_bits(cpumask)[0], apicid);
-}
-
-static inline int
-flat_cpu_mask_to_apicid_and(const struct cpumask *cpumask,
- const struct cpumask *andmask,
- unsigned int *apicid)
-{
- unsigned long mask1 = cpumask_bits(cpumask)[0];
- unsigned long mask2 = cpumask_bits(andmask)[0];
- return __flat_cpu_mask_to_apicid(mask1 & mask2, apicid);
-}
-
-extern int
-default_cpu_mask_to_apicid(const struct cpumask *cpumask,
- unsigned int *apicid);
-
extern int
default_cpu_mask_to_apicid_and(const struct cpumask *cpumask,
const struct cpumask *andmask,
diff --git a/arch/x86/kernel/apic/apic.c b/arch/x86/kernel/apic/apic.c
index 7e9bbe7..048a4f8 100644
--- a/arch/x86/kernel/apic/apic.c
+++ b/arch/x86/kernel/apic/apic.c
@@ -2123,23 +2123,6 @@ void default_init_apic_ldr(void)
apic_write(APIC_LDR, val);
}
-static inline int __default_cpu_to_apicid(int cpu, unsigned int *apicid)
-{
- if (likely((unsigned int)cpu < nr_cpu_ids)) {
- *apicid = per_cpu(x86_cpu_to_apicid, cpu);
- return 0;
- } else {
- return -EINVAL;
- }
-}
-
-int default_cpu_mask_to_apicid(const struct cpumask *cpumask,
- unsigned int *apicid)
-{
- int cpu = cpumask_first_and(cpumask, cpu_online_mask);
- return __default_cpu_to_apicid(cpu, apicid);
-}
-
int default_cpu_mask_to_apicid_and(const struct cpumask *cpumask,
const struct cpumask *andmask,
unsigned int *apicid)
@@ -2151,7 +2134,12 @@ int default_cpu_mask_to_apicid_and(const struct cpumask *cpumask,
break;
}
- return __default_cpu_to_apicid(cpu, apicid);
+ if (likely((unsigned int)cpu < nr_cpu_ids)) {
+ *apicid = per_cpu(x86_cpu_to_apicid, cpu);
+ return 0;
+ } else {
+ return -EINVAL;
+ }
}
/*
diff --git a/arch/x86/kernel/apic/apic_flat_64.c b/arch/x86/kernel/apic/apic_flat_64.c
index bddc925..00c77cf 100644
--- a/arch/x86/kernel/apic/apic_flat_64.c
+++ b/arch/x86/kernel/apic/apic_flat_64.c
@@ -191,7 +191,6 @@ static struct apic apic_flat = {
.set_apic_id = set_apic_id,
.apic_id_mask = 0xFFu << 24,
- .cpu_mask_to_apicid = flat_cpu_mask_to_apicid,
.cpu_mask_to_apicid_and = flat_cpu_mask_to_apicid_and,
.send_IPI_mask = flat_send_IPI_mask,
@@ -308,7 +307,6 @@ static struct apic apic_physflat = {
.set_apic_id = set_apic_id,
.apic_id_mask = 0xFFu << 24,
- .cpu_mask_to_apicid = default_cpu_mask_to_apicid,
.cpu_mask_to_apicid_and = default_cpu_mask_to_apicid_and,
.send_IPI_mask = physflat_send_IPI_mask,
diff --git a/arch/x86/kernel/apic/apic_noop.c b/arch/x86/kernel/apic/apic_noop.c
index ac9edf2..65c07fc 100644
--- a/arch/x86/kernel/apic/apic_noop.c
+++ b/arch/x86/kernel/apic/apic_noop.c
@@ -159,7 +159,6 @@ struct apic apic_noop = {
.set_apic_id = NULL,
.apic_id_mask = 0x0F << 24,
- .cpu_mask_to_apicid = flat_cpu_mask_to_apicid,
.cpu_mask_to_apicid_and = flat_cpu_mask_to_apicid_and,
.send_IPI_mask = noop_send_IPI_mask,
diff --git a/arch/x86/kernel/apic/apic_numachip.c b/arch/x86/kernel/apic/apic_numachip.c
index c028132..bc552cf 100644
--- a/arch/x86/kernel/apic/apic_numachip.c
+++ b/arch/x86/kernel/apic/apic_numachip.c
@@ -234,7 +234,6 @@ static struct apic apic_numachip __refconst = {
.set_apic_id = set_apic_id,
.apic_id_mask = 0xffU << 24,
- .cpu_mask_to_apicid = default_cpu_mask_to_apicid,
.cpu_mask_to_apicid_and = default_cpu_mask_to_apicid_and,
.send_IPI_mask = numachip_send_IPI_mask,
diff --git a/arch/x86/kernel/apic/bigsmp_32.c b/arch/x86/kernel/apic/bigsmp_32.c
index df342fe..d50e364 100644
--- a/arch/x86/kernel/apic/bigsmp_32.c
+++ b/arch/x86/kernel/apic/bigsmp_32.c
@@ -188,7 +188,6 @@ static struct apic apic_bigsmp = {
.set_apic_id = NULL,
.apic_id_mask = 0xFF << 24,
- .cpu_mask_to_apicid = default_cpu_mask_to_apicid,
.cpu_mask_to_apicid_and = default_cpu_mask_to_apicid_and,
.send_IPI_mask = bigsmp_send_IPI_mask,
diff --git a/arch/x86/kernel/apic/es7000_32.c b/arch/x86/kernel/apic/es7000_32.c
index b35cfb9..2c5317e 100644
--- a/arch/x86/kernel/apic/es7000_32.c
+++ b/arch/x86/kernel/apic/es7000_32.c
@@ -525,7 +525,7 @@ static int es7000_check_phys_apicid_present(int cpu_physical_apicid)
return 1;
}
-static int
+static inline int
es7000_cpu_mask_to_apicid(const struct cpumask *cpumask, unsigned int *dest_id)
{
unsigned int round = 0;
@@ -643,7 +643,6 @@ static struct apic __refdata apic_es7000_cluster = {
.set_apic_id = NULL,
.apic_id_mask = 0xFF << 24,
- .cpu_mask_to_apicid = es7000_cpu_mask_to_apicid,
.cpu_mask_to_apicid_and = es7000_cpu_mask_to_apicid_and,
.send_IPI_mask = es7000_send_IPI_mask,
@@ -710,7 +709,6 @@ static struct apic __refdata apic_es7000 = {
.set_apic_id = NULL,
.apic_id_mask = 0xFF << 24,
- .cpu_mask_to_apicid = es7000_cpu_mask_to_apicid,
.cpu_mask_to_apicid_and = es7000_cpu_mask_to_apicid_and,
.send_IPI_mask = es7000_send_IPI_mask,
diff --git a/arch/x86/kernel/apic/io_apic.c b/arch/x86/kernel/apic/io_apic.c
index 0deb773..0540f08 100644
--- a/arch/x86/kernel/apic/io_apic.c
+++ b/arch/x86/kernel/apic/io_apic.c
@@ -1492,7 +1492,8 @@ static void __init setup_timer_IRQ0_pin(unsigned int ioapic_idx,
* We use logical delivery to get the timer IRQ
* to the first CPU.
*/
- if (unlikely(apic->cpu_mask_to_apicid(apic->target_cpus(), &dest)))
+ if (unlikely(apic->cpu_mask_to_apicid_and(apic->target_cpus(),
+ apic->target_cpus(), &dest)))
dest = BAD_APICID;
entry.dest_mode = apic->irq_dest_mode;
diff --git a/arch/x86/kernel/apic/numaq_32.c b/arch/x86/kernel/apic/numaq_32.c
index 2b55514..d661ee9 100644
--- a/arch/x86/kernel/apic/numaq_32.c
+++ b/arch/x86/kernel/apic/numaq_32.c
@@ -407,13 +407,6 @@ static inline int numaq_check_phys_apicid_present(int phys_apicid)
* physical broadcast to stop people from breaking us
*/
static int
-numaq_cpu_mask_to_apicid(const struct cpumask *cpumask, unsigned int *apicid)
-{
- *apicid = 0x0F;
- return 0;
-}
-
-static int
numaq_cpu_mask_to_apicid_and(const struct cpumask *cpumask,
const struct cpumask *andmask,
unsigned int *apicid)
@@ -499,7 +492,6 @@ static struct apic __refdata apic_numaq = {
.set_apic_id = NULL,
.apic_id_mask = 0x0F << 24,
- .cpu_mask_to_apicid = numaq_cpu_mask_to_apicid,
.cpu_mask_to_apicid_and = numaq_cpu_mask_to_apicid_and,
.send_IPI_mask = numaq_send_IPI_mask,
diff --git a/arch/x86/kernel/apic/probe_32.c b/arch/x86/kernel/apic/probe_32.c
index 2c6f003..eef6bcd 100644
--- a/arch/x86/kernel/apic/probe_32.c
+++ b/arch/x86/kernel/apic/probe_32.c
@@ -108,7 +108,6 @@ static struct apic apic_default = {
.set_apic_id = NULL,
.apic_id_mask = 0x0F << 24,
- .cpu_mask_to_apicid = flat_cpu_mask_to_apicid,
.cpu_mask_to_apicid_and = flat_cpu_mask_to_apicid_and,
.send_IPI_mask = default_send_IPI_mask_logical,
diff --git a/arch/x86/kernel/apic/summit_32.c b/arch/x86/kernel/apic/summit_32.c
index 79d360f..bbad180 100644
--- a/arch/x86/kernel/apic/summit_32.c
+++ b/arch/x86/kernel/apic/summit_32.c
@@ -263,7 +263,7 @@ static int summit_check_phys_apicid_present(int physical_apicid)
return 1;
}
-static int
+static inline int
summit_cpu_mask_to_apicid(const struct cpumask *cpumask, unsigned int *dest_id)
{
unsigned int round = 0;
@@ -516,7 +516,6 @@ static struct apic apic_summit = {
.set_apic_id = NULL,
.apic_id_mask = 0xFF << 24,
- .cpu_mask_to_apicid = summit_cpu_mask_to_apicid,
.cpu_mask_to_apicid_and = summit_cpu_mask_to_apicid_and,
.send_IPI_mask = summit_send_IPI_mask,
diff --git a/arch/x86/kernel/apic/x2apic_cluster.c b/arch/x86/kernel/apic/x2apic_cluster.c
index 23a46cf..b5d889b 100644
--- a/arch/x86/kernel/apic/x2apic_cluster.c
+++ b/arch/x86/kernel/apic/x2apic_cluster.c
@@ -97,22 +97,6 @@ static void x2apic_send_IPI_all(int vector)
}
static int
-x2apic_cpu_mask_to_apicid(const struct cpumask *cpumask, unsigned int *apicid)
-{
- int cpu = cpumask_first_and(cpumask, cpu_online_mask);
- int i;
-
- if (cpu >= nr_cpu_ids)
- return -EINVAL;
-
- *apicid = 0;
- for_each_cpu_and(i, cpumask, per_cpu(cpus_in_cluster, cpu))
- *apicid |= per_cpu(x86_cpu_to_logical_apicid, i);
-
- return 0;
-}
-
-static int
x2apic_cpu_mask_to_apicid_and(const struct cpumask *cpumask,
const struct cpumask *andmask,
unsigned int *apicid)
@@ -269,7 +253,6 @@ static struct apic apic_x2apic_cluster = {
.set_apic_id = x2apic_set_apic_id,
.apic_id_mask = 0xFFFFFFFFu,
- .cpu_mask_to_apicid = x2apic_cpu_mask_to_apicid,
.cpu_mask_to_apicid_and = x2apic_cpu_mask_to_apicid_and,
.send_IPI_mask = x2apic_send_IPI_mask,
diff --git a/arch/x86/kernel/apic/x2apic_phys.c b/arch/x86/kernel/apic/x2apic_phys.c
index f109388..e03a1e1 100644
--- a/arch/x86/kernel/apic/x2apic_phys.c
+++ b/arch/x86/kernel/apic/x2apic_phys.c
@@ -123,7 +123,6 @@ static struct apic apic_x2apic_phys = {
.set_apic_id = x2apic_set_apic_id,
.apic_id_mask = 0xFFFFFFFFu,
- .cpu_mask_to_apicid = default_cpu_mask_to_apicid,
.cpu_mask_to_apicid_and = default_cpu_mask_to_apicid_and,
.send_IPI_mask = x2apic_send_IPI_mask,
diff --git a/arch/x86/kernel/apic/x2apic_uv_x.c b/arch/x86/kernel/apic/x2apic_uv_x.c
index 307aa07..026de01 100644
--- a/arch/x86/kernel/apic/x2apic_uv_x.c
+++ b/arch/x86/kernel/apic/x2apic_uv_x.c
@@ -269,27 +269,6 @@ static void uv_init_apic_ldr(void)
{
}
-static inline int __uv_cpu_to_apicid(int cpu, unsigned int *apicid)
-{
- if (likely((unsigned int)cpu < nr_cpu_ids)) {
- *apicid = per_cpu(x86_cpu_to_apicid, cpu) | uv_apicid_hibits;
- return 0;
- } else {
- return -EINVAL;
- }
-}
-
-static int
-uv_cpu_mask_to_apicid(const struct cpumask *cpumask, unsigned int *apicid)
-{
- /*
- * We're using fixed IRQ delivery, can only return one phys APIC ID.
- * May as well be the first.
- */
- int cpu = cpumask_first_and(cpumask, cpu_online_mask);
- return __uv_cpu_to_apicid(cpu, apicid);
-}
-
static int
uv_cpu_mask_to_apicid_and(const struct cpumask *cpumask,
const struct cpumask *andmask,
@@ -306,7 +285,12 @@ uv_cpu_mask_to_apicid_and(const struct cpumask *cpumask,
break;
}
- return __uv_cpu_to_apicid(cpu, apicid);
+ if (likely((unsigned int)cpu < nr_cpu_ids)) {
+ *apicid = per_cpu(x86_cpu_to_apicid, cpu) | uv_apicid_hibits;
+ return 0;
+ } else {
+ return -EINVAL;
+ }
}
static unsigned int x2apic_get_apic_id(unsigned long x)
@@ -384,7 +368,6 @@ static struct apic __refdata apic_x2apic_uv_x = {
.set_apic_id = set_apic_id,
.apic_id_mask = 0xFFFFFFFFu,
- .cpu_mask_to_apicid = uv_cpu_mask_to_apicid,
.cpu_mask_to_apicid_and = uv_cpu_mask_to_apicid_and,
.send_IPI_mask = uv_send_IPI_mask,
diff --git a/arch/x86/platform/uv/uv_irq.c b/arch/x86/platform/uv/uv_irq.c
index dd1ff39..a67c7a6 100644
--- a/arch/x86/platform/uv/uv_irq.c
+++ b/arch/x86/platform/uv/uv_irq.c
@@ -144,7 +144,7 @@ arch_enable_uv_irq(char *irq_name, unsigned int irq, int cpu, int mmr_blade,
if (err != 0)
return err;
- err = apic->cpu_mask_to_apicid(eligible_cpu, &dest);
+ err = apic->cpu_mask_to_apicid_and(eligible_cpu, eligible_cpu, &dest);
if (err != 0)
return err;
--
1.7.7.6
--
Regards,
Alexander Gordeev
agordeev@redhat.com
^ permalink raw reply related [flat|nested] 26+ messages in thread* Re: [PATCH] x86/apic: Eliminate cpu_mask_to_apicid() operation
2012-06-08 16:53 ` [PATCH] x86/apic: Eliminate cpu_mask_to_apicid() operation Alexander Gordeev
@ 2012-06-08 18:24 ` Suresh Siddha
2012-06-08 22:36 ` Yinghai Lu
2012-06-11 8:22 ` Ingo Molnar
2 siblings, 0 replies; 26+ messages in thread
From: Suresh Siddha @ 2012-06-08 18:24 UTC (permalink / raw)
To: Alexander Gordeev; +Cc: Ingo Molnar, linux-kernel, x86, Yinghai Lu
On Fri, 2012-06-08 at 18:53 +0200, Alexander Gordeev wrote:
> Since there are only two locations where cpu_mask_to_apicid() is called
> from, remove the operation and use only cpu_mask_to_apicid_and() instead.
>
> Signed-off-by: Alexander Gordeev <agordeev@redhat.com>
> ---
> arch/x86/include/asm/apic.h | 33 ++++++++-------------------------
> arch/x86/kernel/apic/apic.c | 24 ++++++------------------
> arch/x86/kernel/apic/apic_flat_64.c | 2 --
> arch/x86/kernel/apic/apic_noop.c | 1 -
> arch/x86/kernel/apic/apic_numachip.c | 1 -
> arch/x86/kernel/apic/bigsmp_32.c | 1 -
> arch/x86/kernel/apic/es7000_32.c | 4 +---
> arch/x86/kernel/apic/io_apic.c | 3 ++-
> arch/x86/kernel/apic/numaq_32.c | 8 --------
> arch/x86/kernel/apic/probe_32.c | 1 -
> arch/x86/kernel/apic/summit_32.c | 3 +--
> arch/x86/kernel/apic/x2apic_cluster.c | 17 -----------------
> arch/x86/kernel/apic/x2apic_phys.c | 1 -
> arch/x86/kernel/apic/x2apic_uv_x.c | 29 ++++++-----------------------
> arch/x86/platform/uv/uv_irq.c | 2 +-
> 15 files changed, 25 insertions(+), 105 deletions(-)
>
Looks good to me.
Acked-by: Suresh Siddha <suresh.b.siddha@intel.com>
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH] x86/apic: Eliminate cpu_mask_to_apicid() operation
2012-06-08 16:53 ` [PATCH] x86/apic: Eliminate cpu_mask_to_apicid() operation Alexander Gordeev
2012-06-08 18:24 ` Suresh Siddha
@ 2012-06-08 22:36 ` Yinghai Lu
2012-06-11 8:22 ` Ingo Molnar
2 siblings, 0 replies; 26+ messages in thread
From: Yinghai Lu @ 2012-06-08 22:36 UTC (permalink / raw)
To: Alexander Gordeev; +Cc: Suresh Siddha, Ingo Molnar, linux-kernel, x86
On Fri, Jun 8, 2012 at 9:53 AM, Alexander Gordeev <agordeev@redhat.com> wrote:
> Since there are only two locations where cpu_mask_to_apicid() is called
> from, remove the operation and use only cpu_mask_to_apicid_and() instead.
>
> Signed-off-by: Alexander Gordeev <agordeev@redhat.com>
> ---
> arch/x86/include/asm/apic.h | 33 ++++++++-------------------------
> arch/x86/kernel/apic/apic.c | 24 ++++++------------------
> arch/x86/kernel/apic/apic_flat_64.c | 2 --
> arch/x86/kernel/apic/apic_noop.c | 1 -
> arch/x86/kernel/apic/apic_numachip.c | 1 -
> arch/x86/kernel/apic/bigsmp_32.c | 1 -
> arch/x86/kernel/apic/es7000_32.c | 4 +---
> arch/x86/kernel/apic/io_apic.c | 3 ++-
> arch/x86/kernel/apic/numaq_32.c | 8 --------
> arch/x86/kernel/apic/probe_32.c | 1 -
> arch/x86/kernel/apic/summit_32.c | 3 +--
> arch/x86/kernel/apic/x2apic_cluster.c | 17 -----------------
> arch/x86/kernel/apic/x2apic_phys.c | 1 -
> arch/x86/kernel/apic/x2apic_uv_x.c | 29 ++++++-----------------------
> arch/x86/platform/uv/uv_irq.c | 2 +-
> 15 files changed, 25 insertions(+), 105 deletions(-)
good, kill 80 lines.
Acked-by: Yinghai Lu <yinghai@kernel.org>
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH] x86/apic: Eliminate cpu_mask_to_apicid() operation
2012-06-08 16:53 ` [PATCH] x86/apic: Eliminate cpu_mask_to_apicid() operation Alexander Gordeev
2012-06-08 18:24 ` Suresh Siddha
2012-06-08 22:36 ` Yinghai Lu
@ 2012-06-11 8:22 ` Ingo Molnar
2012-06-11 10:51 ` Alexander Gordeev
2 siblings, 1 reply; 26+ messages in thread
From: Ingo Molnar @ 2012-06-11 8:22 UTC (permalink / raw)
To: Alexander Gordeev; +Cc: Suresh Siddha, linux-kernel, x86, Yinghai Lu
* Alexander Gordeev <agordeev@redhat.com> wrote:
> @@ -2151,7 +2134,12 @@ int default_cpu_mask_to_apicid_and(const struct cpumask *cpumask,
> break;
> }
>
> - return __default_cpu_to_apicid(cpu, apicid);
> + if (likely((unsigned int)cpu < nr_cpu_ids)) {
> + *apicid = per_cpu(x86_cpu_to_apicid, cpu);
> + return 0;
> + } else {
> + return -EINVAL;
> + }
The type cast is rather ugly - why not change the cpu type to
unsigned int?
Also, the else block is superfluous, just make it a return
-EINVAL?
Thanks,
Ingo
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [PATCH] x86/apic: Eliminate cpu_mask_to_apicid() operation
2012-06-11 8:22 ` Ingo Molnar
@ 2012-06-11 10:51 ` Alexander Gordeev
0 siblings, 0 replies; 26+ messages in thread
From: Alexander Gordeev @ 2012-06-11 10:51 UTC (permalink / raw)
To: Ingo Molnar; +Cc: Suresh Siddha, linux-kernel, x86, Yinghai Lu
On Mon, Jun 11, 2012 at 10:22:49AM +0200, Ingo Molnar wrote:
> > @@ -2151,7 +2134,12 @@ int default_cpu_mask_to_apicid_and(const struct cpumask *cpumask,
> > break;
> > }
> >
> > - return __default_cpu_to_apicid(cpu, apicid);
> > + if (likely((unsigned int)cpu < nr_cpu_ids)) {
> > + *apicid = per_cpu(x86_cpu_to_apicid, cpu);
> > + return 0;
> > + } else {
> > + return -EINVAL;
> > + }
>
> The type cast is rather ugly - why not change the cpu type to
> unsigned int?
Tried to preserve in changelog as much original code as possible.
Suppose, it has been int to match cpumask_* functions.
> Also, the else block is superfluous, just make it a return
> -EINVAL?
What about posting a separate patch, with the rest of apic drivers that have
the same branch/cast fixed, altogether?
>
> Thanks,
>
> Ingo
--
Regards,
Alexander Gordeev
agordeev@redhat.com
^ permalink raw reply [flat|nested] 26+ messages in thread
* [tip:x86/apic] x86/apic: Make cpu_mask_to_apicid() operations return error code
2012-06-07 13:15 ` [PATCH 7/8] x86/apic: Make cpu_mask_to_apicid() operations return error code Alexander Gordeev
2012-06-07 22:24 ` Suresh Siddha
@ 2012-06-08 14:51 ` tip-bot for Alexander Gordeev
1 sibling, 0 replies; 26+ messages in thread
From: tip-bot for Alexander Gordeev @ 2012-06-08 14:51 UTC (permalink / raw)
To: linux-tip-commits
Cc: linux-kernel, agordeev, hpa, mingo, yinghai, suresh.b.siddha,
tglx
Commit-ID: ff164324123c0fe181d8de7dadcc7b3fbe25f2cf
Gitweb: http://git.kernel.org/tip/ff164324123c0fe181d8de7dadcc7b3fbe25f2cf
Author: Alexander Gordeev <agordeev@redhat.com>
AuthorDate: Thu, 7 Jun 2012 15:15:59 +0200
Committer: Ingo Molnar <mingo@kernel.org>
CommitDate: Fri, 8 Jun 2012 11:44:29 +0200
x86/apic: Make cpu_mask_to_apicid() operations return error code
Current cpu_mask_to_apicid() and cpu_mask_to_apicid_and()
implementations have few shortcomings:
1. A value returned by cpu_mask_to_apicid() is written to
hardware registers unconditionally. Should BAD_APICID get ever
returned it will be written to a hardware too. But the value of
BAD_APICID is not universal across all hardware in all modes and
might cause unexpected results, i.e. interrupts might get routed
to CPUs that are not configured to receive it.
2. Because the value of BAD_APICID is not universal it is
counter- intuitive to return it for a hardware where it does not
make sense (i.e. x2apic).
3. cpu_mask_to_apicid_and() operation is thought as an
complement to cpu_mask_to_apicid() that only applies a AND mask
on top of a cpumask being passed. Yet, as consequence of 18374d8
commit the two operations are inconsistent in that of:
cpu_mask_to_apicid() should not get a offline CPU with the cpumask
cpu_mask_to_apicid_and() should not fail and return BAD_APICID
These limitations are impossible to realize just from looking at
the operations prototypes.
Most of these shortcomings are resolved by returning a error
code instead of BAD_APICID. As the result, faults are reported
back early rather than possibilities to cause a unexpected
behaviour exist (in case of [1]).
The only exception is setup_timer_IRQ0_pin() routine. Although
obviously controversial to this fix, its existing behaviour is
preserved to not break the fragile check_timer() and would
better addressed in a separate fix.
Signed-off-by: Alexander Gordeev <agordeev@redhat.com>
Acked-by: Suresh Siddha <suresh.b.siddha@intel.com>
Cc: Yinghai Lu <yinghai@kernel.org>
Link: http://lkml.kernel.org/r/20120607131559.GF4759@dhcp-26-207.brq.redhat.com
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
arch/x86/include/asm/apic.h | 44 ++++++++++++-----
arch/x86/kernel/apic/apic.c | 33 +++++++------
arch/x86/kernel/apic/es7000_32.c | 21 +++++---
arch/x86/kernel/apic/io_apic.c | 88 +++++++++++++++++++++------------
arch/x86/kernel/apic/numaq_32.c | 14 +++--
arch/x86/kernel/apic/summit_32.c | 22 +++++---
arch/x86/kernel/apic/x2apic_cluster.c | 24 +++++----
arch/x86/kernel/apic/x2apic_uv_x.c | 27 +++++++---
arch/x86/platform/uv/uv_irq.c | 7 ++-
drivers/iommu/intel_irq_remapping.c | 13 ++++-
10 files changed, 188 insertions(+), 105 deletions(-)
diff --git a/arch/x86/include/asm/apic.h b/arch/x86/include/asm/apic.h
index e3fecd5..ae91f9c 100644
--- a/arch/x86/include/asm/apic.h
+++ b/arch/x86/include/asm/apic.h
@@ -331,9 +331,11 @@ struct apic {
unsigned long (*set_apic_id)(unsigned int id);
unsigned long apic_id_mask;
- unsigned int (*cpu_mask_to_apicid)(const struct cpumask *cpumask);
- unsigned int (*cpu_mask_to_apicid_and)(const struct cpumask *cpumask,
- const struct cpumask *andmask);
+ int (*cpu_mask_to_apicid)(const struct cpumask *cpumask,
+ unsigned int *apicid);
+ int (*cpu_mask_to_apicid_and)(const struct cpumask *cpumask,
+ const struct cpumask *andmask,
+ unsigned int *apicid);
/* ipi */
void (*send_IPI_mask)(const struct cpumask *mask, int vector);
@@ -591,29 +593,45 @@ static inline int default_phys_pkg_id(int cpuid_apic, int index_msb)
#endif
-static inline unsigned int
-flat_cpu_mask_to_apicid(const struct cpumask *cpumask)
+static inline int
+__flat_cpu_mask_to_apicid(unsigned long cpu_mask, unsigned int *apicid)
{
- return cpumask_bits(cpumask)[0] & APIC_ALL_CPUS;
+ cpu_mask &= APIC_ALL_CPUS;
+ if (likely(cpu_mask)) {
+ *apicid = (unsigned int)cpu_mask;
+ return 0;
+ } else {
+ return -EINVAL;
+ }
}
-static inline unsigned int
+static inline int
+flat_cpu_mask_to_apicid(const struct cpumask *cpumask,
+ unsigned int *apicid)
+{
+ return __flat_cpu_mask_to_apicid(cpumask_bits(cpumask)[0], apicid);
+}
+
+static inline int
flat_cpu_mask_to_apicid_and(const struct cpumask *cpumask,
- const struct cpumask *andmask)
+ const struct cpumask *andmask,
+ unsigned int *apicid)
{
unsigned long mask1 = cpumask_bits(cpumask)[0];
unsigned long mask2 = cpumask_bits(andmask)[0];
unsigned long mask3 = cpumask_bits(cpu_online_mask)[0];
- return (unsigned int)(mask1 & mask2 & mask3);
+ return __flat_cpu_mask_to_apicid(mask1 & mask2 & mask3, apicid);
}
-extern unsigned int
-default_cpu_mask_to_apicid(const struct cpumask *cpumask);
+extern int
+default_cpu_mask_to_apicid(const struct cpumask *cpumask,
+ unsigned int *apicid);
-extern unsigned int
+extern int
default_cpu_mask_to_apicid_and(const struct cpumask *cpumask,
- const struct cpumask *andmask);
+ const struct cpumask *andmask,
+ unsigned int *apicid);
static inline bool
flat_vector_allocation_domain(int cpu, struct cpumask *retmask)
diff --git a/arch/x86/kernel/apic/apic.c b/arch/x86/kernel/apic/apic.c
index 96a2608..b8d9260 100644
--- a/arch/x86/kernel/apic/apic.c
+++ b/arch/x86/kernel/apic/apic.c
@@ -2123,24 +2123,26 @@ void default_init_apic_ldr(void)
apic_write(APIC_LDR, val);
}
-unsigned int default_cpu_mask_to_apicid(const struct cpumask *cpumask)
+static inline int __default_cpu_to_apicid(int cpu, unsigned int *apicid)
{
- int cpu;
-
- /*
- * We're using fixed IRQ delivery, can only return one phys APIC ID.
- * May as well be the first.
- */
- cpu = cpumask_first(cpumask);
- if (likely((unsigned)cpu < nr_cpu_ids))
- return per_cpu(x86_cpu_to_apicid, cpu);
+ if (likely((unsigned int)cpu < nr_cpu_ids)) {
+ *apicid = per_cpu(x86_cpu_to_apicid, cpu);
+ return 0;
+ } else {
+ return -EINVAL;
+ }
+}
- return BAD_APICID;
+int default_cpu_mask_to_apicid(const struct cpumask *cpumask,
+ unsigned int *apicid)
+{
+ int cpu = cpumask_first(cpumask);
+ return __default_cpu_to_apicid(cpu, apicid);
}
-unsigned int
-default_cpu_mask_to_apicid_and(const struct cpumask *cpumask,
- const struct cpumask *andmask)
+int default_cpu_mask_to_apicid_and(const struct cpumask *cpumask,
+ const struct cpumask *andmask,
+ unsigned int *apicid)
{
int cpu;
@@ -2148,7 +2150,8 @@ default_cpu_mask_to_apicid_and(const struct cpumask *cpumask,
if (cpumask_test_cpu(cpu, cpu_online_mask))
break;
}
- return per_cpu(x86_cpu_to_apicid, cpu);
+
+ return __default_cpu_to_apicid(cpu, apicid);
}
/*
diff --git a/arch/x86/kernel/apic/es7000_32.c b/arch/x86/kernel/apic/es7000_32.c
index 3c42865..515ebb0 100644
--- a/arch/x86/kernel/apic/es7000_32.c
+++ b/arch/x86/kernel/apic/es7000_32.c
@@ -525,7 +525,8 @@ static int es7000_check_phys_apicid_present(int cpu_physical_apicid)
return 1;
}
-static unsigned int es7000_cpu_mask_to_apicid(const struct cpumask *cpumask)
+static int
+es7000_cpu_mask_to_apicid(const struct cpumask *cpumask, unsigned int *dest_id)
{
unsigned int round = 0;
int cpu, uninitialized_var(apicid);
@@ -539,31 +540,33 @@ static unsigned int es7000_cpu_mask_to_apicid(const struct cpumask *cpumask)
if (round && APIC_CLUSTER(apicid) != APIC_CLUSTER(new_apicid)) {
WARN(1, "Not a valid mask!");
- return BAD_APICID;
+ return -EINVAL;
}
apicid = new_apicid;
round++;
}
- return apicid;
+ *dest_id = apicid;
+ return 0;
}
-static unsigned int
+static int
es7000_cpu_mask_to_apicid_and(const struct cpumask *inmask,
- const struct cpumask *andmask)
+ const struct cpumask *andmask,
+ unsigned int *apicid)
{
- int apicid = early_per_cpu(x86_cpu_to_logical_apicid, 0);
+ *apicid = early_per_cpu(x86_cpu_to_logical_apicid, 0);
cpumask_var_t cpumask;
if (!alloc_cpumask_var(&cpumask, GFP_ATOMIC))
- return apicid;
+ return 0;
cpumask_and(cpumask, inmask, andmask);
cpumask_and(cpumask, cpumask, cpu_online_mask);
- apicid = es7000_cpu_mask_to_apicid(cpumask);
+ es7000_cpu_mask_to_apicid(cpumask, apicid);
free_cpumask_var(cpumask);
- return apicid;
+ return 0;
}
static int es7000_phys_pkg_id(int cpuid_apic, int index_msb)
diff --git a/arch/x86/kernel/apic/io_apic.c b/arch/x86/kernel/apic/io_apic.c
index 4061a7d..0deb773 100644
--- a/arch/x86/kernel/apic/io_apic.c
+++ b/arch/x86/kernel/apic/io_apic.c
@@ -1359,7 +1359,14 @@ static void setup_ioapic_irq(unsigned int irq, struct irq_cfg *cfg,
if (assign_irq_vector(irq, cfg, apic->target_cpus()))
return;
- dest = apic->cpu_mask_to_apicid_and(cfg->domain, apic->target_cpus());
+ if (apic->cpu_mask_to_apicid_and(cfg->domain, apic->target_cpus(),
+ &dest)) {
+ pr_warn("Failed to obtain apicid for ioapic %d, pin %d\n",
+ mpc_ioapic_id(attr->ioapic), attr->ioapic_pin);
+ __clear_irq_vector(irq, cfg);
+
+ return;
+ }
apic_printk(APIC_VERBOSE,KERN_DEBUG
"IOAPIC[%d]: Set routing entry (%d-%d -> 0x%x -> "
@@ -1474,6 +1481,7 @@ static void __init setup_timer_IRQ0_pin(unsigned int ioapic_idx,
unsigned int pin, int vector)
{
struct IO_APIC_route_entry entry;
+ unsigned int dest;
if (irq_remapping_enabled)
return;
@@ -1484,9 +1492,12 @@ static void __init setup_timer_IRQ0_pin(unsigned int ioapic_idx,
* We use logical delivery to get the timer IRQ
* to the first CPU.
*/
+ if (unlikely(apic->cpu_mask_to_apicid(apic->target_cpus(), &dest)))
+ dest = BAD_APICID;
+
entry.dest_mode = apic->irq_dest_mode;
entry.mask = 0; /* don't mask IRQ for edge */
- entry.dest = apic->cpu_mask_to_apicid(apic->target_cpus());
+ entry.dest = dest;
entry.delivery_mode = apic->irq_delivery_mode;
entry.polarity = 0;
entry.trigger = 0;
@@ -2245,16 +2256,25 @@ int __ioapic_set_affinity(struct irq_data *data, const struct cpumask *mask,
unsigned int *dest_id)
{
struct irq_cfg *cfg = data->chip_data;
+ unsigned int irq = data->irq;
+ int err;
if (!cpumask_intersects(mask, cpu_online_mask))
- return -1;
+ return -EINVAL;
- if (assign_irq_vector(data->irq, data->chip_data, mask))
- return -1;
+ err = assign_irq_vector(irq, cfg, mask);
+ if (err)
+ return err;
+
+ err = apic->cpu_mask_to_apicid_and(mask, cfg->domain, dest_id);
+ if (err) {
+ if (assign_irq_vector(irq, cfg, data->affinity))
+ pr_err("Failed to recover vector for irq %d\n", irq);
+ return err;
+ }
cpumask_copy(data->affinity, mask);
- *dest_id = apic->cpu_mask_to_apicid_and(mask, cfg->domain);
return 0;
}
@@ -3040,7 +3060,10 @@ static int msi_compose_msg(struct pci_dev *pdev, unsigned int irq,
if (err)
return err;
- dest = apic->cpu_mask_to_apicid_and(cfg->domain, apic->target_cpus());
+ err = apic->cpu_mask_to_apicid_and(cfg->domain,
+ apic->target_cpus(), &dest);
+ if (err)
+ return err;
if (irq_remapped(cfg)) {
compose_remapped_msi_msg(pdev, irq, dest, msg, hpet_id);
@@ -3361,6 +3384,8 @@ static struct irq_chip ht_irq_chip = {
int arch_setup_ht_irq(unsigned int irq, struct pci_dev *dev)
{
struct irq_cfg *cfg;
+ struct ht_irq_msg msg;
+ unsigned dest;
int err;
if (disable_apic)
@@ -3368,36 +3393,37 @@ int arch_setup_ht_irq(unsigned int irq, struct pci_dev *dev)
cfg = irq_cfg(irq);
err = assign_irq_vector(irq, cfg, apic->target_cpus());
- if (!err) {
- struct ht_irq_msg msg;
- unsigned dest;
+ if (err)
+ return err;
- dest = apic->cpu_mask_to_apicid_and(cfg->domain,
- apic->target_cpus());
+ err = apic->cpu_mask_to_apicid_and(cfg->domain,
+ apic->target_cpus(), &dest);
+ if (err)
+ return err;
- msg.address_hi = HT_IRQ_HIGH_DEST_ID(dest);
+ msg.address_hi = HT_IRQ_HIGH_DEST_ID(dest);
- msg.address_lo =
- HT_IRQ_LOW_BASE |
- HT_IRQ_LOW_DEST_ID(dest) |
- HT_IRQ_LOW_VECTOR(cfg->vector) |
- ((apic->irq_dest_mode == 0) ?
- HT_IRQ_LOW_DM_PHYSICAL :
- HT_IRQ_LOW_DM_LOGICAL) |
- HT_IRQ_LOW_RQEOI_EDGE |
- ((apic->irq_delivery_mode != dest_LowestPrio) ?
- HT_IRQ_LOW_MT_FIXED :
- HT_IRQ_LOW_MT_ARBITRATED) |
- HT_IRQ_LOW_IRQ_MASKED;
+ msg.address_lo =
+ HT_IRQ_LOW_BASE |
+ HT_IRQ_LOW_DEST_ID(dest) |
+ HT_IRQ_LOW_VECTOR(cfg->vector) |
+ ((apic->irq_dest_mode == 0) ?
+ HT_IRQ_LOW_DM_PHYSICAL :
+ HT_IRQ_LOW_DM_LOGICAL) |
+ HT_IRQ_LOW_RQEOI_EDGE |
+ ((apic->irq_delivery_mode != dest_LowestPrio) ?
+ HT_IRQ_LOW_MT_FIXED :
+ HT_IRQ_LOW_MT_ARBITRATED) |
+ HT_IRQ_LOW_IRQ_MASKED;
- write_ht_irq_msg(irq, &msg);
+ write_ht_irq_msg(irq, &msg);
- irq_set_chip_and_handler_name(irq, &ht_irq_chip,
- handle_edge_irq, "edge");
+ irq_set_chip_and_handler_name(irq, &ht_irq_chip,
+ handle_edge_irq, "edge");
- dev_printk(KERN_DEBUG, &dev->dev, "irq %d for HT\n", irq);
- }
- return err;
+ dev_printk(KERN_DEBUG, &dev->dev, "irq %d for HT\n", irq);
+
+ return 0;
}
#endif /* CONFIG_HT_IRQ */
diff --git a/arch/x86/kernel/apic/numaq_32.c b/arch/x86/kernel/apic/numaq_32.c
index eb2d466..2b55514 100644
--- a/arch/x86/kernel/apic/numaq_32.c
+++ b/arch/x86/kernel/apic/numaq_32.c
@@ -406,16 +406,20 @@ static inline int numaq_check_phys_apicid_present(int phys_apicid)
* We use physical apicids here, not logical, so just return the default
* physical broadcast to stop people from breaking us
*/
-static unsigned int numaq_cpu_mask_to_apicid(const struct cpumask *cpumask)
+static int
+numaq_cpu_mask_to_apicid(const struct cpumask *cpumask, unsigned int *apicid)
{
- return 0x0F;
+ *apicid = 0x0F;
+ return 0;
}
-static inline unsigned int
+static int
numaq_cpu_mask_to_apicid_and(const struct cpumask *cpumask,
- const struct cpumask *andmask)
+ const struct cpumask *andmask,
+ unsigned int *apicid)
{
- return 0x0F;
+ *apicid = 0x0F;
+ return 0;
}
/* No NUMA-Q box has a HT CPU, but it can't hurt to use the default code. */
diff --git a/arch/x86/kernel/apic/summit_32.c b/arch/x86/kernel/apic/summit_32.c
index 35d254c..5766d84 100644
--- a/arch/x86/kernel/apic/summit_32.c
+++ b/arch/x86/kernel/apic/summit_32.c
@@ -263,7 +263,8 @@ static int summit_check_phys_apicid_present(int physical_apicid)
return 1;
}
-static unsigned int summit_cpu_mask_to_apicid(const struct cpumask *cpumask)
+static int
+summit_cpu_mask_to_apicid(const struct cpumask *cpumask, unsigned int *dest_id)
{
unsigned int round = 0;
int cpu, apicid = 0;
@@ -276,30 +277,33 @@ static unsigned int summit_cpu_mask_to_apicid(const struct cpumask *cpumask)
if (round && APIC_CLUSTER(apicid) != APIC_CLUSTER(new_apicid)) {
printk("%s: Not a valid mask!\n", __func__);
- return BAD_APICID;
+ return -EINVAL;
}
apicid |= new_apicid;
round++;
}
- return apicid;
+ *dest_id = apicid;
+ return 0;
}
-static unsigned int summit_cpu_mask_to_apicid_and(const struct cpumask *inmask,
- const struct cpumask *andmask)
+static int
+summit_cpu_mask_to_apicid_and(const struct cpumask *inmask,
+ const struct cpumask *andmask,
+ unsigned int *apicid)
{
- int apicid = early_per_cpu(x86_cpu_to_logical_apicid, 0);
+ *apicid = early_per_cpu(x86_cpu_to_logical_apicid, 0);
cpumask_var_t cpumask;
if (!alloc_cpumask_var(&cpumask, GFP_ATOMIC))
- return apicid;
+ return 0;
cpumask_and(cpumask, inmask, andmask);
cpumask_and(cpumask, cpumask, cpu_online_mask);
- apicid = summit_cpu_mask_to_apicid(cpumask);
+ summit_cpu_mask_to_apicid(cpumask, apicid);
free_cpumask_var(cpumask);
- return apicid;
+ return 0;
}
/*
diff --git a/arch/x86/kernel/apic/x2apic_cluster.c b/arch/x86/kernel/apic/x2apic_cluster.c
index 612622c..5f86f79 100644
--- a/arch/x86/kernel/apic/x2apic_cluster.c
+++ b/arch/x86/kernel/apic/x2apic_cluster.c
@@ -96,24 +96,26 @@ static void x2apic_send_IPI_all(int vector)
__x2apic_send_IPI_mask(cpu_online_mask, vector, APIC_DEST_ALLINC);
}
-static unsigned int x2apic_cpu_mask_to_apicid(const struct cpumask *cpumask)
+static int
+x2apic_cpu_mask_to_apicid(const struct cpumask *cpumask, unsigned int *apicid)
{
int cpu = cpumask_first(cpumask);
- u32 dest = 0;
int i;
- if (cpu > nr_cpu_ids)
- return BAD_APICID;
+ if (cpu >= nr_cpu_ids)
+ return -EINVAL;
+ *apicid = 0;
for_each_cpu_and(i, cpumask, per_cpu(cpus_in_cluster, cpu))
- dest |= per_cpu(x86_cpu_to_logical_apicid, i);
+ *apicid |= per_cpu(x86_cpu_to_logical_apicid, i);
- return dest;
+ return 0;
}
-static unsigned int
+static int
x2apic_cpu_mask_to_apicid_and(const struct cpumask *cpumask,
- const struct cpumask *andmask)
+ const struct cpumask *andmask,
+ unsigned int *apicid)
{
u32 dest = 0;
u16 cluster;
@@ -128,7 +130,7 @@ x2apic_cpu_mask_to_apicid_and(const struct cpumask *cpumask,
}
if (!dest)
- return BAD_APICID;
+ return -EINVAL;
for_each_cpu_and(i, cpumask, andmask) {
if (!cpumask_test_cpu(i, cpu_online_mask))
@@ -138,7 +140,9 @@ x2apic_cpu_mask_to_apicid_and(const struct cpumask *cpumask,
dest |= per_cpu(x86_cpu_to_logical_apicid, i);
}
- return dest;
+ *apicid = dest;
+
+ return 0;
}
static void init_x2apic_ldr(void)
diff --git a/arch/x86/kernel/apic/x2apic_uv_x.c b/arch/x86/kernel/apic/x2apic_uv_x.c
index df89a7d..2f3030f 100644
--- a/arch/x86/kernel/apic/x2apic_uv_x.c
+++ b/arch/x86/kernel/apic/x2apic_uv_x.c
@@ -269,23 +269,31 @@ static void uv_init_apic_ldr(void)
{
}
-static unsigned int uv_cpu_mask_to_apicid(const struct cpumask *cpumask)
+static inline int __uv_cpu_to_apicid(int cpu, unsigned int *apicid)
+{
+ if (likely((unsigned int)cpu < nr_cpu_ids)) {
+ *apicid = per_cpu(x86_cpu_to_apicid, cpu) | uv_apicid_hibits;
+ return 0;
+ } else {
+ return -EINVAL;
+ }
+}
+
+static int
+uv_cpu_mask_to_apicid(const struct cpumask *cpumask, unsigned int *apicid)
{
/*
* We're using fixed IRQ delivery, can only return one phys APIC ID.
* May as well be the first.
*/
int cpu = cpumask_first(cpumask);
-
- if ((unsigned)cpu < nr_cpu_ids)
- return per_cpu(x86_cpu_to_apicid, cpu) | uv_apicid_hibits;
- else
- return BAD_APICID;
+ return __uv_cpu_to_apicid(cpu, apicid);
}
-static unsigned int
+static int
uv_cpu_mask_to_apicid_and(const struct cpumask *cpumask,
- const struct cpumask *andmask)
+ const struct cpumask *andmask,
+ unsigned int *apicid)
{
int cpu;
@@ -297,7 +305,8 @@ uv_cpu_mask_to_apicid_and(const struct cpumask *cpumask,
if (cpumask_test_cpu(cpu, cpu_online_mask))
break;
}
- return per_cpu(x86_cpu_to_apicid, cpu) | uv_apicid_hibits;
+
+ return __uv_cpu_to_apicid(cpu, apicid);
}
static unsigned int x2apic_get_apic_id(unsigned long x)
diff --git a/arch/x86/platform/uv/uv_irq.c b/arch/x86/platform/uv/uv_irq.c
index f25c276..dd1ff39 100644
--- a/arch/x86/platform/uv/uv_irq.c
+++ b/arch/x86/platform/uv/uv_irq.c
@@ -135,6 +135,7 @@ arch_enable_uv_irq(char *irq_name, unsigned int irq, int cpu, int mmr_blade,
unsigned long mmr_value;
struct uv_IO_APIC_route_entry *entry;
int mmr_pnode, err;
+ unsigned int dest;
BUILD_BUG_ON(sizeof(struct uv_IO_APIC_route_entry) !=
sizeof(unsigned long));
@@ -143,6 +144,10 @@ arch_enable_uv_irq(char *irq_name, unsigned int irq, int cpu, int mmr_blade,
if (err != 0)
return err;
+ err = apic->cpu_mask_to_apicid(eligible_cpu, &dest);
+ if (err != 0)
+ return err;
+
if (limit == UV_AFFINITY_CPU)
irq_set_status_flags(irq, IRQ_NO_BALANCING);
else
@@ -159,7 +164,7 @@ arch_enable_uv_irq(char *irq_name, unsigned int irq, int cpu, int mmr_blade,
entry->polarity = 0;
entry->trigger = 0;
entry->mask = 0;
- entry->dest = apic->cpu_mask_to_apicid(eligible_cpu);
+ entry->dest = dest;
mmr_pnode = uv_blade_to_pnode(mmr_blade);
uv_write_global_mmr64(mmr_pnode, mmr_offset, mmr_value);
diff --git a/drivers/iommu/intel_irq_remapping.c b/drivers/iommu/intel_irq_remapping.c
index 6d34706..dafbad0 100644
--- a/drivers/iommu/intel_irq_remapping.c
+++ b/drivers/iommu/intel_irq_remapping.c
@@ -924,6 +924,7 @@ intel_ioapic_set_affinity(struct irq_data *data, const struct cpumask *mask,
struct irq_cfg *cfg = data->chip_data;
unsigned int dest, irq = data->irq;
struct irte irte;
+ int err;
if (!cpumask_intersects(mask, cpu_online_mask))
return -EINVAL;
@@ -931,10 +932,16 @@ intel_ioapic_set_affinity(struct irq_data *data, const struct cpumask *mask,
if (get_irte(irq, &irte))
return -EBUSY;
- if (assign_irq_vector(irq, cfg, mask))
- return -EBUSY;
+ err = assign_irq_vector(irq, cfg, mask);
+ if (err)
+ return err;
- dest = apic->cpu_mask_to_apicid_and(cfg->domain, mask);
+ err = apic->cpu_mask_to_apicid_and(cfg->domain, mask, &dest);
+ if (err) {
+ if (assign_irq_vector(irq, cfg, data->affinity));
+ pr_err("Failed to recover vector for irq %d\n", irq);
+ return err;
+ }
irte.vector = cfg->vector;
irte.dest_id = IRTE_DEST(dest);
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH 8/8] x86/apic: Make cpu_mask_to_apicid() operations check cpu_online_mask
2012-06-06 8:22 ` Ingo Molnar
` (4 preceding siblings ...)
2012-06-07 13:15 ` [PATCH 7/8] x86/apic: Make cpu_mask_to_apicid() operations return error code Alexander Gordeev
@ 2012-06-07 13:16 ` Alexander Gordeev
2012-06-08 14:52 ` [tip:x86/apic] " tip-bot for Alexander Gordeev
5 siblings, 1 reply; 26+ messages in thread
From: Alexander Gordeev @ 2012-06-07 13:16 UTC (permalink / raw)
To: Ingo Molnar; +Cc: linux-kernel, x86, Suresh Siddha, Yinghai Lu
Currently cpu_mask_to_apicid() should not get a offline CPU with the
cpumask. Otherwise some apic drivers might try to access non-existent
per-cpu variables (i.e. x2apic). In that regard cpu_mask_to_apicid() and
cpu_mask_to_apicid_and() operations are inconsistent.
This fix makes the two operations do not rely on calling functions and
always return the apicid for only online CPUs. As result, the meaning
and implementations of cpu_mask_to_apicid() and cpu_mask_to_apicid_and()
operations become straight.
Signed-off-by: Alexander Gordeev <agordeev@redhat.com>
---
arch/x86/include/asm/apic.h | 6 ++----
arch/x86/kernel/apic/apic.c | 2 +-
arch/x86/kernel/apic/es7000_32.c | 3 +--
arch/x86/kernel/apic/summit_32.c | 3 +--
arch/x86/kernel/apic/x2apic_cluster.c | 2 +-
arch/x86/kernel/apic/x2apic_uv_x.c | 2 +-
6 files changed, 7 insertions(+), 11 deletions(-)
diff --git a/arch/x86/include/asm/apic.h b/arch/x86/include/asm/apic.h
index ae91f9c..1ed3eea 100644
--- a/arch/x86/include/asm/apic.h
+++ b/arch/x86/include/asm/apic.h
@@ -596,7 +596,7 @@ static inline int default_phys_pkg_id(int cpuid_apic, int index_msb)
static inline int
__flat_cpu_mask_to_apicid(unsigned long cpu_mask, unsigned int *apicid)
{
- cpu_mask &= APIC_ALL_CPUS;
+ cpu_mask = cpu_mask & APIC_ALL_CPUS & cpumask_bits(cpu_online_mask)[0];
if (likely(cpu_mask)) {
*apicid = (unsigned int)cpu_mask;
return 0;
@@ -619,9 +619,7 @@ flat_cpu_mask_to_apicid_and(const struct cpumask *cpumask,
{
unsigned long mask1 = cpumask_bits(cpumask)[0];
unsigned long mask2 = cpumask_bits(andmask)[0];
- unsigned long mask3 = cpumask_bits(cpu_online_mask)[0];
-
- return __flat_cpu_mask_to_apicid(mask1 & mask2 & mask3, apicid);
+ return __flat_cpu_mask_to_apicid(mask1 & mask2, apicid);
}
extern int
diff --git a/arch/x86/kernel/apic/apic.c b/arch/x86/kernel/apic/apic.c
index b8d9260..7e9bbe7 100644
--- a/arch/x86/kernel/apic/apic.c
+++ b/arch/x86/kernel/apic/apic.c
@@ -2136,7 +2136,7 @@ static inline int __default_cpu_to_apicid(int cpu, unsigned int *apicid)
int default_cpu_mask_to_apicid(const struct cpumask *cpumask,
unsigned int *apicid)
{
- int cpu = cpumask_first(cpumask);
+ int cpu = cpumask_first_and(cpumask, cpu_online_mask);
return __default_cpu_to_apicid(cpu, apicid);
}
diff --git a/arch/x86/kernel/apic/es7000_32.c b/arch/x86/kernel/apic/es7000_32.c
index 515ebb0..b35cfb9 100644
--- a/arch/x86/kernel/apic/es7000_32.c
+++ b/arch/x86/kernel/apic/es7000_32.c
@@ -534,7 +534,7 @@ es7000_cpu_mask_to_apicid(const struct cpumask *cpumask, unsigned int *dest_id)
/*
* The cpus in the mask must all be on the apic cluster.
*/
- for_each_cpu(cpu, cpumask) {
+ for_each_cpu_and(cpu, cpumask, cpu_online_mask) {
int new_apicid = early_per_cpu(x86_cpu_to_logical_apicid, cpu);
if (round && APIC_CLUSTER(apicid) != APIC_CLUSTER(new_apicid)) {
@@ -561,7 +561,6 @@ es7000_cpu_mask_to_apicid_and(const struct cpumask *inmask,
return 0;
cpumask_and(cpumask, inmask, andmask);
- cpumask_and(cpumask, cpumask, cpu_online_mask);
es7000_cpu_mask_to_apicid(cpumask, apicid);
free_cpumask_var(cpumask);
diff --git a/arch/x86/kernel/apic/summit_32.c b/arch/x86/kernel/apic/summit_32.c
index 5766d84..79d360f 100644
--- a/arch/x86/kernel/apic/summit_32.c
+++ b/arch/x86/kernel/apic/summit_32.c
@@ -272,7 +272,7 @@ summit_cpu_mask_to_apicid(const struct cpumask *cpumask, unsigned int *dest_id)
/*
* The cpus in the mask must all be on the apic cluster.
*/
- for_each_cpu(cpu, cpumask) {
+ for_each_cpu_and(cpu, cpumask, cpu_online_mask) {
int new_apicid = early_per_cpu(x86_cpu_to_logical_apicid, cpu);
if (round && APIC_CLUSTER(apicid) != APIC_CLUSTER(new_apicid)) {
@@ -298,7 +298,6 @@ summit_cpu_mask_to_apicid_and(const struct cpumask *inmask,
return 0;
cpumask_and(cpumask, inmask, andmask);
- cpumask_and(cpumask, cpumask, cpu_online_mask);
summit_cpu_mask_to_apicid(cpumask, apicid);
free_cpumask_var(cpumask);
diff --git a/arch/x86/kernel/apic/x2apic_cluster.c b/arch/x86/kernel/apic/x2apic_cluster.c
index 5f86f79..23a46cf 100644
--- a/arch/x86/kernel/apic/x2apic_cluster.c
+++ b/arch/x86/kernel/apic/x2apic_cluster.c
@@ -99,7 +99,7 @@ static void x2apic_send_IPI_all(int vector)
static int
x2apic_cpu_mask_to_apicid(const struct cpumask *cpumask, unsigned int *apicid)
{
- int cpu = cpumask_first(cpumask);
+ int cpu = cpumask_first_and(cpumask, cpu_online_mask);
int i;
if (cpu >= nr_cpu_ids)
diff --git a/arch/x86/kernel/apic/x2apic_uv_x.c b/arch/x86/kernel/apic/x2apic_uv_x.c
index 2f3030f..307aa07 100644
--- a/arch/x86/kernel/apic/x2apic_uv_x.c
+++ b/arch/x86/kernel/apic/x2apic_uv_x.c
@@ -286,7 +286,7 @@ uv_cpu_mask_to_apicid(const struct cpumask *cpumask, unsigned int *apicid)
* We're using fixed IRQ delivery, can only return one phys APIC ID.
* May as well be the first.
*/
- int cpu = cpumask_first(cpumask);
+ int cpu = cpumask_first_and(cpumask, cpu_online_mask);
return __uv_cpu_to_apicid(cpu, apicid);
}
--
1.7.7.6
--
Regards,
Alexander Gordeev
agordeev@redhat.com
^ permalink raw reply related [flat|nested] 26+ messages in thread* [tip:x86/apic] x86/apic: Make cpu_mask_to_apicid() operations check cpu_online_mask
2012-06-07 13:16 ` [PATCH 8/8] x86/apic: Make cpu_mask_to_apicid() operations check cpu_online_mask Alexander Gordeev
@ 2012-06-08 14:52 ` tip-bot for Alexander Gordeev
0 siblings, 0 replies; 26+ messages in thread
From: tip-bot for Alexander Gordeev @ 2012-06-08 14:52 UTC (permalink / raw)
To: linux-tip-commits
Cc: linux-kernel, agordeev, hpa, mingo, yinghai, suresh.b.siddha,
tglx
Commit-ID: 4988a40c3981212fa8c64da68722affc1cb6697a
Gitweb: http://git.kernel.org/tip/4988a40c3981212fa8c64da68722affc1cb6697a
Author: Alexander Gordeev <agordeev@redhat.com>
AuthorDate: Thu, 7 Jun 2012 15:16:25 +0200
Committer: Ingo Molnar <mingo@kernel.org>
CommitDate: Fri, 8 Jun 2012 11:44:30 +0200
x86/apic: Make cpu_mask_to_apicid() operations check cpu_online_mask
Currently cpu_mask_to_apicid() should not get a offline CPU with
the cpumask. Otherwise some apic drivers might try to access
non-existent per-cpu variables (i.e. x2apic). In that regard
cpu_mask_to_apicid() and cpu_mask_to_apicid_and() operations are
inconsistent.
This fix makes the two operations do not rely on calling
functions and always return the apicid for only online CPUs. As
result, the meaning and implementations of cpu_mask_to_apicid()
and cpu_mask_to_apicid_and() operations become straight.
Signed-off-by: Alexander Gordeev <agordeev@redhat.com>
Acked-by: Suresh Siddha <suresh.b.siddha@intel.com>
Cc: Yinghai Lu <yinghai@kernel.org>
Link: http://lkml.kernel.org/r/20120607131624.GG4759@dhcp-26-207.brq.redhat.com
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
arch/x86/include/asm/apic.h | 6 ++----
arch/x86/kernel/apic/apic.c | 2 +-
arch/x86/kernel/apic/es7000_32.c | 3 +--
arch/x86/kernel/apic/summit_32.c | 3 +--
arch/x86/kernel/apic/x2apic_cluster.c | 2 +-
arch/x86/kernel/apic/x2apic_uv_x.c | 2 +-
6 files changed, 7 insertions(+), 11 deletions(-)
diff --git a/arch/x86/include/asm/apic.h b/arch/x86/include/asm/apic.h
index ae91f9c..1ed3eea 100644
--- a/arch/x86/include/asm/apic.h
+++ b/arch/x86/include/asm/apic.h
@@ -596,7 +596,7 @@ static inline int default_phys_pkg_id(int cpuid_apic, int index_msb)
static inline int
__flat_cpu_mask_to_apicid(unsigned long cpu_mask, unsigned int *apicid)
{
- cpu_mask &= APIC_ALL_CPUS;
+ cpu_mask = cpu_mask & APIC_ALL_CPUS & cpumask_bits(cpu_online_mask)[0];
if (likely(cpu_mask)) {
*apicid = (unsigned int)cpu_mask;
return 0;
@@ -619,9 +619,7 @@ flat_cpu_mask_to_apicid_and(const struct cpumask *cpumask,
{
unsigned long mask1 = cpumask_bits(cpumask)[0];
unsigned long mask2 = cpumask_bits(andmask)[0];
- unsigned long mask3 = cpumask_bits(cpu_online_mask)[0];
-
- return __flat_cpu_mask_to_apicid(mask1 & mask2 & mask3, apicid);
+ return __flat_cpu_mask_to_apicid(mask1 & mask2, apicid);
}
extern int
diff --git a/arch/x86/kernel/apic/apic.c b/arch/x86/kernel/apic/apic.c
index b8d9260..7e9bbe7 100644
--- a/arch/x86/kernel/apic/apic.c
+++ b/arch/x86/kernel/apic/apic.c
@@ -2136,7 +2136,7 @@ static inline int __default_cpu_to_apicid(int cpu, unsigned int *apicid)
int default_cpu_mask_to_apicid(const struct cpumask *cpumask,
unsigned int *apicid)
{
- int cpu = cpumask_first(cpumask);
+ int cpu = cpumask_first_and(cpumask, cpu_online_mask);
return __default_cpu_to_apicid(cpu, apicid);
}
diff --git a/arch/x86/kernel/apic/es7000_32.c b/arch/x86/kernel/apic/es7000_32.c
index 515ebb0..b35cfb9 100644
--- a/arch/x86/kernel/apic/es7000_32.c
+++ b/arch/x86/kernel/apic/es7000_32.c
@@ -534,7 +534,7 @@ es7000_cpu_mask_to_apicid(const struct cpumask *cpumask, unsigned int *dest_id)
/*
* The cpus in the mask must all be on the apic cluster.
*/
- for_each_cpu(cpu, cpumask) {
+ for_each_cpu_and(cpu, cpumask, cpu_online_mask) {
int new_apicid = early_per_cpu(x86_cpu_to_logical_apicid, cpu);
if (round && APIC_CLUSTER(apicid) != APIC_CLUSTER(new_apicid)) {
@@ -561,7 +561,6 @@ es7000_cpu_mask_to_apicid_and(const struct cpumask *inmask,
return 0;
cpumask_and(cpumask, inmask, andmask);
- cpumask_and(cpumask, cpumask, cpu_online_mask);
es7000_cpu_mask_to_apicid(cpumask, apicid);
free_cpumask_var(cpumask);
diff --git a/arch/x86/kernel/apic/summit_32.c b/arch/x86/kernel/apic/summit_32.c
index 5766d84..79d360f 100644
--- a/arch/x86/kernel/apic/summit_32.c
+++ b/arch/x86/kernel/apic/summit_32.c
@@ -272,7 +272,7 @@ summit_cpu_mask_to_apicid(const struct cpumask *cpumask, unsigned int *dest_id)
/*
* The cpus in the mask must all be on the apic cluster.
*/
- for_each_cpu(cpu, cpumask) {
+ for_each_cpu_and(cpu, cpumask, cpu_online_mask) {
int new_apicid = early_per_cpu(x86_cpu_to_logical_apicid, cpu);
if (round && APIC_CLUSTER(apicid) != APIC_CLUSTER(new_apicid)) {
@@ -298,7 +298,6 @@ summit_cpu_mask_to_apicid_and(const struct cpumask *inmask,
return 0;
cpumask_and(cpumask, inmask, andmask);
- cpumask_and(cpumask, cpumask, cpu_online_mask);
summit_cpu_mask_to_apicid(cpumask, apicid);
free_cpumask_var(cpumask);
diff --git a/arch/x86/kernel/apic/x2apic_cluster.c b/arch/x86/kernel/apic/x2apic_cluster.c
index 5f86f79..23a46cf 100644
--- a/arch/x86/kernel/apic/x2apic_cluster.c
+++ b/arch/x86/kernel/apic/x2apic_cluster.c
@@ -99,7 +99,7 @@ static void x2apic_send_IPI_all(int vector)
static int
x2apic_cpu_mask_to_apicid(const struct cpumask *cpumask, unsigned int *apicid)
{
- int cpu = cpumask_first(cpumask);
+ int cpu = cpumask_first_and(cpumask, cpu_online_mask);
int i;
if (cpu >= nr_cpu_ids)
diff --git a/arch/x86/kernel/apic/x2apic_uv_x.c b/arch/x86/kernel/apic/x2apic_uv_x.c
index 2f3030f..307aa07 100644
--- a/arch/x86/kernel/apic/x2apic_uv_x.c
+++ b/arch/x86/kernel/apic/x2apic_uv_x.c
@@ -286,7 +286,7 @@ uv_cpu_mask_to_apicid(const struct cpumask *cpumask, unsigned int *apicid)
* We're using fixed IRQ delivery, can only return one phys APIC ID.
* May as well be the first.
*/
- int cpu = cpumask_first(cpumask);
+ int cpu = cpumask_first_and(cpumask, cpu_online_mask);
return __uv_cpu_to_apicid(cpu, apicid);
}
^ permalink raw reply related [flat|nested] 26+ messages in thread