From: Milton Miller <miltonm@bga.com>
To: Gilad Ben-Yossef <gilad@benyossef.com>, <linux-kernel@vger.kernel.org>
Cc: Christoph Lameter <cl@linux.com>,
Chris Metcalf <cmetcalf@tilera.com>,
Peter Zijlstra <a.p.zijlstra@chello.nl>,
Frederic Weisbecker <fweisbec@gmail.com>,
Russell King <linux@arm.linux.org.uk>, <linux-mm@kvack.org>,
Pekka Enberg <penberg@kernel.org>, Matt Mackall <mpm@selenic.com>,
Rik van Riel <riel@redhat.com>, Andi Kleen <andi@firstfloor.org>,
Sasha Levin <levinsasha928@gmail.com>, Mel Gorman <mel@csn.ul.ie>,
Andrew Morton <akpm@linux-foundation.org>,
Alexander Viro <viro@zeniv.linux.org.uk>,
<linux-fsdevel@vger.kernel.org>, Avi Kivity <avi@redhat.com>,
Michal Nazarewicz <mina86@mina86.org>,
Kosaki Motohiro <kosaki.motohiro@gmail.com>
Subject: Re: [PATCH v6 0/8] Reduce cross CPU IPI interference
Date: Wed, 11 Jan 2012 01:04:09 -0600 [thread overview]
Message-ID: <1326265449_1658@mail4.comsite.net> (raw)
In-Reply-To: <1326040026-7285-1-git-send-email-gilad@benyossef.com>
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@bga.com>
RCPT TO: <miltonm@bga.com>
RCPT TO: <gilad@benyossef.com>
RCPT TO: <linux-kernel@vger.kernel.org>
RCPT TO: <cl@linux.com>
RCPT TO: <cmetcalf@tilera.com>
RCPT TO: <a.p.zijlstra@chello.nl>
RCPT TO: <fweisbec@gmail.com>
RCPT TO: <linux@arm.linux.org.uk>
RCPT TO: <linux-mm@kvack.org>
RCPT TO: <penberg@kernel.org>
RCPT TO: <mpm@selenic.com>
RCPT TO: <riel@redhat.com>
RCPT TO: <andi@firstfloor.org>
RCPT TO: <levinsasha928@gmail.com>
RCPT TO: <mel@csn.ul.ie>
RCPT TO: <akpm@linux-foundation.org>
RCPT TO: <viro@zeniv.linux.org.uk>
RCPT TO: <linux-fsdevel@vger.kernel.org>
RCPT TO: <avi@redhat.com>
RCPT TO: <mina86@mina86.org>
RCPT TO: <kosaki.motohiro@gmail.com>
DATA
From: Milton Miller <miltonm@bga.com>
Bcc: Milton Miller <miltonm@bga.com>
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@benyossef.com>
References: <1326040026-7285-2-git-send-email-gilad@benyossef.com>
To: Gilad Ben-Yossef <gilad@benyossef.com>,
<linux-kernel@vger.kernel.org>
Cc: Christoph Lameter <cl@linux.com>,
Chris Metcalf <cmetcalf@tilera.com>,
Peter Zijlstra <a.p.zijlstra@chello.nl>,
Frederic Weisbecker <fweisbec@gmail.com>,
Russell King <linux@arm.linux.org.uk>,
<linux-mm@kvack.org>,
Pekka Enberg <penberg@kernel.org>,
Matt Mackall <mpm@selenic.com>,
Rik van Riel <riel@redhat.com>,
Andi Kleen <andi@firstfloor.org>,
Sasha Levin <levinsasha928@gmail.com>,
Mel Gorman <mel@csn.ul.ie>,
Andrew Morton <akpm@linux-foundation.org>,
Alexander Viro <viro@zeniv.linux.org.uk>,
<linux-fsdevel@vger.kernel.org>,
Avi Kivity <avi@redhat.com>,
Michal Nazarewicz <mina86@mina86.org>,
Kosaki Motohiro <kosaki.motohiro@gmail.com>
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@benyossef.com>
> Reviewed-by: Christoph Lameter <cl@linux.com>
..
> ---
> 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
next prev parent reply other threads:[~2012-01-11 7:04 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-01-08 16:26 [PATCH v6 0/8] Reduce cross CPU IPI interference Gilad Ben-Yossef
2012-01-08 16:32 ` Gilad Ben-Yossef
2012-01-09 9:30 ` Peter Zijlstra
2012-01-09 17:57 ` Chris Metcalf
2012-01-11 7:04 ` Milton Miller [this message]
2012-01-11 8:28 ` Gilad Ben-Yossef
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1326265449_1658@mail4.comsite.net \
--to=miltonm@bga.com \
--cc=a.p.zijlstra@chello.nl \
--cc=akpm@linux-foundation.org \
--cc=andi@firstfloor.org \
--cc=avi@redhat.com \
--cc=cl@linux.com \
--cc=cmetcalf@tilera.com \
--cc=fweisbec@gmail.com \
--cc=gilad@benyossef.com \
--cc=kosaki.motohiro@gmail.com \
--cc=levinsasha928@gmail.com \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=linux@arm.linux.org.uk \
--cc=mel@csn.ul.ie \
--cc=mina86@mina86.org \
--cc=mpm@selenic.com \
--cc=penberg@kernel.org \
--cc=riel@redhat.com \
--cc=viro@zeniv.linux.org.uk \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).