public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
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: Tue, 20 Aug 2013 19:31:51 -0700	[thread overview]
Message-ID: <20130821023151.GC4051@kmo-pixel> (raw)
In-Reply-To: <20130821020742.GA3495@htj.dyndns.org>

On Tue, Aug 20, 2013 at 10:07:42PM -0400, Tejun Heo wrote:
> Hello, Kent.
> 
> On Tue, Aug 20, 2013 at 07:01:32PM -0700, Kent Overstreet wrote:
> > I think Tejun and I might be at a bit of an impasse with the ida rewrite
> > itself, but I don't think there were any outstanding objections to the
> > percpu ida code itself - and this is a standalone version.
> 
> The percpu ida code can be applied separately from the ida rewrite?

Yes - at the cost of using significantly more memory for the global
freelist

> > I was meaning to ask you Andrew, if you could take a look at the ida
> > discussion and lend your opinion - I don't think there's any _specific_
> > technical objections left to my ida code, and it's now on a more
> > philisophical "complexity vs. ..." level.
> 
> Hmmm... the objection was pretty specific - don't depend on high-order
> or vmalloc allocations when it can be easily avoided by using proper
> radix tree.

We only do vmalloc allocations for CONFIG_COMPACTION=n, and then only
when we need to allocate more than almost 1 _billion_ ids from a single
ida (twice than on 32 bit, so never because that gets us just about to
INT_MAX) - and then it's only 32k of vmalloc memory, for the entire ida.

This is with max allocation order of 4 for COMPACTION=y, 2 for
COMPACTION=n. 

All this for a performance improvement of 10x to 50x (or more), for the
ida sizes I measured.

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.

(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)

  reply	other threads:[~2013-08-21  2: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 [this message]
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
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=20130821023151.GC4051@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