[PATCH 0/4] notify userspace of offline->running transitions (v2)

[PATCH 0/4] notify userspace of offline->running transitions (v2)

[michaelc]

The following patches were made over the misc branch of the scsi tree.

The patches fix a issue where if the device is offlined or IO is
failed due to fast_io_fail (fc) /recovery_tmo (iscsi) then comes
back, apps do not have a way a nice way to figure out the state
has transitioned to running. Apps have to either poll the sysfs state
file or send a SG IO to figure it out. With the patch apps can listen
for the KOBJ CHANGE event like some of them (at least udev does) do
already.

v2:
- Rebased to misc.

[PATCH 1/4] SCSI: add new SDEV_TRANSPORT_OFFLINE state

[michaelc]

From: Mike Christie <michaelc@cs.wisc.edu>

This patch adds a new state SDEV_TRANSPORT_OFFLINE. It will
be used by transport classes to offline devices for cases like
when the fast_io_fail/recovery_tmo fires. In those cases we
want all IO to fail, and we have not yet escalated to dev_loss_tmo
behavior where we are removing the devices.

Currently to handle this state, transport classes are setting
the scsi_device's state to running, setting their internal
session/port structs state to something that indicates failed,
and then failing IO from some transport check in the queuecommand.

The reason for the new value is so that users can distinguish
between a device failure that is a result of a transport problem
vs the wide range of errors that devices get offlined for
when a scsi command times out and we offline the devices there.
It also fixes the confusion as to why the transport class is
failing IO, but has set the device state from blocked to running.

Signed-off-by: Mike Christie <michaelc@cs.wisc.edu>
---
 drivers/scsi/scsi_lib.c    |    8 +++++++-
 drivers/scsi/scsi_sysfs.c  |    1 +
 include/scsi/scsi_device.h |    2 ++
 3 files changed, 10 insertions(+), 1 deletions(-)

diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index dbe4392..7a3bb74 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -1173,6 +1173,7 @@ int scsi_prep_state_check(struct scsi_device *sdev, struct request *req)
 	if (unlikely(sdev->sdev_state != SDEV_RUNNING)) {
 		switch (sdev->sdev_state) {
 		case SDEV_OFFLINE:
+		case SDEV_TRANSPORT_OFFLINE:
 			/*
 			 * If the device is offline we refuse to process any
 			 * commands.  The device must be brought online
@@ -2076,10 +2077,11 @@ scsi_device_set_state(struct scsi_device *sdev, enum scsi_device_state state)
 			
 	case SDEV_RUNNING:
 		switch (oldstate) {
-		case SDEV_CREATED:
 		case SDEV_OFFLINE:
+		case SDEV_TRANSPORT_OFFLINE:
 		case SDEV_QUIESCE:
 		case SDEV_BLOCK:
+		case SDEV_CREATED:
 			break;
 		default:
 			goto illegal;
@@ -2090,6 +2092,7 @@ scsi_device_set_state(struct scsi_device *sdev, enum scsi_device_state state)
 		switch (oldstate) {
 		case SDEV_RUNNING:
 		case SDEV_OFFLINE:
+		case SDEV_TRANSPORT_OFFLINE:
 			break;
 		default:
 			goto illegal;
@@ -2097,6 +2100,7 @@ scsi_device_set_state(struct scsi_device *sdev, enum scsi_device_state state)
 		break;
 
 	case SDEV_OFFLINE:
+	case SDEV_TRANSPORT_OFFLINE:
 		switch (oldstate) {
 		case SDEV_CREATED:
 		case SDEV_RUNNING:
@@ -2133,6 +2137,7 @@ scsi_device_set_state(struct scsi_device *sdev, enum scsi_device_state state)
 		case SDEV_RUNNING:
 		case SDEV_QUIESCE:
 		case SDEV_OFFLINE:
+		case SDEV_TRANSPORT_OFFLINE:
 		case SDEV_BLOCK:
 			break;
 		default:
@@ -2145,6 +2150,7 @@ scsi_device_set_state(struct scsi_device *sdev, enum scsi_device_state state)
 		case SDEV_CREATED:
 		case SDEV_RUNNING:
 		case SDEV_OFFLINE:
+		case SDEV_TRANSPORT_OFFLINE:
 		case SDEV_CANCEL:
 			break;
 		default:
diff --git a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c
index 04c2a27..5747478 100644
--- a/drivers/scsi/scsi_sysfs.c
+++ b/drivers/scsi/scsi_sysfs.c
@@ -35,6 +35,7 @@ static const struct {
 	{ SDEV_DEL, "deleted" },
 	{ SDEV_QUIESCE, "quiesce" },
 	{ SDEV_OFFLINE,	"offline" },
+	{ SDEV_TRANSPORT_OFFLINE, "transport-offline" },
 	{ SDEV_BLOCK,	"blocked" },
 	{ SDEV_CREATED_BLOCK, "created-blocked" },
 };
diff --git a/include/scsi/scsi_device.h b/include/scsi/scsi_device.h
index 6efb2e1..4acc1c3 100644
--- a/include/scsi/scsi_device.h
+++ b/include/scsi/scsi_device.h
@@ -42,6 +42,7 @@ enum scsi_device_state {
 				 * originate in the mid-layer) */
 	SDEV_OFFLINE,		/* Device offlined (by error handling or
 				 * user request */
+	SDEV_TRANSPORT_OFFLINE,	/* Offlined by transport class error handler */
 	SDEV_BLOCK,		/* Device blocked by scsi lld.  No
 				 * scsi commands from user or midlayer
 				 * should be issued to the scsi
@@ -420,6 +421,7 @@ static inline unsigned int sdev_id(struct scsi_device *sdev)
 static inline int scsi_device_online(struct scsi_device *sdev)
 {
 	return (sdev->sdev_state != SDEV_OFFLINE &&
+		sdev->sdev_state != SDEV_TRANSPORT_OFFLINE &&
 		sdev->sdev_state != SDEV_DEL);
 }
 static inline int scsi_device_blocked(struct scsi_device *sdev)
-- 
1.7.7.6

Re: [PATCH 1/4] SCSI: add new SDEV_TRANSPORT_OFFLINE state

[Hannes Reinecke]

On 05/18/2012 06:56 AM, michaelc@cs.wisc.edu wrote:
> From: Mike Christie <michaelc@cs.wisc.edu>
> 
> This patch adds a new state SDEV_TRANSPORT_OFFLINE. It will
> be used by transport classes to offline devices for cases like
> when the fast_io_fail/recovery_tmo fires. In those cases we
> want all IO to fail, and we have not yet escalated to dev_loss_tmo
> behavior where we are removing the devices.
> 
> Currently to handle this state, transport classes are setting
> the scsi_device's state to running, setting their internal
> session/port structs state to something that indicates failed,
> and then failing IO from some transport check in the queuecommand.
> 
> The reason for the new value is so that users can distinguish
> between a device failure that is a result of a transport problem
> vs the wide range of errors that devices get offlined for
> when a scsi command times out and we offline the devices there.
> It also fixes the confusion as to why the transport class is
> failing IO, but has set the device state from blocked to running.
> 
Very neat.

Acked-by: Hannes Reinecke <hare@suse.de>

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

[PATCH 2/4] SCSI, classes, mpt2sas: have scsi_internal_device_unblock take new state (v2)

[michaelc]

From: Mike Christie <michaelc@cs.wisc.edu>

This has scsi_internal_device_unblock/scsi_target_unblock take
the new state to set the devices as an argument instead of
always setting to running. The patch also converts users of these
functions.

This allows the FC and iSCSI class to transition devices from blocked
to transport-offline, so that when fast_io_fail/replacement_timeout
has fired we do not set the devices back to running. Instead, we
set them to SDEV_TRANSPORT_OFFLINE.

v2:
- Rebase to scsi-misc.

Signed-off-by: Mike Christie <michaelc@cs.wisc.edu>
---
 drivers/scsi/mpt2sas/mpt2sas_base.h  |    3 +-
 drivers/scsi/mpt2sas/mpt2sas_scsih.c |    4 +-
 drivers/scsi/scsi_lib.c              |   40 +++++++++++++++++++--------------
 drivers/scsi/scsi_priv.h             |    4 ++-
 drivers/scsi/scsi_transport_fc.c     |   16 ++++++-------
 drivers/scsi/scsi_transport_iscsi.c  |    6 ++--
 include/scsi/scsi_device.h           |    2 +-
 7 files changed, 41 insertions(+), 34 deletions(-)

diff --git a/drivers/scsi/mpt2sas/mpt2sas_base.h b/drivers/scsi/mpt2sas/mpt2sas_base.h
index b6dd3a5..b3a1a30 100644
--- a/drivers/scsi/mpt2sas/mpt2sas_base.h
+++ b/drivers/scsi/mpt2sas/mpt2sas_base.h
@@ -1158,6 +1158,7 @@ extern struct scsi_transport_template *mpt2sas_transport_template;
 extern int scsi_internal_device_block(struct scsi_device *sdev);
 extern u8 mpt2sas_stm_zero_smid_handler(struct MPT2SAS_ADAPTER *ioc,
     u8 msix_index, u32 reply);
-extern int scsi_internal_device_unblock(struct scsi_device *sdev);
+extern int scsi_internal_device_unblock(struct scsi_device *sdev,
+					enum scsi_device_state new_state);
 
 #endif /* MPT2SAS_BASE_H_INCLUDED */
diff --git a/drivers/scsi/mpt2sas/mpt2sas_scsih.c b/drivers/scsi/mpt2sas/mpt2sas_scsih.c
index 76973e8..b1ebd6f 100644
--- a/drivers/scsi/mpt2sas/mpt2sas_scsih.c
+++ b/drivers/scsi/mpt2sas/mpt2sas_scsih.c
@@ -2904,7 +2904,7 @@ _scsih_ublock_io_all_device(struct MPT2SAS_ADAPTER *ioc)
 		dewtprintk(ioc, sdev_printk(KERN_INFO, sdev, "device_running, "
 		    "handle(0x%04x)\n",
 		    sas_device_priv_data->sas_target->handle));
-		scsi_internal_device_unblock(sdev);
+		scsi_internal_device_unblock(sdev, SDEV_RUNNING);
 	}
 }
 /**
@@ -2933,7 +2933,7 @@ _scsih_ublock_io_device(struct MPT2SAS_ADAPTER *ioc, u64 sas_address)
 			    "sas address(0x%016llx)\n", ioc->name,
 				(unsigned long long)sas_address));
 			sas_device_priv_data->block = 0;
-			scsi_internal_device_unblock(sdev);
+			scsi_internal_device_unblock(sdev, SDEV_RUNNING);
 		}
 	}
 }
diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index 7a3bb74..9fa7bc3 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -2441,6 +2441,7 @@ EXPORT_SYMBOL_GPL(scsi_internal_device_block);
 /**
  * scsi_internal_device_unblock - resume a device after a block request
  * @sdev:	device to resume
+ * @new_state:	state to set devices to after unblocking
  *
  * Called by scsi lld's or the midlayer to restart the device queue
  * for the previously suspended scsi device.  Called from interrupt or
@@ -2450,25 +2451,30 @@ EXPORT_SYMBOL_GPL(scsi_internal_device_block);
  *
  * Notes:       
  *	This routine transitions the device to the SDEV_RUNNING state
- *	(which must be a legal transition) allowing the midlayer to
- *	goose the queue for this device.  This routine assumes the 
- *	host_lock is held upon entry.
+ *	or to one of the offline states (which must be a legal transition)
+ *	allowing the midlayer to goose the queue for this device. This
+ *	routine assumes the host_lock is held upon entry.
  */
 int
-scsi_internal_device_unblock(struct scsi_device *sdev)
+scsi_internal_device_unblock(struct scsi_device *sdev,
+			     enum scsi_device_state new_state)
 {
 	struct request_queue *q = sdev->request_queue; 
 	unsigned long flags;
-	
-	/* 
-	 * Try to transition the scsi device to SDEV_RUNNING
-	 * and goose the device queue if successful.  
+
+	/*
+	 * Try to transition the scsi device to SDEV_RUNNING or one of the
+	 * offlined states and goose the device queue if successful.
 	 */
 	if (sdev->sdev_state == SDEV_BLOCK)
-		sdev->sdev_state = SDEV_RUNNING;
-	else if (sdev->sdev_state == SDEV_CREATED_BLOCK)
-		sdev->sdev_state = SDEV_CREATED;
-	else if (sdev->sdev_state != SDEV_CANCEL &&
+		sdev->sdev_state = new_state;
+	else if (sdev->sdev_state == SDEV_CREATED_BLOCK) {
+		if (new_state == SDEV_TRANSPORT_OFFLINE ||
+		    new_state == SDEV_OFFLINE)
+			sdev->sdev_state = new_state;
+		else
+			sdev->sdev_state = SDEV_CREATED;
+	} else if (sdev->sdev_state != SDEV_CANCEL &&
 		 sdev->sdev_state != SDEV_OFFLINE)
 		return -EINVAL;
 
@@ -2509,26 +2515,26 @@ EXPORT_SYMBOL_GPL(scsi_target_block);
 static void
 device_unblock(struct scsi_device *sdev, void *data)
 {
-	scsi_internal_device_unblock(sdev);
+	scsi_internal_device_unblock(sdev, *(enum scsi_device_state *)data);
 }
 
 static int
 target_unblock(struct device *dev, void *data)
 {
 	if (scsi_is_target_device(dev))
-		starget_for_each_device(to_scsi_target(dev), NULL,
+		starget_for_each_device(to_scsi_target(dev), data,
 					device_unblock);
 	return 0;
 }
 
 void
-scsi_target_unblock(struct device *dev)
+scsi_target_unblock(struct device *dev, enum scsi_device_state new_state)
 {
 	if (scsi_is_target_device(dev))
-		starget_for_each_device(to_scsi_target(dev), NULL,
+		starget_for_each_device(to_scsi_target(dev), &new_state,
 					device_unblock);
 	else
-		device_for_each_child(dev, NULL, target_unblock);
+		device_for_each_child(dev, &new_state, target_unblock);
 }
 EXPORT_SYMBOL_GPL(scsi_target_unblock);
 
diff --git a/drivers/scsi/scsi_priv.h b/drivers/scsi/scsi_priv.h
index 07ce3f5..cbfe5df 100644
--- a/drivers/scsi/scsi_priv.h
+++ b/drivers/scsi/scsi_priv.h
@@ -2,6 +2,7 @@
 #define _SCSI_PRIV_H
 
 #include <linux/device.h>
+#include <scsi/scsi_device.h>
 
 struct request_queue;
 struct request;
@@ -172,6 +173,7 @@ extern struct list_head scsi_sd_probe_domain;
 
 #define SCSI_DEVICE_BLOCK_MAX_TIMEOUT	600	/* units in seconds */
 extern int scsi_internal_device_block(struct scsi_device *sdev);
-extern int scsi_internal_device_unblock(struct scsi_device *sdev);
+extern int scsi_internal_device_unblock(struct scsi_device *sdev,
+					enum scsi_device_state new_state);
 
 #endif /* _SCSI_PRIV_H */
diff --git a/drivers/scsi/scsi_transport_fc.c b/drivers/scsi/scsi_transport_fc.c
index 5797604..b27be8b 100644
--- a/drivers/scsi/scsi_transport_fc.c
+++ b/drivers/scsi/scsi_transport_fc.c
@@ -2477,11 +2477,9 @@ static void fc_terminate_rport_io(struct fc_rport *rport)
 		i->f->terminate_rport_io(rport);
 
 	/*
-	 * must unblock to flush queued IO. The caller will have set
-	 * the port_state or flags, so that fc_remote_port_chkready will
-	 * fail IO.
+	 * Must unblock to flush queued IO. scsi-ml will fail incoming reqs.
 	 */
-	scsi_target_unblock(&rport->dev);
+	scsi_target_unblock(&rport->dev, SDEV_TRANSPORT_OFFLINE);
 }
 
 /**
@@ -2812,8 +2810,8 @@ fc_remote_port_add(struct Scsi_Host *shost, int channel,
 
 				/* if target, initiate a scan */
 				if (rport->scsi_target_id != -1) {
-					scsi_target_unblock(&rport->dev);
-
+					scsi_target_unblock(&rport->dev,
+							    SDEV_RUNNING);
 					spin_lock_irqsave(shost->host_lock,
 							  flags);
 					rport->flags |= FC_RPORT_SCAN_PENDING;
@@ -2882,7 +2880,7 @@ fc_remote_port_add(struct Scsi_Host *shost, int channel,
 			spin_unlock_irqrestore(shost->host_lock, flags);
 
 			if (ids->roles & FC_PORT_ROLE_FCP_TARGET) {
-				scsi_target_unblock(&rport->dev);
+				scsi_target_unblock(&rport->dev, SDEV_RUNNING);
 
 				/* initiate a scan of the target */
 				spin_lock_irqsave(shost->host_lock, flags);
@@ -3087,7 +3085,7 @@ fc_remote_port_rolechg(struct fc_rport  *rport, u32 roles)
 		/* ensure any stgt delete functions are done */
 		fc_flush_work(shost);
 
-		scsi_target_unblock(&rport->dev);
+		scsi_target_unblock(&rport->dev, SDEV_RUNNING);
 		/* initiate a scan of the target */
 		spin_lock_irqsave(shost->host_lock, flags);
 		rport->flags |= FC_RPORT_SCAN_PENDING;
@@ -3131,7 +3129,7 @@ fc_timeout_deleted_rport(struct work_struct *work)
 			"blocked FC remote port time out: no longer"
 			" a FCP target, removing starget\n");
 		spin_unlock_irqrestore(shost->host_lock, flags);
-		scsi_target_unblock(&rport->dev);
+		scsi_target_unblock(&rport->dev, SDEV_TRANSPORT_OFFLINE);
 		fc_queue_work(shost, &rport->stgt_delete_work);
 		return;
 	}
diff --git a/drivers/scsi/scsi_transport_iscsi.c b/drivers/scsi/scsi_transport_iscsi.c
index 1cf640e..96ec21a 100644
--- a/drivers/scsi/scsi_transport_iscsi.c
+++ b/drivers/scsi/scsi_transport_iscsi.c
@@ -907,7 +907,7 @@ static void session_recovery_timedout(struct work_struct *work)
 		session->transport->session_recovery_timedout(session);
 
 	ISCSI_DBG_TRANS_SESSION(session, "Unblocking SCSI target\n");
-	scsi_target_unblock(&session->dev);
+	scsi_target_unblock(&session->dev, SDEV_TRANSPORT_OFFLINE);
 	ISCSI_DBG_TRANS_SESSION(session, "Completed unblocking SCSI target\n");
 }
 
@@ -930,7 +930,7 @@ static void __iscsi_unblock_session(struct work_struct *work)
 	session->state = ISCSI_SESSION_LOGGED_IN;
 	spin_unlock_irqrestore(&session->lock, flags);
 	/* start IO */
-	scsi_target_unblock(&session->dev);
+	scsi_target_unblock(&session->dev, SDEV_RUNNING);
 	/*
 	 * Only do kernel scanning if the driver is properly hooked into
 	 * the async scanning code (drivers like iscsi_tcp do login and
@@ -1180,7 +1180,7 @@ void iscsi_remove_session(struct iscsi_cls_session *session)
 	session->state = ISCSI_SESSION_FREE;
 	spin_unlock_irqrestore(&session->lock, flags);
 
-	scsi_target_unblock(&session->dev);
+	scsi_target_unblock(&session->dev, SDEV_TRANSPORT_OFFLINE);
 	/* flush running scans then delete devices */
 	scsi_flush_work(shost);
 	__iscsi_unbind_session(&session->unbind_work);
diff --git a/include/scsi/scsi_device.h b/include/scsi/scsi_device.h
index 4acc1c3..5ff8180 100644
--- a/include/scsi/scsi_device.h
+++ b/include/scsi/scsi_device.h
@@ -373,7 +373,7 @@ extern void scsi_scan_target(struct device *parent, unsigned int channel,
 			     unsigned int id, unsigned int lun, int rescan);
 extern void scsi_target_reap(struct scsi_target *);
 extern void scsi_target_block(struct device *);
-extern void scsi_target_unblock(struct device *);
+extern void scsi_target_unblock(struct device *, enum scsi_device_state);
 extern void scsi_remove_target(struct device *);
 extern void int_to_scsilun(unsigned int, struct scsi_lun *);
 extern int scsilun_to_int(struct scsi_lun *);
-- 
1.7.7.6

[PATCH 3/4] SCSI: remove old comment from block/unblock functions (v2)

[michaelc]

From: Mike Christie <michaelc@cs.wisc.edu>

We do not hold the host lock when calling these functions,
so remove comment.

v2:
- Forgot to remove "This".

Signed-off-by: Mike Christie <michaelc@cs.wisc.edu>
---
 drivers/scsi/scsi_lib.c |    4 +---
 1 files changed, 1 insertions(+), 3 deletions(-)

diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index 9fa7bc3..d15b243 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -2408,7 +2408,6 @@ EXPORT_SYMBOL(scsi_target_resume);
  *	(which must be a legal transition).  When the device is in this
  *	state, all commands are deferred until the scsi lld reenables
  *	the device with scsi_device_unblock or device_block_tmo fires.
- *	This routine assumes the host_lock is held on entry.
  */
 int
 scsi_internal_device_block(struct scsi_device *sdev)
@@ -2452,8 +2451,7 @@ EXPORT_SYMBOL_GPL(scsi_internal_device_block);
  * Notes:       
  *	This routine transitions the device to the SDEV_RUNNING state
  *	or to one of the offline states (which must be a legal transition)
- *	allowing the midlayer to goose the queue for this device. This
- *	routine assumes the host_lock is held upon entry.
+ *	allowing the midlayer to goose the queue for this device.
  */
 int
 scsi_internal_device_unblock(struct scsi_device *sdev,
-- 
1.7.7.6

[PATCH 4/4] SCSI: send KOBJ_CHANGE when device is set to running v2

[michaelc]

From: Mike Christie <michaelc@cs.wisc.edu>

If a device goes offline while the device is opened then closed
while it is still offline, udev will remove the /dev/disk/by-id
link. If the device comes back and is set to running, userspace
is not notified, and the by-id link will not get remade.

This patch has scsi-ml send a KOBJ_CHANGE event so tools like udev
will know that it can being to use the device again. With this patch
udev see the KOBJ_CHANGE event and will reprobe the device and recreate
a /dev/disk/by-id link.

v2
- Added SDEV_EVT_STATE_RUNNING evt type.

Signed-off-by: Mike Christie <michaelc@cs.wisc.edu>
---
 drivers/scsi/scsi_lib.c    |    9 +++++++++
 include/scsi/scsi_device.h |    2 ++
 2 files changed, 11 insertions(+), 0 deletions(-)

diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index d15b243..b54030d 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -2061,6 +2061,7 @@ int
 scsi_device_set_state(struct scsi_device *sdev, enum scsi_device_state state)
 {
 	enum scsi_device_state oldstate = sdev->sdev_state;
+	int change_evt = 0;
 
 	if (state == oldstate)
 		return 0;
@@ -2079,6 +2080,11 @@ scsi_device_set_state(struct scsi_device *sdev, enum scsi_device_state state)
 		switch (oldstate) {
 		case SDEV_OFFLINE:
 		case SDEV_TRANSPORT_OFFLINE:
+			/*
+			 * Notify userspace we can accept IO by sending
+			 * change event.
+			 */
+			change_evt = 1;
 		case SDEV_QUIESCE:
 		case SDEV_BLOCK:
 		case SDEV_CREATED:
@@ -2160,6 +2166,8 @@ scsi_device_set_state(struct scsi_device *sdev, enum scsi_device_state state)
 
 	}
 	sdev->sdev_state = state;
+	if (change_evt)
+		sdev_evt_send_simple(sdev, SDEV_EVT_STATE_RUNNING, GFP_ATOMIC);
 	return 0;
 
  illegal:
@@ -2283,6 +2291,7 @@ struct scsi_event *sdev_evt_alloc(enum scsi_device_event evt_type,
 	/* evt_type-specific initialization, if any */
 	switch (evt_type) {
 	case SDEV_EVT_MEDIA_CHANGE:
+	case SDEV_EVT_STATE_RUNNING:
 	default:
 		/* do nothing */
 		break;
diff --git a/include/scsi/scsi_device.h b/include/scsi/scsi_device.h
index 5ff8180..0759433 100644
--- a/include/scsi/scsi_device.h
+++ b/include/scsi/scsi_device.h
@@ -52,6 +52,8 @@ enum scsi_device_state {
 
 enum scsi_device_event {
 	SDEV_EVT_MEDIA_CHANGE	= 1,	/* media has changed */
+	SDEV_EVT_STATE_RUNNING	= 2,	/* state has gone from offline to
+					 * running */
 
 	SDEV_EVT_LAST		= SDEV_EVT_MEDIA_CHANGE,
 	SDEV_EVT_MAXBITS	= SDEV_EVT_LAST + 1
-- 
1.7.7.6

Re: [PATCH 4/4] SCSI: send KOBJ_CHANGE when device is set to running v2

[Hannes Reinecke]

On 05/18/2012 06:56 AM, michaelc@cs.wisc.edu wrote:
> From: Mike Christie <michaelc@cs.wisc.edu>
> 
> If a device goes offline while the device is opened then closed
> while it is still offline, udev will remove the /dev/disk/by-id
> link. If the device comes back and is set to running, userspace
> is not notified, and the by-id link will not get remade.
> 
> This patch has scsi-ml send a KOBJ_CHANGE event so tools like udev
> will know that it can being to use the device again. With this patch
> udev see the KOBJ_CHANGE event and will reprobe the device and recreate
> a /dev/disk/by-id link.
> 
> v2
> - Added SDEV_EVT_STATE_RUNNING evt type.
> 
> Signed-off-by: Mike Christie <michaelc@cs.wisc.edu>
> ---
>  drivers/scsi/scsi_lib.c    |    9 +++++++++
>  include/scsi/scsi_device.h |    2 ++
>  2 files changed, 11 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
> index d15b243..b54030d 100644
> --- a/drivers/scsi/scsi_lib.c
> +++ b/drivers/scsi/scsi_lib.c
> @@ -2061,6 +2061,7 @@ int
>  scsi_device_set_state(struct scsi_device *sdev, enum scsi_device_state state)
>  {
>  	enum scsi_device_state oldstate = sdev->sdev_state;
> +	int change_evt = 0;
>  
>  	if (state == oldstate)
>  		return 0;
> @@ -2079,6 +2080,11 @@ scsi_device_set_state(struct scsi_device *sdev, enum scsi_device_state state)
>  		switch (oldstate) {
>  		case SDEV_OFFLINE:
>  		case SDEV_TRANSPORT_OFFLINE:
> +			/*
> +			 * Notify userspace we can accept IO by sending
> +			 * change event.
> +			 */
> +			change_evt = 1;
>  		case SDEV_QUIESCE:
>  		case SDEV_BLOCK:
>  		case SDEV_CREATED:
> @@ -2160,6 +2166,8 @@ scsi_device_set_state(struct scsi_device *sdev, enum scsi_device_state state)
>  
>  	}
>  	sdev->sdev_state = state;
> +	if (change_evt)
> +		sdev_evt_send_simple(sdev, SDEV_EVT_STATE_RUNNING, GFP_ATOMIC);
>  	return 0;
>  
>   illegal:
> @@ -2283,6 +2291,7 @@ struct scsi_event *sdev_evt_alloc(enum scsi_device_event evt_type,
>  	/* evt_type-specific initialization, if any */
>  	switch (evt_type) {
>  	case SDEV_EVT_MEDIA_CHANGE:
> +	case SDEV_EVT_STATE_RUNNING:
>  	default:
>  		/* do nothing */
>  		break;
Hmm. So we're only getting notified if the device switched to OFFLINE?
Weird. We'll get notified about this eventually anyway (aborting
I/Os etc).
I'd be more interested to get notified if the device becomes
_available_ again, ie when entering RUNNING state. That's really
hard to figure out without polling.

Care to add the 'change_evt' thing to the SDEV_RUNNING case
statement, too?

Or am I missing something?

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

Re: [PATCH 4/4] SCSI: send KOBJ_CHANGE when device is set to running v2

[Mike Christie]

On 05/21/2012 06:45 AM, Hannes Reinecke wrote:
> On 05/18/2012 06:56 AM, michaelc@cs.wisc.edu wrote:
>> From: Mike Christie <michaelc@cs.wisc.edu>
>>
>> If a device goes offline while the device is opened then closed
>> while it is still offline, udev will remove the /dev/disk/by-id
>> link. If the device comes back and is set to running, userspace
>> is not notified, and the by-id link will not get remade.
>>
>> This patch has scsi-ml send a KOBJ_CHANGE event so tools like udev
>> will know that it can being to use the device again. With this patch
>> udev see the KOBJ_CHANGE event and will reprobe the device and recreate
>> a /dev/disk/by-id link.
>>
>> v2
>> - Added SDEV_EVT_STATE_RUNNING evt type.
>>
>> Signed-off-by: Mike Christie <michaelc@cs.wisc.edu>
>> ---
>>  drivers/scsi/scsi_lib.c    |    9 +++++++++
>>  include/scsi/scsi_device.h |    2 ++
>>  2 files changed, 11 insertions(+), 0 deletions(-)
>>
>> diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
>> index d15b243..b54030d 100644
>> --- a/drivers/scsi/scsi_lib.c
>> +++ b/drivers/scsi/scsi_lib.c
>> @@ -2061,6 +2061,7 @@ int
>>  scsi_device_set_state(struct scsi_device *sdev, enum scsi_device_state state)
>>  {
>>  	enum scsi_device_state oldstate = sdev->sdev_state;
>> +	int change_evt = 0;
>>  
>>  	if (state == oldstate)
>>  		return 0;
>> @@ -2079,6 +2080,11 @@ scsi_device_set_state(struct scsi_device *sdev, enum scsi_device_state state)
>>  		switch (oldstate) {
>>  		case SDEV_OFFLINE:
>>  		case SDEV_TRANSPORT_OFFLINE:
>> +			/*
>> +			 * Notify userspace we can accept IO by sending
>> +			 * change event.
>> +			 */
>> +			change_evt = 1;
>>  		case SDEV_QUIESCE:
>>  		case SDEV_BLOCK:
>>  		case SDEV_CREATED:
>> @@ -2160,6 +2166,8 @@ scsi_device_set_state(struct scsi_device *sdev, enum scsi_device_state state)
>>  
>>  	}
>>  	sdev->sdev_state = state;
>> +	if (change_evt)
>> +		sdev_evt_send_simple(sdev, SDEV_EVT_STATE_RUNNING, GFP_ATOMIC);
>>  	return 0;
>>  
>>   illegal:
>> @@ -2283,6 +2291,7 @@ struct scsi_event *sdev_evt_alloc(enum scsi_device_event evt_type,
>>  	/* evt_type-specific initialization, if any */
>>  	switch (evt_type) {
>>  	case SDEV_EVT_MEDIA_CHANGE:
>> +	case SDEV_EVT_STATE_RUNNING:
>>  	default:
>>  		/* do nothing */
>>  		break;
> Hmm. So we're only getting notified if the device switched to OFFLINE?

No from one of the offlines to running.

> Weird. We'll get notified about this eventually anyway (aborting
> I/Os etc).
> I'd be more interested to get notified if the device becomes
> _available_ again, ie when entering RUNNING state. That's really
> hard to figure out without polling.

That is what is happening.

> 
> Care to add the 'change_evt' thing to the SDEV_RUNNING case
> statement, too?
> 
> Or am I missing something?
> 

You are :)  I hope :)

Re: [PATCH 4/4] SCSI: send KOBJ_CHANGE when device is set to running v2

[Mike Christie]

On 05/21/2012 10:32 AM, Mike Christie wrote:
> On 05/21/2012 06:45 AM, Hannes Reinecke wrote:
>> On 05/18/2012 06:56 AM, michaelc@cs.wisc.edu wrote:
>>> From: Mike Christie <michaelc@cs.wisc.edu>
>>>
>>> If a device goes offline while the device is opened then closed
>>> while it is still offline, udev will remove the /dev/disk/by-id
>>> link. If the device comes back and is set to running, userspace
>>> is not notified, and the by-id link will not get remade.
>>>
>>> This patch has scsi-ml send a KOBJ_CHANGE event so tools like udev
>>> will know that it can being to use the device again. With this patch
>>> udev see the KOBJ_CHANGE event and will reprobe the device and recreate
>>> a /dev/disk/by-id link.
>>>
>>> v2
>>> - Added SDEV_EVT_STATE_RUNNING evt type.
>>>
>>> Signed-off-by: Mike Christie <michaelc@cs.wisc.edu>
>>> ---
>>>  drivers/scsi/scsi_lib.c    |    9 +++++++++
>>>  include/scsi/scsi_device.h |    2 ++
>>>  2 files changed, 11 insertions(+), 0 deletions(-)
>>>
>>> diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
>>> index d15b243..b54030d 100644
>>> --- a/drivers/scsi/scsi_lib.c
>>> +++ b/drivers/scsi/scsi_lib.c
>>> @@ -2061,6 +2061,7 @@ int
>>>  scsi_device_set_state(struct scsi_device *sdev, enum scsi_device_state state)
>>>  {
>>>  	enum scsi_device_state oldstate = sdev->sdev_state;
>>> +	int change_evt = 0;
>>>  
>>>  	if (state == oldstate)
>>>  		return 0;
>>> @@ -2079,6 +2080,11 @@ scsi_device_set_state(struct scsi_device *sdev, enum scsi_device_state state)
>>>  		switch (oldstate) {
>>>  		case SDEV_OFFLINE:
>>>  		case SDEV_TRANSPORT_OFFLINE:
>>> +			/*
>>> +			 * Notify userspace we can accept IO by sending
>>> +			 * change event.
>>> +			 */
>>> +			change_evt = 1;
>>>  		case SDEV_QUIESCE:
>>>  		case SDEV_BLOCK:
>>>  		case SDEV_CREATED:
>>> @@ -2160,6 +2166,8 @@ scsi_device_set_state(struct scsi_device *sdev, enum scsi_device_state state)
>>>  
>>>  	}
>>>  	sdev->sdev_state = state;
>>> +	if (change_evt)
>>> +		sdev_evt_send_simple(sdev, SDEV_EVT_STATE_RUNNING, GFP_ATOMIC);
>>>  	return 0;
>>>  
>>>   illegal:
>>> @@ -2283,6 +2291,7 @@ struct scsi_event *sdev_evt_alloc(enum scsi_device_event evt_type,
>>>  	/* evt_type-specific initialization, if any */
>>>  	switch (evt_type) {
>>>  	case SDEV_EVT_MEDIA_CHANGE:
>>> +	case SDEV_EVT_STATE_RUNNING:
>>>  	default:
>>>  		/* do nothing */
>>>  		break;
>> Hmm. So we're only getting notified if the device switched to OFFLINE?
> 
> No from one of the offlines to running.
> 

I think I know what happened. Maybe you did not reivew all the patches
at the same time. With all patches in the set applied we have this:


scsi_device_set_state()
......

        case SDEV_RUNNING:	<- so we are going into running
                switch (oldstate) {
                case SDEV_OFFLINE:    <- we were in one of the offlines
                case SDEV_TRANSPORT_OFFLINE:
                        /*
                         * Notify userspace we can accept IO by sending
                         * change event.
                         */
                        change_evt = 1;   <- so we send evt for *offline
to running transistions

Re: [PATCH 0/4] notify userspace of offline->running transitions (v2)

[Hannes Reinecke]

On 05/18/2012 06:56 AM, michaelc@cs.wisc.edu wrote:
> The following patches were made over the misc branch of the scsi tree.
> 
> The patches fix a issue where if the device is offlined or IO is
> failed due to fast_io_fail (fc) /recovery_tmo (iscsi) then comes
> back, apps do not have a way a nice way to figure out the state
> has transitioned to running. Apps have to either poll the sysfs state
> file or send a SG IO to figure it out. With the patch apps can listen
> for the KOBJ CHANGE event like some of them (at least udev does) do
> already.
> 
> v2:
> - Rebased to misc.
> 
In principle, yes.

However, when doing this, we're now sending 'CHANGE' uevents from
SCSI devices. With the potential of putting _quite_ some strain on udev.
Kay explicitely debarred me from using uevents for my SCSI sense
code handling stuff, on the grounds that the SCSI subsystem might
flood the netlink socket and thereby starving udev from handling
really important messages.

(Keeping in mind that Kay doesn't believe in multipathing. But even
so, he has a point. Even more so with him being the udev maintainer :-)

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

Re: [PATCH 0/4] notify userspace of offline->running transitions (v2)

[Mike Christie]

On 05/21/2012 06:50 AM, Hannes Reinecke wrote:
> On 05/18/2012 06:56 AM, michaelc@cs.wisc.edu wrote:
>> The following patches were made over the misc branch of the scsi tree.
>>
>> The patches fix a issue where if the device is offlined or IO is
>> failed due to fast_io_fail (fc) /recovery_tmo (iscsi) then comes
>> back, apps do not have a way a nice way to figure out the state
>> has transitioned to running. Apps have to either poll the sysfs state
>> file or send a SG IO to figure it out. With the patch apps can listen
>> for the KOBJ CHANGE event like some of them (at least udev does) do
>> already.
>>
>> v2:
>> - Rebased to misc.
>>
> In principle, yes.
> 

ccing Kay.

> However, when doing this, we're now sending 'CHANGE' uevents from
> SCSI devices. With the potential of putting _quite_ some strain on udev.
> Kay explicitely debarred me from using uevents for my SCSI sense

Kay told me to do it this way :) In this case udev was the app we
discovered the issue with, so maybe that is the diff.

Re: [PATCH 0/4] notify userspace of offline->running transitions (v2)

[Kay Sievers]

On Mon, May 21, 2012 at 5:35 PM, Mike Christie <michaelc@cs.wisc.edu> wrote:
> On 05/21/2012 06:50 AM, Hannes Reinecke wrote:
>> On 05/18/2012 06:56 AM, michaelc@cs.wisc.edu wrote:
>>> The following patches were made over the misc branch of the scsi tree.
>>>
>>> The patches fix a issue where if the device is offlined or IO is
>>> failed due to fast_io_fail (fc) /recovery_tmo (iscsi) then comes
>>> back, apps do not have a way a nice way to figure out the state
>>> has transitioned to running. Apps have to either poll the sysfs state
>>> file or send a SG IO to figure it out. With the patch apps can listen
>>> for the KOBJ CHANGE event like some of them (at least udev does) do
>>> already.
>>>
>>> v2:
>>> - Rebased to misc.
>>>
>> In principle, yes.
>>
>
> ccing Kay.
>
>> However, when doing this, we're now sending 'CHANGE' uevents from
>> SCSI devices. With the potential of putting _quite_ some strain on udev.
>> Kay explicitely debarred me from using uevents for my SCSI sense
>
> Kay told me to do it this way :) In this case udev was the app we
> discovered the issue with, so maybe that is the diff.

Hah, I basically only told you not to use online/offline events. :)

Uevents are ok to use if we can be sure there is _never_ a storm of
events. Uevents are not meant to handle large amounts of events
happening at the same time. They must never be used to handle things
like reporting errors which are not limited in their rate, or where
many devices might send similar events at the same time in a row.

Kay

Re: [PATCH 0/4] notify userspace of offline->running transitions (v2)

[Mike Christie]

On 05/21/2012 11:49 AM, Kay Sievers wrote:
> On Mon, May 21, 2012 at 5:35 PM, Mike Christie <michaelc@cs.wisc.edu> wrote:
>> On 05/21/2012 06:50 AM, Hannes Reinecke wrote:
>>> On 05/18/2012 06:56 AM, michaelc@cs.wisc.edu wrote:
>>>> The following patches were made over the misc branch of the scsi tree.
>>>>
>>>> The patches fix a issue where if the device is offlined or IO is
>>>> failed due to fast_io_fail (fc) /recovery_tmo (iscsi) then comes
>>>> back, apps do not have a way a nice way to figure out the state
>>>> has transitioned to running. Apps have to either poll the sysfs state
>>>> file or send a SG IO to figure it out. With the patch apps can listen
>>>> for the KOBJ CHANGE event like some of them (at least udev does) do
>>>> already.
>>>>
>>>> v2:
>>>> - Rebased to misc.
>>>>
>>> In principle, yes.
>>>
>>
>> ccing Kay.
>>
>>> However, when doing this, we're now sending 'CHANGE' uevents from
>>> SCSI devices. With the potential of putting _quite_ some strain on udev.
>>> Kay explicitely debarred me from using uevents for my SCSI sense
>>
>> Kay told me to do it this way :) In this case udev was the app we
>> discovered the issue with, so maybe that is the diff.
> 
> Hah, I basically only told you not to use online/offline events. :)

Yeah, I guess you guys got me confused :) Harold said to send an event
for this issue. Then later you said "please always use change". I
thought you and Harold were in sync :(

> 
> Uevents are ok to use if we can be sure there is _never_ a storm of
> events. Uevents are not meant to handle large amounts of events
> happening at the same time. They must never be used to handle things
> like reporting errors which are not limited in their rate, or where
> many devices might send similar events at the same time in a row.
> 

In this case you can get many devices sending the same events at the
same time, because when the transport/connection comes back you all the
device accessed through that connection will be set to running at the
same time.

So what is the fix for this case? Just send one event for the host or
transport/connection then have udev loop over all devices accessed
through that object and fix things up? If so, what type of event do you
want? CHANGE event plus some other info to indicate to look at child
devices?

Re: [PATCH 0/4] notify userspace of offline->running transitions (v2)

[Kay Sievers]

On Mon, May 21, 2012 at 7:04 PM, Mike Christie <michaelc@cs.wisc.edu> wrote:
> On 05/21/2012 11:49 AM, Kay Sievers wrote:

>>> Kay told me to do it this way :) In this case udev was the app we
>>> discovered the issue with, so maybe that is the diff.
>>
>> Hah, I basically only told you not to use online/offline events. :)
>
> Yeah, I guess you guys got me confused :) Harold said to send an event
> for this issue. Then later you said "please always use change". I
> thought you and Harold were in sync :(

Yeah, Harald and I think that sometimes ourselves, but we are not always. :)

>> Uevents are ok to use if we can be sure there is _never_ a storm of
>> events. Uevents are not meant to handle large amounts of events
>> happening at the same time. They must never be used to handle things
>> like reporting errors which are not limited in their rate, or where
>> many devices might send similar events at the same time in a row.
>
> In this case you can get many devices sending the same events at the
> same time, because when the transport/connection comes back you all the
> device accessed through that connection will be set to running at the
> same time.

The problem would be more the possible highest frequency of the
change. One event per device in a longer time frame would be ok, it's
like coldplug at boot time, where we enumerate all devices once in one
step. That is known though, to take minutes on very large boxes with
thousands of devices.

If we get such state change for lots of devices in a row (the state
flips forth and back for all devices), faster than we handle them (and
they are not that fast on older versions of udev, or if people plugged
silly things into udev rules), things will break or get lost one way
or the other.

> So what is the fix for this case? Just send one event for the host or
> transport/connection then have udev loop over all devices accessed
> through that object and fix things up? If so, what type of event do you
> want? CHANGE event plus some other info to indicate to look at child
> devices?

 We do not really support such logic in udev. And it would cause
exactly the same problem, just at a different layer.

Kay

Re: [PATCH 0/4] notify userspace of offline->running transitions (v2)

[Hannes Reinecke]

On 05/21/2012 07:04 PM, Mike Christie wrote:
> On 05/21/2012 11:49 AM, Kay Sievers wrote:
>> On Mon, May 21, 2012 at 5:35 PM, Mike Christie <michaelc@cs.wisc.edu> wrote:
>>> On 05/21/2012 06:50 AM, Hannes Reinecke wrote:
>>>> On 05/18/2012 06:56 AM, michaelc@cs.wisc.edu wrote:
>>>>> The following patches were made over the misc branch of the scsi tree.
>>>>>
>>>>> The patches fix a issue where if the device is offlined or IO is
>>>>> failed due to fast_io_fail (fc) /recovery_tmo (iscsi) then comes
>>>>> back, apps do not have a way a nice way to figure out the state
>>>>> has transitioned to running. Apps have to either poll the sysfs state
>>>>> file or send a SG IO to figure it out. With the patch apps can listen
>>>>> for the KOBJ CHANGE event like some of them (at least udev does) do
>>>>> already.
>>>>>
>>>>> v2:
>>>>> - Rebased to misc.
>>>>>
>>>> In principle, yes.
>>>>
>>>
>>> ccing Kay.
>>>
>>>> However, when doing this, we're now sending 'CHANGE' uevents from
>>>> SCSI devices. With the potential of putting _quite_ some strain on udev.
>>>> Kay explicitely debarred me from using uevents for my SCSI sense
>>>
>>> Kay told me to do it this way :) In this case udev was the app we
>>> discovered the issue with, so maybe that is the diff.
>>
>> Hah, I basically only told you not to use online/offline events. :)
> 
> Yeah, I guess you guys got me confused :) Harold said to send an event
> for this issue. Then later you said "please always use change". I
> thought you and Harold were in sync :(
> 
>>
>> Uevents are ok to use if we can be sure there is _never_ a storm of
>> events. Uevents are not meant to handle large amounts of events
>> happening at the same time. They must never be used to handle things
>> like reporting errors which are not limited in their rate, or where
>> many devices might send similar events at the same time in a row.
>>
> 
> In this case you can get many devices sending the same events at the
> same time, because when the transport/connection comes back you all the
> device accessed through that connection will be set to running at the
> same time.
> 
> So what is the fix for this case? Just send one event for the host or
> transport/connection then have udev loop over all devices accessed
> through that object and fix things up? If so, what type of event do you
> want? CHANGE event plus some other info to indicate to look at child
> devices?

The 'correct' way would be to send a 'CHANGE' event for the rport /
iSCSI session.
That way we'll be in sync with what's actually happening, and won't
get a spurious duplication of events.

And letting udev rules / programs figure out what's happening and
which devices are affected. Bit like dev_loss_tmo setting, only the
other way around.

Alternative would be to hook into the yet-to-be submitted SCSI sense
code infrastructure.

I'll update the patch for that and send an RFC.

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

Re: [PATCH 0/4] notify userspace of offline->running transitions (v2)

[Mike Christie]

Hey,

Would it be ok to still merge patches 1 - 3? I think they are still
useful on their own. They fix that issue where during fast io
fail/replacement_timeout failures IO is failed, but the scsi_device
state indicates the devices are still running.

It seems Hannes was ok with them right?

Drop patch 4 since we all seemed to agree we will do it a different way.


On 05/17/2012 11:56 PM, michaelc@cs.wisc.edu wrote:
> The following patches were made over the misc branch of the scsi tree.
> 
> The patches fix a issue where if the device is offlined or IO is
> failed due to fast_io_fail (fc) /recovery_tmo (iscsi) then comes
> back, apps do not have a way a nice way to figure out the state
> has transitioned to running. Apps have to either poll the sysfs state
> file or send a SG IO to figure it out. With the patch apps can listen
> for the KOBJ CHANGE event like some of them (at least udev does) do
> already.
> 
> v2:
> - Rebased to misc.
> 
> --
> 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

Re: [PATCH 0/4] notify userspace of offline->running transitions (v2)

[Mike Snitzer]

On Wed, Jun 6, 2012 at 5:18 PM, Mike Christie <michaelc@cs.wisc.edu> wrote:
> Hey,
>
> Would it be ok to still merge patches 1 - 3? I think they are still
> useful on their own. They fix that issue where during fast io
> fail/replacement_timeout failures IO is failed, but the scsi_device
> state indicates the devices are still running.
>
> It seems Hannes was ok with them right?
>
> Drop patch 4 since we all seemed to agree we will do it a different way.

Hannes, James,

Please advise on including patches 1 -> 3 during the 3.6 merge.

Thanks,
Mike

[PATCH 0/4] notify userspace of offline->running transitions

[michaelc]

The following patches were made over the misc branch of the scsi tree.

The patches fix a issue where if the device is offlined or IO is
failed due to fast_io_fail (fc) /recovery_tmo (iscsi) then comes
back, apps do not have a way a nice way to figure out the state
has transitioned to running. Apps have to either poll the sysfs state
file or send a SG IO to figure it out. With the patch apps can listen
for the KOBJ CHANGE event like some of them (at least udev does) do
already.

[PATCH 4/4] SCSI: send KOBJ_CHANGE when device is set to running v2

[michaelc]

From: Mike Christie <michaelc@cs.wisc.edu>

If a device goes offline while the device is opened then closed
while it is still offline, udev will remove the /dev/disk/by-id
link. If the device comes back and is set to running, userspace
is not notified, and the by-id link will not get remade.

This patch has scsi-ml send a KOBJ_CHANGE event so tools like udev
will know that it can being to use the device again. With this patch
udev see the KOBJ_CHANGE event and will reprobe the device and recreate
a /dev/disk/by-id link.

v2
- Added SDEV_EVT_STATE_RUNNING evt type.

Signed-off-by: Mike Christie <michaelc@cs.wisc.edu>
---
 drivers/scsi/scsi_lib.c    |    9 +++++++++
 include/scsi/scsi_device.h |    2 ++
 2 files changed, 11 insertions(+), 0 deletions(-)

diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index c61ab0c..ed5bcb2 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -2061,6 +2061,7 @@ int
 scsi_device_set_state(struct scsi_device *sdev, enum scsi_device_state state)
 {
 	enum scsi_device_state oldstate = sdev->sdev_state;
+	int change_evt = 0;
 
 	if (state == oldstate)
 		return 0;
@@ -2079,6 +2080,11 @@ scsi_device_set_state(struct scsi_device *sdev, enum scsi_device_state state)
 		switch (oldstate) {
 		case SDEV_OFFLINE:
 		case SDEV_TRANSPORT_OFFLINE:
+			/*
+			 * Notify userspace we can accept IO by sending
+			 * change event.
+			 */
+			change_evt = 1;
 		case SDEV_QUIESCE:
 		case SDEV_BLOCK:
 		case SDEV_CREATED:
@@ -2160,6 +2166,8 @@ scsi_device_set_state(struct scsi_device *sdev, enum scsi_device_state state)
 
 	}
 	sdev->sdev_state = state;
+	if (change_evt)
+		sdev_evt_send_simple(sdev, SDEV_EVT_STATE_RUNNING, GFP_ATOMIC);
 	return 0;
 
  illegal:
@@ -2283,6 +2291,7 @@ struct scsi_event *sdev_evt_alloc(enum scsi_device_event evt_type,
 	/* evt_type-specific initialization, if any */
 	switch (evt_type) {
 	case SDEV_EVT_MEDIA_CHANGE:
+	case SDEV_EVT_STATE_RUNNING:
 	default:
 		/* do nothing */
 		break;
diff --git a/include/scsi/scsi_device.h b/include/scsi/scsi_device.h
index 5ff8180..0759433 100644
--- a/include/scsi/scsi_device.h
+++ b/include/scsi/scsi_device.h
@@ -52,6 +52,8 @@ enum scsi_device_state {
 
 enum scsi_device_event {
 	SDEV_EVT_MEDIA_CHANGE	= 1,	/* media has changed */
+	SDEV_EVT_STATE_RUNNING	= 2,	/* state has gone from offline to
+					 * running */
 
 	SDEV_EVT_LAST		= SDEV_EVT_MEDIA_CHANGE,
 	SDEV_EVT_MAXBITS	= SDEV_EVT_LAST + 1
-- 
1.7.7.6