[PATCH 0/5] Reduce cross CPU IPI interference

[PATCH 0/5] Reduce cross CPU IPI interference

[Gilad Ben-Yossef]

We have lots of infrastructure in place to partition a multi-core system such 
that we have a group of CPUs that are dedicated to specific task: cgroups, 
scheduler and interrupt affinity and cpuisol boot parameter. Still, kernel 
code will some time interrupt all CPUs in the system via IPIs for various 
needs. These IPIs are useful and cannot be avoided altogether, but in certain 
cases it is possible to interrupt only specific CPUs that have useful work to 
do and not the entire system.

This patch set, inspired by discussions with Peter Zijlstra and Frederic 
Weisbecker when testing the nohz task patch set, is a first stab at trying to 
explore doing this by locating the places where such global IPI calls are 
being made and turning a global IPI into an IPI for a specific group of CPUs. 
The purpose of the patch set is to get  feedback if this is the right way to 
go for dealing with this issue and indeed, if the issue is even worth dealing 
with at all.

This first version creates an on_each_cpu_mask infrastructure API (derived from 
existing arch specific versions in Tile and Arm) and uses it to turn two global 
IPI invocation to per CPU group invocations.

The patch is against 3.1-rc4 and was compiled for x86 and arm in both UP and 
SMP mode (I could not get Tile to build, regardless of this patch) and was 
further tested by running hackbench on x86 in SMP mode in a 4 CPUs VM. No
obvious regression where noted, but I obviously did not test this quite enough.

Signed-off-by: Gilad Ben-Yossef <gilad@benyossef.com>
CC: Peter Zijlstra <a.p.zijlstra@chello.nl>
CC: Frederic Weisbecker <fweisbec@gmail.com>
CC: Russell King <linux@arm.linux.org.uk>
CC: Chris Metcalf <cmetcalf@tilera.com>
CC: linux-mm@kvack.org
CC: Christoph Lameter <cl@linux-foundation.org>
CC: Pekka Enberg <penberg@kernel.org>
CC: Matt Mackall <mpm@selenic.com>

 Ben-Yossef (5):
  Introduce a generic on_each_cpu_mask function
  Move arm over to generic on_each_cpu_mask
  Move tile to use generic on_each_cpu_mask
  Only IPI CPUs to drain local pages if they exist
  slub: only IPI CPUs that have per cpu obj to flush

 arch/arm/kernel/smp_tlb.c   |   20 ++++------------
 arch/tile/include/asm/smp.h |    7 -----
 arch/tile/kernel/smp.c      |   19 ---------------
 include/linux/smp.h         |   14 +++++++++++
 kernel/smp.c                |   20 ++++++++++++++++
 mm/page_alloc.c             |   53 +++++++++++++++++++++++++++++++++++-------
 mm/slub.c                   |   15 +++++++++++-
 7 files changed, 97 insertions(+), 51 deletions(-)

[PATCH 0/5] Reduce cross CPU IPI interference

[Gilad Ben-Yossef]

We have lots of infrastructure in place to partition a multi-core system such 
that we have a group of CPUs that are dedicated to specific task: cgroups, 
scheduler and interrupt affinity and cpuisol boot parameter. Still, kernel 
code will some time interrupt all CPUs in the system via IPIs for various 
needs. These IPIs are useful and cannot be avoided altogether, but in certain 
cases it is possible to interrupt only specific CPUs that have useful work to 
do and not the entire system.

This patch set, inspired by discussions with Peter Zijlstra and Frederic 
Weisbecker when testing the nohz task patch set, is a first stab at trying to 
explore doing this by locating the places where such global IPI calls are 
being made and turning a global IPI into an IPI for a specific group of CPUs. 
The purpose of the patch set is to get  feedback if this is the right way to 
go for dealing with this issue and indeed, if the issue is even worth dealing 
with at all.

This first version creates an on_each_cpu_mask infrastructure API (derived from 
existing arch specific versions in Tile and Arm) and uses it to turn two global 
IPI invocation to per CPU group invocations.

The patch is against 3.1-rc4 and was compiled for x86 and arm in both UP and 
SMP mode (I could not get Tile to build, regardless of this patch) and was 
further tested by running hackbench on x86 in SMP mode in a 4 CPUs VM. No
obvious regression where noted, but I obviously did not test this quite enough.

Signed-off-by: Gilad Ben-Yossef <gilad@benyossef.com>
CC: Peter Zijlstra <a.p.zijlstra@chello.nl>
CC: Frederic Weisbecker <fweisbec@gmail.com>
CC: Russell King <linux@arm.linux.org.uk>
CC: Chris Metcalf <cmetcalf@tilera.com>
CC: linux-mm@kvack.org
CC: Christoph Lameter <cl@linux-foundation.org>
CC: Pekka Enberg <penberg@kernel.org>
CC: Matt Mackall <mpm@selenic.com>

 Ben-Yossef (5):
  Introduce a generic on_each_cpu_mask function
  Move arm over to generic on_each_cpu_mask
  Move tile to use generic on_each_cpu_mask
  Only IPI CPUs to drain local pages if they exist
  slub: only IPI CPUs that have per cpu obj to flush

 arch/arm/kernel/smp_tlb.c   |   20 ++++------------
 arch/tile/include/asm/smp.h |    7 -----
 arch/tile/kernel/smp.c      |   19 ---------------
 include/linux/smp.h         |   14 +++++++++++
 kernel/smp.c                |   20 ++++++++++++++++
 mm/page_alloc.c             |   53 +++++++++++++++++++++++++++++++++++-------
 mm/slub.c                   |   15 +++++++++++-
 7 files changed, 97 insertions(+), 51 deletions(-)

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

[PATCH 1/5] smp: Introduce a generic on_each_cpu_mask function

[Gilad Ben-Yossef]

on_each_cpu_mask calls a function on processors specified my cpumask,
which may include the local processor.

All the limitation specified in smp_call_function_many apply.

Signed-off-by: Gilad Ben-Yossef <gilad@benyossef.com>
CC: Peter Zijlstra <a.p.zijlstra@chello.nl>
CC: Frederic Weisbecker <fweisbec@gmail.com>
CC: Russell King <linux@arm.linux.org.uk>
CC: Chris Metcalf <cmetcalf@tilera.com>
CC: linux-mm@kvack.org
CC: Christoph Lameter <cl@linux-foundation.org>
CC: Pekka Enberg <penberg@kernel.org>
CC: Matt Mackall <mpm@selenic.com>
---
 include/linux/smp.h |   14 ++++++++++++++
 kernel/smp.c        |   20 ++++++++++++++++++++
 2 files changed, 34 insertions(+), 0 deletions(-)

diff --git a/include/linux/smp.h b/include/linux/smp.h
index 8cc38d3..02a8377 100644
--- a/include/linux/smp.h
+++ b/include/linux/smp.h
@@ -102,6 +102,13 @@ static inline void call_function_init(void) { }
 int on_each_cpu(smp_call_func_t func, void *info, int wait);
 
 /*
+ * Call a function on processors specified by mask, which might include
+ * the local one.
+ */
+void on_each_cpu_mask(const struct cpumask *mask, void (*func)(void *),
+		void *info, bool wait);
+
+/*
  * Mark the boot cpu "online" so that it can call console drivers in
  * printk() and can access its per-cpu storage.
  */
@@ -132,6 +139,13 @@ static inline int up_smp_call_function(smp_call_func_t func, void *info)
 		local_irq_enable();		\
 		0;				\
 	})
+#define on_each_cpu_mask(mask, func, info, wait) \
+	if (cpumask_test_cpu(0, (mask))) {	\
+		local_irq_disable();		\
+		func(info);			\
+		local_irq_enable();		\
+	}
+
 static inline void smp_send_reschedule(int cpu) { }
 #define num_booting_cpus()			1
 #define smp_prepare_boot_cpu()			do {} while (0)
diff --git a/kernel/smp.c b/kernel/smp.c
index fb67dfa..df37c08 100644
--- a/kernel/smp.c
+++ b/kernel/smp.c
@@ -701,3 +701,23 @@ int on_each_cpu(void (*func) (void *info), void *info, int wait)
 	return ret;
 }
 EXPORT_SYMBOL(on_each_cpu);
+
+/*
+ * Call a function on processors specified by cpumask, which may include
+ * the local processor. All the limitation specified in smp_call_function_many
+ * apply.
+ */
+void on_each_cpu_mask(const struct cpumask *mask, void (*func)(void *),
+			void *info, bool wait)
+{
+	int cpu = get_cpu();
+
+	smp_call_function_many(mask, func, info, wait);
+	if (cpumask_test_cpu(cpu, mask)) {
+		local_irq_disable();
+		func(info);
+		local_irq_enable();
+	}
+	put_cpu();
+}
+EXPORT_SYMBOL(on_each_cpu_mask);
-- 
1.7.0.4

[PATCH 1/5] smp: Introduce a generic on_each_cpu_mask function

[Gilad Ben-Yossef]

on_each_cpu_mask calls a function on processors specified my cpumask,
which may include the local processor.

All the limitation specified in smp_call_function_many apply.

Signed-off-by: Gilad Ben-Yossef <gilad@benyossef.com>
CC: Peter Zijlstra <a.p.zijlstra@chello.nl>
CC: Frederic Weisbecker <fweisbec@gmail.com>
CC: Russell King <linux@arm.linux.org.uk>
CC: Chris Metcalf <cmetcalf@tilera.com>
CC: linux-mm@kvack.org
CC: Christoph Lameter <cl@linux-foundation.org>
CC: Pekka Enberg <penberg@kernel.org>
CC: Matt Mackall <mpm@selenic.com>
---
 include/linux/smp.h |   14 ++++++++++++++
 kernel/smp.c        |   20 ++++++++++++++++++++
 2 files changed, 34 insertions(+), 0 deletions(-)

diff --git a/include/linux/smp.h b/include/linux/smp.h
index 8cc38d3..02a8377 100644
--- a/include/linux/smp.h
+++ b/include/linux/smp.h
@@ -102,6 +102,13 @@ static inline void call_function_init(void) { }
 int on_each_cpu(smp_call_func_t func, void *info, int wait);
 
 /*
+ * Call a function on processors specified by mask, which might include
+ * the local one.
+ */
+void on_each_cpu_mask(const struct cpumask *mask, void (*func)(void *),
+		void *info, bool wait);
+
+/*
  * Mark the boot cpu "online" so that it can call console drivers in
  * printk() and can access its per-cpu storage.
  */
@@ -132,6 +139,13 @@ static inline int up_smp_call_function(smp_call_func_t func, void *info)
 		local_irq_enable();		\
 		0;				\
 	})
+#define on_each_cpu_mask(mask, func, info, wait) \
+	if (cpumask_test_cpu(0, (mask))) {	\
+		local_irq_disable();		\
+		func(info);			\
+		local_irq_enable();		\
+	}
+
 static inline void smp_send_reschedule(int cpu) { }
 #define num_booting_cpus()			1
 #define smp_prepare_boot_cpu()			do {} while (0)
diff --git a/kernel/smp.c b/kernel/smp.c
index fb67dfa..df37c08 100644
--- a/kernel/smp.c
+++ b/kernel/smp.c
@@ -701,3 +701,23 @@ int on_each_cpu(void (*func) (void *info), void *info, int wait)
 	return ret;
 }
 EXPORT_SYMBOL(on_each_cpu);
+
+/*
+ * Call a function on processors specified by cpumask, which may include
+ * the local processor. All the limitation specified in smp_call_function_many
+ * apply.
+ */
+void on_each_cpu_mask(const struct cpumask *mask, void (*func)(void *),
+			void *info, bool wait)
+{
+	int cpu = get_cpu();
+
+	smp_call_function_many(mask, func, info, wait);
+	if (cpumask_test_cpu(cpu, mask)) {
+		local_irq_disable();
+		func(info);
+		local_irq_enable();
+	}
+	put_cpu();
+}
+EXPORT_SYMBOL(on_each_cpu_mask);
-- 
1.7.0.4

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

[PATCH 2/5] arm: Move arm over to generic on_each_cpu_mask

[Gilad Ben-Yossef]

Note the generic version has the mask as first parameter

Signed-off-by: Gilad Ben-Yossef <gilad@benyossef.com>
CC: Peter Zijlstra <a.p.zijlstra@chello.nl>
CC: Frederic Weisbecker <fweisbec@gmail.com>
CC: Russell King <linux@arm.linux.org.uk>
CC: Chris Metcalf <cmetcalf@tilera.com>
CC: linux-mm@kvack.org
CC: Christoph Lameter <cl@linux-foundation.org>
CC: Pekka Enberg <penberg@kernel.org>
CC: Matt Mackall <mpm@selenic.com>
---
 arch/arm/kernel/smp_tlb.c |   20 +++++---------------
 1 files changed, 5 insertions(+), 15 deletions(-)

diff --git a/arch/arm/kernel/smp_tlb.c b/arch/arm/kernel/smp_tlb.c
index 7dcb352..02c5d2c 100644
--- a/arch/arm/kernel/smp_tlb.c
+++ b/arch/arm/kernel/smp_tlb.c
@@ -13,18 +13,6 @@
 #include <asm/smp_plat.h>
 #include <asm/tlbflush.h>
 
-static void on_each_cpu_mask(void (*func)(void *), void *info, int wait,
-	const struct cpumask *mask)
-{
-	preempt_disable();
-
-	smp_call_function_many(mask, func, info, wait);
-	if (cpumask_test_cpu(smp_processor_id(), mask))
-		func(info);
-
-	preempt_enable();
-}
-
 /**********************************************************************/
 
 /*
@@ -87,7 +75,7 @@ void flush_tlb_all(void)
 void flush_tlb_mm(struct mm_struct *mm)
 {
 	if (tlb_ops_need_broadcast())
-		on_each_cpu_mask(ipi_flush_tlb_mm, mm, 1, mm_cpumask(mm));
+		on_each_cpu_mask(mm_cpumask(mm), ipi_flush_tlb_mm, mm, 1);
 	else
 		local_flush_tlb_mm(mm);
 }
@@ -98,7 +86,8 @@ void flush_tlb_page(struct vm_area_struct *vma, unsigned long uaddr)
 		struct tlb_args ta;
 		ta.ta_vma = vma;
 		ta.ta_start = uaddr;
-		on_each_cpu_mask(ipi_flush_tlb_page, &ta, 1, mm_cpumask(vma->vm_mm));
+		on_each_cpu_mask(mm_cpumask(vma->vm_mm), ipi_flush_tlb_page,
+					&ta, 1);
 	} else
 		local_flush_tlb_page(vma, uaddr);
 }
@@ -121,7 +110,8 @@ void flush_tlb_range(struct vm_area_struct *vma,
 		ta.ta_vma = vma;
 		ta.ta_start = start;
 		ta.ta_end = end;
-		on_each_cpu_mask(ipi_flush_tlb_range, &ta, 1, mm_cpumask(vma->vm_mm));
+		on_each_cpu_mask(mm_cpumask(vma->vm_mm), ipi_flush_tlb_range,
+					&ta, 1);
 	} else
 		local_flush_tlb_range(vma, start, end);
 }
-- 
1.7.0.4

[PATCH 2/5] arm: Move arm over to generic on_each_cpu_mask

[Gilad Ben-Yossef]

Note the generic version has the mask as first parameter

Signed-off-by: Gilad Ben-Yossef <gilad@benyossef.com>
CC: Peter Zijlstra <a.p.zijlstra@chello.nl>
CC: Frederic Weisbecker <fweisbec@gmail.com>
CC: Russell King <linux@arm.linux.org.uk>
CC: Chris Metcalf <cmetcalf@tilera.com>
CC: linux-mm@kvack.org
CC: Christoph Lameter <cl@linux-foundation.org>
CC: Pekka Enberg <penberg@kernel.org>
CC: Matt Mackall <mpm@selenic.com>
---
 arch/arm/kernel/smp_tlb.c |   20 +++++---------------
 1 files changed, 5 insertions(+), 15 deletions(-)

diff --git a/arch/arm/kernel/smp_tlb.c b/arch/arm/kernel/smp_tlb.c
index 7dcb352..02c5d2c 100644
--- a/arch/arm/kernel/smp_tlb.c
+++ b/arch/arm/kernel/smp_tlb.c
@@ -13,18 +13,6 @@
 #include <asm/smp_plat.h>
 #include <asm/tlbflush.h>
 
-static void on_each_cpu_mask(void (*func)(void *), void *info, int wait,
-	const struct cpumask *mask)
-{
-	preempt_disable();
-
-	smp_call_function_many(mask, func, info, wait);
-	if (cpumask_test_cpu(smp_processor_id(), mask))
-		func(info);
-
-	preempt_enable();
-}
-
 /**********************************************************************/
 
 /*
@@ -87,7 +75,7 @@ void flush_tlb_all(void)
 void flush_tlb_mm(struct mm_struct *mm)
 {
 	if (tlb_ops_need_broadcast())
-		on_each_cpu_mask(ipi_flush_tlb_mm, mm, 1, mm_cpumask(mm));
+		on_each_cpu_mask(mm_cpumask(mm), ipi_flush_tlb_mm, mm, 1);
 	else
 		local_flush_tlb_mm(mm);
 }
@@ -98,7 +86,8 @@ void flush_tlb_page(struct vm_area_struct *vma, unsigned long uaddr)
 		struct tlb_args ta;
 		ta.ta_vma = vma;
 		ta.ta_start = uaddr;
-		on_each_cpu_mask(ipi_flush_tlb_page, &ta, 1, mm_cpumask(vma->vm_mm));
+		on_each_cpu_mask(mm_cpumask(vma->vm_mm), ipi_flush_tlb_page,
+					&ta, 1);
 	} else
 		local_flush_tlb_page(vma, uaddr);
 }
@@ -121,7 +110,8 @@ void flush_tlb_range(struct vm_area_struct *vma,
 		ta.ta_vma = vma;
 		ta.ta_start = start;
 		ta.ta_end = end;
-		on_each_cpu_mask(ipi_flush_tlb_range, &ta, 1, mm_cpumask(vma->vm_mm));
+		on_each_cpu_mask(mm_cpumask(vma->vm_mm), ipi_flush_tlb_range,
+					&ta, 1);
 	} else
 		local_flush_tlb_range(vma, start, end);
 }
-- 
1.7.0.4

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

[PATCH 3/5] tile: Move tile to use generic on_each_cpu_mask

[Gilad Ben-Yossef]

The API is the same as the tile private one, so just remove
the private version of the functions

Signed-off-by: Gilad Ben-Yossef <gilad@benyossef.com>
CC: Peter Zijlstra <a.p.zijlstra@chello.nl>
CC: Frederic Weisbecker <fweisbec@gmail.com>
CC: Russell King <linux@arm.linux.org.uk>
CC: Chris Metcalf <cmetcalf@tilera.com>
CC: linux-mm@kvack.org
CC: Christoph Lameter <cl@linux-foundation.org>
CC: Pekka Enberg <penberg@kernel.org>
CC: Matt Mackall <mpm@selenic.com>
---
 arch/tile/include/asm/smp.h |    7 -------
 arch/tile/kernel/smp.c      |   19 -------------------
 2 files changed, 0 insertions(+), 26 deletions(-)

diff --git a/arch/tile/include/asm/smp.h b/arch/tile/include/asm/smp.h
index 532124a..1aa759a 100644
--- a/arch/tile/include/asm/smp.h
+++ b/arch/tile/include/asm/smp.h
@@ -43,10 +43,6 @@ void evaluate_message(int tag);
 /* Boot a secondary cpu */
 void online_secondary(void);
 
-/* Call a function on a specified set of CPUs (may include this one). */
-extern void on_each_cpu_mask(const struct cpumask *mask,
-			     void (*func)(void *), void *info, bool wait);
-
 /* Topology of the supervisor tile grid, and coordinates of boot processor */
 extern HV_Topology smp_topology;
 
@@ -91,9 +87,6 @@ void print_disabled_cpus(void);
 
 #else /* !CONFIG_SMP */
 
-#define on_each_cpu_mask(mask, func, info, wait)		\
-  do { if (cpumask_test_cpu(0, (mask))) func(info); } while (0)
-
 #define smp_master_cpu		0
 #define smp_height		1
 #define smp_width		1
diff --git a/arch/tile/kernel/smp.c b/arch/tile/kernel/smp.c
index c52224d..a44e103 100644
--- a/arch/tile/kernel/smp.c
+++ b/arch/tile/kernel/smp.c
@@ -87,25 +87,6 @@ void send_IPI_allbutself(int tag)
 	send_IPI_many(&mask, tag);
 }
 
-
-/*
- * Provide smp_call_function_mask, but also run function locally
- * if specified in the mask.
- */
-void on_each_cpu_mask(const struct cpumask *mask, void (*func)(void *),
-		      void *info, bool wait)
-{
-	int cpu = get_cpu();
-	smp_call_function_many(mask, func, info, wait);
-	if (cpumask_test_cpu(cpu, mask)) {
-		local_irq_disable();
-		func(info);
-		local_irq_enable();
-	}
-	put_cpu();
-}
-
-
 /*
  * Functions related to starting/stopping cpus.
  */
-- 
1.7.0.4

[PATCH 3/5] tile: Move tile to use generic on_each_cpu_mask

[Gilad Ben-Yossef]

The API is the same as the tile private one, so just remove
the private version of the functions

Signed-off-by: Gilad Ben-Yossef <gilad@benyossef.com>
CC: Peter Zijlstra <a.p.zijlstra@chello.nl>
CC: Frederic Weisbecker <fweisbec@gmail.com>
CC: Russell King <linux@arm.linux.org.uk>
CC: Chris Metcalf <cmetcalf@tilera.com>
CC: linux-mm@kvack.org
CC: Christoph Lameter <cl@linux-foundation.org>
CC: Pekka Enberg <penberg@kernel.org>
CC: Matt Mackall <mpm@selenic.com>
---
 arch/tile/include/asm/smp.h |    7 -------
 arch/tile/kernel/smp.c      |   19 -------------------
 2 files changed, 0 insertions(+), 26 deletions(-)

diff --git a/arch/tile/include/asm/smp.h b/arch/tile/include/asm/smp.h
index 532124a..1aa759a 100644
--- a/arch/tile/include/asm/smp.h
+++ b/arch/tile/include/asm/smp.h
@@ -43,10 +43,6 @@ void evaluate_message(int tag);
 /* Boot a secondary cpu */
 void online_secondary(void);
 
-/* Call a function on a specified set of CPUs (may include this one). */
-extern void on_each_cpu_mask(const struct cpumask *mask,
-			     void (*func)(void *), void *info, bool wait);
-
 /* Topology of the supervisor tile grid, and coordinates of boot processor */
 extern HV_Topology smp_topology;
 
@@ -91,9 +87,6 @@ void print_disabled_cpus(void);
 
 #else /* !CONFIG_SMP */
 
-#define on_each_cpu_mask(mask, func, info, wait)		\
-  do { if (cpumask_test_cpu(0, (mask))) func(info); } while (0)
-
 #define smp_master_cpu		0
 #define smp_height		1
 #define smp_width		1
diff --git a/arch/tile/kernel/smp.c b/arch/tile/kernel/smp.c
index c52224d..a44e103 100644
--- a/arch/tile/kernel/smp.c
+++ b/arch/tile/kernel/smp.c
@@ -87,25 +87,6 @@ void send_IPI_allbutself(int tag)
 	send_IPI_many(&mask, tag);
 }
 
-
-/*
- * Provide smp_call_function_mask, but also run function locally
- * if specified in the mask.
- */
-void on_each_cpu_mask(const struct cpumask *mask, void (*func)(void *),
-		      void *info, bool wait)
-{
-	int cpu = get_cpu();
-	smp_call_function_many(mask, func, info, wait);
-	if (cpumask_test_cpu(cpu, mask)) {
-		local_irq_disable();
-		func(info);
-		local_irq_enable();
-	}
-	put_cpu();
-}
-
-
 /*
  * Functions related to starting/stopping cpus.
  */
-- 
1.7.0.4

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

[PATCH 4/5] mm: Only IPI CPUs to drain local pages if they exist

[Gilad Ben-Yossef]

Use a cpumask to track CPUs with per-cpu pages in any zone
and only send an IPI requesting CPUs to drain these pages
to the buddy allocator if they actually have pages.

Signed-off-by: Gilad Ben-Yossef <gilad@benyossef.com>
CC: Peter Zijlstra <a.p.zijlstra@chello.nl>
CC: Frederic Weisbecker <fweisbec@gmail.com>
CC: Russell King <linux@arm.linux.org.uk>
CC: Chris Metcalf <cmetcalf@tilera.com>
CC: linux-mm@kvack.org
CC: Christoph Lameter <cl@linux-foundation.org>
CC: Pekka Enberg <penberg@kernel.org>
CC: Matt Mackall <mpm@selenic.com>
---
 mm/page_alloc.c |   53 ++++++++++++++++++++++++++++++++++++++++++++---------
 1 files changed, 44 insertions(+), 9 deletions(-)

diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 6e8ecb6..3c079ab 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -62,6 +62,10 @@
 #include <asm/div64.h>
 #include "internal.h"
 
+/* Which CPUs have per cpu pages  */
+cpumask_var_t cpus_with_pcp;
+static DEFINE_PER_CPU(long, total_cpu_pcp_count);
+
 #ifdef CONFIG_USE_PERCPU_NUMA_NODE_ID
 DEFINE_PER_CPU(int, numa_node);
 EXPORT_PER_CPU_SYMBOL(numa_node);
@@ -224,6 +228,25 @@ EXPORT_SYMBOL(nr_online_nodes);
 
 int page_group_by_mobility_disabled __read_mostly;
 
+static inline void inc_pcp_count(int cpu, struct per_cpu_pages *pcp, int count)
+{
+	if (unlikely(!total_cpu_pcp_count))
+		cpumask_set_cpu(cpu, cpus_with_pcp);
+
+	total_cpu_pcp_count += count;
+	pcp->count += count;
+}
+
+static inline void dec_pcp_count(int cpu, struct per_cpu_pages *pcp, int count)
+{
+	pcp->count -= count;
+	total_cpu_pcp_count -= count;
+
+	if (unlikely(!total_cpu_pcp_count))
+		cpumask_clear_cpu(cpu, cpus_with_pcp);
+}
+
+
 static void set_pageblock_migratetype(struct page *page, int migratetype)
 {
 
@@ -1072,7 +1095,7 @@ void drain_zone_pages(struct zone *zone, struct per_cpu_pages *pcp)
 	else
 		to_drain = pcp->count;
 	free_pcppages_bulk(zone, to_drain, pcp);
-	pcp->count -= to_drain;
+	dec_pcp_count(smp_processor_id(), pcp, to_drain);
 	local_irq_restore(flags);
 }
 #endif
@@ -1099,7 +1122,7 @@ static void drain_pages(unsigned int cpu)
 		pcp = &pset->pcp;
 		if (pcp->count) {
 			free_pcppages_bulk(zone, pcp->count, pcp);
-			pcp->count = 0;
+			dec_pcp_count(cpu, pcp, pcp->count);
 		}
 		local_irq_restore(flags);
 	}
@@ -1118,7 +1141,7 @@ void drain_local_pages(void *arg)
  */
 void drain_all_pages(void)
 {
-	on_each_cpu(drain_local_pages, NULL, 1);
+	on_each_cpu_mask(cpus_with_pcp, drain_local_pages, NULL, 1);
 }
 
 #ifdef CONFIG_HIBERNATION
@@ -1166,7 +1189,7 @@ void free_hot_cold_page(struct page *page, int cold)
 	struct zone *zone = page_zone(page);
 	struct per_cpu_pages *pcp;
 	unsigned long flags;
-	int migratetype;
+	int migratetype, cpu;
 	int wasMlocked = __TestClearPageMlocked(page);
 
 	if (!free_pages_prepare(page, 0))
@@ -1194,15 +1217,16 @@ void free_hot_cold_page(struct page *page, int cold)
 		migratetype = MIGRATE_MOVABLE;
 	}
 
+	cpu = smp_processor_id();
 	pcp = &this_cpu_ptr(zone->pageset)->pcp;
 	if (cold)
 		list_add_tail(&page->lru, &pcp->lists[migratetype]);
 	else
 		list_add(&page->lru, &pcp->lists[migratetype]);
-	pcp->count++;
+	inc_pcp_count(cpu, pcp, 1);
 	if (pcp->count >= pcp->high) {
 		free_pcppages_bulk(zone, pcp->batch, pcp);
-		pcp->count -= pcp->batch;
+		dec_pcp_count(cpu, pcp, pcp->batch);
 	}
 
 out:
@@ -1305,9 +1329,10 @@ again:
 		pcp = &this_cpu_ptr(zone->pageset)->pcp;
 		list = &pcp->lists[migratetype];
 		if (list_empty(list)) {
-			pcp->count += rmqueue_bulk(zone, 0,
+			inc_pcp_count(smp_processor_id(), pcp,
+					rmqueue_bulk(zone, 0,
 					pcp->batch, list,
-					migratetype, cold);
+					migratetype, cold));
 			if (unlikely(list_empty(list)))
 				goto failed;
 		}
@@ -1318,7 +1343,7 @@ again:
 			page = list_entry(list->next, struct page, lru);
 
 		list_del(&page->lru);
-		pcp->count--;
+		dec_pcp_count(smp_processor_id(), pcp, 1);
 	} else {
 		if (unlikely(gfp_flags & __GFP_NOFAIL)) {
 			/*
@@ -3553,6 +3578,8 @@ static int zone_batchsize(struct zone *zone)
 #endif
 }
 
+/* NOTE: If you call this function it is very likely you want to clear
+   cpus_with_pcp as well. */
 static void setup_pageset(struct per_cpu_pageset *p, unsigned long batch)
 {
 	struct per_cpu_pages *pcp;
@@ -3591,6 +3618,8 @@ static void setup_zone_pageset(struct zone *zone)
 
 	zone->pageset = alloc_percpu(struct per_cpu_pageset);
 
+	cpumask_clear(cpus_with_pcp);
+
 	for_each_possible_cpu(cpu) {
 		struct per_cpu_pageset *pcp = per_cpu_ptr(zone->pageset, cpu);
 
@@ -3613,6 +3642,10 @@ void __init setup_per_cpu_pageset(void)
 
 	for_each_populated_zone(zone)
 		setup_zone_pageset(zone);
+
+	/* Allocate the cpus_with_pcp var if CONFIG_CPUMASK_OFFSTACK */
+	zalloc_cpumask_var(&cpus_with_pcp, GFP_NOWAIT);
+
 }
 
 static noinline __init_refok
@@ -3664,6 +3697,8 @@ static int __zone_pcp_update(void *data)
 	int cpu;
 	unsigned long batch = zone_batchsize(zone), flags;
 
+	cpumask_clear(cpus_with_pcp);
+
 	for_each_possible_cpu(cpu) {
 		struct per_cpu_pageset *pset;
 		struct per_cpu_pages *pcp;
-- 
1.7.0.4

[PATCH 4/5] mm: Only IPI CPUs to drain local pages if they exist

[Gilad Ben-Yossef]

Use a cpumask to track CPUs with per-cpu pages in any zone
and only send an IPI requesting CPUs to drain these pages
to the buddy allocator if they actually have pages.

Signed-off-by: Gilad Ben-Yossef <gilad@benyossef.com>
CC: Peter Zijlstra <a.p.zijlstra@chello.nl>
CC: Frederic Weisbecker <fweisbec@gmail.com>
CC: Russell King <linux@arm.linux.org.uk>
CC: Chris Metcalf <cmetcalf@tilera.com>
CC: linux-mm@kvack.org
CC: Christoph Lameter <cl@linux-foundation.org>
CC: Pekka Enberg <penberg@kernel.org>
CC: Matt Mackall <mpm@selenic.com>
---
 mm/page_alloc.c |   53 ++++++++++++++++++++++++++++++++++++++++++++---------
 1 files changed, 44 insertions(+), 9 deletions(-)

diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 6e8ecb6..3c079ab 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -62,6 +62,10 @@
 #include <asm/div64.h>
 #include "internal.h"
 
+/* Which CPUs have per cpu pages  */
+cpumask_var_t cpus_with_pcp;
+static DEFINE_PER_CPU(long, total_cpu_pcp_count);
+
 #ifdef CONFIG_USE_PERCPU_NUMA_NODE_ID
 DEFINE_PER_CPU(int, numa_node);
 EXPORT_PER_CPU_SYMBOL(numa_node);
@@ -224,6 +228,25 @@ EXPORT_SYMBOL(nr_online_nodes);
 
 int page_group_by_mobility_disabled __read_mostly;
 
+static inline void inc_pcp_count(int cpu, struct per_cpu_pages *pcp, int count)
+{
+	if (unlikely(!total_cpu_pcp_count))
+		cpumask_set_cpu(cpu, cpus_with_pcp);
+
+	total_cpu_pcp_count += count;
+	pcp->count += count;
+}
+
+static inline void dec_pcp_count(int cpu, struct per_cpu_pages *pcp, int count)
+{
+	pcp->count -= count;
+	total_cpu_pcp_count -= count;
+
+	if (unlikely(!total_cpu_pcp_count))
+		cpumask_clear_cpu(cpu, cpus_with_pcp);
+}
+
+
 static void set_pageblock_migratetype(struct page *page, int migratetype)
 {
 
@@ -1072,7 +1095,7 @@ void drain_zone_pages(struct zone *zone, struct per_cpu_pages *pcp)
 	else
 		to_drain = pcp->count;
 	free_pcppages_bulk(zone, to_drain, pcp);
-	pcp->count -= to_drain;
+	dec_pcp_count(smp_processor_id(), pcp, to_drain);
 	local_irq_restore(flags);
 }
 #endif
@@ -1099,7 +1122,7 @@ static void drain_pages(unsigned int cpu)
 		pcp = &pset->pcp;
 		if (pcp->count) {
 			free_pcppages_bulk(zone, pcp->count, pcp);
-			pcp->count = 0;
+			dec_pcp_count(cpu, pcp, pcp->count);
 		}
 		local_irq_restore(flags);
 	}
@@ -1118,7 +1141,7 @@ void drain_local_pages(void *arg)
  */
 void drain_all_pages(void)
 {
-	on_each_cpu(drain_local_pages, NULL, 1);
+	on_each_cpu_mask(cpus_with_pcp, drain_local_pages, NULL, 1);
 }
 
 #ifdef CONFIG_HIBERNATION
@@ -1166,7 +1189,7 @@ void free_hot_cold_page(struct page *page, int cold)
 	struct zone *zone = page_zone(page);
 	struct per_cpu_pages *pcp;
 	unsigned long flags;
-	int migratetype;
+	int migratetype, cpu;
 	int wasMlocked = __TestClearPageMlocked(page);
 
 	if (!free_pages_prepare(page, 0))
@@ -1194,15 +1217,16 @@ void free_hot_cold_page(struct page *page, int cold)
 		migratetype = MIGRATE_MOVABLE;
 	}
 
+	cpu = smp_processor_id();
 	pcp = &this_cpu_ptr(zone->pageset)->pcp;
 	if (cold)
 		list_add_tail(&page->lru, &pcp->lists[migratetype]);
 	else
 		list_add(&page->lru, &pcp->lists[migratetype]);
-	pcp->count++;
+	inc_pcp_count(cpu, pcp, 1);
 	if (pcp->count >= pcp->high) {
 		free_pcppages_bulk(zone, pcp->batch, pcp);
-		pcp->count -= pcp->batch;
+		dec_pcp_count(cpu, pcp, pcp->batch);
 	}
 
 out:
@@ -1305,9 +1329,10 @@ again:
 		pcp = &this_cpu_ptr(zone->pageset)->pcp;
 		list = &pcp->lists[migratetype];
 		if (list_empty(list)) {
-			pcp->count += rmqueue_bulk(zone, 0,
+			inc_pcp_count(smp_processor_id(), pcp,
+					rmqueue_bulk(zone, 0,
 					pcp->batch, list,
-					migratetype, cold);
+					migratetype, cold));
 			if (unlikely(list_empty(list)))
 				goto failed;
 		}
@@ -1318,7 +1343,7 @@ again:
 			page = list_entry(list->next, struct page, lru);
 
 		list_del(&page->lru);
-		pcp->count--;
+		dec_pcp_count(smp_processor_id(), pcp, 1);
 	} else {
 		if (unlikely(gfp_flags & __GFP_NOFAIL)) {
 			/*
@@ -3553,6 +3578,8 @@ static int zone_batchsize(struct zone *zone)
 #endif
 }
 
+/* NOTE: If you call this function it is very likely you want to clear
+   cpus_with_pcp as well. */
 static void setup_pageset(struct per_cpu_pageset *p, unsigned long batch)
 {
 	struct per_cpu_pages *pcp;
@@ -3591,6 +3618,8 @@ static void setup_zone_pageset(struct zone *zone)
 
 	zone->pageset = alloc_percpu(struct per_cpu_pageset);
 
+	cpumask_clear(cpus_with_pcp);
+
 	for_each_possible_cpu(cpu) {
 		struct per_cpu_pageset *pcp = per_cpu_ptr(zone->pageset, cpu);
 
@@ -3613,6 +3642,10 @@ void __init setup_per_cpu_pageset(void)
 
 	for_each_populated_zone(zone)
 		setup_zone_pageset(zone);
+
+	/* Allocate the cpus_with_pcp var if CONFIG_CPUMASK_OFFSTACK */
+	zalloc_cpumask_var(&cpus_with_pcp, GFP_NOWAIT);
+
 }
 
 static noinline __init_refok
@@ -3664,6 +3697,8 @@ static int __zone_pcp_update(void *data)
 	int cpu;
 	unsigned long batch = zone_batchsize(zone), flags;
 
+	cpumask_clear(cpus_with_pcp);
+
 	for_each_possible_cpu(cpu) {
 		struct per_cpu_pageset *pset;
 		struct per_cpu_pages *pcp;
-- 
1.7.0.4

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

[PATCH 5/5] slub: Only IPI CPUs that have per cpu obj to flush

[Gilad Ben-Yossef]

Try to send IPI to flush per cpu objects back to free lists
to CPUs to seems to have such objects.

The check which CPU to IPI is racy but we don't care since
asking a CPU without per cpu objects to flush does no
damage and as far as I can tell the flush_all by itself is
racy against allocs on remote CPUs anyway, so if you meant
the flush_all to be determinstic, you had to arrange for
locking regardless.

Signed-off-by: Gilad Ben-Yossef <gilad@benyossef.com>
CC: Peter Zijlstra <a.p.zijlstra@chello.nl>
CC: Frederic Weisbecker <fweisbec@gmail.com>
CC: Russell King <linux@arm.linux.org.uk>
CC: Chris Metcalf <cmetcalf@tilera.com>
CC: linux-mm@kvack.org
CC: Christoph Lameter <cl@linux-foundation.org>
CC: Pekka Enberg <penberg@kernel.org>
CC: Matt Mackall <mpm@selenic.com>
---
 mm/slub.c |   15 ++++++++++++++-
 1 files changed, 14 insertions(+), 1 deletions(-)

diff --git a/mm/slub.c b/mm/slub.c
index 9f662d7..8baae30 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -1948,7 +1948,20 @@ static void flush_cpu_slab(void *d)
 
 static void flush_all(struct kmem_cache *s)
 {
-	on_each_cpu(flush_cpu_slab, s, 1);
+	cpumask_var_t cpus;
+	struct kmem_cache_cpu *c;
+	int cpu;
+
+	if (likely(zalloc_cpumask_var(&cpus, GFP_ATOMIC))) {
+		for_each_online_cpu(cpu) {
+			c = per_cpu_ptr(s->cpu_slab, cpu);
+			if (c && c->page)
+				cpumask_set_cpu(cpu, cpus);
+		}
+		on_each_cpu_mask(cpus, flush_cpu_slab, s, 1);
+		free_cpumask_var(cpus);
+	} else
+		on_each_cpu(flush_cpu_slab, s, 1);
 }
 
 /*
-- 
1.7.0.4

[PATCH 5/5] slub: Only IPI CPUs that have per cpu obj to flush

[Gilad Ben-Yossef]

Try to send IPI to flush per cpu objects back to free lists
to CPUs to seems to have such objects.

The check which CPU to IPI is racy but we don't care since
asking a CPU without per cpu objects to flush does no
damage and as far as I can tell the flush_all by itself is
racy against allocs on remote CPUs anyway, so if you meant
the flush_all to be determinstic, you had to arrange for
locking regardless.

Signed-off-by: Gilad Ben-Yossef <gilad@benyossef.com>
CC: Peter Zijlstra <a.p.zijlstra@chello.nl>
CC: Frederic Weisbecker <fweisbec@gmail.com>
CC: Russell King <linux@arm.linux.org.uk>
CC: Chris Metcalf <cmetcalf@tilera.com>
CC: linux-mm@kvack.org
CC: Christoph Lameter <cl@linux-foundation.org>
CC: Pekka Enberg <penberg@kernel.org>
CC: Matt Mackall <mpm@selenic.com>
---
 mm/slub.c |   15 ++++++++++++++-
 1 files changed, 14 insertions(+), 1 deletions(-)

diff --git a/mm/slub.c b/mm/slub.c
index 9f662d7..8baae30 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -1948,7 +1948,20 @@ static void flush_cpu_slab(void *d)
 
 static void flush_all(struct kmem_cache *s)
 {
-	on_each_cpu(flush_cpu_slab, s, 1);
+	cpumask_var_t cpus;
+	struct kmem_cache_cpu *c;
+	int cpu;
+
+	if (likely(zalloc_cpumask_var(&cpus, GFP_ATOMIC))) {
+		for_each_online_cpu(cpu) {
+			c = per_cpu_ptr(s->cpu_slab, cpu);
+			if (c && c->page)
+				cpumask_set_cpu(cpu, cpus);
+		}
+		on_each_cpu_mask(cpus, flush_cpu_slab, s, 1);
+		free_cpumask_var(cpus);
+	} else
+		on_each_cpu(flush_cpu_slab, s, 1);
 }
 
 /*
-- 
1.7.0.4

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 1/5] smp: Introduce a generic on_each_cpu_mask function

[Sasha Levin]

On Sun, 2011-09-25 at 11:54 +0300, Gilad Ben-Yossef wrote:
> +#define on_each_cpu_mask(mask, func, info, wait) \
> +       if (cpumask_test_cpu(0, (mask))) {      \
> +               local_irq_disable();            \
> +               func(info);                     \
		  (func)(info);
> +               local_irq_enable();             \
> +       }
> + 

Maybe it's worth changing it to be so in case someone passes a func ptr
such as 'ptr[0] + 3'.

-- 

Sasha.

Re: [PATCH 1/5] smp: Introduce a generic on_each_cpu_mask function

[Sasha Levin]

On Sun, 2011-09-25 at 11:54 +0300, Gilad Ben-Yossef wrote:
> +#define on_each_cpu_mask(mask, func, info, wait) \
> +       if (cpumask_test_cpu(0, (mask))) {      \
> +               local_irq_disable();            \
> +               func(info);                     \
		  (func)(info);
> +               local_irq_enable();             \
> +       }
> + 

Maybe it's worth changing it to be so in case someone passes a func ptr
such as 'ptr[0] + 3'.

-- 

Sasha.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 4/5] mm: Only IPI CPUs to drain local pages if they exist

[Shaohua Li]

On Sun, 2011-09-25 at 16:54 +0800, Gilad Ben-Yossef wrote:
> Use a cpumask to track CPUs with per-cpu pages in any zone
> and only send an IPI requesting CPUs to drain these pages
> to the buddy allocator if they actually have pages.
Did you have evaluation why the fine-grained ipi is required? I suppose
every CPU has local pages here.

Re: [PATCH 4/5] mm: Only IPI CPUs to drain local pages if they exist

[Shaohua Li]

On Sun, 2011-09-25 at 16:54 +0800, Gilad Ben-Yossef wrote:
> Use a cpumask to track CPUs with per-cpu pages in any zone
> and only send an IPI requesting CPUs to drain these pages
> to the buddy allocator if they actually have pages.
Did you have evaluation why the fine-grained ipi is required? I suppose
every CPU has local pages here.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 4/5] mm: Only IPI CPUs to drain local pages if they exist

[Gilad Ben-Yossef]

Hi Li,

Thank you for the feedback!

On Mon, Sep 26, 2011 at 4:52 AM, Shaohua Li <shaohua.li@intel.com> wrote:
> On Sun, 2011-09-25 at 16:54 +0800, Gilad Ben-Yossef wrote:
>> Use a cpumask to track CPUs with per-cpu pages in any zone
>> and only send an IPI requesting CPUs to drain these pages
>> to the buddy allocator if they actually have pages.
> Did you have evaluation why the fine-grained ipi is required? I suppose
> every CPU has local pages here.


I have given it a lot of though and I believe It's a question of work
load - in a "classic" symmetric work load on a small SMP system I
would indeed expect each CPU to have a per cpu pages cache in some
zone.  However, we are seeing more and more push towards massively
multi core systems and we add support for using them (e.g. cpusets,
Frederic's dynamic tick task patch set etc.). For these work loads,
things can be different:

In a system where you have many core (or hardware threads) and you
dedicate processors to run a singe CPU bound task that performs
virtually no system calls (quite typical for some high performance
computing set ups), you can very well have situations where the per
cpu released page is empty on many processors, since the working set
per cpu rarely changes, so there was now release since the last drain.

Or just consider a multicore machine where a lot of processors are
simply idle with no activity (and we now have cores with 8 cores / 128
hw threads in a single package) - again, no per CPU local page cache
since there was 0 activity since the last drain, but the IPI will be
yanking cores out of low power states to do the check.

I do not know if these scenarios warrant the additional overhead,
certainly not in all situations. Maybe the right thing is to make it a
config option dependent. As I stated in the patch description, that is
one of the thing I'm interested in feedback on.

Thanks,
Gilad

-- 
Gilad Ben-Yossef
Chief Coffee Drinker
gilad@benyossef.com
Israel Cell: +972-52-8260388
US Cell: +1-973-8260388
http://benyossef.com

"I've seen things you people wouldn't believe. Goto statements used to
implement co-routines. I watched C structures being stored in
registers. All those moments will be lost in time... like tears in
rain... Time to die. "

Re: [PATCH 4/5] mm: Only IPI CPUs to drain local pages if they exist

[Gilad Ben-Yossef]

Hi Li,

Thank you for the feedback!

On Mon, Sep 26, 2011 at 4:52 AM, Shaohua Li <shaohua.li@intel.com> wrote:
> On Sun, 2011-09-25 at 16:54 +0800, Gilad Ben-Yossef wrote:
>> Use a cpumask to track CPUs with per-cpu pages in any zone
>> and only send an IPI requesting CPUs to drain these pages
>> to the buddy allocator if they actually have pages.
> Did you have evaluation why the fine-grained ipi is required? I suppose
> every CPU has local pages here.


I have given it a lot of though and I believe It's a question of work
load - in a "classic" symmetric work load on a small SMP system I
would indeed expect each CPU to have a per cpu pages cache in some
zone.  However, we are seeing more and more push towards massively
multi core systems and we add support for using them (e.g. cpusets,
Frederic's dynamic tick task patch set etc.). For these work loads,
things can be different:

In a system where you have many core (or hardware threads) and you
dedicate processors to run a singe CPU bound task that performs
virtually no system calls (quite typical for some high performance
computing set ups), you can very well have situations where the per
cpu released page is empty on many processors, since the working set
per cpu rarely changes, so there was now release since the last drain.

Or just consider a multicore machine where a lot of processors are
simply idle with no activity (and we now have cores with 8 cores / 128
hw threads in a single package) - again, no per CPU local page cache
since there was 0 activity since the last drain, but the IPI will be
yanking cores out of low power states to do the check.

I do not know if these scenarios warrant the additional overhead,
certainly not in all situations. Maybe the right thing is to make it a
config option dependent. As I stated in the patch description, that is
one of the thing I'm interested in feedback on.

Thanks,
Gilad

-- 
Gilad Ben-Yossef
Chief Coffee Drinker
gilad@benyossef.com
Israel Cell: +972-52-8260388
US Cell: +1-973-8260388
http://benyossef.com

"I've seen things you people wouldn't believe. Goto statements used to
implement co-routines. I watched C structures being stored in
registers. All those moments will be lost in time... like tears in
rain... Time to die. "

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 5/5] slub: Only IPI CPUs that have per cpu obj to flush

[Pekka Enberg]

On Sun, Sep 25, 2011 at 11:54 AM, Gilad Ben-Yossef <gilad@benyossef.com> wrote:
> Try to send IPI to flush per cpu objects back to free lists
> to CPUs to seems to have such objects.
>
> The check which CPU to IPI is racy but we don't care since
> asking a CPU without per cpu objects to flush does no
> damage and as far as I can tell the flush_all by itself is
> racy against allocs on remote CPUs anyway, so if you meant
> the flush_all to be determinstic, you had to arrange for
> locking regardless.
>
> Signed-off-by: Gilad Ben-Yossef <gilad@benyossef.com>
> CC: Peter Zijlstra <a.p.zijlstra@chello.nl>
> CC: Frederic Weisbecker <fweisbec@gmail.com>
> CC: Russell King <linux@arm.linux.org.uk>
> CC: Chris Metcalf <cmetcalf@tilera.com>
> CC: linux-mm@kvack.org
> CC: Christoph Lameter <cl@linux-foundation.org>
> CC: Pekka Enberg <penberg@kernel.org>
> CC: Matt Mackall <mpm@selenic.com>
> ---
>  mm/slub.c |   15 ++++++++++++++-
>  1 files changed, 14 insertions(+), 1 deletions(-)
>
> diff --git a/mm/slub.c b/mm/slub.c
> index 9f662d7..8baae30 100644
> --- a/mm/slub.c
> +++ b/mm/slub.c
> @@ -1948,7 +1948,20 @@ static void flush_cpu_slab(void *d)
>
>  static void flush_all(struct kmem_cache *s)
>  {
> -       on_each_cpu(flush_cpu_slab, s, 1);
> +       cpumask_var_t cpus;
> +       struct kmem_cache_cpu *c;
> +       int cpu;
> +
> +       if (likely(zalloc_cpumask_var(&cpus, GFP_ATOMIC))) {
> +               for_each_online_cpu(cpu) {
> +                       c = per_cpu_ptr(s->cpu_slab, cpu);
> +                       if (c && c->page)
> +                               cpumask_set_cpu(cpu, cpus);
> +               }
> +               on_each_cpu_mask(cpus, flush_cpu_slab, s, 1);
> +               free_cpumask_var(cpus);
> +       } else
> +               on_each_cpu(flush_cpu_slab, s, 1);
>  }

AFAICT, flush_all() isn't all that performance sensitive. Why do we
want to reduce IPIs here? Also, I'm somewhat unhappy about introducing
memory allocations in memory shrinking code paths. If we really want
to do this, can we preallocate cpumask in struct kmem_cache, for
example?

                        Pekka

Re: [PATCH 5/5] slub: Only IPI CPUs that have per cpu obj to flush

[Pekka Enberg]

On Sun, Sep 25, 2011 at 11:54 AM, Gilad Ben-Yossef <gilad@benyossef.com> wrote:
> Try to send IPI to flush per cpu objects back to free lists
> to CPUs to seems to have such objects.
>
> The check which CPU to IPI is racy but we don't care since
> asking a CPU without per cpu objects to flush does no
> damage and as far as I can tell the flush_all by itself is
> racy against allocs on remote CPUs anyway, so if you meant
> the flush_all to be determinstic, you had to arrange for
> locking regardless.
>
> Signed-off-by: Gilad Ben-Yossef <gilad@benyossef.com>
> CC: Peter Zijlstra <a.p.zijlstra@chello.nl>
> CC: Frederic Weisbecker <fweisbec@gmail.com>
> CC: Russell King <linux@arm.linux.org.uk>
> CC: Chris Metcalf <cmetcalf@tilera.com>
> CC: linux-mm@kvack.org
> CC: Christoph Lameter <cl@linux-foundation.org>
> CC: Pekka Enberg <penberg@kernel.org>
> CC: Matt Mackall <mpm@selenic.com>
> ---
>  mm/slub.c |   15 ++++++++++++++-
>  1 files changed, 14 insertions(+), 1 deletions(-)
>
> diff --git a/mm/slub.c b/mm/slub.c
> index 9f662d7..8baae30 100644
> --- a/mm/slub.c
> +++ b/mm/slub.c
> @@ -1948,7 +1948,20 @@ static void flush_cpu_slab(void *d)
>
>  static void flush_all(struct kmem_cache *s)
>  {
> -       on_each_cpu(flush_cpu_slab, s, 1);
> +       cpumask_var_t cpus;
> +       struct kmem_cache_cpu *c;
> +       int cpu;
> +
> +       if (likely(zalloc_cpumask_var(&cpus, GFP_ATOMIC))) {
> +               for_each_online_cpu(cpu) {
> +                       c = per_cpu_ptr(s->cpu_slab, cpu);
> +                       if (c && c->page)
> +                               cpumask_set_cpu(cpu, cpus);
> +               }
> +               on_each_cpu_mask(cpus, flush_cpu_slab, s, 1);
> +               free_cpumask_var(cpus);
> +       } else
> +               on_each_cpu(flush_cpu_slab, s, 1);
>  }

AFAICT, flush_all() isn't all that performance sensitive. Why do we
want to reduce IPIs here? Also, I'm somewhat unhappy about introducing
memory allocations in memory shrinking code paths. If we really want
to do this, can we preallocate cpumask in struct kmem_cache, for
example?

                        Pekka

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 0/5] Reduce cross CPU IPI interference

[Peter Zijlstra]

On Sun, 2011-09-25 at 11:54 +0300, Gilad Ben-Yossef wrote:
> This first version creates an on_each_cpu_mask infrastructure API

But we already have the existing smp_call_function_many() doing that.
The on_each_cpu() thing is mostly a hysterical relic and could be
completely depricated

Re: [PATCH 0/5] Reduce cross CPU IPI interference

[Peter Zijlstra]

On Sun, 2011-09-25 at 11:54 +0300, Gilad Ben-Yossef wrote:
> This first version creates an on_each_cpu_mask infrastructure API

But we already have the existing smp_call_function_many() doing that.
The on_each_cpu() thing is mostly a hysterical relic and could be
completely depricated

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 4/5] mm: Only IPI CPUs to drain local pages if they exist

[Peter Zijlstra]

On Sun, 2011-09-25 at 11:54 +0300, Gilad Ben-Yossef wrote:
> +static inline void inc_pcp_count(int cpu, struct per_cpu_pages *pcp, int count)
> +{
> +       if (unlikely(!total_cpu_pcp_count))

	if (unlikely(!__this_cpu_read(total_cpu_pco_count))

> +               cpumask_set_cpu(cpu, cpus_with_pcp);
> +
> +       total_cpu_pcp_count += count;

	__this_cpu_add(total_cpu_pcp_count, count);

> +       pcp->count += count;
> +}
> +
> +static inline void dec_pcp_count(int cpu, struct per_cpu_pages *pcp, int count)
> +{
> +       pcp->count -= count;
> +       total_cpu_pcp_count -= count;

	__this_cpu_sub(total_cpu_pcp_count, count);

> +
> +       if (unlikely(!total_cpu_pcp_count))

	if (unlikely(!__this_cpu_read(total_cpu_pcp_count))

> +               cpumask_clear_cpu(cpu, cpus_with_pcp);
> +} 

Re: [PATCH 4/5] mm: Only IPI CPUs to drain local pages if they exist

[Peter Zijlstra]

On Sun, 2011-09-25 at 11:54 +0300, Gilad Ben-Yossef wrote:
> +static inline void inc_pcp_count(int cpu, struct per_cpu_pages *pcp, int count)
> +{
> +       if (unlikely(!total_cpu_pcp_count))

	if (unlikely(!__this_cpu_read(total_cpu_pco_count))

> +               cpumask_set_cpu(cpu, cpus_with_pcp);
> +
> +       total_cpu_pcp_count += count;

	__this_cpu_add(total_cpu_pcp_count, count);

> +       pcp->count += count;
> +}
> +
> +static inline void dec_pcp_count(int cpu, struct per_cpu_pages *pcp, int count)
> +{
> +       pcp->count -= count;
> +       total_cpu_pcp_count -= count;

	__this_cpu_sub(total_cpu_pcp_count, count);

> +
> +       if (unlikely(!total_cpu_pcp_count))

	if (unlikely(!__this_cpu_read(total_cpu_pcp_count))

> +               cpumask_clear_cpu(cpu, cpus_with_pcp);
> +} 


--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 4/5] mm: Only IPI CPUs to drain local pages if they exist

[Peter Zijlstra]

On Sun, 2011-09-25 at 11:54 +0300, Gilad Ben-Yossef wrote:
> +/* NOTE: If you call this function it is very likely you want to clear
> +   cpus_with_pcp as well. */
>  static void setup_pageset(struct per_cpu_pageset *p, unsigned long batch)
>  { 

  /*
   * Multi-line comment
   * style is like
   * this.
   */

Re: [PATCH 4/5] mm: Only IPI CPUs to drain local pages if they exist

[Peter Zijlstra]

On Sun, 2011-09-25 at 11:54 +0300, Gilad Ben-Yossef wrote:
> +/* NOTE: If you call this function it is very likely you want to clear
> +   cpus_with_pcp as well. */
>  static void setup_pageset(struct per_cpu_pageset *p, unsigned long batch)
>  { 

  /*
   * Multi-line comment
   * style is like
   * this.
   */


--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 5/5] slub: Only IPI CPUs that have per cpu obj to flush

[Peter Zijlstra]

On Sun, 2011-09-25 at 11:54 +0300, Gilad Ben-Yossef wrote:
> +       if (likely(zalloc_cpumask_var(&cpus, GFP_ATOMIC))) {
> +               for_each_online_cpu(cpu) {
> +                       c = per_cpu_ptr(s->cpu_slab, cpu);
> +                       if (c && c->page)
> +                               cpumask_set_cpu(cpu, cpus);
> +               }
> +               on_each_cpu_mask(cpus, flush_cpu_slab, s, 1);
> +               free_cpumask_var(cpus); 

Right, having to do that for_each_oneline_cpu() loop only to then IPI
them can cause a massive cacheline bounce fest.. Ideally you'd want to
keep a cpumask per kmem_cache, although I bet the memory overhead of
that isn't attractive.

Also, what Pekka says, having that alloc here isn't good either.

Re: [PATCH 5/5] slub: Only IPI CPUs that have per cpu obj to flush

[Peter Zijlstra]

On Sun, 2011-09-25 at 11:54 +0300, Gilad Ben-Yossef wrote:
> +       if (likely(zalloc_cpumask_var(&cpus, GFP_ATOMIC))) {
> +               for_each_online_cpu(cpu) {
> +                       c = per_cpu_ptr(s->cpu_slab, cpu);
> +                       if (c && c->page)
> +                               cpumask_set_cpu(cpu, cpus);
> +               }
> +               on_each_cpu_mask(cpus, flush_cpu_slab, s, 1);
> +               free_cpumask_var(cpus); 

Right, having to do that for_each_oneline_cpu() loop only to then IPI
them can cause a massive cacheline bounce fest.. Ideally you'd want to
keep a cpumask per kmem_cache, although I bet the memory overhead of
that isn't attractive.

Also, what Pekka says, having that alloc here isn't good either.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 5/5] slub: Only IPI CPUs that have per cpu obj to flush

[Peter Zijlstra]

On Mon, 2011-09-26 at 09:54 +0300, Pekka Enberg wrote:
> 
> AFAICT, flush_all() isn't all that performance sensitive. Why do we
> want to reduce IPIs here? 

Because it can wake up otherwise idle CPUs, wasting power. Or for the
case I care more about, unnecessarily perturb a CPU that didn't actually
have anything to flush but was running something, introducing jitter.

on_each_cpu() things are bad when you have a ton of CPUs (which is
pretty normal these days). 

Re: [PATCH 5/5] slub: Only IPI CPUs that have per cpu obj to flush

[Peter Zijlstra]

On Mon, 2011-09-26 at 09:54 +0300, Pekka Enberg wrote:
> 
> AFAICT, flush_all() isn't all that performance sensitive. Why do we
> want to reduce IPIs here? 

Because it can wake up otherwise idle CPUs, wasting power. Or for the
case I care more about, unnecessarily perturb a CPU that didn't actually
have anything to flush but was running something, introducing jitter.

on_each_cpu() things are bad when you have a ton of CPUs (which is
pretty normal these days). 

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 5/5] slub: Only IPI CPUs that have per cpu obj to flush

[Gilad Ben-Yossef]

Hi,

On Mon, Sep 26, 2011 at 10:36 AM, Peter Zijlstra <a.p.zijlstra@chello.nl> wrote:
> On Mon, 2011-09-26 at 09:54 +0300, Pekka Enberg wrote:
>>
>> AFAICT, flush_all() isn't all that performance sensitive. Why do we
>> want to reduce IPIs here?
>
> Because it can wake up otherwise idle CPUs, wasting power. Or for the
> case I care more about, unnecessarily perturb a CPU that didn't actually
> have anything to flush but was running something, introducing jitter.
>
> on_each_cpu() things are bad when you have a ton of CPUs (which is
> pretty normal these days).
>


Peter basically already answered better then I could :-)

All I have to add is an example -

flush_all() is called for each kmem_cahce_destroy(). So every cache
being destroyed dynamically ends up sending an IPI to each CPU in the
system, regardless if the cache has ever been used there.

For example, if you close the Infinband ipath driver char device file,
the close file ops calls kmem_cache_destroy().So, if I understand
correctly, running some infiniband config tool on one a single CPU
dedicated to system tasks might interrupt the rest of the 127 CPUs I
dedicated to some CPU intensive task. This is the scenario I'm
tryingto avoid.

I suspect there is a good chance that every line in the output of "git
grep kmem_cache_destroy linux/ | grep '\->'" has a similar scenario
(there are 42 of them).

I hope this sheds some light on the motive of the work.

Thanks!
Gilad
-- 
Gilad Ben-Yossef
Chief Coffee Drinker
gilad@benyossef.com
Israel Cell: +972-52-8260388
US Cell: +1-973-8260388
http://benyossef.com

"I've seen things you people wouldn't believe. Goto statements used to
implement co-routines. I watched C structures being stored in
registers. All those moments will be lost in time... like tears in
rain... Time to die. "

Re: [PATCH 5/5] slub: Only IPI CPUs that have per cpu obj to flush

[Gilad Ben-Yossef]

Hi,

On Mon, Sep 26, 2011 at 10:36 AM, Peter Zijlstra <a.p.zijlstra@chello.nl> wrote:
> On Mon, 2011-09-26 at 09:54 +0300, Pekka Enberg wrote:
>>
>> AFAICT, flush_all() isn't all that performance sensitive. Why do we
>> want to reduce IPIs here?
>
> Because it can wake up otherwise idle CPUs, wasting power. Or for the
> case I care more about, unnecessarily perturb a CPU that didn't actually
> have anything to flush but was running something, introducing jitter.
>
> on_each_cpu() things are bad when you have a ton of CPUs (which is
> pretty normal these days).
>


Peter basically already answered better then I could :-)

All I have to add is an example -

flush_all() is called for each kmem_cahce_destroy(). So every cache
being destroyed dynamically ends up sending an IPI to each CPU in the
system, regardless if the cache has ever been used there.

For example, if you close the Infinband ipath driver char device file,
the close file ops calls kmem_cache_destroy().So, if I understand
correctly, running some infiniband config tool on one a single CPU
dedicated to system tasks might interrupt the rest of the 127 CPUs I
dedicated to some CPU intensive task. This is the scenario I'm
tryingto avoid.

I suspect there is a good chance that every line in the output of "git
grep kmem_cache_destroy linux/ | grep '\->'" has a similar scenario
(there are 42 of them).

I hope this sheds some light on the motive of the work.

Thanks!
Gilad
-- 
Gilad Ben-Yossef
Chief Coffee Drinker
gilad@benyossef.com
Israel Cell: +972-52-8260388
US Cell: +1-973-8260388
http://benyossef.com

"I've seen things you people wouldn't believe. Goto statements used to
implement co-routines. I watched C structures being stored in
registers. All those moments will be lost in time... like tears in
rain... Time to die. "

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 5/5] slub: Only IPI CPUs that have per cpu obj to flush

[Gilad Ben-Yossef]

Hi,

On Mon, Sep 26, 2011 at 9:54 AM, Pekka Enberg <penberg@cs.helsinki.fi> wrote:


>  Also, I'm somewhat unhappy about introducing
> memory allocations in memory shrinking code paths. If we really want
> to do this, can we preallocate cpumask in struct kmem_cache, for
> example?

Yes, I will.

Thanks!
Gilad.


-- 
Gilad Ben-Yossef
Chief Coffee Drinker
gilad@benyossef.com
Israel Cell: +972-52-8260388
US Cell: +1-973-8260388
http://benyossef.com

"I've seen things you people wouldn't believe. Goto statements used to
implement co-routines. I watched C structures being stored in
registers. All those moments will be lost in time... like tears in
rain... Time to die. "

Re: [PATCH 5/5] slub: Only IPI CPUs that have per cpu obj to flush

[Gilad Ben-Yossef]

Hi,

On Mon, Sep 26, 2011 at 9:54 AM, Pekka Enberg <penberg@cs.helsinki.fi> wrote:


>  Also, I'm somewhat unhappy about introducing
> memory allocations in memory shrinking code paths. If we really want
> to do this, can we preallocate cpumask in struct kmem_cache, for
> example?

Yes, I will.

Thanks!
Gilad.


-- 
Gilad Ben-Yossef
Chief Coffee Drinker
gilad@benyossef.com
Israel Cell: +972-52-8260388
US Cell: +1-973-8260388
http://benyossef.com

"I've seen things you people wouldn't believe. Goto statements used to
implement co-routines. I watched C structures being stored in
registers. All those moments will be lost in time... like tears in
rain... Time to die. "

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 5/5] slub: Only IPI CPUs that have per cpu obj to flush

[Gilad Ben-Yossef]

On Mon, Sep 26, 2011 at 10:33 AM, Peter Zijlstra <a.p.zijlstra@chello.nl> wrote:
> On Sun, 2011-09-25 at 11:54 +0300, Gilad Ben-Yossef wrote:
>> +       if (likely(zalloc_cpumask_var(&cpus, GFP_ATOMIC))) {
>> +               for_each_online_cpu(cpu) {
>> +                       c = per_cpu_ptr(s->cpu_slab, cpu);
>> +                       if (c && c->page)
>> +                               cpumask_set_cpu(cpu, cpus);
>> +               }
>> +               on_each_cpu_mask(cpus, flush_cpu_slab, s, 1);
>> +               free_cpumask_var(cpus);
>
> Right, having to do that for_each_oneline_cpu() loop only to then IPI
> them can cause a massive cacheline bounce fest.. Ideally you'd want to
> keep a cpumask per kmem_cache, although I bet the memory overhead of
> that isn't attractive.
>
> Also, what Pekka says, having that alloc here isn't good either.

Yes, the alloc in the flush_all path definitively needs to go. I
wonder if just to resolve that allocating the mask per cpu and not in
kmem_cache itself is not better - after all, all we need is a single
mask per cpu when we wish to do a flush_all and no per cache. The
memory overhead of that is slightly better. This doesn't cover the
cahce bounce issue.

My thoughts regarding that were that since the flush_all() was a
rather rare operation it is preferable to do some more
work/interference here, if it allows us to avoid having to do more
work in the hotter alloc/dealloc paths, especially since it allows us
to have less IPIs that I figured are more intrusive then cacheline
steals (are they?)

After all, for each CPU that actually needs to do a flush, we are
making the flush a bit more expensive because of the cache bounce just
before we send the IPI, but that IPI and further operations are an
expensive operations anyway. For CPUs that don't need to do a flush, I
replaced an IPI for a cacheline(s) steal. I figured it was still a
good bargain

I will spin a new patch that moves this to kmem_cache if you believe
this is the right way to go.

Thanks!
Gilad


-- 
Gilad Ben-Yossef
Chief Coffee Drinker
gilad@benyossef.com
Israel Cell: +972-52-8260388
US Cell: +1-973-8260388
http://benyossef.com

"I've seen things you people wouldn't believe. Goto statements used to
implement co-routines. I watched C structures being stored in
registers. All those moments will be lost in time... like tears in
rain... Time to die. "

Re: [PATCH 5/5] slub: Only IPI CPUs that have per cpu obj to flush

[Gilad Ben-Yossef]

On Mon, Sep 26, 2011 at 10:33 AM, Peter Zijlstra <a.p.zijlstra@chello.nl> wrote:
> On Sun, 2011-09-25 at 11:54 +0300, Gilad Ben-Yossef wrote:
>> +       if (likely(zalloc_cpumask_var(&cpus, GFP_ATOMIC))) {
>> +               for_each_online_cpu(cpu) {
>> +                       c = per_cpu_ptr(s->cpu_slab, cpu);
>> +                       if (c && c->page)
>> +                               cpumask_set_cpu(cpu, cpus);
>> +               }
>> +               on_each_cpu_mask(cpus, flush_cpu_slab, s, 1);
>> +               free_cpumask_var(cpus);
>
> Right, having to do that for_each_oneline_cpu() loop only to then IPI
> them can cause a massive cacheline bounce fest.. Ideally you'd want to
> keep a cpumask per kmem_cache, although I bet the memory overhead of
> that isn't attractive.
>
> Also, what Pekka says, having that alloc here isn't good either.

Yes, the alloc in the flush_all path definitively needs to go. I
wonder if just to resolve that allocating the mask per cpu and not in
kmem_cache itself is not better - after all, all we need is a single
mask per cpu when we wish to do a flush_all and no per cache. The
memory overhead of that is slightly better. This doesn't cover the
cahce bounce issue.

My thoughts regarding that were that since the flush_all() was a
rather rare operation it is preferable to do some more
work/interference here, if it allows us to avoid having to do more
work in the hotter alloc/dealloc paths, especially since it allows us
to have less IPIs that I figured are more intrusive then cacheline
steals (are they?)

After all, for each CPU that actually needs to do a flush, we are
making the flush a bit more expensive because of the cache bounce just
before we send the IPI, but that IPI and further operations are an
expensive operations anyway. For CPUs that don't need to do a flush, I
replaced an IPI for a cacheline(s) steal. I figured it was still a
good bargain

I will spin a new patch that moves this to kmem_cache if you believe
this is the right way to go.

Thanks!
Gilad


-- 
Gilad Ben-Yossef
Chief Coffee Drinker
gilad@benyossef.com
Israel Cell: +972-52-8260388
US Cell: +1-973-8260388
http://benyossef.com

"I've seen things you people wouldn't believe. Goto statements used to
implement co-routines. I watched C structures being stored in
registers. All those moments will be lost in time... like tears in
rain... Time to die. "

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 4/5] mm: Only IPI CPUs to drain local pages if they exist

[Gilad Ben-Yossef]

On Mon, Sep 26, 2011 at 10:28 AM, Peter Zijlstra <a.p.zijlstra@chello.nl> wrote:
> On Sun, 2011-09-25 at 11:54 +0300, Gilad Ben-Yossef wrote:
>> +static inline void inc_pcp_count(int cpu, struct per_cpu_pages *pcp, int count)
>> +{
>> +       if (unlikely(!total_cpu_pcp_count))
>
>        if (unlikely(!__this_cpu_read(total_cpu_pco_count))
>

Thanks for the feedback. I will correct this and the comment style in
the next spin of the patch.

Thanks,
Gilad

-- 
Gilad Ben-Yossef
Chief Coffee Drinker
gilad@benyossef.com
Israel Cell: +972-52-8260388
US Cell: +1-973-8260388
http://benyossef.com

"I've seen things you people wouldn't believe. Goto statements used to
implement co-routines. I watched C structures being stored in
registers. All those moments will be lost in time... like tears in
rain... Time to die. "

Re: [PATCH 4/5] mm: Only IPI CPUs to drain local pages if they exist

[Gilad Ben-Yossef]

On Mon, Sep 26, 2011 at 10:28 AM, Peter Zijlstra <a.p.zijlstra@chello.nl> wrote:
> On Sun, 2011-09-25 at 11:54 +0300, Gilad Ben-Yossef wrote:
>> +static inline void inc_pcp_count(int cpu, struct per_cpu_pages *pcp, int count)
>> +{
>> +       if (unlikely(!total_cpu_pcp_count))
>
>        if (unlikely(!__this_cpu_read(total_cpu_pco_count))
>

Thanks for the feedback. I will correct this and the comment style in
the next spin of the patch.

Thanks,
Gilad

-- 
Gilad Ben-Yossef
Chief Coffee Drinker
gilad@benyossef.com
Israel Cell: +972-52-8260388
US Cell: +1-973-8260388
http://benyossef.com

"I've seen things you people wouldn't believe. Goto statements used to
implement co-routines. I watched C structures being stored in
registers. All those moments will be lost in time... like tears in
rain... Time to die. "

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 0/5] Reduce cross CPU IPI interference

[Gilad Ben-Yossef]

On Mon, Sep 26, 2011 at 10:20 AM, Peter Zijlstra <a.p.zijlstra@chello.nl> wrote:
> On Sun, 2011-09-25 at 11:54 +0300, Gilad Ben-Yossef wrote:
>> This first version creates an on_each_cpu_mask infrastructure API
>
> But we already have the existing smp_call_function_many() doing that.

I might be wrong but my understanding is that smp_call_function_many()
does not invoke the IPI handler on the current processor. The original
code I replaced uses on_each_cpu() which does, so I figured a wrapper
was in order and then I discovered the same wrapper in arch specific
code.

> The on_each_cpu() thing is mostly a hysterical relic and could be
> completely depricated

Wont this require each caller to call smp_call_function_* and then
check to see if it needs to also invoke the IPI handler locally ? I
thought that was the reason for on_each_cpu existence... What have I
missed?

Thanks,
Gilad





-- 
Gilad Ben-Yossef
Chief Coffee Drinker
gilad@benyossef.com
Israel Cell: +972-52-8260388
US Cell: +1-973-8260388
http://benyossef.com

"I've seen things you people wouldn't believe. Goto statements used to
implement co-routines. I watched C structures being stored in
registers. All those moments will be lost in time... like tears in
rain... Time to die. "

Re: [PATCH 0/5] Reduce cross CPU IPI interference

[Gilad Ben-Yossef]

On Mon, Sep 26, 2011 at 10:20 AM, Peter Zijlstra <a.p.zijlstra@chello.nl> wrote:
> On Sun, 2011-09-25 at 11:54 +0300, Gilad Ben-Yossef wrote:
>> This first version creates an on_each_cpu_mask infrastructure API
>
> But we already have the existing smp_call_function_many() doing that.

I might be wrong but my understanding is that smp_call_function_many()
does not invoke the IPI handler on the current processor. The original
code I replaced uses on_each_cpu() which does, so I figured a wrapper
was in order and then I discovered the same wrapper in arch specific
code.

> The on_each_cpu() thing is mostly a hysterical relic and could be
> completely depricated

Wont this require each caller to call smp_call_function_* and then
check to see if it needs to also invoke the IPI handler locally ? I
thought that was the reason for on_each_cpu existence... What have I
missed?

Thanks,
Gilad





-- 
Gilad Ben-Yossef
Chief Coffee Drinker
gilad@benyossef.com
Israel Cell: +972-52-8260388
US Cell: +1-973-8260388
http://benyossef.com

"I've seen things you people wouldn't believe. Goto statements used to
implement co-routines. I watched C structures being stored in
registers. All those moments will be lost in time... like tears in
rain... Time to die. "

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 0/5] Reduce cross CPU IPI interference

[Peter Zijlstra]

On Mon, 2011-09-26 at 11:43 +0300, Gilad Ben-Yossef wrote:
> On Mon, Sep 26, 2011 at 10:20 AM, Peter Zijlstra <a.p.zijlstra@chello.nl> wrote:
> > On Sun, 2011-09-25 at 11:54 +0300, Gilad Ben-Yossef wrote:
> >> This first version creates an on_each_cpu_mask infrastructure API
> >
> > But we already have the existing smp_call_function_many() doing that.
> 
> I might be wrong but my understanding is that smp_call_function_many()
> does not invoke the IPI handler on the current processor. The original
> code I replaced uses on_each_cpu() which does, so I figured a wrapper
> was in order and then I discovered the same wrapper in arch specific
> code.
> 
> > The on_each_cpu() thing is mostly a hysterical relic and could be
> > completely depricated
> 
> Wont this require each caller to call smp_call_function_* and then
> check to see if it needs to also invoke the IPI handler locally ? I
> thought that was the reason for on_each_cpu existence... What have I
> missed?

Gah, you're right.. early.. tea.. more. 

Looks fine then.

Re: [PATCH 0/5] Reduce cross CPU IPI interference

[Peter Zijlstra]

On Mon, 2011-09-26 at 11:43 +0300, Gilad Ben-Yossef wrote:
> On Mon, Sep 26, 2011 at 10:20 AM, Peter Zijlstra <a.p.zijlstra@chello.nl> wrote:
> > On Sun, 2011-09-25 at 11:54 +0300, Gilad Ben-Yossef wrote:
> >> This first version creates an on_each_cpu_mask infrastructure API
> >
> > But we already have the existing smp_call_function_many() doing that.
> 
> I might be wrong but my understanding is that smp_call_function_many()
> does not invoke the IPI handler on the current processor. The original
> code I replaced uses on_each_cpu() which does, so I figured a wrapper
> was in order and then I discovered the same wrapper in arch specific
> code.
> 
> > The on_each_cpu() thing is mostly a hysterical relic and could be
> > completely depricated
> 
> Wont this require each caller to call smp_call_function_* and then
> check to see if it needs to also invoke the IPI handler locally ? I
> thought that was the reason for on_each_cpu existence... What have I
> missed?

Gah, you're right.. early.. tea.. more. 

Looks fine then.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 1/5] smp: Introduce a generic on_each_cpu_mask function

[Peter Zijlstra]

On Sun, 2011-09-25 at 11:54 +0300, Gilad Ben-Yossef wrote:
> +#define on_each_cpu_mask(mask, func, info, wait) \
> +       if (cpumask_test_cpu(0, (mask))) {      \
> +               local_irq_disable();            \
> +               func(info);                     \
> +               local_irq_enable();             \
> +       } 

Typically we wrap that in an extra do { } while(0) loop to properly
consume the trailing ;

Re: [PATCH 1/5] smp: Introduce a generic on_each_cpu_mask function

[Peter Zijlstra]

On Sun, 2011-09-25 at 11:54 +0300, Gilad Ben-Yossef wrote:
> +#define on_each_cpu_mask(mask, func, info, wait) \
> +       if (cpumask_test_cpu(0, (mask))) {      \
> +               local_irq_disable();            \
> +               func(info);                     \
> +               local_irq_enable();             \
> +       } 

Typically we wrap that in an extra do { } while(0) loop to properly
consume the trailing ;

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 5/5] slub: Only IPI CPUs that have per cpu obj to flush

[Pekka Enberg]

On Mon, Sep 26, 2011 at 11:35 AM, Gilad Ben-Yossef <gilad@benyossef.com> wrote:
>> Right, having to do that for_each_oneline_cpu() loop only to then IPI
>> them can cause a massive cacheline bounce fest.. Ideally you'd want to
>> keep a cpumask per kmem_cache, although I bet the memory overhead of
>> that isn't attractive.
>>
>> Also, what Pekka says, having that alloc here isn't good either.
>
> Yes, the alloc in the flush_all path definitively needs to go. I
> wonder if just to resolve that allocating the mask per cpu and not in
> kmem_cache itself is not better - after all, all we need is a single
> mask per cpu when we wish to do a flush_all and no per cache. The
> memory overhead of that is slightly better. This doesn't cover the
> cahce bounce issue.

I'm fine with whatever works for you as long as you don't add a
kmalloc() call in flush_all().

                        Pekka

Re: [PATCH 5/5] slub: Only IPI CPUs that have per cpu obj to flush

[Pekka Enberg]

On Mon, Sep 26, 2011 at 11:35 AM, Gilad Ben-Yossef <gilad@benyossef.com> wrote:
>> Right, having to do that for_each_oneline_cpu() loop only to then IPI
>> them can cause a massive cacheline bounce fest.. Ideally you'd want to
>> keep a cpumask per kmem_cache, although I bet the memory overhead of
>> that isn't attractive.
>>
>> Also, what Pekka says, having that alloc here isn't good either.
>
> Yes, the alloc in the flush_all path definitively needs to go. I
> wonder if just to resolve that allocating the mask per cpu and not in
> kmem_cache itself is not better - after all, all we need is a single
> mask per cpu when we wish to do a flush_all and no per cache. The
> memory overhead of that is slightly better. This doesn't cover the
> cahce bounce issue.

I'm fine with whatever works for you as long as you don't add a
kmalloc() call in flush_all().

                        Pekka

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 5/5] slub: Only IPI CPUs that have per cpu obj to flush

[Peter Zijlstra]

On Mon, 2011-09-26 at 11:35 +0300, Gilad Ben-Yossef wrote:
> Yes, the alloc in the flush_all path definitively needs to go. I
> wonder if just to resolve that allocating the mask per cpu and not in
> kmem_cache itself is not better - after all, all we need is a single
> mask per cpu when we wish to do a flush_all and no per cache. The
> memory overhead of that is slightly better. This doesn't cover the
> cahce bounce issue.
> 
> My thoughts regarding that were that since the flush_all() was a
> rather rare operation it is preferable to do some more
> work/interference here, if it allows us to avoid having to do more
> work in the hotter alloc/dealloc paths, especially since it allows us
> to have less IPIs that I figured are more intrusive then cacheline
> steals (are they?)
> 
> After all, for each CPU that actually needs to do a flush, we are
> making the flush a bit more expensive because of the cache bounce just
> before we send the IPI, but that IPI and further operations are an
> expensive operations anyway. For CPUs that don't need to do a flush, I
> replaced an IPI for a cacheline(s) steal. I figured it was still a
> good bargain

Hard to tell really, I've never really worked with these massive
machines, biggest I've got is 2 nodes and for that I think your
for_each_online_cpu() loop might indeed still be a win when compared to
extra accounting on the alloc/free paths.

The problem with a per-cpu cpumask is that you need to disable
preemption over the whole for_each_online_cpu() scan and that's not
really sane on very large machines as that can easily take a very long
time indeed.

Re: [PATCH 5/5] slub: Only IPI CPUs that have per cpu obj to flush

[Peter Zijlstra]

On Mon, 2011-09-26 at 11:35 +0300, Gilad Ben-Yossef wrote:
> Yes, the alloc in the flush_all path definitively needs to go. I
> wonder if just to resolve that allocating the mask per cpu and not in
> kmem_cache itself is not better - after all, all we need is a single
> mask per cpu when we wish to do a flush_all and no per cache. The
> memory overhead of that is slightly better. This doesn't cover the
> cahce bounce issue.
> 
> My thoughts regarding that were that since the flush_all() was a
> rather rare operation it is preferable to do some more
> work/interference here, if it allows us to avoid having to do more
> work in the hotter alloc/dealloc paths, especially since it allows us
> to have less IPIs that I figured are more intrusive then cacheline
> steals (are they?)
> 
> After all, for each CPU that actually needs to do a flush, we are
> making the flush a bit more expensive because of the cache bounce just
> before we send the IPI, but that IPI and further operations are an
> expensive operations anyway. For CPUs that don't need to do a flush, I
> replaced an IPI for a cacheline(s) steal. I figured it was still a
> good bargain

Hard to tell really, I've never really worked with these massive
machines, biggest I've got is 2 nodes and for that I think your
for_each_online_cpu() loop might indeed still be a win when compared to
extra accounting on the alloc/free paths.

The problem with a per-cpu cpumask is that you need to disable
preemption over the whole for_each_online_cpu() scan and that's not
really sane on very large machines as that can easily take a very long
time indeed.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 5/5] slub: Only IPI CPUs that have per cpu obj to flush

[Pekka Enberg]

On Mon, 26 Sep 2011, Gilad Ben-Yossef wrote:
> Peter basically already answered better then I could :-)
>
> All I have to add is an example -
>
> flush_all() is called for each kmem_cahce_destroy(). So every cache
> being destroyed dynamically ends up sending an IPI to each CPU in the
> system, regardless if the cache has ever been used there.
>
> For example, if you close the Infinband ipath driver char device file,
> the close file ops calls kmem_cache_destroy().So, if I understand
> correctly, running some infiniband config tool on one a single CPU
> dedicated to system tasks might interrupt the rest of the 127 CPUs I
> dedicated to some CPU intensive task. This is the scenario I'm
> tryingto avoid.
>
> I suspect there is a good chance that every line in the output of "git
> grep kmem_cache_destroy linux/ | grep '\->'" has a similar scenario
> (there are 42 of them).
>
> I hope this sheds some light on the motive of the work.

Sure.

If you write down such information in the changelog for future patches, I 
don't need to waste your time asking for an explanation. ;-)

 			Pekka

Re: [PATCH 5/5] slub: Only IPI CPUs that have per cpu obj to flush

[Pekka Enberg]

On Mon, 26 Sep 2011, Gilad Ben-Yossef wrote:
> Peter basically already answered better then I could :-)
>
> All I have to add is an example -
>
> flush_all() is called for each kmem_cahce_destroy(). So every cache
> being destroyed dynamically ends up sending an IPI to each CPU in the
> system, regardless if the cache has ever been used there.
>
> For example, if you close the Infinband ipath driver char device file,
> the close file ops calls kmem_cache_destroy().So, if I understand
> correctly, running some infiniband config tool on one a single CPU
> dedicated to system tasks might interrupt the rest of the 127 CPUs I
> dedicated to some CPU intensive task. This is the scenario I'm
> tryingto avoid.
>
> I suspect there is a good chance that every line in the output of "git
> grep kmem_cache_destroy linux/ | grep '\->'" has a similar scenario
> (there are 42 of them).
>
> I hope this sheds some light on the motive of the work.

Sure.

If you write down such information in the changelog for future patches, I 
don't need to waste your time asking for an explanation. ;-)

 			Pekka

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 5/5] slub: Only IPI CPUs that have per cpu obj to flush

[Gilad Ben-Yossef]

On Mon, Sep 26, 2011 at 12:45 PM, Peter Zijlstra <a.p.zijlstra@chello.nl> wrote:
> On Mon, 2011-09-26 at 11:35 +0300, Gilad Ben-Yossef wrote:
>> Yes, the alloc in the flush_all path definitively needs to go. I
>> wonder if just to resolve that allocating the mask per cpu and not in
>> kmem_cache itself is not better - after all, all we need is a single
>> mask per cpu when we wish to do a flush_all and no per cache. The
>> memory overhead of that is slightly better. This doesn't cover the
>> cahce bounce issue.
>>
>> My thoughts regarding that were that since the flush_all() was a
>> rather rare operation it is preferable to do some more
>> work/interference here, if it allows us to avoid having to do more
>> work in the hotter alloc/dealloc paths, especially since it allows us
>> to have less IPIs that I figured are more intrusive then cacheline
>> steals (are they?)
>>
>> After all, for each CPU that actually needs to do a flush, we are
>> making the flush a bit more expensive because of the cache bounce just
>> before we send the IPI, but that IPI and further operations are an
>> expensive operations anyway. For CPUs that don't need to do a flush, I
>> replaced an IPI for a cacheline(s) steal. I figured it was still a
>> good bargain
>
> Hard to tell really, I've never really worked with these massive
> machines, biggest I've got is 2 nodes and for that I think your
> for_each_online_cpu() loop might indeed still be a win when compared to
> extra accounting on the alloc/free paths.

 I've worked on the SGI Altix which supports up to 1,024 cores in a
single Linux instance, although my dev. machine
at the time only had 32 cores, if I remember correctly,  but I still
don't know the answer either... :-)

> The problem with a per-cpu cpumask is that you need to disable
> preemption over the whole for_each_online_cpu() scan and that's not
> really sane on very large machines as that can easily take a very long
> time indeed.

hmm... I might be thick, but why disable the preemption with the
per-cpu cpumask at all?
Are you worried about re-entering flush_all()? because I don't think
that is a problem -

Let's say we're in the middle of flush_all doing for_each_online_cpu
and updating our cpumask
and then we get preempted and go again into flush_all again. We'll run
over whatever values we
started to put into the cpumask and do the IPIs. When we get back to
our original context
we'll continue updating the tail of the cpumask and send IPIs based on
the updated mask again.

True, we'll end up sending the IPI twice to the head of the cpumask
(the part we managed to update
in the first part of flush_all before the preemption), but that's
doesn't really do any harm and it is still
better then what we have now - in a flush_all preempting flush_all,
we're 100% sure to double IPI all the
processors in the system currently.

Actually, continuing the same line of though - why not have only  one
single global cpumask  that we update
in flush_all before sending the IPI? Using the same reasoning, it
might get updated in parallel from one
processor when  we IPI from another or being doubly updated in
parallel in a few - but we don't really care as
long as cpumask_set_cpu() does its work atomically and
on_each_cpu_mask() is OK with the cpumask
changing under its feet (which I believe both are). We might end up
doing the IPIs based on more updated
values then the one that "our" for_each_online_cpu() walk calculated,
but that isn't a problem.

Sure, it will generate some cacheline bouncing for the global cpumask
variable if several processor do flush_all()
in parallel, but that's supposed to be rare and frankly, if more then
one CPU is about send a bunch of cross system
IPIs you're bound for hell more then some cacheline bouncing anyway.

Does that makes sense or have I've gone over board with this concept? :-)

Thanks,
Gilad

-- 
Gilad Ben-Yossef
Chief Coffee Drinker
gilad@benyossef.com
Israel Cell: +972-52-8260388
US Cell: +1-973-8260388
http://benyossef.com

"I've seen things you people wouldn't believe. Goto statements used to
implement co-routines. I watched C structures being stored in
registers. All those moments will be lost in time... like tears in
rain... Time to die. "

Re: [PATCH 5/5] slub: Only IPI CPUs that have per cpu obj to flush

[Gilad Ben-Yossef]

On Mon, Sep 26, 2011 at 12:45 PM, Peter Zijlstra <a.p.zijlstra@chello.nl> wrote:
> On Mon, 2011-09-26 at 11:35 +0300, Gilad Ben-Yossef wrote:
>> Yes, the alloc in the flush_all path definitively needs to go. I
>> wonder if just to resolve that allocating the mask per cpu and not in
>> kmem_cache itself is not better - after all, all we need is a single
>> mask per cpu when we wish to do a flush_all and no per cache. The
>> memory overhead of that is slightly better. This doesn't cover the
>> cahce bounce issue.
>>
>> My thoughts regarding that were that since the flush_all() was a
>> rather rare operation it is preferable to do some more
>> work/interference here, if it allows us to avoid having to do more
>> work in the hotter alloc/dealloc paths, especially since it allows us
>> to have less IPIs that I figured are more intrusive then cacheline
>> steals (are they?)
>>
>> After all, for each CPU that actually needs to do a flush, we are
>> making the flush a bit more expensive because of the cache bounce just
>> before we send the IPI, but that IPI and further operations are an
>> expensive operations anyway. For CPUs that don't need to do a flush, I
>> replaced an IPI for a cacheline(s) steal. I figured it was still a
>> good bargain
>
> Hard to tell really, I've never really worked with these massive
> machines, biggest I've got is 2 nodes and for that I think your
> for_each_online_cpu() loop might indeed still be a win when compared to
> extra accounting on the alloc/free paths.

 I've worked on the SGI Altix which supports up to 1,024 cores in a
single Linux instance, although my dev. machine
at the time only had 32 cores, if I remember correctly,  but I still
don't know the answer either... :-)

> The problem with a per-cpu cpumask is that you need to disable
> preemption over the whole for_each_online_cpu() scan and that's not
> really sane on very large machines as that can easily take a very long
> time indeed.

hmm... I might be thick, but why disable the preemption with the
per-cpu cpumask at all?
Are you worried about re-entering flush_all()? because I don't think
that is a problem -

Let's say we're in the middle of flush_all doing for_each_online_cpu
and updating our cpumask
and then we get preempted and go again into flush_all again. We'll run
over whatever values we
started to put into the cpumask and do the IPIs. When we get back to
our original context
we'll continue updating the tail of the cpumask and send IPIs based on
the updated mask again.

True, we'll end up sending the IPI twice to the head of the cpumask
(the part we managed to update
in the first part of flush_all before the preemption), but that's
doesn't really do any harm and it is still
better then what we have now - in a flush_all preempting flush_all,
we're 100% sure to double IPI all the
processors in the system currently.

Actually, continuing the same line of though - why not have only  one
single global cpumask  that we update
in flush_all before sending the IPI? Using the same reasoning, it
might get updated in parallel from one
processor when  we IPI from another or being doubly updated in
parallel in a few - but we don't really care as
long as cpumask_set_cpu() does its work atomically and
on_each_cpu_mask() is OK with the cpumask
changing under its feet (which I believe both are). We might end up
doing the IPIs based on more updated
values then the one that "our" for_each_online_cpu() walk calculated,
but that isn't a problem.

Sure, it will generate some cacheline bouncing for the global cpumask
variable if several processor do flush_all()
in parallel, but that's supposed to be rare and frankly, if more then
one CPU is about send a bunch of cross system
IPIs you're bound for hell more then some cacheline bouncing anyway.

Does that makes sense or have I've gone over board with this concept? :-)

Thanks,
Gilad

-- 
Gilad Ben-Yossef
Chief Coffee Drinker
gilad@benyossef.com
Israel Cell: +972-52-8260388
US Cell: +1-973-8260388
http://benyossef.com

"I've seen things you people wouldn't believe. Goto statements used to
implement co-routines. I watched C structures being stored in
registers. All those moments will be lost in time... like tears in
rain... Time to die. "

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 5/5] slub: Only IPI CPUs that have per cpu obj to flush

[Gilad Ben-Yossef]

On Mon, Sep 26, 2011 at 3:05 PM, Gilad Ben-Yossef <gilad@benyossef.com> wrote:


>> The problem with a per-cpu cpumask is that you need to disable
>> preemption over the whole for_each_online_cpu() scan and that's not
>> really sane on very large machines as that can easily take a very long
>> time indeed.
>
> hmm... I might be thick, but why disable the preemption with the
> per-cpu cpumask at all?
...
>
> Does that makes sense or have I've gone over board with this concept? :-)

Scratch that. The cpumask must be per cache or the patch doesn't make
sense at all.
So sadly the only sane place to put it is in struct kmem_cache.

I think we can still update the cpumask field without caring about
preemption for the reasons
stated above, but the per cache memory overhead is still there I'm afraid.

Gilad

-- 
Gilad Ben-Yossef
Chief Coffee Drinker
gilad@benyossef.com
Israel Cell: +972-52-8260388
US Cell: +1-973-8260388
http://benyossef.com

"I've seen things you people wouldn't believe. Goto statements used to
implement co-routines. I watched C structures being stored in
registers. All those moments will be lost in time... like tears in
rain... Time to die. "

Re: [PATCH 5/5] slub: Only IPI CPUs that have per cpu obj to flush

[Gilad Ben-Yossef]

On Mon, Sep 26, 2011 at 3:05 PM, Gilad Ben-Yossef <gilad@benyossef.com> wrote:


>> The problem with a per-cpu cpumask is that you need to disable
>> preemption over the whole for_each_online_cpu() scan and that's not
>> really sane on very large machines as that can easily take a very long
>> time indeed.
>
> hmm... I might be thick, but why disable the preemption with the
> per-cpu cpumask at all?
...
>
> Does that makes sense or have I've gone over board with this concept? :-)

Scratch that. The cpumask must be per cache or the patch doesn't make
sense at all.
So sadly the only sane place to put it is in struct kmem_cache.

I think we can still update the cpumask field without caring about
preemption for the reasons
stated above, but the per cache memory overhead is still there I'm afraid.

Gilad

-- 
Gilad Ben-Yossef
Chief Coffee Drinker
gilad@benyossef.com
Israel Cell: +972-52-8260388
US Cell: +1-973-8260388
http://benyossef.com

"I've seen things you people wouldn't believe. Goto statements used to
implement co-routines. I watched C structures being stored in
registers. All those moments will be lost in time... like tears in
rain... Time to die. "

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 4/5] mm: Only IPI CPUs to drain local pages if they exist

[Christoph Lameter]

On Mon, 26 Sep 2011, Gilad Ben-Yossef wrote:

> I do not know if these scenarios warrant the additional overhead,
> certainly not in all situations. Maybe the right thing is to make it a
> config option dependent. As I stated in the patch description, that is
> one of the thing I'm interested in feedback on.

The flushing of the per cpu pages only done when kmem_cache_shrink() is
run or when a slab cache is closed. And for diagnostics. So its rare and
not performance critical.

Re: [PATCH 4/5] mm: Only IPI CPUs to drain local pages if they exist

[Christoph Lameter]

On Mon, 26 Sep 2011, Gilad Ben-Yossef wrote:

> I do not know if these scenarios warrant the additional overhead,
> certainly not in all situations. Maybe the right thing is to make it a
> config option dependent. As I stated in the patch description, that is
> one of the thing I'm interested in feedback on.

The flushing of the per cpu pages only done when kmem_cache_shrink() is
run or when a slab cache is closed. And for diagnostics. So its rare and
not performance critical.


--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 4/5] mm: Only IPI CPUs to drain local pages if they exist

[Gilad Ben-Yossef]

Hi,

On Mon, Sep 26, 2011 at 6:24 PM, Christoph Lameter <cl@gentwo.org> wrote:
> On Mon, 26 Sep 2011, Gilad Ben-Yossef wrote:
>
>> I do not know if these scenarios warrant the additional overhead,
>> certainly not in all situations. Maybe the right thing is to make it a
>> config option dependent. As I stated in the patch description, that is
>> one of the thing I'm interested in feedback on.
>
> The flushing of the per cpu pages only done when kmem_cache_shrink() is
> run or when a slab cache is closed. And for diagnostics. So its rare and
> not performance critical.

Yes, I understand. The problem is it pops up in the oddest of place.
An example is in order:

The Ipath Infiniband hardware exports a char device interface to user space.
When a user opens the char device and configures a port, a kmem_cache
is created.
When the user later closes the fd, the release method of the char
device destroys
the kmem_cache.

So, if I have some high performance server with 128 processors, and
I've dedicated
127 processors to run my CPU bound computing tasks (and made sure the interrupt
are serviced by the last CPU)  and then run a shell on  the lone admin
CPU to do a backup,
I can interrupt my 127 CPUs doing computational tasks, even though
they have nothing to do
with Infiniband, or backup and have never allocated a single buffer
form that cache.

I believe there is a similar scenario with software raid if I change
RAID configs and several
other places. In fact, I'm guessing many of the lines that pop up when
doing the following
grep hide a similar scenario:

$ git grep kmem_cache_destroy . | grep '\->'

I hope this explains my interest.

My hope is to come up with a way to do more code on the CPU doing the
flush_all (which
as you said is a rare and none performance critical code path anyway)
and by that gain the ability
to do the job without interrupting CPUs that do not need to flush
their per cpu pages.

Thanks,
Gilad







>
>
>



-- 
Gilad Ben-Yossef
Chief Coffee Drinker
gilad@benyossef.com
Israel Cell: +972-52-8260388
US Cell: +1-973-8260388
http://benyossef.com

"I've seen things you people wouldn't believe. Goto statements used to
implement co-routines. I watched C structures being stored in
registers. All those moments will be lost in time... like tears in
rain... Time to die. "

Re: [PATCH 4/5] mm: Only IPI CPUs to drain local pages if they exist

[Gilad Ben-Yossef]

Hi,

On Mon, Sep 26, 2011 at 6:24 PM, Christoph Lameter <cl@gentwo.org> wrote:
> On Mon, 26 Sep 2011, Gilad Ben-Yossef wrote:
>
>> I do not know if these scenarios warrant the additional overhead,
>> certainly not in all situations. Maybe the right thing is to make it a
>> config option dependent. As I stated in the patch description, that is
>> one of the thing I'm interested in feedback on.
>
> The flushing of the per cpu pages only done when kmem_cache_shrink() is
> run or when a slab cache is closed. And for diagnostics. So its rare and
> not performance critical.

Yes, I understand. The problem is it pops up in the oddest of place.
An example is in order:

The Ipath Infiniband hardware exports a char device interface to user space.
When a user opens the char device and configures a port, a kmem_cache
is created.
When the user later closes the fd, the release method of the char
device destroys
the kmem_cache.

So, if I have some high performance server with 128 processors, and
I've dedicated
127 processors to run my CPU bound computing tasks (and made sure the interrupt
are serviced by the last CPU)  and then run a shell on  the lone admin
CPU to do a backup,
I can interrupt my 127 CPUs doing computational tasks, even though
they have nothing to do
with Infiniband, or backup and have never allocated a single buffer
form that cache.

I believe there is a similar scenario with software raid if I change
RAID configs and several
other places. In fact, I'm guessing many of the lines that pop up when
doing the following
grep hide a similar scenario:

$ git grep kmem_cache_destroy . | grep '\->'

I hope this explains my interest.

My hope is to come up with a way to do more code on the CPU doing the
flush_all (which
as you said is a rare and none performance critical code path anyway)
and by that gain the ability
to do the job without interrupting CPUs that do not need to flush
their per cpu pages.

Thanks,
Gilad







>
>
>



-- 
Gilad Ben-Yossef
Chief Coffee Drinker
gilad@benyossef.com
Israel Cell: +972-52-8260388
US Cell: +1-973-8260388
http://benyossef.com

"I've seen things you people wouldn't believe. Goto statements used to
implement co-routines. I watched C structures being stored in
registers. All those moments will be lost in time... like tears in
rain... Time to die. "

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 4/5] mm: Only IPI CPUs to drain local pages if they exist

[Christoph Lameter]

On Tue, 27 Sep 2011, Gilad Ben-Yossef wrote:

> My hope is to come up with a way to do more code on the CPU doing the
> flush_all (which
> as you said is a rare and none performance critical code path anyway)
> and by that gain the ability
> to do the job without interrupting CPUs that do not need to flush
> their per cpu pages.

You may not need that for the kmem_cache_destroy(). At close time there
are no users left and no one should be accessing the cache anyways. You
could flush the whole shebang without IPIs.

Problem is that there is no guarantee that other processes will not still
try to access the cache. If you want to guarantee that then change some
settings in either struct kmem_cache or struct kmem_cache_cpu that makes
allocation and freeing impossible before flushing the per cpu pages.

Re: [PATCH 4/5] mm: Only IPI CPUs to drain local pages if they exist

[Christoph Lameter]

On Tue, 27 Sep 2011, Gilad Ben-Yossef wrote:

> My hope is to come up with a way to do more code on the CPU doing the
> flush_all (which
> as you said is a rare and none performance critical code path anyway)
> and by that gain the ability
> to do the job without interrupting CPUs that do not need to flush
> their per cpu pages.

You may not need that for the kmem_cache_destroy(). At close time there
are no users left and no one should be accessing the cache anyways. You
could flush the whole shebang without IPIs.

Problem is that there is no guarantee that other processes will not still
try to access the cache. If you want to guarantee that then change some
settings in either struct kmem_cache or struct kmem_cache_cpu that makes
allocation and freeing impossible before flushing the per cpu pages.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 0/5] Reduce cross CPU IPI interference

[Chris Metcalf]

On 9/25/2011 4:54 AM, Gilad Ben-Yossef wrote:
> We have lots of infrastructure in place to partition a multi-core system such
> that we have a group of CPUs that are dedicated to specific task: cgroups,
> scheduler and interrupt affinity and cpuisol boot parameter. Still, kernel
> code will some time interrupt all CPUs in the system via IPIs for various
> needs. These IPIs are useful and cannot be avoided altogether, but in certain
> cases it is possible to interrupt only specific CPUs that have useful work to
> do and not the entire system.
>
> This patch set, inspired by discussions with Peter Zijlstra and Frederic
> Weisbecker when testing the nohz task patch set, is a first stab at trying to
> explore doing this by locating the places where such global IPI calls are
> being made and turning a global IPI into an IPI for a specific group of CPUs.
> The purpose of the patch set is to get  feedback if this is the right way to
> go for dealing with this issue and indeed, if the issue is even worth dealing
> with at all.

I strongly concur with your motivation in looking for and removing sources 
of unnecessary cross-cpu interrupts.  We have some code in our tree (not 
yet returned to the community) that tries to deal with some sources of 
interrupt jitter on tiles that are running isolcpu and want to be 100% in 
userspace.

> This first version creates an on_each_cpu_mask infrastructure API (derived from
> existing arch specific versions in Tile and Arm) and uses it to turn two global
> IPI invocation to per CPU group invocations.

The global version looks fine; I would probably make on_each_cpu() an 
inline in the !SMP case now that you are (correctly, I suspect) disabling 
interrupts when calling the function.

> The patch is against 3.1-rc4 and was compiled for x86 and arm in both UP and
> SMP mode (I could not get Tile to build, regardless of this patch)

Yes, our gcc changes are still being prepped for return to the community, 
so unless you want to grab the source code on the http://www.tilera.com/scm 
website, you won't have tile support in gcc yet.  (binutils has been 
returned, so gcc is next up.)
-- 
Chris Metcalf, Tilera Corp.
http://www.tilera.com

Re: [PATCH 0/5] Reduce cross CPU IPI interference

[Chris Metcalf]

On 9/25/2011 4:54 AM, Gilad Ben-Yossef wrote:
> We have lots of infrastructure in place to partition a multi-core system such
> that we have a group of CPUs that are dedicated to specific task: cgroups,
> scheduler and interrupt affinity and cpuisol boot parameter. Still, kernel
> code will some time interrupt all CPUs in the system via IPIs for various
> needs. These IPIs are useful and cannot be avoided altogether, but in certain
> cases it is possible to interrupt only specific CPUs that have useful work to
> do and not the entire system.
>
> This patch set, inspired by discussions with Peter Zijlstra and Frederic
> Weisbecker when testing the nohz task patch set, is a first stab at trying to
> explore doing this by locating the places where such global IPI calls are
> being made and turning a global IPI into an IPI for a specific group of CPUs.
> The purpose of the patch set is to get  feedback if this is the right way to
> go for dealing with this issue and indeed, if the issue is even worth dealing
> with at all.

I strongly concur with your motivation in looking for and removing sources 
of unnecessary cross-cpu interrupts.  We have some code in our tree (not 
yet returned to the community) that tries to deal with some sources of 
interrupt jitter on tiles that are running isolcpu and want to be 100% in 
userspace.

> This first version creates an on_each_cpu_mask infrastructure API (derived from
> existing arch specific versions in Tile and Arm) and uses it to turn two global
> IPI invocation to per CPU group invocations.

The global version looks fine; I would probably make on_each_cpu() an 
inline in the !SMP case now that you are (correctly, I suspect) disabling 
interrupts when calling the function.

> The patch is against 3.1-rc4 and was compiled for x86 and arm in both UP and
> SMP mode (I could not get Tile to build, regardless of this patch)

Yes, our gcc changes are still being prepped for return to the community, 
so unless you want to grab the source code on the http://www.tilera.com/scm 
website, you won't have tile support in gcc yet.  (binutils has been 
returned, so gcc is next up.)
-- 
Chris Metcalf, Tilera Corp.
http://www.tilera.com

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 0/5] Reduce cross CPU IPI interference

[Gilad Ben-Yossef]

On Wed, Sep 28, 2011 at 3:00 PM, Chris Metcalf <cmetcalf@tilera.com> wrote:
> On 9/25/2011 4:54 AM, Gilad Ben-Yossef wrote:
>
> I strongly concur with your motivation in looking for and removing sources
> of unnecessary cross-cpu interrupts.

Thanks for the support :-)

> We have some code in our tree (not yet
> returned to the community) that tries to deal with some sources of interrupt
> jitter on tiles that are running isolcpu and want to be 100% in user space.

Yes, I think this work will benefit this kind of use case (CPU/user
space bound on a dedicated CPU)
the most, although other use cases can benefit as well (e.g. power
management with idle cores).

Btw, do you have any plan to share the patches you mentioned? it could
save me a lot of time. Not wanting to
re-invent the wheel and all that...


>> This first version creates an on_each_cpu_mask infrastructure API (derived
>> from
>> existing arch specific versions in Tile and Arm) and uses it to turn two
>> global
>> IPI invocation to per CPU group invocations.
>
> The global version looks fine; I would probably make on_each_cpu() an inline
> in the !SMP case now that you are (correctly, I suspect) disabling
> interrupts when calling the function.
>

Good point. Will do.

I will take this email as an ACK to the tile relevant changes, if that
is OK with you.

Thanks!
Gilad


-- 
Gilad Ben-Yossef
Chief Coffee Drinker
gilad@benyossef.com
Israel Cell: +972-52-8260388
US Cell: +1-973-8260388
http://benyossef.com

"I've seen things you people wouldn't believe. Goto statements used to
implement co-routines. I watched C structures being stored in
registers. All those moments will be lost in time... like tears in
rain... Time to die. "

Re: [PATCH 0/5] Reduce cross CPU IPI interference

[Gilad Ben-Yossef]

On Wed, Sep 28, 2011 at 3:00 PM, Chris Metcalf <cmetcalf@tilera.com> wrote:
> On 9/25/2011 4:54 AM, Gilad Ben-Yossef wrote:
>
> I strongly concur with your motivation in looking for and removing sources
> of unnecessary cross-cpu interrupts.

Thanks for the support :-)

> We have some code in our tree (not yet
> returned to the community) that tries to deal with some sources of interrupt
> jitter on tiles that are running isolcpu and want to be 100% in user space.

Yes, I think this work will benefit this kind of use case (CPU/user
space bound on a dedicated CPU)
the most, although other use cases can benefit as well (e.g. power
management with idle cores).

Btw, do you have any plan to share the patches you mentioned? it could
save me a lot of time. Not wanting to
re-invent the wheel and all that...


>> This first version creates an on_each_cpu_mask infrastructure API (derived
>> from
>> existing arch specific versions in Tile and Arm) and uses it to turn two
>> global
>> IPI invocation to per CPU group invocations.
>
> The global version looks fine; I would probably make on_each_cpu() an inline
> in the !SMP case now that you are (correctly, I suspect) disabling
> interrupts when calling the function.
>

Good point. Will do.

I will take this email as an ACK to the tile relevant changes, if that
is OK with you.

Thanks!
Gilad


-- 
Gilad Ben-Yossef
Chief Coffee Drinker
gilad@benyossef.com
Israel Cell: +972-52-8260388
US Cell: +1-973-8260388
http://benyossef.com

"I've seen things you people wouldn't believe. Goto statements used to
implement co-routines. I watched C structures being stored in
registers. All those moments will be lost in time... like tears in
rain... Time to die. "

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 0/5] Reduce cross CPU IPI interference

[Chris Metcalf]

On 10/2/2011 4:44 AM, Gilad Ben-Yossef wrote:
>> We have some code in our tree (not yet
>> returned to the community) that tries to deal with some sources of interrupt
>> jitter on tiles that are running isolcpu and want to be 100% in user space.
> Yes, I think this work will benefit this kind of use case (CPU/user
> space bound on a dedicated CPU)
> the most, although other use cases can benefit as well (e.g. power
> management with idle cores).
>
> Btw, do you have any plan to share the patches you mentioned? it could
> save me a lot of time. Not wanting to
> re-invent the wheel and all that...

I'd like to, but getting the patch put together cleanly is still on my list
behind a number of other things (glibc community return, kernel catch-up
with a backlog of less controversial changes, customer crises, enhancements
targeted to forthcoming releases, etc.; I'm sure you know the drill...)

>>> This first version creates an on_each_cpu_mask infrastructure API (derived
>>> from
>>> existing arch specific versions in Tile and Arm) and uses it to turn two
>>> global
>>> IPI invocation to per CPU group invocations.
>> The global version looks fine; I would probably make on_each_cpu() an inline
>> in the !SMP case now that you are (correctly, I suspect) disabling
>> interrupts when calling the function.
>>
> Good point. Will do.
>
> I will take this email as an ACK to the tile relevant changes, if that
> is OK with you.

Yes, definitely.

Acked-by: Chris Metcalf <cmetcalf@tilera.com>

-- 
Chris Metcalf, Tilera Corp.
http://www.tilera.com

Re: [PATCH 0/5] Reduce cross CPU IPI interference

[Chris Metcalf]

On 10/2/2011 4:44 AM, Gilad Ben-Yossef wrote:
>> We have some code in our tree (not yet
>> returned to the community) that tries to deal with some sources of interrupt
>> jitter on tiles that are running isolcpu and want to be 100% in user space.
> Yes, I think this work will benefit this kind of use case (CPU/user
> space bound on a dedicated CPU)
> the most, although other use cases can benefit as well (e.g. power
> management with idle cores).
>
> Btw, do you have any plan to share the patches you mentioned? it could
> save me a lot of time. Not wanting to
> re-invent the wheel and all that...

I'd like to, but getting the patch put together cleanly is still on my list
behind a number of other things (glibc community return, kernel catch-up
with a backlog of less controversial changes, customer crises, enhancements
targeted to forthcoming releases, etc.; I'm sure you know the drill...)

>>> This first version creates an on_each_cpu_mask infrastructure API (derived
>>> from
>>> existing arch specific versions in Tile and Arm) and uses it to turn two
>>> global
>>> IPI invocation to per CPU group invocations.
>> The global version looks fine; I would probably make on_each_cpu() an inline
>> in the !SMP case now that you are (correctly, I suspect) disabling
>> interrupts when calling the function.
>>
> Good point. Will do.
>
> I will take this email as an ACK to the tile relevant changes, if that
> is OK with you.

Yes, definitely.

Acked-by: Chris Metcalf <cmetcalf@tilera.com>

-- 
Chris Metcalf, Tilera Corp.
http://www.tilera.com

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>