[PATCH 2/3] SCSI core and sd: early detection of medium not present

[PATCH 2/3] SCSI core and sd: early detection of medium not present

[Alan Stern]

This patch (as695) changes the scsi_test_unit_ready() routine in the SCSI 
core to set a new flag when no medium is present.  The sd driver is 
changed to use this new flag for reporting -ENOMEDIUM in from the 
sd_media_changed method.



Signed-off-by: Alan Stern <stern@rowland.harvard.edu>

---

Index: usb-2.6/drivers/scsi/sd.c
===================================================================
--- usb-2.6.orig/drivers/scsi/sd.c
+++ usb-2.6/drivers/scsi/sd.c
@@ -744,9 +744,8 @@ static int sd_media_changed(struct gendi
 	 * and we will figure it out later once the drive is
 	 * available again.
 	 */
-	if (retval)
+	if (retval || sdp->medium_not_present)
 		 goto not_present;
-
 	/*
 	 * For removable scsi disk we have to recognise the presence
 	 * of a disk in the drive. This is kept in the struct scsi_disk
@@ -761,7 +760,7 @@ static int sd_media_changed(struct gendi
 
 not_present:
 	set_media_not_present(sdkp);
-	return 1;
+	return -ENOMEDIUM;
 }
 
 static int sd_sync_cache(struct scsi_device *sdp)
Index: usb-2.6/include/scsi/scsi_device.h
===================================================================
--- usb-2.6.orig/include/scsi/scsi_device.h
+++ usb-2.6/include/scsi/scsi_device.h
@@ -92,6 +92,7 @@ struct scsi_device {
 	unsigned writeable:1;
 	unsigned removable:1;
 	unsigned changed:1;	/* Data invalid due to media change */
+	unsigned medium_not_present:1;	/* Set by scsi_test_unit_ready() */
 	unsigned busy:1;	/* Used to prevent races */
 	unsigned lockable:1;	/* Able to prevent media removal */
 	unsigned locked:1;      /* Media removal disabled */
Index: usb-2.6/drivers/scsi/scsi_lib.c
===================================================================
--- usb-2.6.orig/drivers/scsi/scsi_lib.c
+++ usb-2.6/drivers/scsi/scsi_lib.c
@@ -2026,6 +2026,7 @@ scsi_test_unit_ready(struct scsi_device 
 	result = scsi_execute_req(sdev, cmd, DMA_NONE, NULL, 0, &sshdr,
 				  timeout, retries);
 
+	sdev->medium_not_present = 0;
 	if ((driver_byte(result) & DRIVER_SENSE) && sdev->removable) {
 
 		if ((scsi_sense_valid(&sshdr)) &&
@@ -2033,6 +2034,9 @@ scsi_test_unit_ready(struct scsi_device 
 		     (sshdr.sense_key == NOT_READY))) {
 			sdev->changed = 1;
 			result = 0;
+
+			if (sshdr.asc == 0x3A)
+				sdev->medium_not_present = 1;
 		}
 	}
 	return result;

Re: [PATCH 2/3] SCSI core and sd: early detection of medium not present

[James Bottomley]

On Tue, 2006-06-06 at 11:31 -0400, Alan Stern wrote:
> This patch (as695) changes the scsi_test_unit_ready() routine in the
> SCSI 
> core to set a new flag when no medium is present.  The sd driver is 
> changed to use this new flag for reporting -ENOMEDIUM in from the 
> sd_media_changed method.

This would appear to be duplicating the struct scsi_disk media_present
flag.  Moving the media_present flag from scsi_disk to scsi_device may
make a bit of sense long term ... however, there's also dupication with
the sr driver and the cdrom layer now (that stores media change at the
cdrom level), so is there an argument why it's better in scsi_device
than scsi_disk?

James

Re: [PATCH 2/3] SCSI core and sd: early detection of medium not present

[Alan Stern]

On Thu, 8 Jun 2006, James Bottomley wrote:

> On Tue, 2006-06-06 at 11:31 -0400, Alan Stern wrote:
> > This patch (as695) changes the scsi_test_unit_ready() routine in the
> > SCSI 
> > core to set a new flag when no medium is present.  The sd driver is 
> > changed to use this new flag for reporting -ENOMEDIUM in from the 
> > sd_media_changed method.
> 
> This would appear to be duplicating the struct scsi_disk media_present
> flag.

Correct, although the semantics of the two flags aren't exactly the same.

>  Moving the media_present flag from scsi_disk to scsi_device may
> make a bit of sense long term ... however, there's also dupication with
> the sr driver and the cdrom layer now (that stores media change at the
> cdrom level), so is there an argument why it's better in scsi_device
> than scsi_disk?

I did it that way in the patch because it was the only simple choice.  The 
scsi_test_unit_ready() routine is part of the SCSI core and can be called 
for devices that aren't disks.  Hence any flag it sets cannot be part of 
the scsi_disk structure.

In principle the information could be conveyed in the return value from 
scsi_test_unit_ready() rather than in a static flag.  But the routine has 
several callers and I didn't want to change all of them to recognize a 
-ENOMEDIUM return code.  Now in the long run, perhaps that would be a good 
thing to do.  Or perhaps moving the flag to scsi_device would be better, I 
don't know...

Ultimately this boils down to how you want to represent "No medium 
present" in the SCSI core.  What do you think is the bets way?

Alan Stern

Re: [PATCH 2/3] SCSI core and sd: early detection of medium not present

[James Bottomley]

On Fri, 2006-06-09 at 10:13 -0400, Alan Stern wrote:
> I did it that way in the patch because it was the only simple choice.  The 
> scsi_test_unit_ready() routine is part of the SCSI core and can be called 
> for devices that aren't disks.  Hence any flag it sets cannot be part of 
> the scsi_disk structure.

The slightly more complex choice that would be to extend
scsi_test_unit_ready() to allow the sending of a sense header pointer.
Then any user could use the sense return data for setting local
flags ... and thus, they could be kept local.

> In principle the information could be conveyed in the return value from 
> scsi_test_unit_ready() rather than in a static flag.  But the routine has 
> several callers and I didn't want to change all of them to recognize a 
> -ENOMEDIUM return code.  Now in the long run, perhaps that would be a good 
> thing to do.  Or perhaps moving the flag to scsi_device would be better, I 
> don't know...
> 
> Ultimately this boils down to how you want to represent "No medium 
> present" in the SCSI core.  What do you think is the bets way?

Well ... that's where I think we follow the CD people, since they're the
ones who have this occurring the most often.

James

Re: [PATCH 2/3] SCSI core and sd: early detection of medium not present

[Alan Stern]

On Fri, 9 Jun 2006, James Bottomley wrote:

> > Ultimately this boils down to how you want to represent "No medium 
> > present" in the SCSI core.  What do you think is the bets way?
> 
> Well ... that's where I think we follow the CD people, since they're the
> ones who have this occurring the most often.

Okay, I'll wait for Jens to take a look and make a recommendation.

BTW, the stimulus for this work came from a user with a USB flash card
reader.  The reader implements 4 different LUNs (for the different sorts
of flash technology) and hald polls each LUN every 2 seconds.  Lack of
early detection for no-medium-present means that each poll ends up
generating 8 SCSI commands; hence 16 commands get sent per second.  Not
only is this aesthetically displeasing, on this user's system some
low-quality USB hardware was chewing up a high percentage of the total PCI
bandwidth because of the frequent requests.  Poorly-selected timeouts in
the USB host controller driver contributed to the problem as well.  The
final result was that on an unloaded system doing nothing at all, the CPU
load was nevertheless up to 40%!

Changing hardware and updating the USB host driver helped the user, but I 
still think it's a good idea to eliminate the useless commands.

Alan Stern