[RFC PATCH v2 0/9] Remove useless on_each_cpu return value

on_each_cpu() returns as its own return value the return value of
smp_call_function(). smp_call_function() in turn returns a hard
coded value of zero.

Some callers to on_each_cpu() waste cycles and bloat code space
by checking the return value to on_each_cpu(), probably for
historical reasons.

This patch set refactors callers to not test on_each_cpu()
(fixed) return value and then refactors on_each_cpu to
return void to avoid confusing future users.

In other words, this patch aims to delete 18 source code lines
while not changing any functionality :-)

I tested as best as I could the x86 changes and compiled some
of the others, but I don't have access to all the needed hardware
for testing. Reviewers and testers welcome!

The only change from the first version is the addition of a 
proper Signed-off-by line.

This patch set is also available on branch on_each_cpu_ret_v2
at git://github.com/gby/linux.git

Signed-off-by: Gilad Ben-Yossef <gilad@benyossef.com>
Acked-by: Peter Zijlstra <redacted>
Reviewed-by: Michal Nazarewicz <redacted>
CC: Michal Nazarewicz <redacted>
CC: David Airlie <redacted>
CC: dri-devel@lists.freedesktop.org
CC: Benjamin Herrenschmidt <benh@kernel.crashing.org>
CC: Paul Mackerras <redacted>
CC: Grant Likely <redacted>
CC: Rob Herring <redacted>
CC: linuxppc-dev@lists.ozlabs.org
CC: devicetree-discuss@lists.ozlabs.org
CC: Richard Henderson <redacted>
CC: Ivan Kokshaysky <redacted>
CC: Matt Turner <mattst88@gmail.com>
CC: linux-alpha@vger.kernel.org
CC: Thomas Gleixner <redacted>
CC: Ingo Molnar <mingo@redhat.com>
CC: "H. Peter Anvin" <hpa@zytor.com>
CC: x86@kernel.org
CC: Tony Luck <tony.luck@intel.com>
CC: Fenghua Yu <redacted>
CC: linux-ia64@vger.kernel.org
CC: Will Deacon <redacted>
CC: Peter Zijlstra <redacted>
CC: Arnaldo Carvalho de Melo <redacted>
CC: Russell King <redacted>
CC: linux-arm-kernel@lists.infradead.org


Gilad Ben-Yossef (9):
  arm: avoid using on_each_cpu hard coded ret value
  ia64: avoid using on_each_cpu hard coded ret value
  x86: avoid using on_each_cpu hard coded ret value
  alpha: avoid using on_each_cpu hard coded ret value
  ppc: avoid using on_each_cpu hard coded ret value
  agp: avoid using on_each_cpu hard coded ret value
  drm: avoid using on_each_cpu hard coded ret value
  smp: refactor on_each_cpu to void returning func
  x86: refactor wbinvd_on_all_cpus to void function

 arch/alpha/kernel/smp.c      |    7 ++-----
 arch/arm/kernel/perf_event.c |    2 +-
 arch/ia64/kernel/perfmon.c   |   12 ++----------
 arch/powerpc/kernel/rtas.c   |    3 +--
 arch/x86/include/asm/smp.h   |    5 ++---
 arch/x86/lib/cache-smp.c     |    4 ++--
 drivers/char/agp/generic.c   |    3 +--
 drivers/gpu/drm/drm_cache.c  |    3 +--
 include/linux/smp.h          |    7 +++----
 kernel/smp.c                 |    6 ++----
 10 files changed, 17 insertions(+), 35 deletions(-)

on_each_cpu always returns a hard coded return code of zero.

Removing all tests based on this return value saves run time
cycles for compares and code bloat for branches.

Signed-off-by: Gilad Ben-Yossef <gilad@benyossef.com>
Acked-by: Peter Zijlstra <redacted>
Reviewed-by: Michal Nazarewicz <redacted>
CC: Benjamin Herrenschmidt <benh@kernel.crashing.org>
CC: Paul Mackerras <redacted>
CC: Grant Likely <redacted>
CC: Rob Herring <redacted>
CC: linuxppc-dev@lists.ozlabs.org
CC: devicetree-discuss@lists.ozlabs.org
---
 arch/powerpc/kernel/rtas.c |    3 +--
 1 files changed, 1 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/kernel/rtas.c b/arch/powerpc/kernel/rtas.c
index 517b1d8..829629e 100644
--- a/arch/powerpc/kernel/rtas.c
+++ b/arch/powerpc/kernel/rtas.c

@@ -850,8 +850,7 @@ int rtas_ibm_suspend_me(struct rtas_args *args)
 	/* Call function on all CPUs.  One of us will make the
 	 * rtas call
 	 */
-	if (on_each_cpu(rtas_percpu_suspend_me, &data, 0))
-		atomic_set(&data.error, -EINVAL);
+	on_each_cpu(rtas_percpu_suspend_me, &data, 0);
 
 	wait_for_completion(&done);
 

-- 
1.7.0.4

on_each_cpu returns the retunr value of smp_call_function
which is hard coded to 0.

Refactor on_each_cpu to a void function and the few callers
that check the return value to save compares and branches.

Signed-off-by: Gilad Ben-Yossef <gilad@benyossef.com>
Acked-by: Peter Zijlstra <redacted>
Reviewed-by: Michal Nazarewicz <redacted>
CC: David Airlie <redacted>
CC: dri-devel@lists.freedesktop.org
CC: Benjamin Herrenschmidt <benh@kernel.crashing.org>
CC: Paul Mackerras <redacted>
CC: Grant Likely <redacted>
CC: Rob Herring <redacted>
CC: linuxppc-dev@lists.ozlabs.org
CC: devicetree-discuss@lists.ozlabs.org
CC: Richard Henderson <redacted>
CC: Ivan Kokshaysky <redacted>
CC: Matt Turner <mattst88@gmail.com>
CC: linux-alpha@vger.kernel.org
CC: Thomas Gleixner <redacted>
CC: Ingo Molnar <mingo@redhat.com>
CC: "H. Peter Anvin" <hpa@zytor.com>
CC: x86@kernel.org
CC: Tony Luck <tony.luck@intel.com>
CC: Fenghua Yu <redacted>
CC: linux-ia64@vger.kernel.org
CC: Will Deacon <redacted>
CC: Peter Zijlstra <redacted>
CC: Arnaldo Carvalho de Melo <redacted>
CC: Russell King <redacted>
CC: linux-arm-kernel@lists.infradead.org
---
 include/linux/smp.h |    7 +++----
 kernel/smp.c        |    6 ++----
 2 files changed, 5 insertions(+), 8 deletions(-)

diff --git a/include/linux/smp.h b/include/linux/smp.h
index 8cc38d3..050ddd4 100644
--- a/include/linux/smp.h
+++ b/include/linux/smp.h

@@ -99,7 +99,7 @@ static inline void call_function_init(void) { }
 /*
  * Call a function on all processors
  */
-int on_each_cpu(smp_call_func_t func, void *info, int wait);
+void on_each_cpu(smp_call_func_t func, void *info, int wait);
 
 /*
  * Mark the boot cpu "online" so that it can call console drivers in

@@ -126,12 +126,11 @@ static inline int up_smp_call_function(smp_call_func_t func, void *info)
 #define smp_call_function(func, info, wait) \
 			(up_smp_call_function(func, info))
 #define on_each_cpu(func,info,wait)		\
-	({					\
+	{					\
 		local_irq_disable();		\
 		func(info);			\
 		local_irq_enable();		\
-		0;				\
-	})
+	}
 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 db197d6..f66a1b2 100644
--- a/kernel/smp.c
+++ b/kernel/smp.c

@@ -687,17 +687,15 @@ void __init smp_init(void)
  * early_boot_irqs_disabled is set.  Use local_irq_save/restore() instead
  * of local_irq_disable/enable().
  */
-int on_each_cpu(void (*func) (void *info), void *info, int wait)
+void on_each_cpu(void (*func) (void *info), void *info, int wait)
 {
 	unsigned long flags;
-	int ret = 0;
 
 	preempt_disable();
-	ret = smp_call_function(func, info, wait);
+	smp_call_function(func, info, wait);
 	local_irq_save(flags);
 	func(info);
 	local_irq_restore(flags);
 	preempt_enable();
-	return ret;
 }
 EXPORT_SYMBOL(on_each_cpu);

-- 
1.7.0.4

On Sun, Jan 08, 2012 at 03:32:28PM +0200, Gilad Ben-Yossef wrote:

on_each_cpu returns the retunr value of smp_call_function
which is hard coded to 0.

Refactor on_each_cpu to a void function and the few callers
that check the return value to save compares and branches.

Signed-off-by: Gilad Ben-Yossef <gilad@benyossef.com>
Acked-by: Peter Zijlstra <redacted>
Reviewed-by: Michal Nazarewicz <redacted>
CC: David Airlie <redacted>
CC: dri-devel@lists.freedesktop.org
CC: Benjamin Herrenschmidt <benh@kernel.crashing.org>
CC: Paul Mackerras <redacted>
CC: Grant Likely <redacted>
CC: Rob Herring <redacted>
CC: linuxppc-dev@lists.ozlabs.org
CC: devicetree-discuss@lists.ozlabs.org
CC: Richard Henderson <redacted>
CC: Ivan Kokshaysky <redacted>
CC: Matt Turner <mattst88@gmail.com>
CC: linux-alpha@vger.kernel.org
CC: Thomas Gleixner <redacted>
CC: Ingo Molnar <mingo@redhat.com>
CC: "H. Peter Anvin" <hpa@zytor.com>
CC: x86@kernel.org
CC: Tony Luck <tony.luck@intel.com>
CC: Fenghua Yu <redacted>
CC: linux-ia64@vger.kernel.org
CC: Will Deacon <redacted>
CC: Peter Zijlstra <redacted>
CC: Arnaldo Carvalho de Melo <redacted>
CC: Russell King <redacted>

As there's only one place in the ARM code where we look at the return
value, and you've patched that away in patch 1, this looks fine.  I've
not checked for users outside of arch/arm, so:

Acked-by: Russell King <redacted>

Thanks.