From: Jens Axboe <axboe@kernel.dk>
To: Ming Lei <tom.leiming@gmail.com>
Cc: Alexander Gordeev <agordeev@redhat.com>,
Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
Kent Overstreet <kmo@daterainc.com>, Shaohua Li <shli@kernel.org>,
Nicholas Bellinger <nab@linux-iscsi.org>,
Ingo Molnar <mingo@redhat.com>,
Peter Zijlstra <peterz@infradead.org>
Subject: Re: [PATCH RFC 0/2] percpu_ida: Take into account CPU topology when stealing tags
Date: Fri, 25 Apr 2014 20:03:03 -0600 [thread overview]
Message-ID: <535B13D7.4050202@kernel.dk> (raw)
In-Reply-To: <CACVXFVPkiQT6yxse=sX1yuX8wLbr1sgbT4chQycbbPDR_A6hqA@mail.gmail.com>
On 2014-04-25 18:01, Ming Lei wrote:
> Hi Jens,
>
> On Sat, Apr 26, 2014 at 5:23 AM, Jens Axboe <axboe@kernel.dk> wrote:
>> On 04/25/2014 03:10 AM, Ming Lei wrote:
>>
>> Sorry, I did run it the other day. It has little to no effect here, but
>> that's mostly because there's so much other crap going on in there. The
>> most effective way to currently make it work better, is just to ensure
>> the caching pool is of a sane size.
>
> Yes, that is just what the patch is doing, :-)
But it's not enough. For instance, my test case, it's 255 tags and 64
CPUs. We end up in cross-cpu spinlock nightmare mode.
> From percpu_ida view, it is easy to observe it can improve
> allocation performance. I have several patches to export
> these information by sysfs for monitoring percpu_ida
> performance.
Sounds good!
>> I've got an alternative tagging scheme that I think would be useful for
>> the cases where the tag space to cpu ratio isn't big enough. So I think
>> we'll retain percpu_ida for the cases where it can cache enough, and
>> punt to an alternative scheme when not.
>
> OK, care to comment on the patch or the idea of setting percpu cache
> size as (nr_tags / hctx->nr_ctx)?
I think it's a good idea. The problem is that for percpu_ida to be
effective, you need a bigger cache than the 3 I'd get above. If that
isn't the case, it performs poorly. I'm just not convinced the design
can ever work in the realm of realistic queue depths.
>> That doesn't mean we should not improve percpu_ida. There's quite a bit
>> of low hanging fruit in there.
>
> IMO percpu_max_size in percpu_ida is very important for the
> performance, and it might need to adjust dynamically according
> to the percpu allocation loading, but it is far more complicated
> to implement. And it might be the simplest way to fix the parameter
> before percpu_ida_init().
That's what I did, essentially. Ensuring that the percpu_max_size is at
least 8 makes it a whole lot better here. But still slower than a
regular simple bitmap, which makes me sad. A fairly straight forward
cmpxchg based scheme I tested here is around 20% faster than the bitmap
approach on a basic desktop machine, and around 35% faster on a
4-socket. Outside of NVMe, I can't think of cases where that approach
would not be faster than percpu_ida. That means all of SCSI, basically,
and the basic block drivers.
--
Jens Axboe
next prev parent reply other threads:[~2014-04-26 2:03 UTC|newest]
Thread overview: 26+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-03-26 13:34 [PATCH RFC 0/2] percpu_ida: Take into account CPU topology when stealing tags Alexander Gordeev
2014-03-26 13:34 ` [PATCH RFC 1/2] sched: Introduce topology level masks and for_each_tlm() macro Alexander Gordeev
2014-03-26 13:34 ` [PATCH RFC 2/2] percpu_ida: Use for_each_tlm() macro for CPU lookup in steal_tags() Alexander Gordeev
2014-04-22 7:10 ` [PATCH RFC 0/2] percpu_ida: Take into account CPU topology when stealing tags Alexander Gordeev
2014-04-22 14:03 ` Jens Axboe
2014-04-22 15:57 ` Jens Axboe
2014-04-23 0:53 ` Ming Lei
2014-04-23 1:25 ` Jens Axboe
2014-04-25 9:10 ` Ming Lei
2014-04-25 21:23 ` Jens Axboe
2014-04-26 0:01 ` Ming Lei
2014-04-26 2:03 ` Jens Axboe [this message]
2014-04-29 11:35 ` Ming Lei
2014-04-29 21:13 ` Jens Axboe
2014-04-30 9:40 ` Ming Lei
2014-05-01 22:47 ` Kent Overstreet
2014-05-02 2:19 ` Jens Axboe
2014-05-02 2:38 ` Kent Overstreet
2014-05-02 2:44 ` Jens Axboe
2014-05-02 5:05 ` Christoph Hellwig
2014-05-02 16:41 ` Jens Axboe
2014-05-02 16:43 ` Christoph Hellwig
2014-05-02 16:56 ` Jens Axboe
2014-04-22 11:56 ` Peter Zijlstra
2014-05-01 21:24 ` Alexander Gordeev
2014-05-01 22:04 ` Alexander Gordeev
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=535B13D7.4050202@kernel.dk \
--to=axboe@kernel.dk \
--cc=agordeev@redhat.com \
--cc=kmo@daterainc.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mingo@redhat.com \
--cc=nab@linux-iscsi.org \
--cc=peterz@infradead.org \
--cc=shli@kernel.org \
--cc=tom.leiming@gmail.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.