From: Kent Overstreet <kmo@daterainc.com>
To: Tejun Heo <tj@kernel.org>
Cc: Andrew Morton <akpm@linux-foundation.org>,
"Nicholas A. Bellinger" <nab@linux-iscsi.org>,
Christoph Lameter <cl@gentwo.org>,
linux-kernel@vger.kernel.org, Oleg Nesterov <oleg@redhat.com>,
Ingo Molnar <mingo@redhat.com>, Andi Kleen <andi@firstfloor.org>,
Jens Axboe <axboe@kernel.dk>
Subject: Re: [PATCH] idr: Use this_cpu_ptr() for percpu_ida
Date: Wed, 21 Aug 2013 14:09:01 -0700 [thread overview]
Message-ID: <20130821210901.GA4657@kmo-pixel> (raw)
In-Reply-To: <20130821115941.GA18617@mtj.dyndns.org>
On Wed, Aug 21, 2013 at 07:59:41AM -0400, Tejun Heo wrote:
> Hello, Kent.
>
> On Tue, Aug 20, 2013 at 07:31:51PM -0700, Kent Overstreet wrote:
> > All this for a performance improvement of 10x to 50x (or more), for the
> > ida sizes I measured.
>
> That's misleading, isn't it?
It's comparing it to the existing version that actually exists, instead
of comparing it to a hypothetical approach that doesn't exist yet. I
don't see how that's misleading.
> We should see large performance
> improvements even without the large pages. What matters more is the
> leaf node performance for vast majority of cases and an extra radix
> tree layer on top would cover most of whatever is left. Whether to
> use high order pages or not only affects the extreme niche use cases
> and I don't think going for high order pages to micro optimize those
> extreme use cases is the right trade off.
>
> > So I could see your point if we were allocating gobs of vmalloc memory,
> > or high order allocations big enough to realistically be problematic (I
> > honestly don't think these will be) - but to me, this seems like a
> > pretty reasonable tradeoff for those performance gains.
>
> The trade off is made up as the bulk of the performance benefit can be
> gained without resorting to high order allocations.
I'm more and more skeptical that that's true, and it's a given that it
wouldn't be as fast.
> > (And the performance gains do substantially come from using more
> > contiguous memory and treating the whole data structure as an array, and
> > doing less pointer chasing/looping)
>
> I really have hard time buying that. Let's say you go with single
> page leaf node and an extra single page layer on top. How many IDs
> are we talking about? For the cases which are most performance
> sensitive, this doesn't even matter a bit as percpu caching layer
> would be on top anyway. I really don't think the micro optimization
> is called for at the cost of high order allocations from low level
> tool library.
These "micro optimizations" mean either less pointer chasing or less
branching in the _common_ case; you'd trade common case performance for
avoiding ever doing higher order allocations (and 2 with COMPACTION=n
and 4 with COMPACTION=y is not particularly high order!).
I don't buy that that's a good tradeoff. If you're convinced radix trees
are the way to go and it can be done without much performance cost, why
not code it up and show us?
next prev parent reply other threads:[~2013-08-21 21:08 UTC|newest]
Thread overview: 40+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-08-07 17:34 IDA/IDR rewrite, percpu ida Kent Overstreet
2013-08-07 17:34 ` [PATCH 03/10] idr: Rewrite ida Kent Overstreet
2013-08-07 20:22 ` Tejun Heo
2013-08-07 20:51 ` [PATCH] idr: Document ida tree sections Kent Overstreet
2013-08-09 14:57 ` Tejun Heo
2013-08-13 22:13 ` Kent Overstreet
2013-08-13 22:19 ` Tejun Heo
2013-08-13 22:27 ` Kent Overstreet
2013-08-13 22:44 ` Tejun Heo
2013-08-13 22:59 ` Kent Overstreet
2013-08-13 23:22 ` Tejun Heo
2013-08-13 23:51 ` Kent Overstreet
2013-08-13 23:59 ` Tejun Heo
2013-08-15 0:04 ` Kent Overstreet
2013-08-15 0:22 ` Tejun Heo
2013-08-13 22:33 ` Kent Overstreet
2013-08-07 17:34 ` [PATCH 04/10] idr: Percpu ida Kent Overstreet
2013-08-07 17:56 ` Christoph Lameter
2013-08-07 18:33 ` Kent Overstreet
2013-08-07 19:40 ` Christoph Lameter
2013-08-07 19:57 ` [PATCH] idr: Use this_cpu_ptr() for percpu_ida Kent Overstreet
2013-08-08 14:32 ` Christoph Lameter
2013-08-20 21:19 ` Nicholas A. Bellinger
2013-08-20 21:29 ` Andrew Morton
2013-08-21 2:01 ` Kent Overstreet
2013-08-21 2:07 ` Tejun Heo
2013-08-21 2:31 ` Kent Overstreet
2013-08-21 11:59 ` Tejun Heo
2013-08-21 21:09 ` Kent Overstreet [this message]
2013-08-21 21:16 ` Tejun Heo
2013-08-21 21:24 ` Kent Overstreet
2013-08-21 21:31 ` Tejun Heo
2013-08-21 14:32 ` Christoph Lameter
2013-08-21 17:49 ` Nicholas A. Bellinger
2013-08-21 20:49 ` Andrew Morton
2013-08-22 16:44 ` Christoph Lameter
2013-08-22 16:56 ` Jens Axboe
2013-08-07 17:46 ` [PATCH 05/10] idr: Kill old deprecated idr interfaces Kent Overstreet
2013-08-07 17:46 ` [PATCH 06/10] idr: Rename idr_get_next() -> idr_find_next() Kent Overstreet
2013-08-07 17:46 ` [PATCH 08/10] idr: Reimplement idr on top of ida/radix trees Kent Overstreet
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=20130821210901.GA4657@kmo-pixel \
--to=kmo@daterainc.com \
--cc=akpm@linux-foundation.org \
--cc=andi@firstfloor.org \
--cc=axboe@kernel.dk \
--cc=cl@gentwo.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mingo@redhat.com \
--cc=nab@linux-iscsi.org \
--cc=oleg@redhat.com \
--cc=tj@kernel.org \
/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).