[PATCH] Fix cdrom error handling in 2.4

From: Erik Andersen (andersen_at_codepoet.org)
Date: 08/23/03

  • Next message: Brown, Len: "RE: [patch] noapic should depend on ioapic config not local"
    Date:	Fri, 22 Aug 2003 19:26:09 -0600
    To: Marcelo Tosatti <marcelo@conectiva.com.br>
    
    

    In both 2.4 and in 2.6, error handling for bad cdrom media is
    wrong. And it is my fault I'm afraid, since I botched an earlier
    fix for the problem by putting the fix in the wrong spot.

    My kids have a "Jumpstart Toddlers" cd they have long since
    completely killed, which makes a great test disc. Without this
    fix, the best time projection I can get for completing a dd type
    sector copy is about 2 years... Most of that is spent thrashing
    about in kernel space trying to re-read sectors we already know
    are not correctable.... After the fix, I was able to rip a copy
    the CD (or rather muddle through it getting lots of EIO errors)
    in about 15 minutes.

    Attached is the fix for 2.4.x, which just moves the MEDIUM_ERROR
    handler a bit earlier. I have also included a minor error path
    cleanup from the 2.6.x driver,

     -Erik

    --
    Erik B. Andersen             http://codepoet-consulting.com/
    --This message was written using 73% post-consumer electrons--
    --- linux/drivers/ide/ide-cd.c.orig	2003-08-22 18:48:07.000000000 -0600
    +++ linux/drivers/ide/ide-cd.c	2003-08-22 18:49:07.000000000 -0600
    @@ -765,6 +765,8 @@
     		if ((stat & ERR_STAT) != 0)
     			cdrom_queue_request_sense(drive, wait, pc->sense, pc);
     	} else if (blk_fs_request(rq)) {
    +		int do_end_request = 0;
    +
     		/* Handle errors from READ and WRITE requests. */
     
     		if (sense_key == NOT_READY) {
    @@ -773,7 +775,7 @@
     
     			/* Fail the request. */
     			printk ("%s: tray open\n", drive->name);
    -			cdrom_end_request(drive, 0);
    +			do_end_request = 1;
     		} else if (sense_key == UNIT_ATTENTION) {
     			/* Media change. */
     			cdrom_saw_media_change (drive);
    @@ -782,28 +784,31 @@
     			   But be sure to give up if we've retried
     			   too many times. */
     			if (++rq->errors > ERROR_MAX)
    -				cdrom_end_request(drive, 0);
    +				do_end_request = 1;
     		} else if (sense_key == ILLEGAL_REQUEST ||
     			   sense_key == DATA_PROTECT) {
     			/* No point in retrying after an illegal
     			   request or data protect error.*/
     			ide_dump_status (drive, "command error", stat);
    -			cdrom_end_request(drive, 0);
    +			do_end_request = 1;
    +		} else if (sense_key == MEDIUM_ERROR) {
    +			/* No point in re-trying a zillion times on a bad 
    +			 * sector...  If we got here the error is not correctable */
    +			ide_dump_status (drive, "media error (bad sector)", stat);
    +			do_end_request = 1;
     		} else if ((err & ~ABRT_ERR) != 0) {
     			/* Go to the default handler
     			   for other errors. */
     			*startstop = DRIVER(drive)->error(drive, "cdrom_decode_status", stat);
     			return 1;
    -		} else if (sense_key == MEDIUM_ERROR) {
    -			/* No point in re-trying a zillion times on a bad 
    -			 * sector...  If we got here the error is not correctable */
    -			ide_dump_status (drive, "media error (bad sector)", stat);
    -			cdrom_end_request(drive, 0);
     		} else if ((++rq->errors > ERROR_MAX)) {
     			/* We've racked up too many retries.  Abort. */
    -			cdrom_end_request(drive, 0);
    +			do_end_request = 1;
     		}
     
    +		if (do_end_request)
    +			cdrom_end_request(drive, 0);
    +
     		/* If we got a CHECK_CONDITION status,
     		   queue a request sense command. */
     		if ((stat & ERR_STAT) != 0)
    -
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at  http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at  http://www.tux.org/lkml/
    

  • Next message: Brown, Len: "RE: [patch] noapic should depend on ioapic config not local"

    Relevant Pages

    • [GIT PULL] block changes for 2.6.28
      ... This is the bulk of the block layer changes for 2.6.28. ... Fix up comments about matching flags between bio and rq ... Add 'discard' request handling ... Support 'discard sectors' operation in translation layer support core ...
      (Linux-Kernel)
    • [GIT PULL] first block round for 2.6.31
      ... cciss: use schedule_timeout_interruptible ... mg_disk: fix dependency on libata ... Add blk_make_request(), takes bio, returns a request ... swim3: dequeue in-flight request ...
      (Linux-Kernel)
    • Re: data corruption using raid0+lvm2+jfs with 2.6.0-test3
      ... >> There is a fix for this at ... > I think it does honour the max_sectors restriction, ... > allow a request as big as one chunk, but it will allow such a requests ... send the line "unsubscribe linux-kernel" in ...
      (Linux-Kernel)
    • Re: [PATCH 2.6.15-rc6] block: Make CDROMEJECT more robust
      ... it can't be a write request if there's no data attached. ... >> The bug was in the SCSI layer, and James already has the fix integrated ... >> Do this in the eject tool, if it's required for some devices. ... So the medium removal command does require write permission on the ...
      (Linux-Kernel)
    • Re: Notices
      ... An unchecked checkbox is not sent in $_REQUEST and if you do something with $_REQUESTyou'll get a notice. ... I'm a recovering perl programmer so conciseness is still important. ... It's a bit tedious at times to fix harmless missing index notices when you just want to test something. ...
      (comp.lang.php)