Patch review

[M. Warner Losh]

What do people think of this?  It is very useful when trying to
recover a CD/DVD that's been scratched with ddrescue which handles all
the retry logic.  This patch is modeled after the
kern.cam.da.retry_count sysctl and friends.

This makes ddrescue about between 5x and 20x faster.  The reason for
the large delta is that it can now 'stream' past errors rather than
doing a lot of reseeking when reading a large block size.  I've also
observed fewer hangs in one of the cheap DVD drives I tried this with,
likely due to fewer forced retries frying its brain...  Of course, all
of this was with kern.cam.cd.retry_count = 0.  I've not changed the
default behavior.

Comments?

Warner

P.S.  I have a recollection that Scott Long sent me something akin to
this, but I couldn't find it so I just recreated this trivial patch
from scratch, cheating off of scsi_da.c.

Index: cam/scsi/scsi_cd.c
===================================================================
--- cam/scsi/scsi_cd.c	(revision 186807)
+++ cam/scsi/scsi_cd.c	(working copy)
@@ -292,6 +292,9 @@
 
 PERIPHDRIVER_DECLARE(cd, cddriver);
 
+#ifndef	CD_DEFAULT_RETRY
+#define	CD_DEFAULT_RETRY	4
+#endif
 #ifndef CHANGER_MIN_BUSY_SECONDS
 #define CHANGER_MIN_BUSY_SECONDS	5
 #endif
@@ -299,11 +302,15 @@
 #define CHANGER_MAX_BUSY_SECONDS	15
 #endif
 
+static int cd_retry_count = CD_DEFAULT_RETRY;
 static int changer_min_busy_seconds = CHANGER_MIN_BUSY_SECONDS;
 static int changer_max_busy_seconds = CHANGER_MAX_BUSY_SECONDS;
 
 SYSCTL_NODE(_kern_cam, OID_AUTO, cd, CTLFLAG_RD, 0, "CAM CDROM driver");
 SYSCTL_NODE(_kern_cam_cd, OID_AUTO, changer, CTLFLAG_RD, 0, "CD Changer");
+SYSCTL_INT(_kern_cam_cd, OID_AUTO, retry_count, CTLFLAG_RW,
+           &cd_retry_count, 0, "Normal I/O retry count");
+TUNABLE_INT("kern.cam.cd.retry_count", &cd_retry_count);
 SYSCTL_INT(_kern_cam_cd_changer, OID_AUTO, min_busy_seconds, CTLFLAG_RW,
 	   &changer_min_busy_seconds, 0, "Minimum changer scheduling quantum");
 TUNABLE_INT("kern.cam.cd.changer.min_busy_seconds", &changer_min_busy_seconds);
@@ -1454,7 +1461,7 @@
 			devstat_start_transaction_bio(softc->disk->d_devstat, bp);
 
 			scsi_read_write(&start_ccb->csio,
-					/*retries*/4,
+					/*retries*/cd_retry_count,
 					/* cbfcnp */ cddone,
 					MSG_SIMPLE_Q_TAG,
 					/* read */bp->bio_cmd == BIO_READ,