All of lore.kernel.org
 help / color / mirror / Atom feed
From: James Bottomley <James.Bottomley@HansenPartnership.com>
To: Christoph Hellwig <hch@infradead.org>
Cc: Jens Axboe <axboe@kernel.dk>, Kent Overstreet <kmo@daterainc.com>,
	Alexander Gordeev <agordeev@redhat.com>,
	Shaohua Li <shli@kernel.org>,
	linux-kernel@vger.kernel.org, linux-scsi@vger.kernel.org
Subject: Re: [patch 1/2]percpu_ida: fix a live lock
Date: Tue, 11 Feb 2014 06:42:40 -0800	[thread overview]
Message-ID: <1392129760.2128.15.camel@dabdike.int.hansenpartnership.com> (raw)
In-Reply-To: <20140211091228.GA25567@infradead.org>

On Tue, 2014-02-11 at 01:12 -0800, Christoph Hellwig wrote:
> On Mon, Feb 10, 2014 at 04:06:27PM -0700, Jens Axboe wrote:
> > For the common case, I'd assume that anywhere between 31..256 tags
> > is "normal". That's where the majority of devices will end up being,
> > largely. So single digits would be an anomaly.
> 
> Unfortunately that's not true in SCSI land, where most driver do per-lun
> tagging, and the the cmd_per_lun values are very low and very often
> single digits, as a simple grep for cmd_per_lun will tell.

Remember we do shared (all queue) tags on qla, aic and a few other
drivers (it's actually the mailbox slot tag for the HBA).

>   Now it might
> be that the tag space actually is much bigger in the hardware and the
> driver authors for some reason want to limit the number of outstanding
> commands, but the interface to the drivers doesn't allow them to express
> such a difference at the moment.

Tag space is dependent on SCSI protocol.  It's 256 for SPI, 65536 for
SAS and I'm not sure for FCP.

> > >How about we just make the number of tags that are allowed to be stranded an
> > >explicit parameter (somehow) - then it can be up to device drivers to do
> > >something sensible with it. Half is probably an ideal default for devices where
> > >that works, but this way more constrained devices will be able to futz with it
> > >however they want.
> > 
> > I don't think we should involve device drivers in this, that's
> > punting a complicated issue to someone who likely has little idea
> > what to do about it. This needs to be handled sensibly in the core,
> > not in a device driver. If we can't come up with a sensible
> > algorithm to handle this, how can we expect someone writing a device
> > driver to do so?
> 
> Agreed, punting this to the drivers is a bad idea.  But at least
> exposing variable for the allowed tag space and allowed outstanding
> commands to be able to make a smarter decision might be a good idea.  On
> the other hand this will require us to count the outstanding commands
> again, introducing more cachelines touched than nessecary.  To make
> things worse for complex topologies like SCSI we might have to limit the
> outstanding commands at up to three levels in the hierarchy.

The list seems to be missing prior context but a few SPI drivers use the
clock algorithm for tag starvation in the driver.  The NCR ones are the
ones I know about: tag allocation is the hands of a clock sweeping
around (one for last tag and one for last outstanding tag).  The hands
are never allowed to cross, so if a tag gets starved the hands try to
cross and the driver stops issuing until the missing tag returns.  Tag
starvation used to be a known problem for Parallel devices; I haven't
seen much in the way of tag starvation algorithms for other types of
devices, so I assume the problem went away.

James



  reply	other threads:[~2014-02-11 14:42 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
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 [this message]
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=1392129760.2128.15.camel@dabdike.int.hansenpartnership.com \
    --to=james.bottomley@hansenpartnership.com \
    --cc=agordeev@redhat.com \
    --cc=axboe@kernel.dk \
    --cc=hch@infradead.org \
    --cc=kmo@daterainc.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-scsi@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.