Commit dbe217af3be08346f4b1abb885c2d9ec29c98fac

Authored by Alan Cox
Committed by Linus Torvalds
1 parent 8e3a67a992

[PATCH] IDE CD end-of media error fix

This is a patch from Alan that fixes a real ide-cd.c regression causing
bogus "Media Check" failures for perfectly valid Fedora install ISOs, on
certain CD-ROM drives.

This is a forward port to 2.6.16 (from RHEL) of the minimal changes for the
end of media problem.  It may not be sufficient for some controllers
(promise notably) and it does not touch the locking so the error path
locking is as horked as in mainstream.

From: Ingo Molnar <mingo@elte.hu>

I have ported the patch to 2.6.17-rc4 and tested it by provoking
end-of-media IO errors with an unaligned ISO image.  Unlike the vanilla
kernel, the patched kernel interpreted the error condition correctly with
512 byte granularity:

 hdc: command error: status=0x51 { DriveReady SeekComplete Error }
 hdc: command error: error=0x54 { AbortedCommand LastFailedSense=0x05 }
 ide: failed opcode was: unknown
 ATAPI device hdc:
   Error: Illegal request -- (Sense key=0x05)
   Illegal mode for this track or incompatible medium -- (asc=0x64, ascq=0x00)
   The failed "Read 10" packet command was:
   "28 00 00 04 fb 78 00 00 06 00 00 00 00 00 00 00 "
 end_request: I/O error, dev hdc, sector 1306080
 Buffer I/O error on device hdc, logical block 163260
 Buffer I/O error on device hdc, logical block 163261
 Buffer I/O error on device hdc, logical block 163262

the unpatched kernel produces an incorrect error dump:

 hdc: command error: status=0x51 { DriveReady SeekComplete Error }
 hdc: command error: error=0x54 { AbortedCommand LastFailedSense=0x05 }
 ide: failed opcode was: unknown
 end_request: I/O error, dev hdc, sector 1306080
 Buffer I/O error on device hdc, logical block 163260
 hdc: command error: status=0x51 { DriveReady SeekComplete Error }
 hdc: command error: error=0x54 { AbortedCommand LastFailedSense=0x05 }
 ide: failed opcode was: unknown
 end_request: I/O error, dev hdc, sector 1306088
 Buffer I/O error on device hdc, logical block 163261
 hdc: command error: status=0x51 { DriveReady SeekComplete Error }
 hdc: command error: error=0x54 { AbortedCommand LastFailedSense=0x05 }
 ide: failed opcode was: unknown
 end_request: I/O error, dev hdc, sector 1306096
 Buffer I/O error on device hdc, logical block 163262

I do not have the right type of CD-ROM drive to reproduce the end-of-media
data corruption bug myself, but this same patch in RHEL solved it.

Signed-off-by: Ingo Molnar <mingo@elte.hu>
Cc: Alan Cox <alan@lxorguk.ukuu.org.uk>
Cc: Bartlomiej Zolnierkiewicz <B.Zolnierkiewicz@elka.pw.edu.pl>
Cc: Jens Axboe <axboe@suse.de>
Cc: Matt Mackall <mpm@selenic.com>
Signed-off-by: Andrew Morton <akpm@osdl.org>
Signed-off-by: Linus Torvalds <torvalds@osdl.org>

Showing 3 changed files with 152 additions and 28 deletions Side-by-side Diff

drivers/ide/ide-cd.c
... ... @@ -395,7 +395,8 @@
395 395 * we cannot reliably check if drive can auto-close
396 396 */
397 397 if (rq->cmd[0] == GPCMD_START_STOP_UNIT && sense->asc == 0x24)
398   - log = 0;
  398 + break;
  399 + log = 1;
399 400 break;
400 401 case UNIT_ATTENTION:
401 402 /*
... ... @@ -417,6 +418,11 @@
417 418 struct request *failed_command,
418 419 struct request_sense *sense)
419 420 {
  421 + unsigned long sector;
  422 + unsigned long bio_sectors;
  423 + unsigned long valid;
  424 + struct cdrom_info *info = drive->driver_data;
  425 +
420 426 if (!cdrom_log_sense(drive, failed_command, sense))
421 427 return;
422 428  
... ... @@ -429,6 +435,37 @@
429 435 if (sense->sense_key == 0x05 && sense->asc == 0x24)
430 436 return;
431 437  
  438 + if (sense->error_code == 0x70) { /* Current Error */
  439 + switch(sense->sense_key) {
  440 + case MEDIUM_ERROR:
  441 + case VOLUME_OVERFLOW:
  442 + case ILLEGAL_REQUEST:
  443 + if (!sense->valid)
  444 + break;
  445 + if (failed_command == NULL ||
  446 + !blk_fs_request(failed_command))
  447 + break;
  448 + sector = (sense->information[0] << 24) |
  449 + (sense->information[1] << 16) |
  450 + (sense->information[2] << 8) |
  451 + (sense->information[3]);
  452 +
  453 + bio_sectors = bio_sectors(failed_command->bio);
  454 + if (bio_sectors < 4)
  455 + bio_sectors = 4;
  456 + if (drive->queue->hardsect_size == 2048)
  457 + sector <<= 2; /* Device sector size is 2K */
  458 + sector &= ~(bio_sectors -1);
  459 + valid = (sector - failed_command->sector) << 9;
  460 +
  461 + if (valid < 0)
  462 + valid = 0;
  463 + if (sector < get_capacity(info->disk) &&
  464 + drive->probed_capacity - sector < 4 * 75) {
  465 + set_capacity(info->disk, sector);
  466 + }
  467 + }
  468 + }
432 469 #if VERBOSE_IDE_CD_ERRORS
433 470 {
434 471 int i;
435 472  
... ... @@ -609,17 +646,23 @@
609 646 sense = failed->sense;
610 647 failed->sense_len = rq->sense_len;
611 648 }
612   -
  649 + cdrom_analyze_sense_data(drive, failed, sense);
613 650 /*
614 651 * now end failed request
615 652 */
616   - spin_lock_irqsave(&ide_lock, flags);
617   - end_that_request_chunk(failed, 0, failed->data_len);
618   - end_that_request_last(failed, 0);
619   - spin_unlock_irqrestore(&ide_lock, flags);
620   - }
621   -
622   - cdrom_analyze_sense_data(drive, failed, sense);
  653 + if (blk_fs_request(failed)) {
  654 + if (ide_end_dequeued_request(drive, failed, 0,
  655 + failed->hard_nr_sectors))
  656 + BUG();
  657 + } else {
  658 + spin_lock_irqsave(&ide_lock, flags);
  659 + end_that_request_chunk(failed, 0,
  660 + failed->data_len);
  661 + end_that_request_last(failed, 0);
  662 + spin_unlock_irqrestore(&ide_lock, flags);
  663 + }
  664 + } else
  665 + cdrom_analyze_sense_data(drive, NULL, sense);
623 666 }
624 667  
625 668 if (!rq->current_nr_sectors && blk_fs_request(rq))
... ... @@ -633,6 +676,13 @@
633 676 ide_end_request(drive, uptodate, nsectors);
634 677 }
635 678  
  679 +static void ide_dump_status_no_sense(ide_drive_t *drive, const char *msg, u8 stat)
  680 +{
  681 + if (stat & 0x80)
  682 + return;
  683 + ide_dump_status(drive, msg, stat);
  684 +}
  685 +
636 686 /* Returns 0 if the request should be continued.
637 687 Returns 1 if the request was ended. */
638 688 static int cdrom_decode_status(ide_drive_t *drive, int good_stat, int *stat_ret)
639 689  
640 690  
... ... @@ -761,16 +811,16 @@
761 811 sense_key == DATA_PROTECT) {
762 812 /* No point in retrying after an illegal
763 813 request or data protect error.*/
764   - ide_dump_status (drive, "command error", stat);
  814 + ide_dump_status_no_sense (drive, "command error", stat);
765 815 do_end_request = 1;
766 816 } else if (sense_key == MEDIUM_ERROR) {
767 817 /* No point in re-trying a zillion times on a bad
768 818 * sector... If we got here the error is not correctable */
769   - ide_dump_status (drive, "media error (bad sector)", stat);
  819 + ide_dump_status_no_sense (drive, "media error (bad sector)", stat);
770 820 do_end_request = 1;
771 821 } else if (sense_key == BLANK_CHECK) {
772 822 /* Disk appears blank ?? */
773   - ide_dump_status (drive, "media error (blank)", stat);
  823 + ide_dump_status_no_sense (drive, "media error (blank)", stat);
774 824 do_end_request = 1;
775 825 } else if ((err & ~ABRT_ERR) != 0) {
776 826 /* Go to the default handler
777 827  
... ... @@ -782,13 +832,27 @@
782 832 do_end_request = 1;
783 833 }
784 834  
785   - if (do_end_request)
786   - cdrom_end_request(drive, 0);
  835 + /* End a request through request sense analysis when we have
  836 + sense data. We need this in order to perform end of media
  837 + processing */
787 838  
788   - /* If we got a CHECK_CONDITION status,
789   - queue a request sense command. */
790   - if ((stat & ERR_STAT) != 0)
791   - cdrom_queue_request_sense(drive, NULL, NULL);
  839 + if (do_end_request) {
  840 + if (stat & ERR_STAT) {
  841 + unsigned long flags;
  842 + spin_lock_irqsave(&ide_lock, flags);
  843 + blkdev_dequeue_request(rq);
  844 + HWGROUP(drive)->rq = NULL;
  845 + spin_unlock_irqrestore(&ide_lock, flags);
  846 +
  847 + cdrom_queue_request_sense(drive, rq->sense, rq);
  848 + } else
  849 + cdrom_end_request(drive, 0);
  850 + } else {
  851 + /* If we got a CHECK_CONDITION status,
  852 + queue a request sense command. */
  853 + if (stat & ERR_STAT)
  854 + cdrom_queue_request_sense(drive, NULL, NULL);
  855 + }
792 856 } else {
793 857 blk_dump_rq_flags(rq, "ide-cd: bad rq");
794 858 cdrom_end_request(drive, 0);
... ... @@ -1491,8 +1555,7 @@
1491 1555 }
1492 1556  
1493 1557  
1494   -static
1495   -int cdrom_queue_packet_command(ide_drive_t *drive, struct request *rq)
  1558 +static int cdrom_queue_packet_command(ide_drive_t *drive, struct request *rq)
1496 1559 {
1497 1560 struct request_sense sense;
1498 1561 int retries = 10;
... ... @@ -2220,6 +2283,9 @@
2220 2283 toc->capacity = 0x1fffff;
2221 2284  
2222 2285 set_capacity(info->disk, toc->capacity * sectors_per_frame);
  2286 + /* Save a private copy of te TOC capacity for error handling */
  2287 + drive->probed_capacity = toc->capacity * sectors_per_frame;
  2288 +
2223 2289 blk_queue_hardsect_size(drive->queue,
2224 2290 sectors_per_frame << SECTOR_BITS);
2225 2291  
... ... @@ -2342,6 +2408,7 @@
2342 2408 if (!stat && (last_written > toc->capacity)) {
2343 2409 toc->capacity = last_written;
2344 2410 set_capacity(info->disk, toc->capacity * sectors_per_frame);
  2411 + drive->probed_capacity = toc->capacity * sectors_per_frame;
2345 2412 }
2346 2413  
2347 2414 /* Remember that we've read this stuff. */
2348 2415  
... ... @@ -2698,14 +2765,11 @@
2698 2765 * any other way to detect this...
2699 2766 */
2700 2767 if (sense.sense_key == NOT_READY) {
2701   - if (sense.asc == 0x3a) {
2702   - if (sense.ascq == 1)
2703   - return CDS_NO_DISC;
2704   - else if (sense.ascq == 0 || sense.ascq == 2)
2705   - return CDS_TRAY_OPEN;
2706   - }
  2768 + if (sense.asc == 0x3a && sense.ascq == 1)
  2769 + return CDS_NO_DISC;
  2770 + else
  2771 + return CDS_TRAY_OPEN;
2707 2772 }
2708   -
2709 2773 return CDS_DRIVE_NOT_READY;
2710 2774 }
2711 2775  
drivers/ide/ide-io.c
... ... @@ -223,6 +223,63 @@
223 223 }
224 224  
225 225 /**
  226 + * ide_end_dequeued_request - complete an IDE I/O
  227 + * @drive: IDE device for the I/O
  228 + * @uptodate:
  229 + * @nr_sectors: number of sectors completed
  230 + *
  231 + * Complete an I/O that is no longer on the request queue. This
  232 + * typically occurs when we pull the request and issue a REQUEST_SENSE.
  233 + * We must still finish the old request but we must not tamper with the
  234 + * queue in the meantime.
  235 + *
  236 + * NOTE: This path does not handle barrier, but barrier is not supported
  237 + * on ide-cd anyway.
  238 + */
  239 +
  240 +int ide_end_dequeued_request(ide_drive_t *drive, struct request *rq,
  241 + int uptodate, int nr_sectors)
  242 +{
  243 + unsigned long flags;
  244 + int ret = 1;
  245 +
  246 + spin_lock_irqsave(&ide_lock, flags);
  247 +
  248 + BUG_ON(!(rq->flags & REQ_STARTED));
  249 +
  250 + /*
  251 + * if failfast is set on a request, override number of sectors and
  252 + * complete the whole request right now
  253 + */
  254 + if (blk_noretry_request(rq) && end_io_error(uptodate))
  255 + nr_sectors = rq->hard_nr_sectors;
  256 +
  257 + if (!blk_fs_request(rq) && end_io_error(uptodate) && !rq->errors)
  258 + rq->errors = -EIO;
  259 +
  260 + /*
  261 + * decide whether to reenable DMA -- 3 is a random magic for now,
  262 + * if we DMA timeout more than 3 times, just stay in PIO
  263 + */
  264 + if (drive->state == DMA_PIO_RETRY && drive->retry_pio <= 3) {
  265 + drive->state = 0;
  266 + HWGROUP(drive)->hwif->ide_dma_on(drive);
  267 + }
  268 +
  269 + if (!end_that_request_first(rq, uptodate, nr_sectors)) {
  270 + add_disk_randomness(rq->rq_disk);
  271 + if (blk_rq_tagged(rq))
  272 + blk_queue_end_tag(drive->queue, rq);
  273 + end_that_request_last(rq, uptodate);
  274 + ret = 0;
  275 + }
  276 + spin_unlock_irqrestore(&ide_lock, flags);
  277 + return ret;
  278 +}
  279 +EXPORT_SYMBOL_GPL(ide_end_dequeued_request);
  280 +
  281 +
  282 +/**
226 283 * ide_complete_pm_request - end the current Power Management request
227 284 * @drive: target drive
228 285 * @rq: request
... ... @@ -630,6 +630,7 @@
630 630 unsigned int usage; /* current "open()" count for drive */
631 631 unsigned int failures; /* current failure count */
632 632 unsigned int max_failures; /* maximum allowed failure count */
  633 + u64 probed_capacity;/* initial reported media capacity (ide-cd only currently) */
633 634  
634 635 u64 capacity64; /* total number of sectors */
635 636  
... ... @@ -1005,6 +1006,8 @@
1005 1006 extern int noautodma;
1006 1007  
1007 1008 extern int ide_end_request (ide_drive_t *drive, int uptodate, int nrsecs);
  1009 +int ide_end_dequeued_request(ide_drive_t *drive, struct request *rq,
  1010 + int uptodate, int nr_sectors);
1008 1011  
1009 1012 /*
1010 1013 * This is used on exit from the driver to designate the next irq handler