Hi Gilad. A few minor corrections for several of the patch logs, but some
meater discussions on several of the patches.
Overall I like the series and hope you see it through.
On Sun Jan 08 2012 about 11:27:52 EST, Gilad Ben-Yossef wrote:
>
> We have lots of infrastructure in place to partition a multi-core systems
partition multi-core systems
> such that we have a group of CPUs that are dedicated to specific task:
> cgroups, scheduler and interrupt affinity and cpuisol boot parameter.
interrupt affinity, and isolcpus= boot parameter
> Still, kernel code will some time interrupt all CPUs in the system via IPIs
will at times
> 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
turning the global IPI
> 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. Based on the feedback from this patch set
> I plan to offer further patches that address similar issue in other code
> paths.
>
> The patch creates an on_each_cpu_mask and on_each_cpu_conf infrastructure
on_each_cpu_cond
> API (the former derived from existing arch specific versions in Tile and
> Arm) and and uses them to turn several global IPI invocation to per CPU
> group invocations.
>
> This 6th iteration includes the following changes:
>
> - In case of cpumask allocation failure, have on_each_cpu_cond
> send an IPI to each needed CPU seperately via
> smp_call_function_single so no cpumask var is needed, as
> suggested by Andrew Morton.
> - Document why on_each_cpu_mask need to check the mask even on
> UP in a code comment, as suggested by Andrew Morton.
> - Various typo cleanup in patch descriptions
>
milton
MAIL FROM: <miltonm@xxxxxxx>
RCPT TO: <miltonm@xxxxxxx>
RCPT TO: <gilad@xxxxxxxxxxxxx>
RCPT TO: <linux-kernel@xxxxxxxxxxxxxxx>
RCPT TO: <cl@xxxxxxxxx>
RCPT TO: <cmetcalf@xxxxxxxxxx>
RCPT TO: <a.p.zijlstra@xxxxxxxxx>
RCPT TO: <fweisbec@xxxxxxxxx>
RCPT TO: <linux@xxxxxxxxxxxxxxxx>
RCPT TO: <linux-mm@xxxxxxxxx>
RCPT TO: <penberg@xxxxxxxxxx>
RCPT TO: <mpm@xxxxxxxxxxx>
RCPT TO: <riel@xxxxxxxxxx>
RCPT TO: <andi@xxxxxxxxxxxxxx>
RCPT TO: <levinsasha928@xxxxxxxxx>
RCPT TO: <mel@xxxxxxxxx>
RCPT TO: <akpm@xxxxxxxxxxxxxxxxxxxx>
RCPT TO: <viro@xxxxxxxxxxxxxxxxxx>
RCPT TO: <linux-fsdevel@xxxxxxxxxxxxxxx>
RCPT TO: <avi@xxxxxxxxxx>
RCPT TO: <mina86@xxxxxxxxxx>
RCPT TO: <kosaki.motohiro@xxxxxxxxx>
DATA
From: Milton Miller <miltonm@xxxxxxx>
Bcc: Milton Miller <miltonm@xxxxxxx>
Subject: Re: [PATCH v6 1/8] smp: Introduce a generic on_each_cpu_mask function
In-Reply-To: <1326040026-7285-2-git-send-email-gilad@xxxxxxxxxxxxx>
References: <1326040026-7285-2-git-send-email-gilad@xxxxxxxxxxxxx>
To: Gilad Ben-Yossef <gilad@xxxxxxxxxxxxx>,
<linux-kernel@xxxxxxxxxxxxxxx>
Cc: Christoph Lameter <cl@xxxxxxxxx>,
Chris Metcalf <cmetcalf@xxxxxxxxxx>,
Peter Zijlstra <a.p.zijlstra@xxxxxxxxx>,
Frederic Weisbecker <fweisbec@xxxxxxxxx>,
Russell King <linux@xxxxxxxxxxxxxxxx>,
<linux-mm@xxxxxxxxx>,
Pekka Enberg <penberg@xxxxxxxxxx>,
Matt Mackall <mpm@xxxxxxxxxxx>,
Rik van Riel <riel@xxxxxxxxxx>,
Andi Kleen <andi@xxxxxxxxxxxxxx>,
Sasha Levin <levinsasha928@xxxxxxxxx>,
Mel Gorman <mel@xxxxxxxxx>,
Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx>,
Alexander Viro <viro@xxxxxxxxxxxxxxxxxx>,
<linux-fsdevel@xxxxxxxxxxxxxxx>,
Avi Kivity <avi@xxxxxxxxxx>,
Michal Nazarewicz <mina86@xxxxxxxxxx>,
Kosaki Motohiro <kosaki.motohiro@xxxxxxxxx>
On Sun, 8 Jan 2012 about 18:26:59 +0200, Gilad Ben-Yossef wrote:
>
> on_each_cpu_mask calls a function on processors specified by
> cpumask, which may or may not include the local processor.
>
> All the limitation specified in smp_call_function_many apply.
limitations
Except they don't, see below
>
> Signed-off-by: Gilad Ben-Yossef <gilad@xxxxxxxxxxxxx>
> Reviewed-by: Christoph Lameter <cl@xxxxxxxxx>
..
> ---
> include/linux/smp.h | 22 ++++++++++++++++++++++
> kernel/smp.c | 20 ++++++++++++++++++++
> 2 files changed, 42 insertions(+), 0 deletions(-)
> diff --git a/include/linux/smp.h b/include/linux/smp.h
> index 8cc38d3..a3a14d9 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);
> +
func should be smp_call_func_t
> +/*
> * Mark the boot cpu "online" so that it can call console drivers in
> * printk() and can access its per-cpu storage.
> */
> @@ -132,6 +139,21 @@ static inline int up_smp_call_function(smp_call_func_t func, void *info)
> local_irq_enable(); \
> 0; \
> })
> +/*
> + * Note we still need to test the mask even for UP
> + * because we actually can get an empty mask from
> + * code that on SMP might call us without the local
> + * CPU in the mask.
> + */
> +#define on_each_cpu_mask(mask, func, info, wait) \
> + do { \
> + if (cpumask_test_cpu(0, (mask))) { \
> + local_irq_disable(); \
> + (func)(info); \
> + local_irq_enable(); \
> + } \
> + } while (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..7c0cbd7 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.
> + */
Please turn this comment into kerneldoc like the smp_call_function* family.
Also, this is not accurate, as smp_call_function_many requires
preemption to have been disabled while on_each_cpu_mask disables
preemption (via get_cpu).
> +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);
It should be less code if we rewrite on_each_cpu as the one liner
on_each_cpu_mask(cpu_online_mask). I think the trade off of less
code is worth the cost of the added test of cpu being in online_mask.
That could be a seperate patch, but will be easier to read the result
if on_each_cpu_mask is placed above on_each_cpu in this one.
milton
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@xxxxxxxxx. 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@xxxxxxxxx";> email@xxxxxxxxx </a>