Re: 2.6.8.1-mm3

Re: 2.6.8.1-mm3

[Mikael Pettersson]

On Fri, 20 Aug 2004 03:19:19 -0700, Andrew Morton wrote:
>ftp://ftp.kernel.org/pub/linux/kernel/people/akpm/patches/2.6/2.6.8.1/2.6.8.1-mm3/
...
> bk-scsi.patch

This one is broken. It causes the kernel to emit a bogus
"program $PROG is using a deprecated SCSI ioctl, please convert it to SG_IO"
message whenever user-space open(2)s a SCSI block device, even
though user-space never did any ioctl() on it.

Re: 2.6.8.1-mm3

[James Bottomley]

On Sat, 2004-08-21 at 10:43, Mikael Pettersson wrote:
> On Fri, 20 Aug 2004 03:19:19 -0700, Andrew Morton wrote:
> >ftp://ftp.kernel.org/pub/linux/kernel/people/akpm/patches/2.6/2.6.8.1/2.6.8.1-mm3/
> ...
> > bk-scsi.patch
> 
> This one is broken. It causes the kernel to emit a bogus
> "program $PROG is using a deprecated SCSI ioctl, please convert it to SG_IO"
> message whenever user-space open(2)s a SCSI block device, even
> though user-space never did any ioctl() on it.

A simple open of /dev/sda from userland doesn't exhibit this behaviour
for me.  What sort of device is this?  And what is the program?

James

Re: 2.6.8.1-mm3

[Mikael Pettersson]

On 21 Aug 2004 12:02:24 -0400, James Bottomley wrote:
> On Sat, 2004-08-21 at 10:43, Mikael Pettersson wrote:
> > On Fri, 20 Aug 2004 03:19:19 -0700, Andrew Morton wrote:
> > >ftp://ftp.kernel.org/pub/linux/kernel/people/akpm/patches/2.6/2.6.8.1/2.6.8.1-mm3/
> > ...
> > > bk-scsi.patch
> > 
> > This one is broken. It causes the kernel to emit a bogus
> > "program $PROG is using a deprecated SCSI ioctl, please convert it to SG_IO"
> > message whenever user-space open(2)s a SCSI block device, even
> > though user-space never did any ioctl() on it.
> 
> A simple open of /dev/sda from userland doesn't exhibit this behaviour
> for me.  What sort of device is this?  And what is the program?

It happens on my USB flash memory stick, which uses USB_STORAGE and BLK_DEV_SD.
A simple open(2) is enough to trigger the message. I'm about to try -mm3 on a
different machine which has a "true" SCSI controller/disk combo. (I should
have checked that first, sorry.)

/Mikael

Re: 2.6.8.1-mm3

[Mikael Pettersson]

I wrote:
> > > This one is broken. It causes the kernel to emit a bogus
> > > "program $PROG is using a deprecated SCSI ioctl, please convert it to SG_IO"
> > > message whenever user-space open(2)s a SCSI block device, even
> > > though user-space never did any ioctl() on it.
> > 
> > A simple open of /dev/sda from userland doesn't exhibit this behaviour
> > for me.  What sort of device is this?  And what is the program?
> 
> It happens on my USB flash memory stick, which uses USB_STORAGE and BLK_DEV_SD.
> A simple open(2) is enough to trigger the message. I'm about to try -mm3 on a
> different machine which has a "true" SCSI controller/disk combo. (I should
> have checked that first, sorry.)

I checked now, and I can also trigger the message on ide-scsi + sr
(CD writer), but not on ide-scsi + st (ATAPI tape drive) or on
a SCSI disk on a SYM53C8XX controller.

So it seems both usb storage and ide-scsi (or the cdrom module)
generate these ioctls.

Re: 2.6.8.1-mm3

[Patrick Mansfield]

On Sat, Aug 21, 2004 at 08:38:30PM +0200, Mikael Pettersson wrote:

> I checked now, and I can also trigger the message on ide-scsi + sr
> (CD writer), but not on ide-scsi + st (ATAPI tape drive) or on
> a SCSI disk on a SYM53C8XX controller.
> 
> So it seems both usb storage and ide-scsi (or the cdrom module)
> generate these ioctls.

Looks like it will be hit for any scsi removable media, the removable
media check in sd.c sd_media_changed() uses SCSI_IOCTL_TEST_UNIT_READY.

-- Patrick Mansfield

Re: 2.6.8.1-mm3

[James Bottomley]

On Sat, 2004-08-21 at 15:14, Patrick Mansfield wrote:
> Looks like it will be hit for any scsi removable media, the removable
> media check in sd.c sd_media_changed() uses SCSI_IOCTL_TEST_UNIT_READY.

Yes, I'm in two minds about this one.

Either we could provide a helper routine to do it and convert all the
internal uses over, or we could define a new ioctl that is correctly
unique, something like

#define SCSI_TEST_UNIT_READY	_IOR('S', 8, int)
or perhaps just 0x5388

and convert the internal users over to it.

Opinions?

James

Re: 2.6.8.1-mm3

[Matthew Wilcox]

On Sat, Aug 21, 2004 at 03:24:56PM -0400, James Bottomley wrote:
> Either we could provide a helper routine to do it and convert all the
> internal uses over, or we could define a new ioctl that is correctly
> unique, something like
> 
> #define SCSI_TEST_UNIT_READY	_IOR('S', 8, int)
> or perhaps just 0x5388
> 
> and convert the internal users over to it.
> 
> Opinions?

I prefer the former, something like

int scsi_test_unit_ready(struct scsi_device *sdev);

-- 
"Next the statesmen will invent cheap lies, putting the blame upon 
the nation that is attacked, and every man will be glad of those
conscience-soothing falsities, and will diligently study them, and refuse
to examine any refutations of them; and thus he will by and by convince 
himself that the war is just, and will thank God for the better sleep 
he enjoys after this process of grotesque self-deception." -- Mark Twain

Re: 2.6.8.1-mm3

[James Bottomley]

On Sat, 2004-08-21 at 17:47, Matthew Wilcox wrote:
> I prefer the former, something like
> 
> int scsi_test_unit_ready(struct scsi_device *sdev);

OK, how about the attached.  It seems to work for me, but then I don't
have any of the problem devices ...

James

===== drivers/scsi/scsi_ioctl.c 1.29 vs edited =====
--- 1.29/drivers/scsi/scsi_ioctl.c	2004-08-19 16:11:54 -05:00
+++ edited/drivers/scsi/scsi_ioctl.c	2004-08-24 22:34:39 -05:00
@@ -432,12 +432,8 @@
 	case SCSI_IOCTL_DOORUNLOCK:
 		return scsi_set_medium_removal(sdev, SCSI_REMOVAL_ALLOW);
 	case SCSI_IOCTL_TEST_UNIT_READY:
-		scsi_cmd[0] = TEST_UNIT_READY;
-		scsi_cmd[1] = 0;
-		scsi_cmd[2] = scsi_cmd[3] = scsi_cmd[5] = 0;
-		scsi_cmd[4] = 0;
-		return ioctl_internal_command(sdev, scsi_cmd,
-				   IOCTL_NORMAL_TIMEOUT, NORMAL_RETRIES);
+		return scsi_test_unit_ready(sdev, IOCTL_NORMAL_TIMEOUT,
+					    NORMAL_RETRIES);
 	case SCSI_IOCTL_START_UNIT:
 		scsi_cmd[0] = START_STOP;
 		scsi_cmd[1] = 0;
===== drivers/scsi/scsi_lib.c 1.130 vs edited =====
--- 1.130/drivers/scsi/scsi_lib.c	2004-08-23 03:14:46 -05:00
+++ edited/drivers/scsi/scsi_lib.c	2004-08-24 22:08:51 -05:00
@@ -1572,6 +1572,34 @@
 	return ret;
 }
 
+int
+scsi_test_unit_ready(struct scsi_device *sdev, int timeout, int retries)
+{
+	struct scsi_request *sreq;
+	char cmd[] = {
+		TEST_UNIT_READY, 0, 0, 0, 0, 0,
+	};
+	int result;
+	
+	sreq = scsi_allocate_request(sdev, GFP_KERNEL);
+	if (!sreq)
+		return -ENOMEM;
+
+		sreq->sr_data_direction = DMA_NONE;
+        scsi_wait_req(sreq, cmd, NULL, 0, timeout, retries);
+
+	if ((driver_byte(sreq->sr_result) & DRIVER_SENSE)
+	    && (sreq->sr_sense_buffer[2] & 0x0f) == UNIT_ATTENTION
+	    && sdev->removable) {
+		sdev->changed = 1;
+		sreq->sr_result = 0;
+	}
+	result = sreq->sr_result;
+	scsi_release_request(sreq);
+	return result;
+}
+EXPORT_SYMBOL(scsi_test_unit_ready);
+
 /**
  *	scsi_device_set_state - Take the given device through the device
  *		state model.
===== drivers/scsi/sd.c 1.157 vs edited =====
--- 1.157/drivers/scsi/sd.c	2004-08-24 18:09:03 -05:00
+++ edited/drivers/scsi/sd.c	2004-08-24 22:36:10 -05:00
@@ -648,7 +648,7 @@
 	 */
 	retval = -ENODEV;
 	if (scsi_block_when_processing_errors(sdp))
-		retval = scsi_ioctl(sdp, SCSI_IOCTL_TEST_UNIT_READY, NULL);
+		retval = scsi_test_unit_ready(sdp, SD_TIMEOUT, SD_MAX_RETRIES);
 
 	/*
 	 * Unable to test, unit probably not ready.   This usually
===== drivers/scsi/sr.c 1.114 vs edited =====
--- 1.114/drivers/scsi/sr.c	2004-08-12 19:03:53 -05:00
+++ edited/drivers/scsi/sr.c	2004-08-24 22:39:41 -05:00
@@ -183,7 +183,7 @@
 		return -EINVAL;
 	}
 
-	retval = scsi_ioctl(cd->device, SCSI_IOCTL_TEST_UNIT_READY, NULL);
+	retval = scsi_test_unit_ready(cd->device, SR_TIMEOUT, MAX_RETRIES);
 	if (retval) {
 		/* Unable to test, unit probably not ready.  This usually
 		 * means there is no disc in the drive.  Mark as changed,
===== include/scsi/scsi_device.h 1.21 vs edited =====
--- 1.21/include/scsi/scsi_device.h	2004-08-22 20:06:22 -05:00
+++ edited/include/scsi/scsi_device.h	2004-08-24 22:11:48 -05:00
@@ -185,6 +185,8 @@
 extern int scsi_mode_sense(struct scsi_device *sdev, int dbd, int modepage,
 			   unsigned char *buffer, int len, int timeout,
 			   int retries, struct scsi_mode_data *data);
+extern int scsi_test_unit_ready(struct scsi_device *sdev, int timeout,
+				int retries);
 extern int scsi_device_set_state(struct scsi_device *sdev,
 				 enum scsi_device_state state);
 extern int scsi_device_quiesce(struct scsi_device *sdev);