[patch] SCSI: early detection of medium not present, updated

[patch] SCSI: early detection of medium not present, updated

[Andrew Morton]

Guys, I have this marked as needed-in-2.6.24?





From: Alan Stern <stern@rowland.harvard.edu>

Taken from http://bugzilla.kernel.org/show_bug.cgi?id=8904

An updated (by Albert, I assume) version of the fourteen-month-old patch here:

http://marc.info/?l=linux-kernel&m=115412002912837&w=2

Apparently fixes the bug described at
http://bugzilla.kernel.org/show_bug.cgi?id=8904

Needs some TLC.  Perhaps urgently.

Cc: Albert Lee <albertcc@tw.ibm.com>
Cc: Alan Stern <stern@rowland.harvard.edu>
Cc: James Bottomley <James.Bottomley@steeleye.com>
Cc: Tejun Heo <htejun@gmail.com>
Cc: Jens Axboe <jens.axboe@oracle.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
---

 drivers/scsi/scsi_ioctl.c  |    2 +-
 drivers/scsi/scsi_lib.c    |   20 ++++++++++++++++++--
 drivers/scsi/sd.c          |    2 +-
 drivers/scsi/sr.c          |   15 +++++++++------
 include/scsi/scsi_device.h |    2 +-
 5 files changed, 30 insertions(+), 11 deletions(-)

diff -puN drivers/scsi/scsi_ioctl.c~scsi-early-detection-of-medium-not-present-updated drivers/scsi/scsi_ioctl.c
--- a/drivers/scsi/scsi_ioctl.c~scsi-early-detection-of-medium-not-present-updated
+++ a/drivers/scsi/scsi_ioctl.c
@@ -244,7 +244,7 @@ int scsi_ioctl(struct scsi_device *sdev,
 		return scsi_set_medium_removal(sdev, SCSI_REMOVAL_ALLOW);
 	case SCSI_IOCTL_TEST_UNIT_READY:
 		return scsi_test_unit_ready(sdev, IOCTL_NORMAL_TIMEOUT,
-					    NORMAL_RETRIES);
+					    NORMAL_RETRIES, NULL);
 	case SCSI_IOCTL_START_UNIT:
 		scsi_cmd[0] = START_STOP;
 		scsi_cmd[1] = 0;
diff -puN drivers/scsi/scsi_lib.c~scsi-early-detection-of-medium-not-present-updated drivers/scsi/scsi_lib.c
--- a/drivers/scsi/scsi_lib.c~scsi-early-detection-of-medium-not-present-updated
+++ a/drivers/scsi/scsi_lib.c
@@ -2010,15 +2010,26 @@ scsi_mode_sense(struct scsi_device *sdev
 }
 EXPORT_SYMBOL(scsi_mode_sense);
 
+/**
+ *	scsi_test_unit_ready - test if unit is ready
+ *	@sdev:	scsi device to change the state of.
+ *	@timeout: command timeout
+ *	@retries: number of retries before failing
+ *	@media_maybe_present: 1 if media maybe present or not.
+ *		              0 if media not present.
+ *
+ *	Returns zero if unsuccessful or an error if TUR failed.
+ **/
 int
-scsi_test_unit_ready(struct scsi_device *sdev, int timeout, int retries)
+scsi_test_unit_ready(struct scsi_device *sdev, int timeout, int retries, int *media_maybe_present)
 {
 	char cmd[] = {
 		TEST_UNIT_READY, 0, 0, 0, 0, 0,
 	};
 	struct scsi_sense_hdr sshdr;
 	int result;
-	
+	int maybe_present = 1;
+
 	result = scsi_execute_req(sdev, cmd, DMA_NONE, NULL, 0, &sshdr,
 				  timeout, retries);
 
@@ -2027,10 +2038,15 @@ scsi_test_unit_ready(struct scsi_device 
 		if ((scsi_sense_valid(&sshdr)) &&
 		    ((sshdr.sense_key == UNIT_ATTENTION) ||
 		     (sshdr.sense_key == NOT_READY))) {
+			if (sshdr.asc == 0x3A)
+				maybe_present = 0;
 			sdev->changed = 1;
 			result = 0;
 		}
 	}
+
+	if (media_maybe_present)
+		*media_maybe_present = maybe_present;
 	return result;
 }
 EXPORT_SYMBOL(scsi_test_unit_ready);
diff -puN drivers/scsi/sd.c~scsi-early-detection-of-medium-not-present-updated drivers/scsi/sd.c
--- a/drivers/scsi/sd.c~scsi-early-detection-of-medium-not-present-updated
+++ a/drivers/scsi/sd.c
@@ -767,7 +767,7 @@ static int sd_media_changed(struct gendi
 	retval = -ENODEV;
 
 	if (scsi_block_when_processing_errors(sdp))
-		retval = scsi_test_unit_ready(sdp, SD_TIMEOUT, SD_MAX_RETRIES);
+		retval = scsi_test_unit_ready(sdp, SD_TIMEOUT, SD_MAX_RETRIES, NULL);
 
 	/*
 	 * Unable to test, unit probably not ready.   This usually
diff -puN drivers/scsi/sr.c~scsi-early-detection-of-medium-not-present-updated drivers/scsi/sr.c
--- a/drivers/scsi/sr.c~scsi-early-detection-of-medium-not-present-updated
+++ a/drivers/scsi/sr.c
@@ -179,18 +179,21 @@ static int sr_media_change(struct cdrom_
 {
 	struct scsi_cd *cd = cdi->handle;
 	int retval;
+	int media_maybe_present;
 
 	if (CDSL_CURRENT != slot) {
 		/* no changer support */
 		return -EINVAL;
 	}
 
-	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,
-		 * and we will figure it out later once the drive is
-		 * available again.  */
+	retval = scsi_test_unit_ready(cd->device, SR_TIMEOUT, MAX_RETRIES,
+				      &media_maybe_present);
+	if (retval || !media_maybe_present) {
+		/* Media not present or unable to test, unit probably not
+		 * ready. This usually means there is no disc in the drive.
+		 * Mark as changed, and we will figure it out later once
+		 * the drive is available again.
+		 */
 		cd->device->changed = 1;
 		/* This will force a flush, if called from check_disk_change */
 		retval = 1;
diff -puN include/scsi/scsi_device.h~scsi-early-detection-of-medium-not-present-updated include/scsi/scsi_device.h
--- a/include/scsi/scsi_device.h~scsi-early-detection-of-medium-not-present-updated
+++ a/include/scsi/scsi_device.h
@@ -292,7 +292,7 @@ extern int scsi_mode_select(struct scsi_
 			    struct scsi_mode_data *data,
 			    struct scsi_sense_hdr *);
 extern int scsi_test_unit_ready(struct scsi_device *sdev, int timeout,
-				int retries);
+				int retries, int *media_maybe_present);
 extern int scsi_device_set_state(struct scsi_device *sdev,
 				 enum scsi_device_state state);
 extern struct scsi_event *sdev_evt_alloc(enum scsi_device_event evt_type,
_

Re: [patch] SCSI: early detection of medium not present, updated

[Tejun Heo]

Andrew Morton wrote:
> From: Alan Stern <stern@rowland.harvard.edu>
> 
> Taken from http://bugzilla.kernel.org/show_bug.cgi?id=8904
> 
> An updated (by Albert, I assume) version of the fourteen-month-old patch here:
> 
> http://marc.info/?l=linux-kernel&m=115412002912837&w=2
> 
> Apparently fixes the bug described at
> http://bugzilla.kernel.org/show_bug.cgi?id=8904
> 
> Needs some TLC.  Perhaps urgently.
> 
> Cc: Albert Lee <albertcc@tw.ibm.com>
> Cc: Alan Stern <stern@rowland.harvard.edu>
> Cc: James Bottomley <James.Bottomley@steeleye.com>
> Cc: Tejun Heo <htejun@gmail.com>
> Cc: Jens Axboe <jens.axboe@oracle.com>
> Signed-off-by: Andrew Morton <akpm@linux-foundation.org>

I think I did it last time but...

Acked-by: Tejun Heo <htejun@gmail.com>

-- 
tejun

Re: [patch] SCSI: early detection of medium not present, updated

[Alan Stern]

On Thu, 29 Nov 2007, Andrew Morton wrote:

> Guys, I have this marked as needed-in-2.6.24?

> From: Alan Stern <stern@rowland.harvard.edu>
> 
> Taken from http://bugzilla.kernel.org/show_bug.cgi?id=8904
> 
> An updated (by Albert, I assume) version of the fourteen-month-old patch here:
> 
> http://marc.info/?l=linux-kernel&m=115412002912837&w=2
> 
> Apparently fixes the bug described at
> http://bugzilla.kernel.org/show_bug.cgi?id=8904
> 
> Needs some TLC.  Perhaps urgently.

As far as I'm concerned there's no harm in leaving it until 2.6.25; my
original version was only a slight performance optimization to prevent
excessive querying when the answer is known beforehand.  In fact this
version of the patch includes only a portion of my original submission.  
The part in sd.c that exits the retry loop early when there's no medium
is missing.

But if this really does fix bug #8904 then it should go in sooner.  
The question is, does it fix the bug?  The report mentions only a small 
amount of testing.

Alan Stern

Re: [patch] SCSI: early detection of medium not present, updated

[James Bottomley]

On Thu, 2007-11-29 at 15:18 -0800, Andrew Morton wrote:
> Guys, I have this marked as needed-in-2.6.24?

Could you wait on this a bit ... since it's such an old bug.  The code
in question needs to be reworked (as the comment says).  I think the
best rework is to have the caller pass in an optional struct
scsi_sense_header into scsi_test_unit_ready() instead of the hacky
media_may_be_present, so the one place that needs this will be able to
interpret the sense codes correctly.

I can do this when I get back home ... unfortunately, I'm in Aswan today
at an Internet café ... I'll be back in Cairo tomorrow, but my US system
seems to have fallen off the internet, so I might not be able to test
any patches I come up with.  Worst case, I'll be back in Chicago on
Thursday.

James


-
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] SCSI: early detection of medium not present, updated

[Alan Stern]

On Sat, 1 Dec 2007, James Bottomley wrote:

> On Thu, 2007-11-29 at 15:18 -0800, Andrew Morton wrote:
> > Guys, I have this marked as needed-in-2.6.24?
> 
> Could you wait on this a bit ... since it's such an old bug.  The code
> in question needs to be reworked (as the comment says).  I think the
> best rework is to have the caller pass in an optional struct
> scsi_sense_header into scsi_test_unit_ready() instead of the hacky
> media_may_be_present, so the one place that needs this will be able to
> interpret the sense codes correctly.
> 
> I can do this when I get back home ... unfortunately, I'm in Aswan today
> at an Internet café ... I'll be back in Cairo tomorrow, but my US system
> seems to have fallen off the internet, so I might not be able to test
> any patches I come up with.  Worst case, I'll be back in Chicago on
> Thursday.

That's fine with me.  But the optional scsi_sense_header should be 
added in two places, not just one: There's the medium-detection code in 
sr.c already in this patch, and there's the medium-detection code in 
sd.c (it was changed in my original patch but not in this one).

Alan Stern

-
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] SCSI: early detection of medium not present, updated

[James Bottomley]

On Sat, 2007-12-01 at 16:25 +0200, James Bottomley wrote:
> On Thu, 2007-11-29 at 15:18 -0800, Andrew Morton wrote:
> > Guys, I have this marked as needed-in-2.6.24?
> 
> Could you wait on this a bit ... since it's such an old bug.  The code
> in question needs to be reworked (as the comment says).  I think the
> best rework is to have the caller pass in an optional struct
> scsi_sense_header into scsi_test_unit_ready() instead of the hacky
> media_may_be_present, so the one place that needs this will be able to
> interpret the sense codes correctly.
> 
> I can do this when I get back home ... unfortunately, I'm in Aswan today
> at an Internet café ... I'll be back in Cairo tomorrow, but my US system
> seems to have fallen off the internet, so I might not be able to test
> any patches I come up with.  Worst case, I'll be back in Chicago on
> Thursday.

Actually, the night train is a good place to do coding.  I know this
compiles, but could someone check it out?  It's the form I think we
should do, since the ASC/ASCQ codes for esoteric conditions which we
might eventually check for are different for CDs and removable discs.

James

diff --git a/drivers/scsi/scsi_ioctl.c b/drivers/scsi/scsi_ioctl.c
index 83e1447..28b19ef 100644
--- a/drivers/scsi/scsi_ioctl.c
+++ b/drivers/scsi/scsi_ioctl.c
@@ -244,7 +244,7 @@ int scsi_ioctl(struct scsi_device *sdev, int cmd, void __user *arg)
 		return scsi_set_medium_removal(sdev, SCSI_REMOVAL_ALLOW);
 	case SCSI_IOCTL_TEST_UNIT_READY:
 		return scsi_test_unit_ready(sdev, IOCTL_NORMAL_TIMEOUT,
-					    NORMAL_RETRIES);
+					    NORMAL_RETRIES, NULL);
 	case SCSI_IOCTL_START_UNIT:
 		scsi_cmd[0] = START_STOP;
 		scsi_cmd[1] = 0;
diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index 1148c40..823df63 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -2016,27 +2016,57 @@ scsi_mode_sense(struct scsi_device *sdev, int dbd, int modepage,
 }
 EXPORT_SYMBOL(scsi_mode_sense);
 
+/**
+ *	scsi_test_unit_ready - test if unit is ready
+ *	@sdev:	scsi device to change the state of.
+ *	@timeout: command timeout
+ *	@retries: number of retries before failing
+ *	@sshdr_external: Optional pointer to struct scsi_sense_hdr for
+ *		returning sense. Make sure that this is cleared before passing
+ *		in.
+ *
+ *	Returns zero if unsuccessful or an error if TUR failed.  For
+ *	removable media, a return of NOT_READY or UNIT_ATTENTION is
+ *	translated to success, with the ->changed flag updated.
+ **/
 int
-scsi_test_unit_ready(struct scsi_device *sdev, int timeout, int retries)
+scsi_test_unit_ready(struct scsi_device *sdev, int timeout, int retries,
+		     struct scsi_sense_hdr *sshdr_external)
 {
 	char cmd[] = {
 		TEST_UNIT_READY, 0, 0, 0, 0, 0,
 	};
-	struct scsi_sense_hdr sshdr;
+	struct scsi_sense_hdr *sshdr;
 	int result;
-	
-	result = scsi_execute_req(sdev, cmd, DMA_NONE, NULL, 0, &sshdr,
-				  timeout, retries);
+
+	if (!sshdr_external)
+		sshdr = kzalloc(sizeof(*sshdr), GFP_KERNEL);
+	else
+		sshdr = sshdr_external;
+
+	/* try to eat the UNIT_ATTENTION if there are enough retries */
+	do {
+		result = scsi_execute_req(sdev, cmd, DMA_NONE, NULL, 0, sshdr,
+					  timeout, retries);
+	} while ((driver_byte(result) & DRIVER_SENSE) &&
+		 sshdr && sshdr->sense_key == UNIT_ATTENTION &&
+		 --retries);
+
+	if (!sshdr)
+		/* could not allocate sense buffer, so can't process it */
+		return result;
 
 	if ((driver_byte(result) & DRIVER_SENSE) && sdev->removable) {
 
-		if ((scsi_sense_valid(&sshdr)) &&
-		    ((sshdr.sense_key == UNIT_ATTENTION) ||
-		     (sshdr.sense_key == NOT_READY))) {
+		if ((scsi_sense_valid(sshdr)) &&
+		    ((sshdr->sense_key == UNIT_ATTENTION) ||
+		     (sshdr->sense_key == NOT_READY))) {
 			sdev->changed = 1;
 			result = 0;
 		}
 	}
+	if (!sshdr_external)
+		kfree(sshdr);
 	return result;
 }
 EXPORT_SYMBOL(scsi_test_unit_ready);
diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index 18343a6..212f6bc 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -736,6 +736,7 @@ static int sd_media_changed(struct gendisk *disk)
 {
 	struct scsi_disk *sdkp = scsi_disk(disk);
 	struct scsi_device *sdp = sdkp->device;
+	struct scsi_sense_hdr *sshdr = NULL;
 	int retval;
 
 	SCSI_LOG_HLQUEUE(3, sd_printk(KERN_INFO, sdkp, "sd_media_changed\n"));
@@ -766,8 +767,11 @@ static int sd_media_changed(struct gendisk *disk)
 	 */
 	retval = -ENODEV;
 
-	if (scsi_block_when_processing_errors(sdp))
-		retval = scsi_test_unit_ready(sdp, SD_TIMEOUT, SD_MAX_RETRIES);
+	if (scsi_block_when_processing_errors(sdp)) {
+		sshdr  = kzalloc(sizeof(*sshdr), GFP_KERNEL);
+		retval = scsi_test_unit_ready(sdp, SD_TIMEOUT, SD_MAX_RETRIES,
+					      sshdr);
+	}
 
 	/*
 	 * Unable to test, unit probably not ready.   This usually
@@ -775,7 +779,9 @@ static int sd_media_changed(struct gendisk *disk)
 	 * and we will figure it out later once the drive is
 	 * available again.
 	 */
-	if (retval) {
+	if (retval || (scsi_sense_valid(sshdr) &&
+		       /* 0x3a is medium not present */
+		       sshdr->asc == 0x3a)) {
 		set_media_not_present(sdkp);
 		retval = 1;
 		goto out;
@@ -794,6 +800,7 @@ out:
 	if (retval != sdkp->previous_state)
 		sdev_evt_send_simple(sdp, SDEV_EVT_MEDIA_CHANGE, GFP_KERNEL);
 	sdkp->previous_state = retval;
+	kfree(sshdr);
 	return retval;
 }
 
diff --git a/drivers/scsi/sr.c b/drivers/scsi/sr.c
index 7702681..896be4a 100644
--- a/drivers/scsi/sr.c
+++ b/drivers/scsi/sr.c
@@ -179,18 +179,24 @@ static int sr_media_change(struct cdrom_device_info *cdi, int slot)
 {
 	struct scsi_cd *cd = cdi->handle;
 	int retval;
+	struct scsi_sense_hdr *sshdr;
 
 	if (CDSL_CURRENT != slot) {
 		/* no changer support */
 		return -EINVAL;
 	}
 
-	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,
-		 * and we will figure it out later once the drive is
-		 * available again.  */
+	sshdr =  kzalloc(sizeof(*sshdr), GFP_KERNEL);
+	retval = scsi_test_unit_ready(cd->device, SR_TIMEOUT, MAX_RETRIES,
+				      sshdr);
+	if (retval || (scsi_sense_valid(sshdr) &&
+		       /* 0x3a is medium not present */
+		       sshdr->asc == 0x3a)) {
+		/* Media not present or unable to test, unit probably not
+		 * ready. This usually means there is no disc in the drive.
+		 * Mark as changed, and we will figure it out later once
+		 * the drive is available again.
+		 */
 		cd->device->changed = 1;
 		/* This will force a flush, if called from check_disk_change */
 		retval = 1;
@@ -213,6 +219,7 @@ out:
 		sdev_evt_send_simple(cd->device, SDEV_EVT_MEDIA_CHANGE,
 				     GFP_KERNEL);
 	cd->previous_state = retval;
+	kfree(sshdr);
 
 	return retval;
 }
diff --git a/include/scsi/scsi_device.h b/include/scsi/scsi_device.h
index c7bccf9..b1861c7 100644
--- a/include/scsi/scsi_device.h
+++ b/include/scsi/scsi_device.h
@@ -292,7 +292,7 @@ extern int scsi_mode_select(struct scsi_device *sdev, int pf, int sp,
 			    struct scsi_mode_data *data,
 			    struct scsi_sense_hdr *);
 extern int scsi_test_unit_ready(struct scsi_device *sdev, int timeout,
-				int retries);
+				int retries, struct scsi_sense_hdr *sshdr);
 extern int scsi_device_set_state(struct scsi_device *sdev,
 				 enum scsi_device_state state);
 extern struct scsi_event *sdev_evt_alloc(enum scsi_device_event evt_type,


-
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] SCSI: early detection of medium not present, updated

[Alan Stern]

On Sun, 2 Dec 2007, James Bottomley wrote:

> Actually, the night train is a good place to do coding.  I know this
> compiles, but could someone check it out?  It's the form I think we
> should do, since the ASC/ASCQ codes for esoteric conditions which we
> might eventually check for are different for CDs and removable discs.

This part is puzzling:

> +	/* try to eat the UNIT_ATTENTION if there are enough retries */
> +	do {
> +		result = scsi_execute_req(sdev, cmd, DMA_NONE, NULL, 0, sshdr,
> +					  timeout, retries);
> +	} while ((driver_byte(result) & DRIVER_SENSE) &&
> +		 sshdr && sshdr->sense_key == UNIT_ATTENTION &&
> +		 --retries);

For one thing, you've using retries to control both an outer loop and 
an inner loop -- meaning the total number of attempts could be on the 
order of retries**2.

For another, why do you want to swallow a UNIT_ATTENTION?  Shouldn't 
that be up to the code calling scsi_test_unit_ready()?

Alan Stern

Re: [patch] SCSI: early detection of medium not present, updated

[James Bottomley]

On Sun, 2007-12-02 at 15:02 -0500, Alan Stern wrote:
> On Sun, 2 Dec 2007, James Bottomley wrote:
> 
> > Actually, the night train is a good place to do coding.  I know this
> > compiles, but could someone check it out?  It's the form I think we
> > should do, since the ASC/ASCQ codes for esoteric conditions which we
> > might eventually check for are different for CDs and removable discs.
> 
> This part is puzzling:
> 
> > +	/* try to eat the UNIT_ATTENTION if there are enough retries */
> > +	do {
> > +		result = scsi_execute_req(sdev, cmd, DMA_NONE, NULL, 0, sshdr,
> > +					  timeout, retries);
> > +	} while ((driver_byte(result) & DRIVER_SENSE) &&
> > +		 sshdr && sshdr->sense_key == UNIT_ATTENTION &&
> > +		 --retries);
> 
> For one thing, you've using retries to control both an outer loop and 
> an inner loop -- meaning the total number of attempts could be on the 
> order of retries**2.

Sort of; however, there should only really be one cc/ua before the TUR
goes through.  Also, TUR produces almost no conditions that would be
retryable in scsi_decide_disposition() and the scsi_execute sends it
down with REQ_FAILFAST anyway, so it's really not used much.

> For another, why do you want to swallow a UNIT_ATTENTION?  Shouldn't 
> that be up to the code calling scsi_test_unit_ready()?

No, that's a bug in the old code.  UA doesn't necessarily mean there's a
media change.  It could be asserted for a whole host of reasons.  Until
we get proper UA signalling, we need to resend the TUR so we get the
correct media status and discard the UA (Even if the device is asserting
UA for media change reasons, it will still give the correct sense code
response to the following TUR).

James

Re: [patch] SCSI: early detection of medium not present, updated

[Alan Stern]

On Wed, 5 Dec 2007, James Bottomley wrote:

> > For one thing, you've using retries to control both an outer loop and 
> > an inner loop -- meaning the total number of attempts could be on the 
> > order of retries**2.
> 
> Sort of; however, there should only really be one cc/ua before the TUR
> goes through.  Also, TUR produces almost no conditions that would be
> retryable in scsi_decide_disposition() and the scsi_execute sends it
> down with REQ_FAILFAST anyway, so it's really not used much.
> 
> > For another, why do you want to swallow a UNIT_ATTENTION?  Shouldn't 
> > that be up to the code calling scsi_test_unit_ready()?
> 
> No, that's a bug in the old code.  UA doesn't necessarily mean there's a
> media change.  It could be asserted for a whole host of reasons.  Until
> we get proper UA signalling, we need to resend the TUR so we get the
> correct media status and discard the UA (Even if the device is asserting
> UA for media change reasons, it will still give the correct sense code
> response to the following TUR).

Well then, the patch appears good.  I haven't been able to test it 
because it's based against a source tree I haven't got.  Can you 
provide an equivalent patch against 2.6.24-rc4?

Alan Stern

[PATCH] SCSI: early detection of medium not present, updated

[Alan Stern]

This patch (as757) is meant to apply on top of the
scsi-core-and-sd-early-detection-of-medium-not-present patch currently in
2.6.18-rc2-mm1.  It updates the way the scsi_test_unit_ready() routine
reports medium not present, using the technique recommended by James
Bottomley.

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

---

If James would prefer, I could submit this in a different form: first 
revert the earlier SCSI patch and then add the new stuff from this one.

In a brief conversation during OLS, Jens said that he saw no problem with
allowing the block layer to accept -ENOMEDIUM as a return code from the 
media_changed method.  The patch is already in -mm, named
block-layer-early-detection-of-medium-not-present.

In principle cdrom drivers could use a similar technique to avoid calling 
the revalidate method when no disc is loaded, but that would require more 
far-reaching changes.  For now it's easier to confine this to disk drives.


Index: 2.6.18-rc2-mm1/drivers/scsi/sd.c
===================================================================
--- 2.6.18-rc2-mm1.orig/drivers/scsi/sd.c
+++ 2.6.18-rc2-mm1/drivers/scsi/sd.c
@@ -733,6 +733,7 @@ static int sd_media_changed(struct gendi
 	struct scsi_disk *sdkp = scsi_disk(disk);
 	struct scsi_device *sdp = sdkp->device;
 	int retval;
+	int media_not_present = 0;
 
 	SCSI_LOG_HLQUEUE(3, printk("sd_media_changed: disk=%s\n",
 						disk->disk_name));
@@ -760,7 +761,8 @@ static int sd_media_changed(struct gendi
 	 */
 	retval = -ENODEV;
 	if (scsi_block_when_processing_errors(sdp))
-		retval = scsi_test_unit_ready(sdp, SD_TIMEOUT, SD_MAX_RETRIES);
+		retval = scsi_test_unit_ready(sdp, SD_TIMEOUT, SD_MAX_RETRIES,
+				&media_not_present);
 
 	/*
 	 * Unable to test, unit probably not ready.   This usually
@@ -768,7 +770,7 @@ static int sd_media_changed(struct gendi
 	 * and we will figure it out later once the drive is
 	 * available again.
 	 */
-	if (retval || sdp->medium_not_present)
+	if (retval || media_not_present)
 		 goto not_present;
 	/*
 	 * For removable scsi disk we have to recognise the presence
Index: 2.6.18-rc2-mm1/include/scsi/scsi_device.h
===================================================================
--- 2.6.18-rc2-mm1.orig/include/scsi/scsi_device.h
+++ 2.6.18-rc2-mm1/include/scsi/scsi_device.h
@@ -92,7 +92,6 @@ 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 */
@@ -268,7 +267,7 @@ extern int scsi_mode_select(struct scsi_
 			    struct scsi_mode_data *data,
 			    struct scsi_sense_hdr *);
 extern int scsi_test_unit_ready(struct scsi_device *sdev, int timeout,
-				int retries);
+				int retries, int *media_not_present);
 extern int scsi_device_set_state(struct scsi_device *sdev,
 				 enum scsi_device_state state);
 extern int scsi_device_quiesce(struct scsi_device *sdev);
Index: 2.6.18-rc2-mm1/drivers/scsi/sr.c
===================================================================
--- 2.6.18-rc2-mm1.orig/drivers/scsi/sr.c
+++ 2.6.18-rc2-mm1/drivers/scsi/sr.c
@@ -178,14 +178,16 @@ static int sr_media_change(struct cdrom_
 {
 	struct scsi_cd *cd = cdi->handle;
 	int retval;
+	int media_not_present = 0;
 
 	if (CDSL_CURRENT != slot) {
 		/* no changer support */
 		return -EINVAL;
 	}
 
-	retval = scsi_test_unit_ready(cd->device, SR_TIMEOUT, MAX_RETRIES);
-	if (retval) {
+	retval = scsi_test_unit_ready(cd->device, SR_TIMEOUT, MAX_RETRIES,
+			&media_not_present);
+	if (retval || media_not_present) {
 		/* Unable to test, unit probably not ready.  This usually
 		 * means there is no disc in the drive.  Mark as changed,
 		 * and we will figure it out later once the drive is
Index: 2.6.18-rc2-mm1/drivers/scsi/scsi_lib.c
===================================================================
--- 2.6.18-rc2-mm1.orig/drivers/scsi/scsi_lib.c
+++ 2.6.18-rc2-mm1/drivers/scsi/scsi_lib.c
@@ -1867,7 +1867,8 @@ scsi_mode_sense(struct scsi_device *sdev
 EXPORT_SYMBOL(scsi_mode_sense);
 
 int
-scsi_test_unit_ready(struct scsi_device *sdev, int timeout, int retries)
+scsi_test_unit_ready(struct scsi_device *sdev, int timeout, int retries,
+		int *media_not_present)
 {
 	char cmd[] = {
 		TEST_UNIT_READY, 0, 0, 0, 0, 0,
@@ -1878,7 +1879,6 @@ 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)) &&
@@ -1887,8 +1887,8 @@ scsi_test_unit_ready(struct scsi_device 
 			sdev->changed = 1;
 			result = 0;
 
-			if (sshdr.asc == 0x3A)
-				sdev->medium_not_present = 1;
+			if (sshdr.asc == 0x3A && media_not_present != NULL)
+				*media_not_present = 1;
 		}
 	}
 	return result;
Index: 2.6.18-rc2-mm1/drivers/scsi/scsi_ioctl.c
===================================================================
--- 2.6.18-rc2-mm1.orig/drivers/scsi/scsi_ioctl.c
+++ 2.6.18-rc2-mm1/drivers/scsi/scsi_ioctl.c
@@ -242,7 +242,7 @@ int scsi_ioctl(struct scsi_device *sdev,
 		return scsi_set_medium_removal(sdev, SCSI_REMOVAL_ALLOW);
 	case SCSI_IOCTL_TEST_UNIT_READY:
 		return scsi_test_unit_ready(sdev, IOCTL_NORMAL_TIMEOUT,
-					    NORMAL_RETRIES);
+					    NORMAL_RETRIES, NULL);
 	case SCSI_IOCTL_START_UNIT:
 		scsi_cmd[0] = START_STOP;
 		scsi_cmd[1] = 0;