Re: [patch] SCSI: early detection of medium not present, updated
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 - 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
Re: [patch] SCSI: early detection of medium not present, updated
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
-
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
Re: [patch] SCSI: early detection of medium not present, updated
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)
*/
Re: [patch] SCSI: early detection of medium not present, updated
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 [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch] SCSI: early detection of medium not present, updated
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 [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[patch] SCSI: early detection of medium not present, updated
Guys, I have this marked as needed-in-2.6.24?
From: Alan Stern [EMAIL PROTECTED]
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-kernelm=115412002912837w=2
Apparently fixes the bug described at
http://bugzilla.kernel.org/show_bug.cgi?id=8904
Needs some TLC. Perhaps urgently.
Cc: Albert Lee [EMAIL PROTECTED]
Cc: Alan Stern [EMAIL PROTECTED]
Cc: James Bottomley [EMAIL PROTECTED]
Cc: Tejun Heo [EMAIL PROTECTED]
Cc: Jens Axboe [EMAIL PROTECTED]
Signed-off-by: Andrew Morton [EMAIL PROTECTED]
---
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 ||
Re: [patch] SCSI: early detection of medium not present, updated
Andrew Morton wrote: From: Alan Stern [EMAIL PROTECTED] 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-kernelm=115412002912837w=2 Apparently fixes the bug described at http://bugzilla.kernel.org/show_bug.cgi?id=8904 Needs some TLC. Perhaps urgently. Cc: Albert Lee [EMAIL PROTECTED] Cc: Alan Stern [EMAIL PROTECTED] Cc: James Bottomley [EMAIL PROTECTED] Cc: Tejun Heo [EMAIL PROTECTED] Cc: Jens Axboe [EMAIL PROTECTED] Signed-off-by: Andrew Morton [EMAIL PROTECTED] I think I did it last time but... Acked-by: Tejun Heo [EMAIL PROTECTED] -- tejun - 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
Re: [patch] SCSI: early detection of medium not present, updated
On Thu, 29 Nov 2007, Andrew Morton wrote: Guys, I have this marked as needed-in-2.6.24? From: Alan Stern [EMAIL PROTECTED] 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-kernelm=115412002912837w=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 - 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
