All of lore.kernel.org
 help / color / mirror / Atom feed
From: Kent Overstreet <kmo@daterainc.com>
To: Jens Axboe <axboe@kernel.dk>
Cc: Shaohua Li <shli@kernel.org>,
	linux-kernel@vger.kernel.org, hch@infradead.org
Subject: Re: [patch 1/2]percpu_ida: fix a live lock
Date: Mon, 6 Jan 2014 13:47:26 -0800	[thread overview]
Message-ID: <20140106214726.GD9037@kmo> (raw)
In-Reply-To: <52CB1783.4050205@kernel.dk>

On Mon, Jan 06, 2014 at 01:52:19PM -0700, Jens Axboe wrote:
> On 01/06/2014 01:46 PM, Kent Overstreet wrote:
> > On Sun, Jan 05, 2014 at 09:13:00PM +0800, Shaohua Li wrote:
> 
> >>> - we explicitly don't guarantee that all
> >>> the tags will be available for allocation at any given time, only half
> >>> of them. 
> >>
> >> only half of the tags can be used? this is scaring. Of course we hope all tags
> >> are available.
> > 
> > No: that behaviour is explicitly documented and it's the behaviour we want.
> 
> That is going to end horribly, for the cases (ie most cases) where we
> really want to be able to use all tags. If we can't support that
> reliably or in a fast manner with percpu ida, then that's pretty close
> to a show stopper. And I suspect that will be the case for most use
> cases. We can't just feasibly throw away half of the tag space, that's
> crazy.

Sounds like we're coming at this from different use cases, maybe we're going to
need to add some flexibility for different use cases.

For background, I wrote this code for SSDs where the you get 16 bits for the tag
ID; we really don't want the queue to be anywhere near that deep so potentially
wasting half of however many tags you have is no big deal.

If you have a device where the max tag id is small enough that you really do
need to use the entire tag space... yeah, that's going to be an issue.

> But that wont work at all. To take a concrete example, lets assume the
> device is some sort of NCQ. We have 0..31 tags. If we throw away half of
> those tags, we're at 16. You can't double the tag space in software.

Ok, so I hadn't really given any thought to that kind of use case; insofar as I
had I would've been skeptical percpu tag allocation made sense for 32 different
tags at all.

We really don't want to screw over the users that aren't so constrained by the
size of their tag space; there really is a huge performance tradeoff here
(otherwise you're stealing tags and bouncing cachelines for _every_ tag
allocation when the queue is full, and your percpu tag allocation is no longer
very percpu).

I'm not sure what the best strategy is for NCQ-type max nr_tags, though -
thoughts?

Easy thing to do for now is just to add another parameter to percpu_ida_init()
for the number of tags that are allowed to sit unused on other cpu's freelists -
users that have large relatively unbounded nr_tags can set that to nr_tags / 2,
for NCQ you'd set it to 0.

I suspect we can do better for NCQ though, w.r.t. worst case performance.

  reply	other threads:[~2014-01-06 21:47 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-12-31  3:38 [patch 1/2]percpu_ida: fix a live lock Shaohua Li
2014-01-04 21:08 ` Kent Overstreet
2014-01-05 13:13   ` Shaohua Li
2014-01-06 20:46     ` Kent Overstreet
2014-01-06 20:52       ` Jens Axboe
2014-01-06 21:47         ` Kent Overstreet [this message]
2014-02-09 15:50           ` Alexander Gordeev
2014-02-10 10:32             ` Christoph Hellwig
2014-02-10 12:29               ` Alexander Gordeev
2014-02-10 15:49                 ` Alexander Gordeev
2014-02-10 16:16                   ` Christoph Hellwig
2014-02-10 16:26               ` Jens Axboe
2014-02-10 22:41                 ` Kent Overstreet
2014-02-10 23:06                   ` Jens Axboe
2014-02-11  9:12                     ` Christoph Hellwig
2014-02-11 14:42                       ` James Bottomley
2014-02-11 14:53                         ` Christoph Hellwig
2014-02-14 10:36                     ` 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=20140106214726.GD9037@kmo \
    --to=kmo@daterainc.com \
    --cc=axboe@kernel.dk \
    --cc=hch@infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=shli@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 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.