[PATCH] cdrom: longer timeout for "Read Track Info" command

[PATCH] cdrom: longer timeout for "Read Track Info" command

[Jeremy Higdon]

I have a DVD combo drive and a CD in which the
"READ TRACK INFORMATION" command (implemented in the
cdrom_get_track_info() function) takes about 7 seconds to run.
The current implementation of cdrom_get_track_info() uses the
default timeout of 5 seconds.  So here's a patch that increases
the timeout from 5 to 15 seconds.

The drive in question is a TSSTcorpCD/DVDW SN-S082D, and I have
a Silicon Image 680A adapter, in case that's of interest.

signed-off-by: <jeremy@sgi.com>

diff -ur linux-2.6.20-rc3_ORIG/drivers/cdrom/cdrom.c linux-2.6.20-rc3/drivers/cdrom/cdrom.c
--- linux-2.6.20-rc3_ORIG/drivers/cdrom/cdrom.c	2006-12-31 16:53:20.000000000 -0800
+++ linux-2.6.20-rc3/drivers/cdrom/cdrom.c	2007-01-01 18:13:50.135173456 -0800
@@ -3094,6 +3094,7 @@
 	cgc.cmd[5] = track & 0xff;
 	cgc.cmd[8] = 8;
 	cgc.quiet = 1;
+	cgc.timeout = 15*HZ;
 
 	if ((ret = cdo->generic_packet(cdi, &cgc)))
 		return ret;

Re: [PATCH] cdrom: longer timeout for "Read Track Info" command

[Alan]

On Mon, 1 Jan 2007 18:36:24 -0800
Jeremy Higdon <jeremy@sgi.com> wrote:

> I have a DVD combo drive and a CD in which the
> "READ TRACK INFORMATION" command (implemented in the
> cdrom_get_track_info() function) takes about 7 seconds to run.
> The current implementation of cdrom_get_track_info() uses the
> default timeout of 5 seconds.  So here's a patch that increases
> the timeout from 5 to 15 seconds.
> 
> The drive in question is a TSSTcorpCD/DVDW SN-S082D, and I have
> a Silicon Image 680A adapter, in case that's of interest.
> 
> signed-off-by: <jeremy@sgi.com>

Please test with a seven second timeout rather than fifteen which is way
longer than anyone wants to wait. Seven is the magic value used by
another major vendor so ought to be right for all hardware 8)

Re: [PATCH] cdrom: longer timeout for "Read Track Info" command

[Jens Axboe]

On Tue, Jan 02 2007, Alan wrote:
> On Mon, 1 Jan 2007 18:36:24 -0800
> Jeremy Higdon <jeremy@sgi.com> wrote:
> 
> > I have a DVD combo drive and a CD in which the
> > "READ TRACK INFORMATION" command (implemented in the
> > cdrom_get_track_info() function) takes about 7 seconds to run.
> > The current implementation of cdrom_get_track_info() uses the
> > default timeout of 5 seconds.  So here's a patch that increases
> > the timeout from 5 to 15 seconds.
> > 
> > The drive in question is a TSSTcorpCD/DVDW SN-S082D, and I have
> > a Silicon Image 680A adapter, in case that's of interest.
> > 
> > signed-off-by: <jeremy@sgi.com>
> 
> Please test with a seven second timeout rather than fifteen which is way
> longer than anyone wants to wait. Seven is the magic value used by
> another major vendor so ought to be right for all hardware 8)

Yep, I suspect this patch is long overdue. Jeremy, is this enough to fix
it for you?

diff --git a/drivers/cdrom/cdrom.c b/drivers/cdrom/cdrom.c
index 66d028d..3105ddd 100644
--- a/drivers/cdrom/cdrom.c
+++ b/drivers/cdrom/cdrom.c
@@ -337,6 +337,12 @@ static const char *mrw_address_space[] = { "DMA", "GAA" };
 /* used in the audio ioctls */
 #define CHECKAUDIO if ((ret=check_for_audio_disc(cdi, cdo))) return ret
 
+/*
+ * Another popular OS uses 7 seconds as the hard timeout for default
+ * commands, so it is a good choice for us as well.
+ */
+#define CDROM_DEF_TIMEOUT	(7 * HZ)
+
 /* Not-exported routines. */
 static int open_for_data(struct cdrom_device_info * cdi);
 static int check_for_audio_disc(struct cdrom_device_info * cdi,
@@ -1528,7 +1534,7 @@ void init_cdrom_command(struct packet_command *cgc, void *buf, int len,
 	cgc->buffer = (char *) buf;
 	cgc->buflen = len;
 	cgc->data_direction = type;
-	cgc->timeout = 5*HZ;
+	cgc->timeout = CDROM_DEF_TIMEOUT;
 }
 
 /* DVD handling */

-- 
Jens Axboe

Re: [PATCH] cdrom: longer timeout for "Read Track Info" command

[Jeremy Higdon]

On Tue, Jan 02, 2007 at 02:50:53PM +0100, Jens Axboe wrote:
> Yep, I suspect this patch is long overdue. Jeremy, is this enough to fix
> it for you?

Yes, the 7 second timeout is fine.  It actually takes about 6.7 seconds.
I guess if "another popular OS" has a 7 second timeout that we won't find
multimedia devices out there that take longer than that.  :-)

My 15 seconds assumed that the observed case wasn't the worst case, but
it probably is.

This patch looks good.

Thanks

jeremy

> diff --git a/drivers/cdrom/cdrom.c b/drivers/cdrom/cdrom.c
> index 66d028d..3105ddd 100644
> --- a/drivers/cdrom/cdrom.c
> +++ b/drivers/cdrom/cdrom.c
> @@ -337,6 +337,12 @@ static const char *mrw_address_space[] = { "DMA", "GAA" };
>  /* used in the audio ioctls */
>  #define CHECKAUDIO if ((ret=check_for_audio_disc(cdi, cdo))) return ret
>  
> +/*
> + * Another popular OS uses 7 seconds as the hard timeout for default
> + * commands, so it is a good choice for us as well.
> + */
> +#define CDROM_DEF_TIMEOUT	(7 * HZ)
> +
>  /* Not-exported routines. */
>  static int open_for_data(struct cdrom_device_info * cdi);
>  static int check_for_audio_disc(struct cdrom_device_info * cdi,
> @@ -1528,7 +1534,7 @@ void init_cdrom_command(struct packet_command *cgc, void *buf, int len,
>  	cgc->buffer = (char *) buf;
>  	cgc->buflen = len;
>  	cgc->data_direction = type;
> -	cgc->timeout = 5*HZ;
> +	cgc->timeout = CDROM_DEF_TIMEOUT;
>  }
>  
>  /* DVD handling */