All of lore.kernel.org
 help / color / mirror / Atom feed
From: Christoph Hellwig <hch@infradead.org>
To: James Bottomley <James.Bottomley@HansenPartnership.com>
Cc: Christoph Hellwig <hch@infradead.org>,
	Steven Haber <steven@qumulo.com>,
	linux-scsi@vger.kernel.org, Jens Axboe <axboe@kernel.dk>
Subject: Re: SCSI eats error from flush failure during hot plug
Date: Wed, 25 Jun 2014 06:13:44 -0700	[thread overview]
Message-ID: <20140625131344.GA13094@infradead.org> (raw)
In-Reply-To: <1403201159.17294.13.camel@dabdike.int.hansenpartnership.com>

On Thu, Jun 19, 2014 at 11:05:59AM -0700, James Bottomley wrote:
> That's not really a good idea either ... I did think of it.  We'll end
> up with a cmd_type of REQ_TYPE_FS which because of REQ_FLUSH (or REQ_FUA
> or REQ_DISCARD or any number of other things) we have to treat as though
> it were REQ_TYPE_BLOCK_PC.  It's much better to tune handling
> expectations according to req->cmd_type because that's what we already
> do.  These commands are actually set up by our handlers, so it's up to
> us to mark the request type correctly.

Looking at the places where the SCSI midlayer cares about the request
type:

 - scsi_finish_command to call ->done for non-PC requests.  Given that
   we called into the driver to setup flush/discard/etc we should also
   call into the driver on request completion
 - scsi_eh_action: ditto for error handling
 - scsi_noretry_cmd: I don't see why we'd want to treat flush request
	as having an implicit failfast flag
 - scsi_io_completion: this mostly opts out of all kinds of error
	handling and retries, not really what we'd want either
 - scsi_unprep_fn: calls ->uninit_command only for !PC request,
   	so your patch introduces a leak for discard requests


  reply	other threads:[~2014-06-25 13:13 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-06-05 23:52 SCSI eats error from flush failure during hot plug Steven Haber
2014-06-07 19:29 ` James Bottomley
2014-06-09 17:21   ` Steven Haber
2014-06-09 17:29     ` James Bottomley
2014-06-11 13:37       ` Christoph Hellwig
2014-06-19 18:05         ` James Bottomley
2014-06-25 13:13           ` Christoph Hellwig [this message]
2014-06-26 14:02             ` James Bottomley
2014-06-26 15:00               ` Christoph Hellwig
2014-06-26 15:04                 ` James Bottomley
2014-06-26 15:08                   ` Christoph Hellwig
2014-06-26 18:52                     ` James Bottomley
2014-06-27  7:53                       ` Christoph Hellwig
2014-06-26 15:02 ` Christoph Hellwig
2014-06-26 18:13   ` Steven Haber
2014-06-26 18:52   ` James Bottomley
2014-06-27  7:55     ` Christoph Hellwig
     [not found]       ` <CAPK7rjdpxAALA-oLZJ3wDBPc3kr5Nw4jLALLxEaT0zSiAOD+wg@mail.gmail.com>
2014-06-30 18:45         ` Steven Haber
2014-07-01  8:15           ` Christoph Hellwig
2014-07-03 16:57             ` Steven Haber
2014-07-03 17:18               ` Christoph Hellwig
2014-07-09 22:38                 ` James Bottomley
2014-07-10  7:44                   ` Christoph Hellwig
2014-07-17 15:27                   ` Martin K. Petersen

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=20140625131344.GA13094@infradead.org \
    --to=hch@infradead.org \
    --cc=James.Bottomley@HansenPartnership.com \
    --cc=axboe@kernel.dk \
    --cc=linux-scsi@vger.kernel.org \
    --cc=steven@qumulo.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.