Re: [02/17][PATCH] Implement smp_call_function_mask for ia64 - V8

Re: [02/17][PATCH] Implement smp_call_function_mask for ia64 - V8

[Jes Sorensen]

Zhang, Xiantao wrote:
>>From 697d50286088e98da5ac8653c80aaa96c81abf87 Mon Sep 17 00:00:00 2001
> From: Xiantao Zhang <xiantao.zhang@intel.com>
> Date: Mon, 31 Mar 2008 09:50:24 +0800
> Subject: [PATCH] KVM:IA64: Implement smp_call_function_mask for ia64
> 
> This function provides more flexible interface for smp
> infrastructure.
> Signed-off-by: Xiantao Zhang <xiantao.zhang@intel.com>

Hi Xiantao,

I'm a little wary of the performance impact of this change. Doing a
cpumask compare on all smp_call_function calls seems a little expensive.
Maybe it's just noise in the big picture compared to the actual cost of
the IPIs, but I thought I'd bring it up.

Keep in mind that a cpumask can be fairly big these days, max NR_CPUS
is currently 4096. For those booting a kernel with NR_CPUS at 4096 on
a dual CPU machine, it would be a bit expensive.

Why not keep smp_call_function() the way it was before, rather than
implementing it via the call to smp_call_function_mask()?

Cheers,
Jes

RE: [kvm-devel] [02/17][PATCH] Implement smp_call_function_mask foria64 - V8

[Zhang, Xiantao]

Jes Sorensen wrote:
> Zhang, Xiantao wrote:
>>> From 697d50286088e98da5ac8653c80aaa96c81abf87 Mon Sep 17 00:00:00
>>> 2001 
>> From: Xiantao Zhang <xiantao.zhang@intel.com>
>> Date: Mon, 31 Mar 2008 09:50:24 +0800
>> Subject: [PATCH] KVM:IA64: Implement smp_call_function_mask for ia64
>> 
>> This function provides more flexible interface for smp
>> infrastructure. Signed-off-by: Xiantao Zhang
>> <xiantao.zhang@intel.com> 
> 
> Hi Xiantao,
> 
> I'm a little wary of the performance impact of this change. Doing a
> cpumask compare on all smp_call_function calls seems a little
> expensive. Maybe it's just noise in the big picture compared to the
> actual cost of the IPIs, but I thought I'd bring it up.
> Keep in mind that a cpumask can be fairly big these days, max NR_CPUS
> is currently 4096. For those booting a kernel with NR_CPUS at 4096 on
> a dual CPU machine, it would be a bit expensive.
> 
> Why not keep smp_call_function() the way it was before, rather than
> implementing it via the call to smp_call_function_mask()?

Hi, Jes
   I'm not aware of the performance impact before.  If the worst case
occurs,  it need 64 comparisions ?  Maybe keeping old smp_call_function
is better ? 
Xiantao
> 
>
------------------------------------------------------------------------
-
> Check out the new SourceForge.net Marketplace.
> It's the best place to buy or sell services for
> just about anything Open Source.
>
http://ad.doubleclick.net/clk;164216239;13503038;w?http://sf.net/marketp
lace
> _______________________________________________
> kvm-devel mailing list
> kvm-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/kvm-devel

Re: [02/17][PATCH] Implement smp_call_function_mask for ia64 - V8

[Jeremy Fitzhardinge]

Jes Sorensen wrote:
> I'm a little wary of the performance impact of this change. Doing a
> cpumask compare on all smp_call_function calls seems a little expensive.
> Maybe it's just noise in the big picture compared to the actual cost of
> the IPIs, but I thought I'd bring it up.
>
> Keep in mind that a cpumask can be fairly big these days, max NR_CPUS
> is currently 4096. For those booting a kernel with NR_CPUS at 4096 on
> a dual CPU machine, it would be a bit expensive.
>   

Unless your hardware has remarkably fast IPIs, I think really the cost 
of scanning 512 bytes is going to be in the noise...

This change has been on the x86 side for ages, and not even Ingo made a 
peep about it ;)

> Why not keep smp_call_function() the way it was before, rather than
> implementing it via the call to smp_call_function_mask()?
>   

Because Xen needs a different core implementation (because of its 
different IPI implementation), and it would be better to just have to do 
one of them rather than N.

    J

Re: [02/17][PATCH] Implement smp_call_function_mask for ia64 - V8

[Jes Sorensen]

Jeremy Fitzhardinge wrote:
> Jes Sorensen wrote:
> This change has been on the x86 side for ages, and not even Ingo made a 
> peep about it ;)

Mmmm, last time I looked, x86 didn't scale to any interesting number
of CPUs :-)

>> Why not keep smp_call_function() the way it was before, rather than
>> implementing it via the call to smp_call_function_mask()?
> 
> Because Xen needs a different core implementation (because of its 
> different IPI implementation), and it would be better to just have to do 
> one of them rather than N.

I wasn't suggesting we shouldn't have both interfaces, merely
questioning why adding what to me seems like an unnecessary performance
hit for the classic case of the call.

Cheers,
Jes

Re: [02/17][PATCH] Implement smp_call_function_mask for ia64 - V8

[Jeremy Fitzhardinge]

Jes Sorensen wrote:
> Jeremy Fitzhardinge wrote:
>   
>> Jes Sorensen wrote:
>> This change has been on the x86 side for ages, and not even Ingo made a 
>> peep about it ;)
>>     
>
> Mmmm, last time I looked, x86 didn't scale to any interesting number
> of CPUs :-)
>   

Well, I guess you need all those CPUs if scanning a 64-word bitvector 
takes anything like the time it takes to do an IPI...

> I wasn't suggesting we shouldn't have both interfaces, merely
> questioning why adding what to me seems like an unnecessary performance
> hit for the classic case of the call.

I don't mind how many interfaces there are, so long as there only needs 
to be one place to hook to plug in the Xen version of 
smp_call_function_whatever.  Perhaps the answer is to just hook the IPI 
mechanism itself rather than the whole of smp_call_function_mask...

Have you looked at Jens Axboe's patches to make all this stuff a lot 
more arch-common?

    J

Re: [02/17][PATCH] Implement smp_call_function_mask for ia64 - V8

[Jes Sorensen]

Jeremy Fitzhardinge wrote:
>> I wasn't suggesting we shouldn't have both interfaces, merely
>> questioning why adding what to me seems like an unnecessary performance
>> hit for the classic case of the call.
> 
> I don't mind how many interfaces there are, so long as there only needs 
> to be one place to hook to plug in the Xen version of 
> smp_call_function_whatever.  Perhaps the answer is to just hook the IPI 
> mechanism itself rather than the whole of smp_call_function_mask...

Well we're obviously going to have at least two interfaces given that
we have the traditional Linux one and Xen seems to require something
different :-)

> Have you looked at Jens Axboe's patches to make all this stuff a lot 
> more arch-common?

Nope, do you have a pointer?

Cheers,
Jes

Re: [02/17][PATCH] Implement smp_call_function_mask for ia64 - V8

[Luck, Tony]

> > Have you looked at Jens Axboe's patches to make all this stuff a lot 
> > more arch-common?
>
> Nope, do you have a pointer?

Check your favourite archive for this Subject line:

 Generic smp_call_function(), improvements, and  smp_call_function_single()

-Tony

-------------------------------------------------------------------------
Check out the new SourceForge.net Marketplace.
It's the best place to buy or sell services for
just about anything Open Source.
http://ad.doubleclick.net/clk;164216239;13503038;w?http://sf.net/marketplace

[02/17][PATCH] Implement smp_call_function_mask for ia64 - V8

[Zhang, Xiantao]

[-- Attachment #1: Type: text/plain, Size: 5087 bytes --]

>From 697d50286088e98da5ac8653c80aaa96c81abf87 Mon Sep 17 00:00:00 2001
From: Xiantao Zhang <xiantao.zhang@intel.com>
Date: Mon, 31 Mar 2008 09:50:24 +0800
Subject: [PATCH] KVM:IA64: Implement smp_call_function_mask for ia64

This function provides more flexible interface for smp
infrastructure.
Signed-off-by: Xiantao Zhang <xiantao.zhang@intel.com>
---
 arch/ia64/kernel/smp.c |   84
+++++++++++++++++++++++++++++++++++++----------
 include/asm-ia64/smp.h |    3 ++
 2 files changed, 69 insertions(+), 18 deletions(-)

diff --git a/arch/ia64/kernel/smp.c b/arch/ia64/kernel/smp.c
index 4e446aa..5bb241f 100644
--- a/arch/ia64/kernel/smp.c
+++ b/arch/ia64/kernel/smp.c
@@ -213,6 +213,19 @@ send_IPI_allbutself (int op)
  * Called with preemption disabled.
  */
 static inline void
+send_IPI_mask(cpumask_t mask, int op)
+{
+	unsigned int cpu;
+
+	for_each_cpu_mask(cpu, mask) {
+			send_IPI_single(cpu, op);
+	}
+}
+
+/*
+ * Called with preemption disabled.
+ */
+static inline void
 send_IPI_all (int op)
 {
 	int i;
@@ -401,33 +414,36 @@ smp_call_function_single (int cpuid, void (*func)
(void *info), void *info, int
 }
 EXPORT_SYMBOL(smp_call_function_single);
 
-/*
- * this function sends a 'generic call function' IPI to all other CPUs
- * in the system.
- */
-
-/*
- *  [SUMMARY]	Run a function on all other CPUs.
- *  <func>	The function to run. This must be fast and non-blocking.
- *  <info>	An arbitrary pointer to pass to the function.
- *  <nonatomic>	currently unused.
- *  <wait>	If true, wait (atomically) until function has completed
on other CPUs.
- *  [RETURNS]   0 on success, else a negative status code.
+/**
+ * smp_call_function_mask(): Run a function on a set of other CPUs.
+ * <mask>	The set of cpus to run on.  Must not include the current
cpu.
+ * <func> 	The function to run. This must be fast and non-blocking.
+ * <info>	An arbitrary pointer to pass to the function.
+ * <wait>	If true, wait (atomically) until function
+ *		has completed on other CPUs.
  *
- * Does not return until remote CPUs are nearly ready to execute <func>
or are or have
- * executed.
+ * Returns 0 on success, else a negative status code.
+ *
+ * If @wait is true, then returns once @func has returned; otherwise
+ * it returns just before the target cpu calls @func.
  *
  * You must not call this function with disabled interrupts or from a
  * hardware interrupt handler or from a bottom half handler.
  */
-int
-smp_call_function (void (*func) (void *info), void *info, int
nonatomic, int wait)
+int smp_call_function_mask(cpumask_t mask,
+			   void (*func)(void *), void *info,
+			   int wait)
 {
 	struct call_data_struct data;
+	cpumask_t allbutself;
 	int cpus;
 
 	spin_lock(&call_lock);
-	cpus = num_online_cpus() - 1;
+	allbutself = cpu_online_map;
+	cpu_clear(smp_processor_id(), allbutself);
+
+	cpus_and(mask, mask, allbutself);
+	cpus = cpus_weight(mask);
 	if (!cpus) {
 		spin_unlock(&call_lock);
 		return 0;
@@ -445,7 +461,12 @@ smp_call_function (void (*func) (void *info), void
*info, int nonatomic, int wai
 
 	call_data = &data;
 	mb();	/* ensure store to call_data precedes setting of
IPI_CALL_FUNC */
-	send_IPI_allbutself(IPI_CALL_FUNC);
+
+	/* Send a message to other CPUs */
+	if (cpus_equal(mask, allbutself))
+		send_IPI_allbutself(IPI_CALL_FUNC);
+	else
+		send_IPI_mask(mask, IPI_CALL_FUNC);
 
 	/* Wait for response */
 	while (atomic_read(&data.started) != cpus)
@@ -458,6 +479,33 @@ smp_call_function (void (*func) (void *info), void
*info, int nonatomic, int wai
 
 	spin_unlock(&call_lock);
 	return 0;
+
+}
+EXPORT_SYMBOL(smp_call_function_mask);
+
+/*
+ * this function sends a 'generic call function' IPI to all other CPUs
+ * in the system.
+ */
+
+/*
+ *  [SUMMARY]	Run a function on all other CPUs.
+ *  <func>	The function to run. This must be fast and non-blocking.
+ *  <info>	An arbitrary pointer to pass to the function.
+ *  <nonatomic>	currently unused.
+ *  <wait>	If true, wait (atomically) until function has completed
on other CPUs.
+ *  [RETURNS]   0 on success, else a negative status code.
+ *
+ * Does not return until remote CPUs are nearly ready to execute <func>
or are or have
+ * executed.
+ *
+ * You must not call this function with disabled interrupts or from a
+ * hardware interrupt handler or from a bottom half handler.
+ */
+int
+smp_call_function (void (*func) (void *info), void *info, int
nonatomic, int wait)
+{
+	return smp_call_function_mask(cpu_online_map, func, info, wait);
 }
 EXPORT_SYMBOL(smp_call_function);
 
diff --git a/include/asm-ia64/smp.h b/include/asm-ia64/smp.h
index 4fa733d..ec5f355 100644
--- a/include/asm-ia64/smp.h
+++ b/include/asm-ia64/smp.h
@@ -38,6 +38,9 @@ ia64_get_lid (void)
 	return lid.f.id << 8 | lid.f.eid;
 }
 
+extern int smp_call_function_mask(cpumask_t mask, void (*func)(void *),
+				  void *info, int wait);
+
 #define hard_smp_processor_id()		ia64_get_lid()
 
 #ifdef CONFIG_SMP
-- 
1.5.2

[-- Attachment #2: 0002-KVM-IA64-Implement-smp_call_function_mask-for-ia64.patch --]
[-- Type: application/octet-stream, Size: 4920 bytes --]

From 697d50286088e98da5ac8653c80aaa96c81abf87 Mon Sep 17 00:00:00 2001
From: Xiantao Zhang <xiantao.zhang@intel.com>
Date: Mon, 31 Mar 2008 09:50:24 +0800
Subject: [PATCH] KVM:IA64: Implement smp_call_function_mask for ia64

This function provides more flexible interface for smp
infrastructure.
Signed-off-by: Xiantao Zhang <xiantao.zhang@intel.com>
---
 arch/ia64/kernel/smp.c |   84 +++++++++++++++++++++++++++++++++++++----------
 include/asm-ia64/smp.h |    3 ++
 2 files changed, 69 insertions(+), 18 deletions(-)

diff --git a/arch/ia64/kernel/smp.c b/arch/ia64/kernel/smp.c
index 4e446aa..5bb241f 100644
--- a/arch/ia64/kernel/smp.c
+++ b/arch/ia64/kernel/smp.c
@@ -213,6 +213,19 @@ send_IPI_allbutself (int op)
  * Called with preemption disabled.
  */
 static inline void
+send_IPI_mask(cpumask_t mask, int op)
+{
+	unsigned int cpu;
+
+	for_each_cpu_mask(cpu, mask) {
+			send_IPI_single(cpu, op);
+	}
+}
+
+/*
+ * Called with preemption disabled.
+ */
+static inline void
 send_IPI_all (int op)
 {
 	int i;
@@ -401,33 +414,36 @@ smp_call_function_single (int cpuid, void (*func) (void *info), void *info, int
 }
 EXPORT_SYMBOL(smp_call_function_single);
 
-/*
- * this function sends a 'generic call function' IPI to all other CPUs
- * in the system.
- */
-
-/*
- *  [SUMMARY]	Run a function on all other CPUs.
- *  <func>	The function to run. This must be fast and non-blocking.
- *  <info>	An arbitrary pointer to pass to the function.
- *  <nonatomic>	currently unused.
- *  <wait>	If true, wait (atomically) until function has completed on other CPUs.
- *  [RETURNS]   0 on success, else a negative status code.
+/**
+ * smp_call_function_mask(): Run a function on a set of other CPUs.
+ * <mask>	The set of cpus to run on.  Must not include the current cpu.
+ * <func> 	The function to run. This must be fast and non-blocking.
+ * <info>	An arbitrary pointer to pass to the function.
+ * <wait>	If true, wait (atomically) until function
+ *		has completed on other CPUs.
  *
- * Does not return until remote CPUs are nearly ready to execute <func> or are or have
- * executed.
+ * Returns 0 on success, else a negative status code.
+ *
+ * If @wait is true, then returns once @func has returned; otherwise
+ * it returns just before the target cpu calls @func.
  *
  * You must not call this function with disabled interrupts or from a
  * hardware interrupt handler or from a bottom half handler.
  */
-int
-smp_call_function (void (*func) (void *info), void *info, int nonatomic, int wait)
+int smp_call_function_mask(cpumask_t mask,
+			   void (*func)(void *), void *info,
+			   int wait)
 {
 	struct call_data_struct data;
+	cpumask_t allbutself;
 	int cpus;
 
 	spin_lock(&call_lock);
-	cpus = num_online_cpus() - 1;
+	allbutself = cpu_online_map;
+	cpu_clear(smp_processor_id(), allbutself);
+
+	cpus_and(mask, mask, allbutself);
+	cpus = cpus_weight(mask);
 	if (!cpus) {
 		spin_unlock(&call_lock);
 		return 0;
@@ -445,7 +461,12 @@ smp_call_function (void (*func) (void *info), void *info, int nonatomic, int wai
 
 	call_data = &data;
 	mb();	/* ensure store to call_data precedes setting of IPI_CALL_FUNC */
-	send_IPI_allbutself(IPI_CALL_FUNC);
+
+	/* Send a message to other CPUs */
+	if (cpus_equal(mask, allbutself))
+		send_IPI_allbutself(IPI_CALL_FUNC);
+	else
+		send_IPI_mask(mask, IPI_CALL_FUNC);
 
 	/* Wait for response */
 	while (atomic_read(&data.started) != cpus)
@@ -458,6 +479,33 @@ smp_call_function (void (*func) (void *info), void *info, int nonatomic, int wai
 
 	spin_unlock(&call_lock);
 	return 0;
+
+}
+EXPORT_SYMBOL(smp_call_function_mask);
+
+/*
+ * this function sends a 'generic call function' IPI to all other CPUs
+ * in the system.
+ */
+
+/*
+ *  [SUMMARY]	Run a function on all other CPUs.
+ *  <func>	The function to run. This must be fast and non-blocking.
+ *  <info>	An arbitrary pointer to pass to the function.
+ *  <nonatomic>	currently unused.
+ *  <wait>	If true, wait (atomically) until function has completed on other CPUs.
+ *  [RETURNS]   0 on success, else a negative status code.
+ *
+ * Does not return until remote CPUs are nearly ready to execute <func> or are or have
+ * executed.
+ *
+ * You must not call this function with disabled interrupts or from a
+ * hardware interrupt handler or from a bottom half handler.
+ */
+int
+smp_call_function (void (*func) (void *info), void *info, int nonatomic, int wait)
+{
+	return smp_call_function_mask(cpu_online_map, func, info, wait);
 }
 EXPORT_SYMBOL(smp_call_function);
 
diff --git a/include/asm-ia64/smp.h b/include/asm-ia64/smp.h
index 4fa733d..ec5f355 100644
--- a/include/asm-ia64/smp.h
+++ b/include/asm-ia64/smp.h
@@ -38,6 +38,9 @@ ia64_get_lid (void)
 	return lid.f.id << 8 | lid.f.eid;
 }
 
+extern int smp_call_function_mask(cpumask_t mask, void (*func)(void *),
+				  void *info, int wait);
+
 #define hard_smp_processor_id()		ia64_get_lid()
 
 #ifdef CONFIG_SMP
-- 
1.5.2


[-- Attachment #3: Type: text/plain, Size: 278 bytes --]

-------------------------------------------------------------------------
Check out the new SourceForge.net Marketplace.
It's the best place to buy or sell services for
just about anything Open Source.
http://ad.doubleclick.net/clk;164216239;13503038;w?http://sf.net/marketplace

[-- Attachment #4: Type: text/plain, Size: 158 bytes --]

_______________________________________________
kvm-devel mailing list
kvm-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/kvm-devel