All of lore.kernel.org
 help / color / mirror / Atom feed
From: Mike Christie <michaelc@cs.wisc.edu>
To: James Bottomley <James.Bottomley@HansenPartnership.com>
Cc: Jens Axboe <jens.axboe@oracle.com>,
	Mike Anderson <andmike@linux.vnet.ibm.com>,
	linux-scsi <linux-scsi@vger.kernel.org>
Subject: Re: Block timeouts seem not to be working
Date: Mon, 15 Sep 2008 15:01:30 -0500	[thread overview]
Message-ID: <48CEBF1A.3010400@cs.wisc.edu> (raw)
In-Reply-To: <1221256012.3319.21.camel@localhost.localdomain>

James Bottomley wrote:
> On Thu, 2008-09-11 at 20:35 +0200, Jens Axboe wrote:
>> On Thu, Sep 11 2008, James Bottomley wrote:
>>> I just noticed this with a rather finickey SAS system I have.  It's got
>>> a SATA DVD attached over an expander.  Periodically the DVD just hangs
>>> up, so we wait for the timeout and then send a phy reset which clears
>>> it.
>>>
>>> What I'm seeing with the new block timer code is that the timer never
>>> expires.  I can dig some more into this, but if you wanted to test it as
>>> well, the timer code is easy to excite.  Just throw away one command in
>>> every 128 or so in the queuecommand routine of your favourite HBA
>>> driver.
>> James, I've seen a few oddities as well, I'll be beating on it tomorrow
>> again to shake out the last bug(s).
> 
> Actually, turns out it's nothing to do with block timeouts, it's a
> target reset bug.
> 
> This loop:
> 
> 	for (id = 0; id <= shost->max_id; id++) {
> 
> Never terminates if shost->max_id is set to ~0, like aic94xx does.
> 
> It's also pretty inefficient since you mostly have compact target
> numbers, but the max_id can be very high.  The best way would be to sort
> the recovery list by target id and skip them if they're equal, but even
> a worst case O(N^2) traversal is probably OK here.
> 

Sorry about that. I really screwed up on multiple counts there. I tested 
your patch with Linus's tree here on some drivers by setting the command 
timeout to 1 second and letting IO run against slow targets. I saw the 
eh run multiple times and it ran fine with your patch. Thanks for 
finding that.

      reply	other threads:[~2008-09-15 20:08 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-09-11 15:05 Block timeouts seem not to be working James Bottomley
2008-09-11 15:42 ` Mike Anderson
2008-09-11 18:35 ` Jens Axboe
2008-09-12 21:46   ` James Bottomley
2008-09-15 20:01     ` Mike Christie [this message]

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=48CEBF1A.3010400@cs.wisc.edu \
    --to=michaelc@cs.wisc.edu \
    --cc=James.Bottomley@HansenPartnership.com \
    --cc=andmike@linux.vnet.ibm.com \
    --cc=jens.axboe@oracle.com \
    --cc=linux-scsi@vger.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.