2002-02-13 00:25:26

by J.A. Magallon

[permalink] [raw]
Subject: new aic7xxx 6.2.5 extra hunks

Hi,

I was trying the new 6.2.5 version of AIC driver from Justin Gibss, and
as always it includes changes to the generic SCSI layer.
Are this really needed ????? (some look just cosmetic if -> switch...)

Hunks follow:

--- linux/drivers/scsi/scsi_error.c.orig
+++ linux/drivers/scsi/scsi_error.c
@@ -1009,13 +1009,7 @@
SCpnt->flags &= ~IS_RESETTING;
goto maybe_retry;
}
- /*
- * Examine the sense data to figure out how to proceed from here.
- * If there is no sense data, we will be forced into the error
- * handler thread, where we get to examine the thing in a lot more
- * detail.
- */
- return scsi_check_sense(SCpnt);
+ return SUCCESS;
default:
return FAILED;
}
--- linux/drivers/scsi/sd.c.orig
+++ linux/drivers/scsi/sd.c
@@ -595,6 +595,7 @@
int this_count = SCpnt->bufflen >> 9;
int good_sectors = (result == 0 ? this_count : 0);
int block_sectors = 1;
+ long error_sector;

SCSI_LOG_HLCOMPLETE(1, sd_devname(DEVICE_NR(SCpnt->request.rq_dev), nbuff));

@@ -611,10 +612,11 @@
*/

/* An error occurred */
- if (driver_byte(result) != 0) {
- /* Sense data is valid */
- if (SCpnt->sense_buffer[0] == 0xF0 && SCpnt->sense_buffer[2] == MEDIUM_ERROR) {
- long error_sector = (SCpnt->sense_buffer[3] << 24) |
+ if (driver_byte(result) != 0 && /* An error occured */
+ SCpnt->sense_buffer[0] == 0xF0) { /* Sense data is valid */
+ switch (SCpnt->sense_buffer[2]) {
+ case MEDIUM_ERROR:
+ error_sector = (SCpnt->sense_buffer[3] << 24) |
(SCpnt->sense_buffer[4] << 16) |
(SCpnt->sense_buffer[5] << 8) |
SCpnt->sense_buffer[6];
@@ -647,13 +649,30 @@
good_sectors = error_sector - SCpnt->request.sector;
if (good_sectors < 0 || good_sectors >= this_count)
good_sectors = 0;
- }
- if (SCpnt->sense_buffer[2] == ILLEGAL_REQUEST) {
+ break;
+
+ case RECOVERED_ERROR:
+ /*
+ * An error occured, but it recovered. Inform the
+ * user, but make sure that it's not treated as a
+ * hard error.
+ */
+ print_sense("sd", SCpnt);
+ result = 0;
+ SCpnt->sense_buffer[0] = 0x0;
+ good_sectors = this_count;
+ break;
+
+ case ILLEGAL_REQUEST:
if (SCpnt->device->ten == 1) {
if (SCpnt->cmnd[0] == READ_10 ||
SCpnt->cmnd[0] == WRITE_10)
SCpnt->device->ten = 0;
}
+ break;
+
+ default:
+ break;
}
}
/*
--- linux/drivers/scsi/sr.c.orig
+++ linux/drivers/scsi/sr.c
@@ -198,6 +198,7 @@
int good_sectors = (result == 0 ? this_count : 0);
int block_sectors = 0;
int device_nr = DEVICE_NR(SCpnt->request.rq_dev);
+ long error_sector;

#ifdef DEBUG
printk("sr.c done: %x %p\n", result, SCpnt->request.bh->b_data);
@@ -210,33 +211,52 @@


if (driver_byte(result) != 0 && /* An error occurred */
- SCpnt->sense_buffer[0] == 0xF0 && /* Sense data is valid */
- (SCpnt->sense_buffer[2] == MEDIUM_ERROR ||
- SCpnt->sense_buffer[2] == VOLUME_OVERFLOW ||
- SCpnt->sense_buffer[2] == ILLEGAL_REQUEST)) {
- long error_sector = (SCpnt->sense_buffer[3] << 24) |
- (SCpnt->sense_buffer[4] << 16) |
- (SCpnt->sense_buffer[5] << 8) |
- SCpnt->sense_buffer[6];
- if (SCpnt->request.bh != NULL)
- block_sectors = SCpnt->request.bh->b_size >> 9;
- if (block_sectors < 4)
- block_sectors = 4;
- if (scsi_CDs[device_nr].device->sector_size == 2048)
- error_sector <<= 2;
- error_sector &= ~(block_sectors - 1);
- good_sectors = error_sector - SCpnt->request.sector;
- if (good_sectors < 0 || good_sectors >= this_count)
- good_sectors = 0;
- /*
- * The SCSI specification allows for the value returned by READ
- * CAPACITY to be up to 75 2K sectors past the last readable
- * block. Therefore, if we hit a medium error within the last
- * 75 2K sectors, we decrease the saved size value.
- */
- if ((error_sector >> 1) < sr_sizes[device_nr] &&
- scsi_CDs[device_nr].capacity - error_sector < 4 * 75)
- sr_sizes[device_nr] = error_sector >> 1;
+ SCpnt->sense_buffer[0] == 0xF0) { /* Sense data is valid */
+ switch (SCpnt->sense_buffer[2]) {
+ case MEDIUM_ERROR:
+ case VOLUME_OVERFLOW:
+ case ILLEGAL_REQUEST:
+ error_sector = (SCpnt->sense_buffer[3] << 24) |
+ (SCpnt->sense_buffer[4] << 16) |
+ (SCpnt->sense_buffer[5] << 8) |
+ SCpnt->sense_buffer[6];
+ if (SCpnt->request.bh != NULL)
+ block_sectors = SCpnt->request.bh->b_size >> 9;
+ if (block_sectors < 4)
+ block_sectors = 4;
+ if (scsi_CDs[device_nr].device->sector_size == 2048)
+ error_sector <<= 2;
+ error_sector &= ~(block_sectors - 1);
+ good_sectors = error_sector - SCpnt->request.sector;
+ if (good_sectors < 0 || good_sectors >= this_count)
+ good_sectors = 0;
+ /*
+ * The SCSI specification allows for the value returned
+ * by READ CAPACITY to be up to 75 2K sectors past the
+ * last readable block. Therefore, if we hit a medium
+ * error within the last 75 2K sectors, we decrease the
+ * saved size value.
+ */
+ if ((error_sector >> 1) < sr_sizes[device_nr] &&
+ scsi_CDs[device_nr].capacity - error_sector < 4 *75)
+ sr_sizes[device_nr] = error_sector >> 1;
+ break;
+
+ case RECOVERED_ERROR:
+ /*
+ * An error occured, but it recovered. Inform the
+ * user, but make sure that it's not treated as a
+ * hard error.
+ */
+ print_sense("sr", SCpnt);
+ result = 0;
+ SCpnt->sense_buffer[0] = 0x0;
+ good_sectors = this_count;
+ break;
+
+ default:
+ break;
+ }
}

/*

--
J.A. Magallon # Let the source be with you...
mailto:[email protected]
Mandrake Linux release 8.2 (Cooker) for i586
Linux werewolf 2.4.18-pre9-slb2 #1 SMP Tue Feb 12 20:16:13 CET 2002 i686


2002-02-13 01:15:16

by Douglas Gilbert

[permalink] [raw]
Subject: Re: new aic7xxx 6.2.5 extra hunks

J.A. Magallon wrote:

> I was trying the new 6.2.5 version of AIC driver from
> Justin Gibss, and as always it includes changes to the
> generic SCSI layer. Are this really needed ????? (some
> look just cosmetic if -> switch...)

Yes.

The patch to the mid level code is addressing the current
incorrect treatment of RECOVERED_ERROR. It is _not_ an
error but an advisory that a command (typically a READ)
need to be retried (or some other recovery technique) before
the data was obtained.

It is a "thought you might like to know your disk could
be failing (or why I took a long time to read that)" message
that the current scsi mid level interpretes as a command
failure.

While we are more than happy to have Justin Gibbs maintain
the aic7xxx driver (and Doug Ledford the older one), when
he tries to patch the mid-level he has to compete with
lots of other "cooks" (myself included). So his patches to
the mid level become harder to apply as time elapses from
when he did his patch generation.

Please persevere.

Doug Gilbert