From: Avi Kivity <avi@redhat.com>
To: Gilad Ben-Yossef <gilad@benyossef.com>
Cc: Pekka Enberg <penberg@kernel.org>,
linux-kernel@vger.kernel.org, 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, Matt Mackall <mpm@selenic.com>,
Sasha Levin <levinsasha928@gmail.com>,
Rik van Riel <riel@redhat.com>, Andi Kleen <andi@firstfloor.org>,
apkm@linux-foundation.org
Subject: Re: [PATCH v4 4/5] slub: Only IPI CPUs that have per cpu obj to flush
Date: Sun, 08 Jan 2012 18:15:27 +0200 [thread overview]
Message-ID: <4F09C11F.9060503@redhat.com> (raw)
In-Reply-To: <CAOtvUMcnjfLxQW7iGKWbPJ=MVhBWdyAyeVJB0sCDb-phkXOx-g@mail.gmail.com>
On 01/08/2012 06:13 PM, Gilad Ben-Yossef wrote:
> On Mon, Jan 2, 2012 at 3:30 PM, Avi Kivity <avi@redhat.com> wrote:
> > On 01/02/2012 01:59 PM, Gilad Ben-Yossef wrote:
> >> On Sun, Jan 1, 2012 at 6:50 PM, Avi Kivity <avi@redhat.com> wrote:
> >> > On 01/01/2012 06:12 PM, Gilad Ben-Yossef wrote:
> >> >> >
> >> >> > Since this seems to be a common pattern, how about:
> >> >> >
> >> >> > zalloc_cpumask_var_or_all_online_cpus(&cpus, GFTP_ATOMIC);
> >> >> > ...
> >> >> > free_cpumask_var(cpus);
> >> >> >
> >> >> > The long-named function at the top of the block either returns a newly
> >> >> > allocated zeroed cpumask, or a static cpumask with all online cpus set.
> >> >> > The code in the middle is only allowed to set bits in the cpumask
> >> >> > (should be the common usage). free_cpumask_var() needs to check whether
> >> >> > the freed object is the static variable.
> >> >>
> >> >> Thanks for the feedback and advice! I totally agree the repeating
> >> >> pattern needs abstracting.
> >> >>
> >> >> I ended up chosing to try a different abstraction though - basically a wrapper
> >> >> on_each_cpu_cond that gets a predicate function to run per CPU to
> >> >> build the mask
> >> >> to send the IPI to. It seems cleaner to me not having to mess with
> >> >> free_cpumask_var
> >> >> and it abstracts more of the general pattern.
> >> >>
> >> >
> >> > This converts the algorithm to O(NR_CPUS) from a potentially lower
> >> > complexity algorithm. Also, the existing algorithm may not like to be
> >> > driven by cpu number. Both are true for kvm.
> >> >
> >>
> >> Right, I was only thinking on my own uses, which are O(NR_CPUS) by nature.
> >>
> >> I wonder if it would be better to create a safe_cpumask_var type with
> >> its own alloc function
> >> free and and sset_cpu function but no clear_cpu function so that the
> >> compiler will catch
> >> cases of trying to clear bits off of such a cpumask?
> >>
> >> It seems safer and also makes handling the free function easier.
> >>
> >> Does that makes sense or am I over engineering it? :-)
> >
> > It makes sense. Depends on the number of call sites, really. If there
> > are several, consolidation helps, also makes it easier to further refactor.
>
> As Andrew pointed out code that usually calls just the CPU you wanted but
> sometime (under memory pressure) might call other CPUs is a problem
> waiting to happen, so I took his advice and re factored to use
> smp_call_function_single to IPI each CPU individually in case of alloc failure.
>
> I don't know if that applied to the KVM use case. I'm guessing it
> probably doesn't
> from what you wrote here.
No, it doesn't. But note it isn't the case you're covering anyway. kvm
already only IPIs relevant cpus, it just suffers from that ugly
allocation failure case.
--
error compiling committee.c: too many arguments to function
--
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>
next prev parent reply other threads:[~2012-01-08 16:15 UTC|newest]
Thread overview: 29+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-11-22 11:08 [PATCH v4 0/5] Reduce cross CPU IPI interference Gilad Ben-Yossef
2011-11-22 11:08 ` [PATCH v4 1/5] smp: Introduce a generic on_each_cpu_mask function Gilad Ben-Yossef
2011-11-22 11:08 ` [PATCH v4 2/5] arm: Move arm over to generic on_each_cpu_mask Gilad Ben-Yossef
2011-11-22 21:00 ` Russell King - ARM Linux
2011-11-23 6:47 ` Gilad Ben-Yossef
2011-11-22 11:08 ` [PATCH v4 3/5] tile: Move tile to use " Gilad Ben-Yossef
2011-11-22 11:08 ` [PATCH v4 4/5] slub: Only IPI CPUs that have per cpu obj to flush Gilad Ben-Yossef
2011-11-23 6:23 ` Pekka Enberg
2011-11-23 6:57 ` Pekka Enberg
2011-11-23 7:52 ` Gilad Ben-Yossef
2012-01-01 12:41 ` Avi Kivity
2012-01-01 16:12 ` Gilad Ben-Yossef
2012-01-01 16:50 ` Avi Kivity
2012-01-02 11:59 ` Gilad Ben-Yossef
2012-01-02 13:30 ` Avi Kivity
2012-01-08 16:13 ` Gilad Ben-Yossef
2012-01-08 16:15 ` Avi Kivity [this message]
2011-11-22 11:08 ` [PATCH v4 5/5] mm: Only IPI CPUs to drain local pages if they exist Gilad Ben-Yossef
2011-11-23 7:45 ` Pekka Enberg
2011-12-23 10:28 ` Mel Gorman
2011-12-25 9:39 ` Gilad Ben-Yossef
2011-12-30 15:04 ` Mel Gorman
2011-12-30 15:25 ` Chris Metcalf
2011-12-30 16:08 ` Mel Gorman
2011-12-30 20:29 ` Gilad Ben-Yossef
2012-01-01 8:03 ` Gilad Ben-Yossef
2011-12-30 20:16 ` Gilad Ben-Yossef
2011-11-23 1:36 ` [PATCH v4 0/5] Reduce cross CPU IPI interference Chris Metcalf
2011-11-23 6:52 ` 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=4F09C11F.9060503@redhat.com \
--to=avi@redhat.com \
--cc=a.p.zijlstra@chello.nl \
--cc=andi@firstfloor.org \
--cc=apkm@linux-foundation.org \
--cc=cmetcalf@tilera.com \
--cc=fweisbec@gmail.com \
--cc=gilad@benyossef.com \
--cc=levinsasha928@gmail.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=linux@arm.linux.org.uk \
--cc=mpm@selenic.com \
--cc=penberg@kernel.org \
--cc=riel@redhat.com \
/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).