All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Jörn Engel" <joern@logfs.org>
To: Hannes Reinecke <hare@suse.de>
Cc: James Bottomley <jbottomley@parallels.com>,
	linux-scsi@vger.kernel.org, Ewan Milne <emilne@redhat.com>,
	Ren Mingxin <renmx@cn.fujitsu.com>,
	Bart van Assche <bvanassche@acm.org>
Subject: Re: [PATCHv2 0/7] Limit overall SCSI EH runtime
Date: Mon, 1 Jul 2013 13:44:23 -0400	[thread overview]
Message-ID: <20130701174423.GA10645@logfs.org> (raw)
In-Reply-To: <1372661455-122384-1-git-send-email-hare@suse.de>

On Mon, 1 July 2013 08:50:48 +0200, Hannes Reinecke wrote:
> 
> This patchset implements a new 'eh_deadline' attribute to the
> SCSI host. It will limit the overall SCSI EH runtime by a given
> timeout. If the timeout is reached all intermediate EH steps
> will be skipped and host reset will be scheduled immediately.

I have mixed opinions about the concept.

Having a command timeout is of limited use if you can still spend
several minutes after the timeout in random processing.  Userspace
either needs -EIO reasonably quickly after a command timeout or will
have to implement it's own timeout mechanism.  I prefer having a
single implementation in the kernel, so your patches are a step in the
right direction.

Host reset is an expensive and harmful operation.  You lose access to
all devices behind the host.  At best this is a performance blip, at
worst someone actually cared about some realtime properties.  My main
grump is that a single bad device can trigger this behaviour,
essentially doing a DoS on the rest of the system.  While that problem
is somewhat orthogonal, your patchset can only make matters worse.

Ideally we would have a way to detect the system geometry and next the
error location.  If a single device is bad, don't ever do a host
reset.  If you have redundant paths, never do a host reset on both
controllers at the same time.  Etc, etc.

Getting there will be a lot of work and the result may be too
error-prone to maintain without constantly breaking one exotic setup
or another.  But if someone could pull it off, it would be really nice
to have.

That said, now I should actually read your patches. ;)

Jörn

--
Measure. Don't tune for speed until you've measured, and even then
don't unless one part of the code overwhelms the rest.
-- Rob Pike
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

  parent reply	other threads:[~2013-07-01 19:15 UTC|newest]

Thread overview: 32+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-07-01  6:50 [PATCHv2 0/7] Limit overall SCSI EH runtime Hannes Reinecke
2013-07-01  6:50 ` [PATCH 1/7] dpt_i2o: Remove DPTI_STATE_IOCTL Hannes Reinecke
2013-07-01  6:50 ` [PATCH 2/7] dpt_i2o: return SCSI_MLQUEUE_HOST_BUSY when in reset Hannes Reinecke
2013-07-01  6:50 ` [PATCH 3/7] advansys: Remove 'last_reset' references Hannes Reinecke
2013-07-01  6:50 ` [PATCH 4/7] tmscsim: Move 'last_reset' into host structure Hannes Reinecke
2013-07-01  6:50 ` [PATCH 5/7] dc395: Move 'last_reset' into internal " Hannes Reinecke
2013-07-01  6:50 ` [PATCH 6/7] scsi: remove check for 'resetting' Hannes Reinecke
2013-07-01  6:50 ` [PATCH 7/7] scsi: Add 'eh_deadline' to limit SCSI EH runtime Hannes Reinecke
2013-09-20  7:48   ` Ren Mingxin
2013-10-16 19:22   ` James Bottomley
2013-10-17 14:27     ` Ewan Milne
2013-10-23  9:25     ` Hannes Reinecke
2013-10-23  7:46       ` James Bottomley
2013-10-23  9:49         ` Hannes Reinecke
2013-07-01 17:44 ` Jörn Engel [this message]
2013-07-01 19:23   ` [PATCHv2 0/7] Limit overall " James Bottomley
2013-07-01 20:55     ` Jörn Engel
2013-07-02  5:48       ` Hannes Reinecke
2013-07-02  6:37       ` James Bottomley
2013-07-02 14:58         ` Jörn Engel
2013-07-02 16:33           ` James Bottomley
2013-07-02 15:50             ` Jörn Engel
2013-07-10 20:35 ` Ewan Milne
2013-07-12  5:54   ` Ren Mingxin
2013-07-12 13:30     ` Ewan Milne
2013-07-15 10:33       ` Ren Mingxin
2013-07-26  9:52         ` Ren Mingxin
2013-08-07  6:43   ` Ren Mingxin
2013-08-29 13:06     ` Hannes Reinecke
2013-09-24 20:51       ` Ric Wheeler
2013-09-25  5:48         ` Hannes Reinecke
2013-10-02 16:21           ` Hannes Reinecke

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=20130701174423.GA10645@logfs.org \
    --to=joern@logfs.org \
    --cc=bvanassche@acm.org \
    --cc=emilne@redhat.com \
    --cc=hare@suse.de \
    --cc=jbottomley@parallels.com \
    --cc=linux-scsi@vger.kernel.org \
    --cc=renmx@cn.fujitsu.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.