Alan Stern wrote:
> James:
>
> This revised patch (as572b) improves the code in sr.c that detects where
> read/write errors occurred and moves it to a separate (but inline) routine
> for greater clarity.
>
> The original code contained a glaring mistake: testing
> (SCpnt->sense_buffer[0] & 0x90) to see if the Information field in the
> sense data is valid. The Valid bit is 0x80; the test against 0x90 can
> never fail, thanks to an earlier test: (SCpnt->sense_buffer[0] & 0x7f) ==
> 0x70).
>
> The patch is careful to check that the error sector is within the range of
> blocks that were supposed to be transferred, and it rounds the reported
> number of bytes transferred down to a multiple of the block layer's block
> size.
>
> The major enhancement is that the patch uses the Residue to calculate the
> error sector, if the sense data doesn't already provide it. This helps
> with a drive I use. Since the Residue isn't always reported correctly,
> the patch is careful to check that it has a reasonable value: somewhere
> strictly between 0 and the total transfer length.
Alan,
In include/scsi/scsi_eh.h there are several helper functions
to aid processing SCSI errors. This includes SCSI sense data
descriptor format (which won't be needed for DVD/HD/BD for
some time with a (2**32 * 2048) byte maximum using existing
fixed sense data format). However there is
scsi_get_sense_info_fld() to fetch the info field.
sd, st and sg have been converted to use these helpers,
where appropriate.
MMC-4 does not mention that the valid bit needs to
be set on a MEDIUM/HARDWARE error and I have seen
real life examples of this. [So it's poorly defined
if one gets a medium error on lba 0.] You may also like to
consider deferred errors which can occur according to
MMC-4.
Doug Gilbert
> Signed-off-by: Alan Stern <stern@xxxxxxxxxxxxxxxxxxx>
>
> ---
>
> Index: usb-2.6/drivers/scsi/sr.c
> ===================================================================
> --- usb-2.6.orig/drivers/scsi/sr.c
> +++ usb-2.6/drivers/scsi/sr.c
> @@ -203,7 +203,44 @@ int sr_media_change(struct cdrom_device_
> }
> return retval;
> }
> -
> +
> +/*
> + * Use the information from a failed command to compute how much data
> + * was transferred successfully, and find the sector that caused the
> + * error. Be careful to avoid errors from numerical overflow.
> + */
> +static inline int calc_good_bytes(struct scsi_cmnd *SCpnt,
> + long *error_sector, int hw_blocksize,
> + int sectors_per_bio_block)
> +{
> + int req_bytes = SCpnt->bufflen;
> + unsigned int sectors = 0;
> +
> + if (SCpnt->sense_buffer[0] & 0x80) {
> + /* The Information field is valid; use it */
> + unsigned int start_block, error_block, blocks;
> +
> + start_block = (unsigned int) SCpnt->request->sector /
> + (hw_blocksize >> 9);
> + error_block = (SCpnt->sense_buffer[3] << 24) |
> + (SCpnt->sense_buffer[4] << 16) |
> + (SCpnt->sense_buffer[5] << 8) |
> + SCpnt->sense_buffer[6];
> + blocks = error_block - start_block;
> + if (blocks < req_bytes / hw_blocksize)
> + sectors = blocks * (hw_blocksize >> 9);
> + } else {
> + /* Fall back on the Residue */
> + if (SCpnt->resid > 0 && SCpnt->resid < req_bytes)
> + sectors = (req_bytes - SCpnt->resid) >> 9;
> + }
> + *error_sector = SCpnt->request->sector + sectors;
> +
> + /* Round the amount to a multiple of the block layer's block size */
> + sectors &= ~(sectors_per_bio_block - 1);
> + return sectors << 9;
> +}
> +
> /*
> * rw_intr is the interrupt routine for the device driver.
> *
> @@ -235,25 +272,17 @@ static void rw_intr(struct scsi_cmnd * S
> case MEDIUM_ERROR:
> case VOLUME_OVERFLOW:
> case ILLEGAL_REQUEST:
> - if (!(SCpnt->sense_buffer[0] & 0x90))
> - break;
> if (!blk_fs_request(SCpnt->request))
> break;
> - error_sector = (SCpnt->sense_buffer[3] << 24) |
> - (SCpnt->sense_buffer[4] << 16) |
> - (SCpnt->sense_buffer[5] << 8) |
> - SCpnt->sense_buffer[6];
> if (SCpnt->request->bio != NULL)
> block_sectors =
> bio_sectors(SCpnt->request->bio);
> if (block_sectors < 4)
> block_sectors = 4;
> - if (cd->device->sector_size == 2048)
> - error_sector <<= 2;
> - error_sector &= ~(block_sectors - 1);
> - good_bytes = (error_sector - SCpnt->request->sector) << 9;
> - if (good_bytes < 0 || good_bytes >= this_count)
> - good_bytes = 0;
> +
> + good_bytes = calc_good_bytes(SCpnt, &error_sector,
> + cd->device->sector_size,
> + block_sectors);
> /*
> * The SCSI specification allows for the value
> * returned by READ CAPACITY to be up to 75 2K
>
> -
> : send the line "unsubscribe linux-scsi" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
-
: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
