linux-scsi.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Hannes Reinecke <hare@suse.de>
To: Bart Van Assche <bvanassche@acm.org>
Cc: James Bottomley <jbottomley@parallels.com>,
	linux-scsi@vger.kernel.org, Ewan Milne <emilne@redhat.com>,
	Ren Mingxin <renmx@cn.fujitsu.com>, Joern Engel <joern@logfs.org>,
	James Smart <james.smart@emulex.com>,
	Roland Dreier <roland@purestorage.com>,
	Mike Christie <michaelc@cs.wisc.edu>
Subject: Re: [PATCH 3/9] scsi: improved eh timeout handler
Date: Wed, 04 Sep 2013 09:02:31 +0200	[thread overview]
Message-ID: <5226DB07.40001@suse.de> (raw)
In-Reply-To: <5225A83A.7090700@acm.org>

On 09/03/2013 11:13 AM, Bart Van Assche wrote:
> On 09/02/13 15:11, Hannes Reinecke wrote:
>> On 09/02/2013 02:45 PM, Bart Van Assche wrote:
>>> This patch adds several new calls to LLD EH handlers. Is it
>>> guaranteed that these will only be invoked before scsi_remove_host()
>>> has finished ? For more background information, see also "[PATCH]
>>> Make scsi_remove_host() wait until error handling finished"
>>> (http://thread.gmane.org/gmane.linux.scsi/82572/focus=82779).
>>>
>> Well, that depends how scsi_remove_host() handles outstanding
>> commands. What happens if you call scsi_remove_host() and there is
>> still I/O in flight? I would assume that any HBA would have to kill
>> any outstanding I/O prior to calling scsi_remove_host() (FC most
>> certainly does this).
>> Which would mean that it'll have to wait for scsi_put_command() to
>> be called for all outstanding commands. And as scsi_put_command()
>> will be called only _after_ our routine runs (see the reasoning
>> above) we should be safe.
> 
> Hello Hannes,
> 
> Since fc_remove_host() has to be invoked before scsi_remove_host()
> and since fc_remove_host() changes the port state into
> FC_PORTSTATE_DELETED this means that fc_remote_port_chkready() will
> return DID_NO_CONNECT while scsi_remove_host() is in progress. I
> think this prevents that the SYNCHRONIZE CACHE command submitted by
> sd_remove() reaches SCSI disks over FC since sd_remove() is invoked
> from inside scsi_remove_host(). The SRP transport patch I had posted
> recently makes sure that the SYNCHRONIZE CACHE command submitted by
> sd_remove() reaches the target SCSI disk. This makes me wonder what
> the correct behavior is for SCSI transport drivers - disabling I/O
> before scsi_remove_host() starts or ensuring that I/O is still
> possible while scsi_remove_host() is in progress ? I think the call
> chain is as follows: scsi_remove_host() -> device_del() ->
> bus_remove_device() -> device_release_driver() ->
> __device_release_driver() -> sd_remove() -> sd_shutdown() ->
> sd_sync_cache().
> 

The actual call chain is
scsi_remove_host() -> scsi_forget_host() -> __scsi_remove_device()
-> device_del() etc.

What's important here is that __scsi_remove_device() sets the state
'SDEV_DEL' and calls blk_cleanup_queue().

So after __scsi_remove_device() no further I/O is possible.
However, prior to setting SDEV_DEL I/O should be perfectly okay, so
one would assume that the SYNCHRONIZE CACHE command should be sent
to the device. USB most certainly does it.

A safe practice, however, would be to ensure that no _other_ I/O can
be sent to the device, ie that all I/O coming in via the request
queue or ioctl should be short-circuited and never hit the device at
all. That's what fc_remote_port_chkready() does.

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		      zSeries & Storage
hare@suse.de			      +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg)
--
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

  reply	other threads:[~2013-09-04  7:02 UTC|newest]

Thread overview: 38+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-09-02  7:12 [PATCHv5 0/9] New EH command timeout handler Hannes Reinecke
2013-09-02  7:12 ` [PATCH 1/9] scsi: Fix erratic device offline during EH Hannes Reinecke
2013-09-02  7:12 ` [PATCH 2/9] blk-timeout: add BLK_EH_SCHEDULED return code Hannes Reinecke
2013-09-02  7:12 ` [PATCH 3/9] scsi: improved eh timeout handler Hannes Reinecke
2013-09-02 12:45   ` Bart Van Assche
2013-09-02 13:11     ` Hannes Reinecke
2013-09-02 16:38       ` Bart Van Assche
2013-09-03  9:13       ` Bart Van Assche
2013-09-04  7:02         ` Hannes Reinecke [this message]
2013-09-02  7:13 ` [PATCH 4/9] virtio_scsi: Enable new EH " Hannes Reinecke
2013-09-02  7:13 ` [PATCH 5/9] libsas: " Hannes Reinecke
2013-09-02  7:13 ` [PATCH 6/9] mptsas: " Hannes Reinecke
2013-09-02  7:13 ` [PATCH 7/9] mpt2sas: " Hannes Reinecke
2013-09-02  7:13 ` [PATCH 8/9] mpt3sas: " Hannes Reinecke
2013-09-02  7:13 ` [PATCH 9/9] scsi_transport_fc: " Hannes Reinecke
2013-09-02  8:27 ` [PATCHv5 0/9] New EH command " Christoph Hellwig
2013-09-02  8:54   ` Hannes Reinecke
2013-09-02  9:31     ` Christoph Hellwig
2013-09-02  9:59       ` Hannes Reinecke
2013-09-03 16:36       ` Jörn Engel
  -- strict thread matches above, loose matches on Subject: below --
2013-08-29 13:32 [PATCHv4 " Hannes Reinecke
2013-08-29 13:32 ` [PATCH 3/9] scsi: improved eh " Hannes Reinecke
2013-07-01 14:24 [PATCHv3 0/9] New EH command " Hannes Reinecke
2013-07-01 14:24 ` [PATCH 3/9] scsi: improved eh " Hannes Reinecke
2013-08-22  8:51   ` Ren Mingxin
2013-08-23 12:27     ` Hannes Reinecke
2013-06-10  7:40 [PATCHv2 0/9] New SCSI command " Hannes Reinecke
2013-06-10  7:40 ` [PATCH 3/9] scsi: improved eh " Hannes Reinecke
2013-06-10  8:20   ` Christoph Hellwig
2013-06-10  9:00     ` Hannes Reinecke
2013-06-10 15:19       ` Jörn Engel
2013-06-10 23:24         ` Jörn Engel
2013-06-11  6:18           ` Hannes Reinecke
2013-06-11 16:35             ` Jörn Engel
2013-06-11 18:57     ` James Bottomley
2013-06-11 20:41       ` Ewan Milne
2013-06-11 20:54         ` James Bottomley
2013-06-12  5:54       ` Hannes Reinecke
2013-06-12  6:34       ` Bart Van Assche
2013-06-12  6:42         ` Hannes Reinecke
2013-06-10 15:47   ` Jörn Engel

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=5226DB07.40001@suse.de \
    --to=hare@suse.de \
    --cc=bvanassche@acm.org \
    --cc=emilne@redhat.com \
    --cc=james.smart@emulex.com \
    --cc=jbottomley@parallels.com \
    --cc=joern@logfs.org \
    --cc=linux-scsi@vger.kernel.org \
    --cc=michaelc@cs.wisc.edu \
    --cc=renmx@cn.fujitsu.com \
    --cc=roland@purestorage.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).