[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <1DC35A28-DEBC-4A46-AC35-3AADD23AA40D@vmware.com>
Date: Wed, 3 Jul 2019 18:06:51 +0000
From: Nadav Amit <namit@...are.com>
To: Thomas Gleixner <tglx@...utronix.de>
CC: LKML <linux-kernel@...r.kernel.org>,
"x86@...nel.org" <x86@...nel.org>,
Ricardo Neri <ricardo.neri-calderon@...ux.intel.com>,
Stephane Eranian <eranian@...gle.com>,
Feng Tang <feng.tang@...el.com>
Subject: Re: [patch 16/18] x86/apic: Convert 32bit to IPI shorthand static key
> On Jul 3, 2019, at 3:54 AM, Thomas Gleixner <tglx@...utronix.de> wrote:
>
> Broadcast now depends on the fact that all present CPUs have been booted
> once so handling broadcast IPIs is not doing any harm. In case that a CPU
> is offline, it does not react to regular IPIs and the NMI handler returns
> early.
>
> Signed-off-by: Thomas Gleixner <tglx@...utronix.de>
> ---
> arch/x86/kernel/apic/ipi.c | 12 +++---------
> 1 file changed, 3 insertions(+), 9 deletions(-)
>
> --- a/arch/x86/kernel/apic/ipi.c
> +++ b/arch/x86/kernel/apic/ipi.c
> @@ -8,13 +8,7 @@
> DEFINE_STATIC_KEY_FALSE(apic_use_ipi_shorthand);
>
> #ifdef CONFIG_SMP
> -#ifdef CONFIG_HOTPLUG_CPU
> -#define DEFAULT_SEND_IPI (1)
> -#else
> -#define DEFAULT_SEND_IPI (0)
> -#endif
> -
> -static int apic_ipi_shorthand_off __ro_after_init = DEFAULT_SEND_IPI;
> +static int apic_ipi_shorthand_off __ro_after_init;
>
> static __init int apic_ipi_shorthand(char *str)
> {
> @@ -250,7 +244,7 @@ void default_send_IPI_allbutself(int vec
> if (num_online_cpus() < 2)
> return;
>
> - if (apic_ipi_shorthand_off || vector == NMI_VECTOR) {
> + if (static_branch_likely(&apic_use_ipi_shorthand)) {
> apic->send_IPI_mask_allbutself(cpu_online_mask, vector);
> } else {
> __default_send_IPI_shortcut(APIC_DEST_ALLBUT, vector);
> @@ -259,7 +253,7 @@ void default_send_IPI_allbutself(int vec
>
> void default_send_IPI_all(int vector)
> {
> - if (apic_ipi_shorthand_off || vector == NMI_VECTOR) {
> + if (static_branch_likely(&apic_use_ipi_shorthand)) {
> apic->send_IPI_mask(cpu_online_mask, vector);
> } else {
> __default_send_IPI_shortcut(APIC_DEST_ALLINC, vector);
It may be better to check the static-key in native_send_call_func_ipi() (and
other callers if there are any), and remove all the other checks in
default_send_IPI_all(), x2apic_send_IPI_mask_allbutself(), etc.
This would allow to remove potentially unnecessary checks in
native_send_call_func_ipi()I also have this patch I still did not send to
slightly improve the test in native_send_call_func_ipi().
-- >8 --
From: Nadav Amit <namit@...are.com>
Date: Fri, 7 Jun 2019 15:11:44 -0700
Subject: [PATCH] x86/smp: Better check of allbutself
Introduce for_each_cpu_and_not() for this matter.
Signed-off-by: Nadav Amit <namit@...are.com>
---
arch/x86/kernel/smp.c | 22 +++++++++++-----------
include/asm-generic/bitops/find.h | 17 +++++++++++++++++
include/linux/cpumask.h | 17 +++++++++++++++++
lib/cpumask.c | 20 ++++++++++++++++++++
lib/find_bit.c | 21 ++++++++++++++++++---
5 files changed, 83 insertions(+), 14 deletions(-)
diff --git a/arch/x86/kernel/smp.c b/arch/x86/kernel/smp.c
index 96421f97e75c..7972ab593397 100644
--- a/arch/x86/kernel/smp.c
+++ b/arch/x86/kernel/smp.c
@@ -136,23 +136,23 @@ void native_send_call_func_single_ipi(int cpu)
void native_send_call_func_ipi(const struct cpumask *mask)
{
- cpumask_var_t allbutself;
-
- if (!alloc_cpumask_var(&allbutself, GFP_ATOMIC)) {
- apic->send_IPI_mask(mask, CALL_FUNCTION_VECTOR);
- return;
+ int cpu, this_cpu = smp_processor_id();
+ bool allbutself = true;
+ bool self = false;
+
+ for_each_cpu_and_not(cpu, cpu_online_mask, mask) {
+ if (cpu != this_cpu) {
+ allbutself = false;
+ break;
+ }
+ self = true;
}
- cpumask_copy(allbutself, cpu_online_mask);
- __cpumask_clear_cpu(smp_processor_id(), allbutself);
-
- if (cpumask_equal(mask, allbutself) &&
+ if (allbutself && !self &&
cpumask_equal(cpu_online_mask, cpu_callout_mask))
apic->send_IPI_allbutself(CALL_FUNCTION_VECTOR);
else
apic->send_IPI_mask(mask, CALL_FUNCTION_VECTOR);
-
- free_cpumask_var(allbutself);
}
static int smp_stop_nmi_callback(unsigned int val, struct pt_regs *regs)
diff --git a/include/asm-generic/bitops/find.h b/include/asm-generic/bitops/find.h
index 8a1ee10014de..e5f19eec2737 100644
--- a/include/asm-generic/bitops/find.h
+++ b/include/asm-generic/bitops/find.h
@@ -32,6 +32,23 @@ extern unsigned long find_next_and_bit(const unsigned long *addr1,
unsigned long offset);
#endif
+#ifndef find_next_and_bit
+/**
+ * find_next_and_not_bit - find the next set bit in the the first region
+ * which is clear on the second
+ * @addr1: The first address to base the search on
+ * @addr2: The second address to base the search on
+ * @offset: The bitnumber to start searching at
+ * @size: The bitmap size in bits
+ *
+ * Returns the bit number for the next set bit
+ * If no bits are set, returns @size.
+ */
+extern unsigned long find_next_and_not_bit(const unsigned long *addr1,
+ const unsigned long *addr2, unsigned long size,
+ unsigned long offset);
+#endif
+
#ifndef find_next_zero_bit
/**
* find_next_zero_bit - find the next cleared bit in a memory region
diff --git a/include/linux/cpumask.h b/include/linux/cpumask.h
index 693124900f0a..4648add54fad 100644
--- a/include/linux/cpumask.h
+++ b/include/linux/cpumask.h
@@ -229,6 +229,7 @@ static inline unsigned int cpumask_next_zero(int n, const struct cpumask *srcp)
}
int cpumask_next_and(int n, const struct cpumask *, const struct cpumask *);
+int __pure cpumask_next_and_not(int n, const struct cpumask *, const struct cpumask *);
int cpumask_any_but(const struct cpumask *mask, unsigned int cpu);
unsigned int cpumask_local_spread(unsigned int i, int node);
@@ -291,6 +292,22 @@ extern int cpumask_next_wrap(int n, const struct cpumask *mask, int start, bool
for ((cpu) = -1; \
(cpu) = cpumask_next_and((cpu), (mask), (and)), \
(cpu) < nr_cpu_ids;)
+
+
+/**
+ * for_each_cpu_and_not - iterate over every cpu in @mask & ~@..._not
+ * @cpu: the (optionally unsigned) integer iterator
+ * @mask: the first cpumask pointer
+ * @and_not: the second cpumask pointer
+ *
+ * After the loop, cpu is >= nr_cpu_ids.
+ */
+#define for_each_cpu_and_not(cpu, mask, and_not) \
+ for ((cpu) = -1; \
+ (cpu) = cpumask_next_and_not((cpu), (mask), (and_not)), \
+ (cpu) < nr_cpu_ids;)
+
+
#endif /* SMP */
#define CPU_BITS_NONE \
diff --git a/lib/cpumask.c b/lib/cpumask.c
index 0cb672eb107c..59c98f55c308 100644
--- a/lib/cpumask.c
+++ b/lib/cpumask.c
@@ -42,6 +42,26 @@ int cpumask_next_and(int n, const struct cpumask *src1p,
}
EXPORT_SYMBOL(cpumask_next_and);
+/**
+ * cpumask_next_and_not - get the next cpu in *src1p & ~*src2p
+ * @n: the cpu prior to the place to search (ie. return will be > @n)
+ * @src1p: the first cpumask pointer
+ * @src2p: the second cpumask pointer
+ *
+ * Returns >= nr_cpu_ids if no further cpus set in both.
+ */
+int cpumask_next_and_not(int n, const struct cpumask *src1p,
+ const struct cpumask *src2p)
+{
+ /* -1 is a legal arg here. */
+ if (n != -1)
+ cpumask_check(n);
+ return find_next_and_not_bit(cpumask_bits(src1p), cpumask_bits(src2p),
+ nr_cpumask_bits, n + 1);
+}
+EXPORT_SYMBOL(cpumask_next_and_not);
+
+
/**
* cpumask_any_but - return a "random" in a cpumask, but not this one.
* @mask: the cpumask to search
diff --git a/lib/find_bit.c b/lib/find_bit.c
index 5c51eb45178a..7bd2c567287e 100644
--- a/lib/find_bit.c
+++ b/lib/find_bit.c
@@ -23,7 +23,7 @@
/*
* This is a common helper function for find_next_bit, find_next_zero_bit, and
* find_next_and_bit. The differences are:
- * - The "invert" argument, which is XORed with each fetched word before
+ * - The "invert" argument, which is XORed with each fetched first word before
* searching it for one bits.
* - The optional "addr2", which is anded with "addr1" if present.
*/
@@ -37,9 +37,9 @@ static inline unsigned long _find_next_bit(const unsigned long *addr1,
return nbits;
tmp = addr1[start / BITS_PER_LONG];
+ tmp ^= invert;
if (addr2)
tmp &= addr2[start / BITS_PER_LONG];
- tmp ^= invert;
/* Handle 1st word. */
tmp &= BITMAP_FIRST_WORD_MASK(start);
@@ -51,9 +51,9 @@ static inline unsigned long _find_next_bit(const unsigned long *addr1,
return nbits;
tmp = addr1[start / BITS_PER_LONG];
+ tmp ^= invert;
if (addr2)
tmp &= addr2[start / BITS_PER_LONG];
- tmp ^= invert;
}
return min(start + __ffs(tmp), nbits);
@@ -91,6 +91,21 @@ unsigned long find_next_and_bit(const unsigned long *addr1,
EXPORT_SYMBOL(find_next_and_bit);
#endif
+#if !defined(find_next_and_not_bit)
+unsigned long find_next_and_not_bit(const unsigned long *addr1,
+ const unsigned long *addr2, unsigned long size,
+ unsigned long offset)
+{
+ /*
+ * Switching addr1 and addr2, since the first argument is the one that
+ * will be inverted.
+ */
+ return _find_next_bit(addr2, addr1, size, offset, ~0UL);
+}
+EXPORT_SYMBOL(find_next_and_not_bit);
+#endif
+
+
#ifndef find_first_bit
/*
* Find the first set bit in a memory region.
--
2.20.1
Powered by blists - more mailing lists