add SCSI reset ioctls to scsi_ioctl.c (and sd and sr)

PATCH In review James Bottomley 4 participants 6 messages Sep 18, 2004 – Sep 20, 2004

Subsystems (2)

SCSI SCSI tape driver

lore

Share & tools

lore

James Bottomley

author

2004-09-18 16:57 UTC #0 lore raw

PATCH 1/1 add SCSI reset ioctls to scsi_ioctl.c (and sd and sr)

The purpose of this patch is to allow a SCSI reset to be issued directly to the devices without having to go via sg. Because the reset may be issued even while the device is in recovery, this had to be done via a new api (sd won't invoke scsi_ioctl() while the device is in recovery).

Kai, do you want me to plum this into st as well?

James

===== drivers/scsi/scsi_ioctl.c 1.30 vs edited =====

drivers/scsi/scsi_ioctl.c +49 -0

@@ -20,6 +20,7 @@
 #include <scsi/scsi_host.h>
 #include <scsi/scsi_ioctl.h>
 #include <scsi/scsi_request.h>
+#include <scsi/sg.h>
 
 #include "scsi_logging.h"

@@ -456,3 +457,51 @@
 	}
 	return -EINVAL;
 }
+
+/*
+ * the scsi_nonblock_ioctl() function is designed for ioctls which may
+ * be executed even if the device is in recovery.
+ */
+int scsi_nonblockable_ioctl(struct scsi_device *sdev, int cmd,
+			    void __user *arg, struct file *filp)
+{
+	int val, result;
+
+	/* The first set of iocts may be executed even if we're doing
+	 * error processing, as long as the device was opened
+	 * non-blocking */
+	if (filp && filp->f_flags & O_NONBLOCK) {
+		if (test_bit(SHOST_RECOVERY,
+			     &sdev->host->shost_state))
+			return -ENODEV;
+	} else if (!scsi_block_when_processing_errors(sdev))
+		return -ENODEV;
+
+	switch (cmd) {
+	case SG_SCSI_RESET:
+		result = get_user(val, (int __user *)arg);
+		if (result)
+			return result;
+		if (val == SG_SCSI_RESET_NOTHING)
+			return 0;
+		switch (val) {
+		case SG_SCSI_RESET_DEVICE:
+			val = SCSI_TRY_RESET_DEVICE;
+			break;
+		case SG_SCSI_RESET_BUS:
+			val = SCSI_TRY_RESET_BUS;
+			break;
+		case SG_SCSI_RESET_HOST:
+			val = SCSI_TRY_RESET_HOST;
+			break;
+		default:
+			return -EINVAL;
+		}
+		if (!capable(CAP_SYS_ADMIN) || !capable(CAP_SYS_RAWIO))
+			return -EACCES;
+		return (scsi_reset_provider(sdev, val) ==
+			SUCCESS) ? 0 : -EIO;
+	}
+	return -ENODEV;
+}
+EXPORT_SYMBOL(scsi_nonblockable_ioctl);

===== drivers/scsi/sd.c 1.159 vs edited =====

drivers/scsi/sd.c +1 -1

@@ -575,7 +575,7 @@
 	 * access to the device is prohibited.
 	 */
 	if (!scsi_block_when_processing_errors(sdp))
-		return -ENODEV;
+		return scsi_nonblockable_ioctl(sdp, cmd, p, filp);
 
 	if (cmd == HDIO_GETGEO) {
 		if (!arg)

===== drivers/scsi/sr_ioctl.c 1.36 vs edited =====

drivers/scsi/sr_ioctl.c +12 -0

@@ -549,5 +549,17 @@
 		 unsigned int cmd, unsigned long arg)
 {
 	Scsi_CD *cd = cdi->handle;
+	int ret;
+	
+	ret = scsi_nonblockable_ioctl(cd->device, cmd,
+				      (void __user *)arg, NULL);
+	/*
+	 * ENODEV means that we didn't recognise the ioctl, or that we
+	 * cannot execute it in the current device state.  In either
+	 * case fall through to scsi_ioctl, which will return ENDOEV again
+	 * if it doesn't recognise the ioctl
+	 */
+	if (ret != -ENODEV)
+		return ret;
 	return scsi_ioctl(cd->device, cmd, (void __user *)arg);
 }

===== include/scsi/scsi_ioctl.h 1.4 vs edited =====

include/scsi/scsi_ioctl.h +2 -0

@@ -43,6 +43,8 @@
 extern int scsi_ioctl(struct scsi_device *, int, void __user *);
 extern int scsi_ioctl_send_command(struct scsi_device *,
 				   struct scsi_ioctl_command __user *);
+extern int scsi_nonblockable_ioctl(struct scsi_device *sdev, int cmd,
+				   void __user *arg, struct file *filp);
 
 #endif /* __KERNEL__ */
 #endif /* _SCSI_IOCTL_H */

Kai Makisara

2004-09-18 19:49 UTC #1 lore raw

Quoted 7 lines · James Bottomley

The purpose of this patch is to allow a SCSI reset to be issued directly to the devices without having to go via sg. Because the reset may be issued even while the device is in recovery, this had to be done via a new api (sd won't invoke scsi_ioctl() while the device is in recovery).
Kai, do you want me to plum this into st as well?

Yes, please do that. I would like st to have the same generic capabilities as the other high-level drivers as far as possible and I don't see any problems in adding this feature. It is useful to have an easy way to reset a tape drive. Some drives seem to be lock up easily when they see a bad tape.

Signature · 1 line

Kai

James Bottomley

author

2004-09-19 02:39 UTC #2 lore raw

Quoted 4 lines · Kai Makisara

Yes, please do that. I would like st to have the same generic capabilities as the other high-level drivers as far as possible and I don't see any problems in adding this feature. It is useful to have an easy way to reset a tape drive. Some drives seem to be lock up easily when they see a bad tape.

OK, I added st. The attached actually works (I even tested it) for sd.

James

===== drivers/scsi/scsi_ioctl.c 1.30 vs edited =====

drivers/scsi/scsi_ioctl.c +49 -0

@@ -20,6 +20,7 @@
 #include <scsi/scsi_host.h>
 #include <scsi/scsi_ioctl.h>
 #include <scsi/scsi_request.h>
+#include <scsi/sg.h>
 
 #include "scsi_logging.h"

@@ -456,3 +457,51 @@
 	}
 	return -EINVAL;
 }
+
+/*
+ * the scsi_nonblock_ioctl() function is designed for ioctls which may
+ * be executed even if the device is in recovery.
+ */
+int scsi_nonblockable_ioctl(struct scsi_device *sdev, int cmd,
+			    void __user *arg, struct file *filp)
+{
+	int val, result;
+
+	/* The first set of iocts may be executed even if we're doing
+	 * error processing, as long as the device was opened
+	 * non-blocking */
+	if (filp && filp->f_flags & O_NONBLOCK) {
+		if (test_bit(SHOST_RECOVERY,
+			     &sdev->host->shost_state))
+			return -ENODEV;
+	} else if (!scsi_block_when_processing_errors(sdev))
+		return -ENODEV;
+
+	switch (cmd) {
+	case SG_SCSI_RESET:
+		result = get_user(val, (int __user *)arg);
+		if (result)
+			return result;
+		if (val == SG_SCSI_RESET_NOTHING)
+			return 0;
+		switch (val) {
+		case SG_SCSI_RESET_DEVICE:
+			val = SCSI_TRY_RESET_DEVICE;
+			break;
+		case SG_SCSI_RESET_BUS:
+			val = SCSI_TRY_RESET_BUS;
+			break;
+		case SG_SCSI_RESET_HOST:
+			val = SCSI_TRY_RESET_HOST;
+			break;
+		default:
+			return -EINVAL;
+		}
+		if (!capable(CAP_SYS_ADMIN) || !capable(CAP_SYS_RAWIO))
+			return -EACCES;
+		return (scsi_reset_provider(sdev, val) ==
+			SUCCESS) ? 0 : -EIO;
+	}
+	return -ENODEV;
+}
+EXPORT_SYMBOL(scsi_nonblockable_ioctl);

===== drivers/scsi/sd.c 1.159 vs edited =====

drivers/scsi/sd.c +3 -2

@@ -574,8 +574,9 @@
 	 * may try and take the device offline, in which case all further
 	 * access to the device is prohibited.
 	 */
-	if (!scsi_block_when_processing_errors(sdp))
-		return -ENODEV;
+	error = scsi_nonblockable_ioctl(sdp, cmd, p, filp);
+	if (!scsi_block_when_processing_errors(sdp) || !error)
+		return error;
 
 	if (cmd == HDIO_GETGEO) {
 		if (!arg)

===== drivers/scsi/sr_ioctl.c 1.36 vs edited =====

drivers/scsi/sr_ioctl.c +12 -0

@@ -549,5 +549,17 @@
 		 unsigned int cmd, unsigned long arg)
 {
 	Scsi_CD *cd = cdi->handle;
+	int ret;
+	
+	ret = scsi_nonblockable_ioctl(cd->device, cmd,
+				      (void __user *)arg, NULL);
+	/*
+	 * ENODEV means that we didn't recognise the ioctl, or that we
+	 * cannot execute it in the current device state.  In either
+	 * case fall through to scsi_ioctl, which will return ENDOEV again
+	 * if it doesn't recognise the ioctl
+	 */
+	if (ret != -ENODEV)
+		return ret;
 	return scsi_ioctl(cd->device, cmd, (void __user *)arg);
 }

===== drivers/scsi/st.c 1.94 vs edited =====

drivers/scsi/st.c +3 -3

@@ -3128,10 +3128,10 @@
 	 * may try and take the device offline, in which case all further
 	 * access to the device is prohibited.
 	 */
-	if (!scsi_block_when_processing_errors(STp->device)) {
-		retval = (-ENXIO);
+	retval = scsi_nonblockable_ioctl(STp->device, cmd_in, p, file);
+	if (!scsi_block_when_processing_errors(STp->device) || !retval)
 		goto out;
-	}
+
 	cmd_type = _IOC_TYPE(cmd_in);
 	cmd_nr = _IOC_NR(cmd_in);

===== include/scsi/scsi_ioctl.h 1.4 vs edited =====

include/scsi/scsi_ioctl.h +2 -0

@@ -43,6 +43,8 @@
 extern int scsi_ioctl(struct scsi_device *, int, void __user *);
 extern int scsi_ioctl_send_command(struct scsi_device *,
 				   struct scsi_ioctl_command __user *);
+extern int scsi_nonblockable_ioctl(struct scsi_device *sdev, int cmd,
+				   void __user *arg, struct file *filp);
 
 #endif /* __KERNEL__ */
 #endif /* _SCSI_IOCTL_H */

Douglas Gilbert

2004-09-20 05:49 UTC #3 lore raw

In reply to James Bottomley

Quoted 4 lines · James Bottomley

The purpose of this patch is to allow a SCSI reset to be issued directly to the devices without having to go via sg. Because the reset may be issued even while the device is in recovery, this had to be done via a new api (sd won't invoke scsi_ioctl() while the device is in recovery).

James,
Perhaps there should be a new reset called:
...RESET_LOGICAL_UNIT

as this is now defined as a task management function in SAM-3. If so ...RESET_DEVICE might get a synonym: ...RESET_TARGET. Most transports support a RESET_TARGET while RESET_BUS is SPI specific (is there an FCP equivalent?).

Doug Gilbert

James Bottomley

author

2004-09-20 13:50 UTC #4 lore raw

Quoted 7 lines · Douglas Gilbert

Perhaps there should be a new reset called:
...RESET_LOGICAL_UNIT
as this is now defined as a task management function in SAM-3. If so ...RESET_DEVICE might get a synonym: ...RESET_TARGET. Most transports support a RESET_TARGET while RESET_BUS is SPI specific (is there an FCP equivalent?).

Well, perhaps ... also one that takes an actual value for the reset type instead of a pointer to a value that requires compat processing. However, the goal of this exercise was simply to provide backwards compatibility.

Probably, longer term, the reset ioctls should be provided by the transport class, since they're pretty much transport specific.

James

Luben Tuikov

2004-09-20 14:54 UTC #5 lore raw

In reply to Douglas Gilbert

Quoted 7 lines

Perhaps there should be a new reset called:
...RESET_LOGICAL_UNIT
as this is now defined as a task management function in SAM-3. If so ...RESET_DEVICE might get a synonym: ...RESET_TARGET. Most transports support a RESET_TARGET while RESET_BUS is SPI specific (is there an FCP equivalent?).

Very good idea. Most likely TMF LU Reset should be directly addressable to the LLDD, either via a separate LLDD stub or through a common LLDD::int do_tmf(int tmf, void *data) stub, so that the LLDD knows LU Reset is wanted.

The point is that the LLDD (and the target on the other end) would know best how to do this (and other) TMFs. (More so for newer protocols/transports.)

As to FCP, why not just do I_T Nexus loss or Transport Reset (best). Ref. SAM3r13, sec 6 and FCP3r3c (4.9, 4.11, 4.12). And possibly have the SCSI events into SCSI Core, for all types of transports.

So, recovery would resolve to issuing a TMF (or two) and if this doesn't work _and_ we haven't gotten a SCSI event, we can _force_ an event, else we just act on the event gotten.

Luben

P.S. We have to allow LLDDs, to mask out
"forcing" of those events, as the recovery thread
in SCSI Core shouldn't be so "trigger happy".