scsi_dh: Move dm device handler to SCSI layer

drivers/scsi/scsi_lib.c +16 -1

@@ -1445,9 +1479,24 @@ static void scsi_kill_request(struct req
 static void scsi_softirq_done(struct request *rq)
 {
 	struct scsi_cmnd *cmd = rq->completion_data;
-	unsigned long wait_for = (cmd->allowed + 1) * cmd->timeout_per_command;
 	int disposition;
+	struct request_queue *q;
+	unsigned long wait_for, flags;
 
+	if (blk_linux_request(rq)) {
+		q = rq->q;
+		spin_lock_irqsave(q->queue_lock, flags);
+		/*
+		 * we always return 1 and the caller should
+		 * check rq->errors for the complete status
+		 */
+		end_that_request_last(rq, 1);
+		spin_unlock_irqrestore(q->queue_lock, flags);
+		return;
+	}
+
+
+	wait_for = (cmd->allowed + 1) * cmd->timeout_per_command;
 	INIT_LIST_HEAD(&cmd->eh_entry);
 

drivers/scsi/scsi_lib.c +9 -1

+
 /*
  * Function:    scsi_request_fn()
  *

@@ -1519,7 +1612,23 @@ static void scsi_request_fn(struct reque
 		 * accept it.
 		 */
 		req = elv_next_request(q);
-		if (!req || !scsi_dev_queue_ready(q, sdev))
+		if (!req)
+			break;
+
+		/*
+		 * We do not account for linux blk req in the device
+		 * or host busy accounting because it is not necessarily
+		 * a scsi command that is sent to some object. The lower
+		 * level can translate it into a request/scsi_cmnd, if

Quoted 25 lines · Chandra Seetharaman

drivers/scsi/scsi_lib.c +16 -1

@@ -1445,9 +1479,24 @@ static void scsi_kill_request(struct req
 static void scsi_softirq_done(struct request *rq)
 {
 	struct scsi_cmnd *cmd = rq->completion_data;
-	unsigned long wait_for = (cmd->allowed + 1) * cmd->timeout_per_command;
 	int disposition;
+	struct request_queue *q;
+	unsigned long wait_for, flags;
 
+	if (blk_linux_request(rq)) {
+		q = rq->q;
+		spin_lock_irqsave(q->queue_lock, flags);
+		/*
+		 * we always return 1 and the caller should
+		 * check rq->errors for the complete status
+		 */
+		end_that_request_last(rq, 1);
+		spin_unlock_irqrestore(q->queue_lock, flags);
+		return;
+	}
+
+
+	wait_for = (cmd->allowed + 1) * cmd->timeout_per_command;
 	INIT_LIST_HEAD(&cmd->eh_entry);
 

.....

Quoted 17 lines

drivers/scsi/scsi_lib.c +9 -1

+
 /*
  * Function:    scsi_request_fn()
  *

@@ -1519,7 +1612,23 @@ static void scsi_request_fn(struct reque
 		 * accept it.
 		 */
 		req = elv_next_request(q);
-		if (!req || !scsi_dev_queue_ready(q, sdev))
+		if (!req)
+			break;
+
+		/*
+		 * We do not account for linux blk req in the device
+		 * or host busy accounting because it is not necessarily
+		 * a scsi command that is sent to some object. The lower
+		 * level can translate it into a request/scsi_cmnd, if

drivers/scsi/scsi_lib.c static void scsi_request_fn(struct reque · line 1632

Quoted 10 lines

drivers/scsi/scsi_lib.c +9 -0

+		 * necessary, and then queue that up using REQ_TYPE_BLOCK_PC.
+		 */
+		if (blk_linux_request(req)) {
+			blkdev_dequeue_request(req);
+			scsi_execute_blk_linux_cmd(req);
+			continue;
+		}
+
+		if (!scsi_dev_queue_ready(q, sdev))
 			break;

I think these two pieces are one of the reasons I have not pushed the patches. I thought the completion and execution pieces here are a little ugly and seem to just wedge themselves in where they want to be.

Is there any way to make the insertion of non-scsi commands more common? Do we have the code for being able to send requests directly to something like a fc rport done? Could we maybe inject these special commands to the hw handler using something similar to how bsg would send non scsi commands to weird objects (objects like rport, sessions, and not devices we traditionally associated with queues like scsi_devices). Just a thought with no code :) that is why the ugly code existed still :)

Quoted 25 lines · Chandra Seetharaman

drivers/scsi/scsi_lib.c +16 -1

@@ -1445,9 +1479,24 @@ static void scsi_kill_request(struct req
 static void scsi_softirq_done(struct request *rq)
 {
 	struct scsi_cmnd *cmd = rq->completion_data;
-	unsigned long wait_for = (cmd->allowed + 1) * cmd->timeout_per_command;
 	int disposition;
+	struct request_queue *q;
+	unsigned long wait_for, flags;
 
+	if (blk_linux_request(rq)) {
+		q = rq->q;
+		spin_lock_irqsave(q->queue_lock, flags);
+		/*
+		 * we always return 1 and the caller should
+		 * check rq->errors for the complete status
+		 */
+		end_that_request_last(rq, 1);
+		spin_unlock_irqrestore(q->queue_lock, flags);
+		return;
+	}
+
+
+	wait_for = (cmd->allowed + 1) * cmd->timeout_per_command;
 	INIT_LIST_HEAD(&cmd->eh_entry);
 

.....

Quoted 17 lines

drivers/scsi/scsi_lib.c +9 -1

+
 /*
  * Function:    scsi_request_fn()
  *

@@ -1519,7 +1612,23 @@ static void scsi_request_fn(struct reque
 		 * accept it.
 		 */
 		req = elv_next_request(q);
-		if (!req || !scsi_dev_queue_ready(q, sdev))
+		if (!req)
+			break;
+
+		/*
+		 * We do not account for linux blk req in the device
+		 * or host busy accounting because it is not necessarily
+		 * a scsi command that is sent to some object. The lower
+		 * level can translate it into a request/scsi_cmnd, if

drivers/scsi/scsi_lib.c static void scsi_request_fn(struct reque · line 1632

Quoted 10 lines

drivers/scsi/scsi_lib.c +9 -0

+		 * necessary, and then queue that up using REQ_TYPE_BLOCK_PC.
+		 */
+		if (blk_linux_request(req)) {
+			blkdev_dequeue_request(req);
+			scsi_execute_blk_linux_cmd(req);
+			continue;
+		}
+
+		if (!scsi_dev_queue_ready(q, sdev))
 			break;

I think these two pieces are one of the reasons I have not pushed the patches. I thought the completion and execution pieces here are a little ugly and seem to just wedge themselves in where they want to be.

Is there any way to make the insertion of non-scsi commands more common? Do we have the code for being able to send requests directly to something like a fc rport done? Could we maybe inject these special commands to the hw handler using something similar to how bsg would send non scsi commands to weird objects (objects like rport, sessions, and not devices we traditionally associated with queues like scsi_devices). Just a thought with no code :) that is why the ugly code existed still :)

Quoted 60 lines · Mike Christie

Quoted 25 lines · Chandra Seetharaman

drivers/scsi/scsi_lib.c +16 -1

@@ -1445,9 +1479,24 @@ static void scsi_kill_request(struct req
 static void scsi_softirq_done(struct request *rq)
 {
 	struct scsi_cmnd *cmd = rq->completion_data;
-	unsigned long wait_for = (cmd->allowed + 1) * cmd->timeout_per_command;
 	int disposition;
+	struct request_queue *q;
+	unsigned long wait_for, flags;
 
+	if (blk_linux_request(rq)) {
+		q = rq->q;
+		spin_lock_irqsave(q->queue_lock, flags);
+		/*
+		 * we always return 1 and the caller should
+		 * check rq->errors for the complete status
+		 */
+		end_that_request_last(rq, 1);
+		spin_unlock_irqrestore(q->queue_lock, flags);
+		return;
+	}
+
+
+	wait_for = (cmd->allowed + 1) * cmd->timeout_per_command;
 	INIT_LIST_HEAD(&cmd->eh_entry);
 

.....

Quoted 17 lines

drivers/scsi/scsi_lib.c +9 -1

+
 /*
  * Function:    scsi_request_fn()
  *

@@ -1519,7 +1612,23 @@ static void scsi_request_fn(struct reque
 		 * accept it.
 		 */
 		req = elv_next_request(q);
-		if (!req || !scsi_dev_queue_ready(q, sdev))
+		if (!req)
+			break;
+
+		/*
+		 * We do not account for linux blk req in the device
+		 * or host busy accounting because it is not necessarily
+		 * a scsi command that is sent to some object. The lower
+		 * level can translate it into a request/scsi_cmnd, if

drivers/scsi/scsi_lib.c static void scsi_request_fn(struct reque · line 1632

Quoted 10 lines

drivers/scsi/scsi_lib.c +9 -0

+		 * necessary, and then queue that up using REQ_TYPE_BLOCK_PC.
+		 */
+		if (blk_linux_request(req)) {
+			blkdev_dequeue_request(req);
+			scsi_execute_blk_linux_cmd(req);
+			continue;
+		}
+
+		if (!scsi_dev_queue_ready(q, sdev))
 			break;

I think these two pieces are one of the reasons I have not pushed the patches. I thought the completion and execution pieces here are a little ugly and seem to just wedge themselves in where they want to be.

Quoted 7 lines

Is there any way to make the insertion of non-scsi commands more common? Do we have the code for being able to send requests directly to something like a fc rport done? Could we maybe inject these special commands to the hw handler using something similar to how bsg would send non scsi commands to weird objects (objects like rport, sessions, and not devices we traditionally associated with queues like scsi_devices). Just a thought with no code :) that is why the ugly code existed still :)

We sort of do. The bsg code in scsi_transport_sas to send SMP frames to expander devices would be an example of non-scsi commands going via a mechanism other than being encapsulated in SCSI. I don't know if that's the complete solution in this case, but you could investigate it.

Quoted 70 lines · James Bottomley

Quoted 60 lines · Mike Christie

Quoted 25 lines · Chandra Seetharaman

drivers/scsi/scsi_lib.c +16 -1

@@ -1445,9 +1479,24 @@ static void scsi_kill_request(struct req
 static void scsi_softirq_done(struct request *rq)
 {
 	struct scsi_cmnd *cmd = rq->completion_data;
-	unsigned long wait_for = (cmd->allowed + 1) * cmd->timeout_per_command;
 	int disposition;
+	struct request_queue *q;
+	unsigned long wait_for, flags;
 
+	if (blk_linux_request(rq)) {
+		q = rq->q;
+		spin_lock_irqsave(q->queue_lock, flags);
+		/*
+		 * we always return 1 and the caller should
+		 * check rq->errors for the complete status
+		 */
+		end_that_request_last(rq, 1);
+		spin_unlock_irqrestore(q->queue_lock, flags);
+		return;
+	}
+
+
+	wait_for = (cmd->allowed + 1) * cmd->timeout_per_command;
 	INIT_LIST_HEAD(&cmd->eh_entry);
 

.....

Quoted 17 lines

drivers/scsi/scsi_lib.c +9 -1

+
 /*
  * Function:    scsi_request_fn()
  *

@@ -1519,7 +1612,23 @@ static void scsi_request_fn(struct reque
 		 * accept it.
 		 */
 		req = elv_next_request(q);
-		if (!req || !scsi_dev_queue_ready(q, sdev))
+		if (!req)
+			break;
+
+		/*
+		 * We do not account for linux blk req in the device
+		 * or host busy accounting because it is not necessarily
+		 * a scsi command that is sent to some object. The lower
+		 * level can translate it into a request/scsi_cmnd, if

drivers/scsi/scsi_lib.c static void scsi_request_fn(struct reque · line 1632

Quoted 10 lines

drivers/scsi/scsi_lib.c +9 -0

+		 * necessary, and then queue that up using REQ_TYPE_BLOCK_PC.
+		 */
+		if (blk_linux_request(req)) {
+			blkdev_dequeue_request(req);
+			scsi_execute_blk_linux_cmd(req);
+			continue;
+		}
+
+		if (!scsi_dev_queue_ready(q, sdev))
 			break;

I think these two pieces are one of the reasons I have not pushed the patches. I thought the completion and execution pieces here are a little ugly and seem to just wedge themselves in where they want to be.

Quoted 7 lines

Is there any way to make the insertion of non-scsi commands more common? Do we have the code for being able to send requests directly to something like a fc rport done? Could we maybe inject these special commands to the hw handler using something similar to how bsg would send non scsi commands to weird objects (objects like rport, sessions, and not devices we traditionally associated with queues like scsi_devices). Just a thought with no code :) that is why the ugly code existed still :)

Quoted 4 lines

We sort of do. The bsg code in scsi_transport_sas to send SMP frames to expander devices would be an example of non-scsi commands going via a mechanism other than being encapsulated in SCSI. I don't know if that's the complete solution in this case, but you could investigate it.

I looked at the bsg code in scsi_transport_sas and all I see it doing is calling blk_init_queue to set the request_fn. The request_fn (*smp_request) just processes one cmd_type. Is there code is another tree that has more processing?

A idea to allow for more control / flexibility cmd_type handlers could be added inside request_fn, prep_rq_fn, softirq_done_fn.

I thought about this being at a higher level in the block layer, but it would be hard to handle the request_fn cleanly at the high level. The localized change would reduce impact on users who do not want or need per cmd_type handlers.

* mainly associated with tapes and returned SUCCESS.
Index: linux-2.6.24-rc8/drivers/scsi/scsi_sysfs.c
===================================================================

drivers/scsi/scsi_sysfs.c +33 -0

@@ -951,6 +951,49 @@ int scsi_register_interface(struct class
 }
 EXPORT_SYMBOL(scsi_register_interface);
 
+static int scsi_dh_notifier_add(struct device *dev, void *data)
+{
+	struct scsi_device_handler *sdev_dh = data;
+
+	sdev_dh->nb.notifier_call(&sdev_dh->nb, BUS_NOTIFY_ADD_DEVICE, dev);
+
+	return 0;
+}
+
+int scsi_register_device_handler(struct scsi_device_handler *sdev_dh)
+{
+	int ret;
+
+	ret = bus_register_notifier(&scsi_bus_type, &sdev_dh->nb);
+
+	bus_for_each_dev(&scsi_bus_type, NULL, sdev_dh, scsi_dh_notifier_add);
+
+	return ret;
+}
+EXPORT_SYMBOL(scsi_register_device_handler);
+
+static int scsi_dh_notifier_remove(struct device *dev, void *data)
+{
+	struct scsi_device_handler *sdev_dh = data;
+
+	sdev_dh->nb.notifier_call(&sdev_dh->nb, BUS_NOTIFY_DEL_DEVICE, dev);
+
+	return 0;
+}
+
+int scsi_unregister_device_handler(struct scsi_device_handler *sdev_dh)
+{
+	int ret;

Quoted 48 lines · Chandra Seetharaman

* mainly associated with tapes and returned SUCCESS.
Index: linux-2.6.24-rc8/drivers/scsi/scsi_sysfs.c
===================================================================

drivers/scsi/scsi_sysfs.c +39 -0

@@ -951,6 +951,49 @@ int scsi_register_interface(struct class
 }
 EXPORT_SYMBOL(scsi_register_interface);
 +static int scsi_dh_notifier_add(struct device *dev, void *data)
+{
+	struct scsi_device_handler *sdev_dh = data;
+
+	sdev_dh->nb.notifier_call(&sdev_dh->nb, BUS_NOTIFY_ADD_DEVICE, dev);
+
+	return 0;
+}
+
+int scsi_register_device_handler(struct scsi_device_handler *sdev_dh)
+{
+	int ret;
+
+	ret = bus_register_notifier(&scsi_bus_type, &sdev_dh->nb);
+
+	bus_for_each_dev(&scsi_bus_type, NULL, sdev_dh, scsi_dh_notifier_add);
+
+	return ret;
+}
+EXPORT_SYMBOL(scsi_register_device_handler);
+
+static int scsi_dh_notifier_remove(struct device *dev, void *data)
+{
+	struct scsi_device_handler *sdev_dh = data;
+
+	sdev_dh->nb.notifier_call(&sdev_dh->nb, BUS_NOTIFY_DEL_DEVICE, dev);
+
+	return 0;
+}
+
+int scsi_unregister_device_handler(struct scsi_device_handler *sdev_dh)
+{
+	int ret;
+
+	ret = bus_unregister_notifier(&scsi_bus_type, &sdev_dh->nb);
+
+	bus_for_each_dev(&scsi_bus_type, NULL, sdev_dh,
+					scsi_dh_notifier_remove);
+
+	return ret;

drivers/scsi/scsi_sysfs.c int scsi_register_interface(struct class · line 996

Quoted 3 lines

drivers/scsi/scsi_sysfs.c +3 -0

+}
+EXPORT_SYMBOL(scsi_unregister_device_handler);
+

Did this end up solving the problem I was hitting where when using the other driver model stuff like is used with the SCSI upper layer drivers. The problem where the hw handler module had to be loaded before finding devices and had to be loaded before sd.c?

Quoted 49 lines · Chandra Seetharaman

* mainly associated with tapes and returned SUCCESS.
Index: linux-2.6.24-rc8/drivers/scsi/scsi_sysfs.c
===================================================================

drivers/scsi/scsi_sysfs.c +40 -0

@@ -951,6 +951,49 @@ int scsi_register_interface(struct class
 }
 EXPORT_SYMBOL(scsi_register_interface);
 
+static int scsi_dh_notifier_add(struct device *dev, void *data)
+{
+	struct scsi_device_handler *sdev_dh = data;
+
+	sdev_dh->nb.notifier_call(&sdev_dh->nb, BUS_NOTIFY_ADD_DEVICE, dev);
+
+	return 0;
+}
+
+int scsi_register_device_handler(struct scsi_device_handler *sdev_dh)
+{
+	int ret;
+
+	ret = bus_register_notifier(&scsi_bus_type, &sdev_dh->nb);
+
+	bus_for_each_dev(&scsi_bus_type, NULL, sdev_dh, scsi_dh_notifier_add);
+
+	return ret;
+}
+EXPORT_SYMBOL(scsi_register_device_handler);
+
+static int scsi_dh_notifier_remove(struct device *dev, void *data)
+{
+	struct scsi_device_handler *sdev_dh = data;
+
+	sdev_dh->nb.notifier_call(&sdev_dh->nb, BUS_NOTIFY_DEL_DEVICE, dev);
+
+	return 0;
+}
+
+int scsi_unregister_device_handler(struct scsi_device_handler *sdev_dh)
+{
+	int ret;
+
+	ret = bus_unregister_notifier(&scsi_bus_type, &sdev_dh->nb);
+
+	bus_for_each_dev(&scsi_bus_type, NULL, sdev_dh,
+					scsi_dh_notifier_remove);
+
+	return ret;

drivers/scsi/scsi_sysfs.c int scsi_register_interface(struct class · line 996

Quoted 3 lines

drivers/scsi/scsi_sysfs.c +3 -0

+}
+EXPORT_SYMBOL(scsi_unregister_device_handler);
+

Did this end up solving the problem I was hitting where when using the other driver model stuff like is used with the SCSI upper layer drivers. The problem where the hw handler module had to be loaded before finding devices and had to be loaded before sd.c?

-
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Subject: scsi_dh: Add support for SDEV_PASSIVE

From: Chandra Seetharaman <[email protected]>

This patch adds a new device state SDEV_PASSIVE, to correspond to the passive side access of an active/passive multipathed device.

Quoted 6 lines · Chandra Seetharaman

Subject: scsi_dh: Add support for SDEV_PASSIVE

From: Chandra Seetharaman <[email protected]>

This patch adds a new device state SDEV_PASSIVE, to correspond to the passive side access of an active/passive multipathed device.

Really, no; this isn't right. The state field of a SCSI device is for the SCSI state model. Passive might be a valid device mapper state, but

it's not a valid SCSI state. If these patches can't work except by mucking with the SCSI state model, there's some layering problem elsewhere that needs sorting out.

James

-
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Quoted 8 lines · James Bottomley

Quoted 6 lines · Chandra Seetharaman

Subject: scsi_dh: Add support for SDEV_PASSIVE

From: Chandra Seetharaman <[email protected]>

This patch adds a new device state SDEV_PASSIVE, to correspond to the passive side access of an active/passive multipathed device.

Quoted 2 lines

Really, no; this isn't right. The state field of a SCSI device is for the SCSI state model. Passive might be a valid device mapper state, but

Hi James,

It is not the "device mapper state", it is the state of the device itself. These devices have active/passive paths, the passive paths will be represented by SDEV_PASSIVE device state in SCSI.

Quoted 9 lines · Chandra Seetharaman

Quoted 8 lines · James Bottomley

Quoted 6 lines · Chandra Seetharaman

Subject: scsi_dh: Add support for SDEV_PASSIVE

From: Chandra Seetharaman <[email protected]>

This patch adds a new device state SDEV_PASSIVE, to correspond to the passive side access of an active/passive multipathed device.

Quoted 8 lines

Quoted 2 lines

Really, no; this isn't right. The state field of a SCSI device is for the SCSI state model. Passive might be a valid device mapper state, but

Hi James,

It is not the "device mapper state", it is the state of the device itself. These devices have active/passive paths, the passive paths will be represented by SDEV_PASSIVE device state in SCSI.

Yes, it is .. you're killing commands on the basis of being in this state, which nothing in SCSI ever sets.

A proper return from a passive path is the SCSI standard NOT_READY LOGICAL UNIT NOT READY, INITIALIZING COMMAND REQUIRED. We expect to see this, not the command being killed.

James

-
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

The device does send these error messages currently, but it takes some time to get the check condition back, which adds up the time to boot especially when the # of LUNS is huge.

For example, in my test configuration, I had 40 luns, and the time difference (with this patch and without it) to boot is 171 seconds and 1426 seconds.

Quoted 7 lines · Chandra Seetharaman

The device does send these error messages currently, but it takes some time to get the check condition back, which adds up the time to boot especially when the # of LUNS is huge.

For example, in my test configuration, I had 40 luns, and the time difference (with this patch and without it) to boot is 171 seconds and 1426 seconds.

Was that with sync or async SCSI bus scanning?

Quoted 5 lines · Matthew Wilcox

Quoted 4 lines · Chandra Seetharaman

The device does send these error messages currently, but it takes some time to get the check condition back, which adds up the time to boot especially when the # of LUNS is huge.

Quoted 5 lines

Quoted 3 lines

For example, in my test configuration, I had 40 luns, and the time difference (with this patch and without it) to boot is 171 seconds and 1426 seconds.

Was that with sync or async SCSI bus scanning?

I didn't change anything, IOW, i did default scanning, which I would guess sync ?!

Quoted 12 lines · Chandra Seetharaman

Quoted 10 lines · Matthew Wilcox

Quoted 7 lines · Chandra Seetharaman

The device does send these error messages currently, but it takes some time to get the check condition back, which adds up the time to boot especially when the # of LUNS is huge.

For example, in my test configuration, I had 40 luns, and the time difference (with this patch and without it) to boot is 171 seconds and 1426 seconds.

Was that with sync or async SCSI bus scanning?

Quoted 2 lines

I didn't change anything, IOW, i did default scanning, which I would guess sync ?!

That would depend on your CONFIG_SCSI_SCAN_ASYNC setting.

Try booting with 'scsi_mod.scan=async' and without this patch, and see how long it takes.

Quoted 10 lines · Chandra Seetharaman

Quoted 9 lines · James Bottomley

Quoted 6 lines · Chandra Seetharaman

Subject: scsi_dh: Add support for SDEV_PASSIVE

From: Chandra Seetharaman <[email protected]>

This patch adds a new device state SDEV_PASSIVE, to correspond to the passive side access of an active/passive multipathed device.

Really, no; this isn't right. The state field of a SCSI device is for the SCSI state model. Passive might be a valid device mapper state, but

Quoted 5 lines

Hi James,

It is not the "device mapper state", it is the state of the device itself. These devices have active/passive paths, the passive paths will be represented by SDEV_PASSIVE device state in SCSI.

Yes, it is .. you're killing commands on the basis of being in this state, which nothing in SCSI ever sets.

A proper return from a passive path is the SCSI standard NOT_READY LOGICAL UNIT NOT READY, INITIALIZING COMMAND REQUIRED. We expect to see this, not the command being killed.

When IO is sent to a path that cannot execute IO optimally, the scsi hw handler hook for sense processing (see rdac_check_sense in "[PATCH 8/9]
scsi_dh: add lsi rdac device handler" and the scsi_error.c hook in in "scsi_dh: add skeleton for SCSI Device Handlers") will detect this and set the state to passive so future IO is not execute on the path (SG_IO/passthrough is allowed).

I am not sure about alternatives. If we just exported the port access state in sysfs, but did not fail IO from scsi_prep_state_check, then the users could still check the state before sending IO. Would it be horrible to convert apps to do this?

Quoted 7 lines · Mike Christie

When IO is sent to a path that cannot execute IO optimally, the scsi hw handler hook for sense processing (see rdac_check_sense in "[PATCH 8/9]
scsi_dh: add lsi rdac device handler" and the scsi_error.c hook in in "scsi_dh: add skeleton for SCSI Device Handlers") will detect this and set the state to passive so future IO is not execute on the path (SG_IO/passthrough is allowed).

Quoted 4 lines

I am not sure about alternatives. If we just exported the port access state in sysfs, but did not fail IO from scsi_prep_state_check, then the users could still check the state before sending IO. Would it be horrible to convert apps to do this?

The majority of the boot up delays is caused by the kernel partition scanning and other kernel init code (Chandra please correct if that is not

true). Sysfs attributes would not help here. One option maybe to add handling of the newer BLKERR_ codes in the generators of IO or some similar solution with a rollout possibly focused at the top generators of

IO.

A number of user apps like lvm scanning that execute media access commands already have filter capability to filter devices that one does not want to

scan. Another class of device scanners just use inquiries which are not effected by the passive state (though some could probably use udevinfo and reduce the amount of repeated SCSI inquiries execute on the system.

-andmike
--
Michael Anderson
[email protected]

A number of user apps like lvm scanning that execute media access commands already have filter capability to filter devices that one does not want to scan. Another class of device scanners just use inquiries which are not effected by the passive state (though some could probably use udevinfo and reduce the amount of repeated SCSI inquiries execute on the system.

Quoted 5 lines · Mike Anderson

A number of user apps like lvm scanning that execute media access commands already have filter capability to filter devices that one does not want to scan. Another class of device scanners just use inquiries which are not effected by the passive state (though some could probably use udevinfo and reduce the amount of repeated SCSI inquiries execute on the system.

To expand on this:

At least on desktop systems and SOHO server systems, userspace should _never_ issue INQUIRY. There are too many broken firmwares out there which assume that there will never be more than one INQUIRY sent. They start to return garbled data or crash if they get a second INQUIRY.

Quoted 7 lines · Stefan Richter

Quoted 5 lines · Mike Anderson

A number of user apps like lvm scanning that execute media access commands already have filter capability to filter devices that one does not want to scan. Another class of device scanners just use inquiries which are not effected by the passive state (though some could probably use udevinfo and reduce the amount of repeated SCSI inquiries execute on the system.

Quoted 6 lines

To expand on this:

At least on desktop systems and SOHO server systems, userspace should _never_ issue INQUIRY. There are too many broken firmwares out there which assume that there will never be more than one INQUIRY sent. They start to return garbled data or crash if they get a second INQUIRY.

It's all very well to say this, but I think if you look at what udev does, you'll find that it uses scsi_id to send a VPD inquiry to the device so it can populate /dev/disk/by-id, so the point is already conceded (and I think looking at a recent camera crash that seems to have been precipitated by this, it's already causing us problems).

This is all a tradeoff. If you want userspace *never* to issue raw SCSI commands like INQUIRY, we're going to have to provide the needed information from the kernel via sysfs ... including VPD strings. This is something we've always shovelled off into userspace before.

Quoted 4 lines · James Bottomley

It's all very well to say this, but I think if you look at what udev does, you'll find that it uses scsi_id to send a VPD inquiry to the device so it can populate /dev/disk/by-id, so the point is already conceded

Quoted 5 lines

Quoted 4 lines · James Bottomley

It's all very well to say this, but I think if you look at what udev does, you'll find that it uses scsi_id to send a VPD inquiry to the device so it can populate /dev/disk/by-id, so the point is already conceded

PS: Alas we don't have a practicable way to know how many of the
- doesn't work with Linux but works to some degree with Windows,
- doesn't work with a 2.6 based Linux distro but did work with a 2.4 based distro
kinds of devices are those with this INQUIRY bug or similar bugs.

While non-udev distros slowly went out of fashion on the desktop, there was a certain frequency of reports of the latter kind of FireWire devices, but this was before I became aware of that kind of firmware bug, therefore I don't have any data whether it played a role for these cases.

Quoted 12 lines · Stefan Richter

Quoted 5 lines

Quoted 4 lines · James Bottomley

It's all very well to say this, but I think if you look at what udev does, you'll find that it uses scsi_id to send a VPD inquiry to the device so it can populate /dev/disk/by-id, so the point is already conceded

PS: Alas we don't have a practicable way to know how many of the
- doesn't work with Linux but works to some degree with Windows,
- doesn't work with a 2.6 based Linux distro but did work with a 2.4 based distro
kinds of devices are those with this INQUIRY bug or similar bugs.

Quoted 5 lines

While non-udev distros slowly went out of fashion on the desktop, there was a certain frequency of reports of the latter kind of FireWire devices, but this was before I became aware of that kind of firmware bug, therefore I don't have any data whether it played a role for these cases.

Just a small point here. To my knowledge all devices that understand the SCSI command set will accept a 36 byte INQUIRY (i.e. an INQUIRY with an "allocation length" of 36). That is a practical consideration since they would crash on Windows otherwise.

Now scsi_id (and probably other programs of that ilk (including some of mine)) get excited when they get a response to a "36 byte" INQUIRY and go onto ask for VPD page 0x80 (serial number) or page 0x83 (device id) via an INQUIRY command with the EVPD bit set. Now if they were clever they would make sure that the "allocation length" of the _first_ such VPD inquiry was 36! That way they could have a close look at the response and if it wasn't valid for a VPD page (e.g. requested_VPD_page_num != response[1]) then they would leave the damn device alone **.

<<< Special case: removable media >>>
Since response[1] of a standard INQUIRY has RMB (removable medium bit) set in its top bit and the rest of the byte reserved, 0x80 is a possibility. That could give a false positive when the serial number VPD page is requested. So try looking for VPD page 0x83 (with alloc_len=36) first.

** The logic here is that a brain dead SCSI target interpreter doesn't look at the EVPD bit and can only handle an "allocation length" of 36. So if it receives an INQUIRY cdb with the EVPD bit set it will just respond with its standard INQUIRY response.

This is all a tradeoff. If you want userspace *never* to issue raw SCSI
commands like INQUIRY, we're going to have to provide the needed
information from the kernel via sysfs ... including VPD strings. This
is something we've always shovelled off into userspace before.

Quoted 4 lines · Chandra Seetharaman

Subject: scsi_dh: Add support for SDEV_PASSIVE

From: Chandra Seetharaman <[email protected]>

Quoted 2 lines

This patch adds a new device state SDEV_PASSIVE, to correspond to the passive side access of an active/passive multipathed device.

Really, no; this isn't right. The state field of a SCSI device is for the SCSI state model. Passive might be a valid device mapper state, but it's not a valid SCSI state. If these patches can't work except by mucking with the SCSI state model, there's some layering problem elsewhere that needs sorting out.