mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Tejun Heo <tj@kernel.org>
To: Kent Overstreet <kmo@daterainc.com>
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 17:31:44 -0400	[thread overview]
Message-ID: <20130821213144.GE2436@htj.dyndns.org> (raw)
In-Reply-To: <20130821212442.GC4657@kmo-pixel>

Hello, Kent.

On Wed, Aug 21, 2013 at 02:24:42PM -0700, Kent Overstreet wrote:
> With single page allocations:
> 
> 1 << 15 bits per page
> 
> 1 << 9 pointers per page
> 
> So two layers of pointers does get us to 1 << 33 bits, which is what we
> need.

And single layer - 1 << 15 - would cover most of the use cases, right?
With 1 << (9 + 15) probably covering everyone else but the cyclic ones
doing the full circle.

> But now, since we need two layers of pointers instead of one, we need
> either another pointer deref for a node lookup - _always_, even when
> we've got 8 bytes of bits - or we need to branch on the depth of the
> tree, which is something we don't have now.

A likely() branch which is almost always hit is *extremely* cheap.

> This is extra overhead _no matter the size of the ida_, over my current
> approach.
> I'm assuming the common case is < one page of bits, based on the usage
> I've seen throughout the kernel that's probably way conservative.
> 
> In that case, your approach is going to be slower than mine, and there's
> no difference in the size of the allocations.

By single likely() branch.  I'm not even sure that'd be measureable in
most cases.  I'd take that over custom radix tree implementation which
needs high order allocations.

> I've already shown massive performance gains over the existing radix
> tree approach, you're the one claiming a different approach would be
> better.

So?  What difference does that make?  You should be able to justify
your custom thing.  If you do something unusual, of course someone is
gonna ask you to justify it and justifying that is *your*
responsibility.

Thanks.

-- 
tejun

  reply	other threads:[~2013-08-21 21:31 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
2013-08-21 21:16                           ` Tejun Heo
2013-08-21 21:24                             ` Kent Overstreet
2013-08-21 21:31                               ` Tejun Heo [this message]
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=20130821213144.GE2436@htj.dyndns.org \
    --to=tj@kernel.org \
    --cc=akpm@linux-foundation.org \
    --cc=andi@firstfloor.org \
    --cc=axboe@kernel.dk \
    --cc=cl@gentwo.org \
    --cc=kmo@daterainc.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@redhat.com \
    --cc=nab@linux-iscsi.org \
    --cc=oleg@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

all inboxes | Powered by JetHome®