[PATCH] 2.4.20 drivers/cdrom/cdu31a.c

[PATCH] 2.4.20 drivers/cdrom/cdu31a.c

[Mauricio Martinez]

This patch fixes the kernel oops while trying to read a data CD. Thanks to
Brian Gerst and Faik Uygur for your suggestions.

It looks like the variable nblocks must be <= 4 so the read ahead buffer
size is not exceeded (which is the cause of the oops). Changing its value
doesn't seem to be the right way, but it makes the device work properly.

Feedback of any sort welcome.


--- linux-2.4.20/drivers/cdrom/cdu31a.c.orig	Thu Nov 28 15:53:12 2002
+++ linux-2.4.20/drivers/cdrom/cdu31a.c	Thu Feb  6 20:49:44 2003
@@ -1361,7 +1361,9 @@
 	res_reg[0] = 0;
 	res_reg[1] = 0;
 	*res_size = 0;
-	bytesleft = nblocks * 512;
+	/* Make sure that bytesleft doesn't exceed the buffer size */
+	if (nblocks > 4) nblocks = 4;
+	bytesleft = nblocks * 512;
 	offset = 0;

 	/* If the data in the read-ahead does not match the block offset,
@@ -1384,9 +1386,9 @@
 			       readahead_buffer + (2048 -
 						   readahead_dataleft),
 			       readahead_dataleft);
-			readahead_dataleft = 0;
 			bytesleft -= readahead_dataleft;
 			offset += readahead_dataleft;
+			readahead_dataleft = 0;
 		} else {
 			/* The readahead will fill the whole buffer, get the data
 			   and return. */

Re: [PATCH] 2.4.20 drivers/cdrom/cdu31a.c

[Corey Minyard]

[-- Attachment #1: Type: text/plain, Size: 1097 bytes --]

I don't guess anyone I've sent documentation to has taken up support of 
this drive.  It's amazing that any of these things are still running, 
since I think they stop manufacturing them in 1994.  I don't think I 
have a machine that it will even work in any more.  I guess they were 
well-built drives.

The change you are suggesting is probably not the best, you probably 
need to fix it higher up to do multiple requests if nsect is > 4.  I've 
attached a patch.  It compiles, but I obviously can't test it.

Looking through the code, there's obvious SMP problems and a few other 
things.  I have a new machine with an ISA slot (I think), I might try to 
plug it in.

-Corey

Mauricio Martinez wrote:

>This patch fixes the kernel oops while trying to read a data CD. Thanks to
>Brian Gerst and Faik Uygur for your suggestions.
>
>It looks like the variable nblocks must be <= 4 so the read ahead buffer
>size is not exceeded (which is the cause of the oops). Changing its value
>doesn't seem to be the right way, but it makes the device work properly.
>
>Feedback of any sort welcome.
>
>  
>

[-- Attachment #2: linux-cdu31a.diff --]
[-- Type: text/plain, Size: 2463 bytes --]

--- drivers/cdrom/cdu31a.c.old	Fri Feb  7 09:16:08 2003
+++ drivers/cdrom/cdu31a.c	Fri Feb  7 09:42:53 2003
@@ -1341,7 +1341,7 @@
 #endif
 }
 
-/* read data from the drive.  Note the nsect must be <= 4. */
+/* read data from the drive.  Note the nblocks must be <= 4. */
 static void
 read_data_block(char *buffer,
 		unsigned int block,
@@ -1364,7 +1364,7 @@
 	bytesleft = nblocks * 512;
 	offset = 0;
 
-	/* If the data in the read-ahead does not match the block offset,
+	/* If the data in the read ahead does not match the block offset,
 	   then fix things up. */
 	if (((block % 4) * 512) != ((2048 - readahead_dataleft) % 2048)) {
 		sony_next_block += block % 4;
@@ -1384,9 +1384,9 @@
 			       readahead_buffer + (2048 -
 						   readahead_dataleft),
 			       readahead_dataleft);
-			readahead_dataleft = 0;
 			bytesleft -= readahead_dataleft;
 			offset += readahead_dataleft;
+			readahead_dataleft = 0;
 		} else {
 			/* The readahead will fill the whole buffer, get the data
 			   and return. */
@@ -1533,7 +1533,7 @@
 
 /*
  * The OS calls this to perform a read or write operation to the drive.
- * Write obviously fail.  Reads to a read ahead of sony_buffer_size
+ * Writes obviously fail.  Reads to a read ahead of sony_buffer_size
  * bytes to help speed operations.  This especially helps since the OS
  * uses 1024 byte blocks and the drive uses 2048 byte blocks.  Since most
  * data access on a CD is done sequentially, this saves a lot of operations.
@@ -1546,6 +1546,7 @@
 	unsigned int res_size;
 	int num_retries;
 	unsigned long flags;
+	char *buffer;
 
 
 #if DEBUG
@@ -1616,6 +1617,7 @@
 
 		block = CURRENT->sector;
 		nblock = CURRENT->nr_sectors;
+		buffer = CURRENT->buffer;
 
 		if (!sony_toc_read) {
 			printk("CDU31A: TOC not read\n");
@@ -1695,8 +1697,17 @@
 				}
 			}
 
-			read_data_block(CURRENT->buffer, block, nblock,
-					res_reg, &res_size);
+			if (nblock >= 4) {
+				read_data_block(buffer, block, 4,
+						res_reg, &res_size);
+				nblock -= 4;
+				block += 4;
+				buffer += 4 * 512;
+			} else {
+				read_data_block(buffer, block, nblock,
+						res_reg, &res_size);
+				nblock = 0;
+			}
 			if (res_reg[0] == 0x20) {
 				if (num_retries > MAX_CDU31A_RETRIES) {
 					end_request(0);
@@ -1714,6 +1725,8 @@
 					     translate_error(res_reg[1]),
 					     block, nblock);
 				}
+				goto try_read_again;
+			} else if (nblock > 0) {
 				goto try_read_again;
 			} else {
 				end_request(1);

Re: [PATCH] 2.4.20 drivers/cdrom/cdu31a.c

[Mauricio Martinez]

Thanks for your reply. I guess there are still some drives like this
floating around. I can live without it, but it is good to use it in an old
486 as a jukebox and print server :)

Your patch makes much more sense that mine (I have no experience in Linux
driver development), and it makes the drive work *very well* (excellent
transfer rate and no system overload), but only if I remove the last hunk.

This last hunk tries to read again the data with 4 sectors less each time
(i.e. 16,14,12,...,4) which *i think* overloads the buffer leading to an
oops (and even a system reboot without warning!).

Hope this information helps.

-Mauricio


On Fri, 7 Feb 2003, Corey Minyard wrote:

> I don't guess anyone I've sent documentation to has taken up support of
> this drive.  It's amazing that any of these things are still running,
> since I think they stop manufacturing them in 1994.  I don't think I
> have a machine that it will even work in any more.  I guess they were
> well-built drives.
>
> The change you are suggesting is probably not the best, you probably
> need to fix it higher up to do multiple requests if nsect is > 4.  I've
> attached a patch.  It compiles, but I obviously can't test it.
>
> Looking through the code, there's obvious SMP problems and a few other
> things.  I have a new machine with an ISA slot (I think), I might try to
> plug it in.
>
> -Corey
>
> Mauricio Martinez wrote:
>
> >This patch fixes the kernel oops while trying to read a data CD. Thanks to
> >Brian Gerst and Faik Uygur for your suggestions.
> >
> >It looks like the variable nblocks must be <= 4 so the read ahead buffer
> >size is not exceeded (which is the cause of the oops). Changing its value
> >doesn't seem to be the right way, but it makes the device work properly.
> >
> >Feedback of any sort welcome.
> >
> >
> >
>

Re: [PATCH] 2.4.20 drivers/cdrom/cdu31a.c

[Corey Minyard]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Mauricio Martinez wrote:

|Thanks for your reply. I guess there are still some drives like this
|floating around. I can live without it, but it is good to use it in an old
|486 as a jukebox and print server :)
|
|Your patch makes much more sense that mine (I have no experience in Linux
|driver development), and it makes the drive work *very well* (excellent
|transfer rate and no system overload), but only if I remove the last hunk.
|
|This last hunk tries to read again the data with 4 sectors less each time
|(i.e. 16,14,12,...,4) which *i think* overloads the buffer leading to an
|oops (and even a system reboot without warning!).
|
|Hope this information helps.

That's really wierd.  Can you make the code in question be:
~            } else if (nblock > 0) {
~                printk("Number of blocks left: %d\n", nblock);
~                end_request(1);
~            } else {

and then send the results when it happens?

It turns out my machine does not have an ISA bus slot, so I can't plug 
my drive in anywhere.

Thanks,

- -Corey
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.0.6 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iD8DBQE+SUaAIXnXXONXERcRAurxAKCgATRBLbNDprxCKcKdsmrPuVkQggCdEwFX
ZVMPef8C10TZzcjEbIgz09U=
=RF+b
-----END PGP SIGNATURE-----

Re: [PATCH] 2.4.20 drivers/cdrom/cdu31a.c

[Mauricio Martinez]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 291 bytes --]


Attached is the output you requested, when reading an 80 k file.

Basically, the original patch tries to read 4 sectors less for each retry,
but I'm not sure when is it necessary to try to read again - this may be
the reason of the behavior I described before.

Hope this helps.

-Mauricio

[-- Attachment #2: Type: TEXT/PLAIN, Size: 3626 bytes --]

copley:~> mount /dev/sonycd /cdrom -t iso9660
cdu31a: Trying session 1
cdu31a: Trying session 2
mount: block device /dev/sonycd is write-protected, mounting read-only
copley:~> ls -l /cdrom/
total 249627
dr-xr-xr-x   1 root     root         2048 Mar 29  1994 0msvideo/
dr-xr-xr-x   1 root     root         2048 Mar 29  1994 101avi/
dr-xr-xr-x   1 root     root         2048 Mar 29  1994 301avi/
dr-xr-xr-x   1 root     root         2048 Mar 29  1994 302avi/
dr-xr-xr-x   1 root     root         2048 Mar 29  1994 401avi/
dr-xr-xr-x   1 root     root         2048 Mar 29  1994 501avi/
-r-xr-xr-x   1 root     root        89248 Sep 20  1993 commdlg.dll*
-r-xr-xr-x   1 root     root        11776 Sep 20  1993 eesc.dll*
dr-xr-xr-x   1 root     root         8192 Mar 29  1994 gbut16/
dr-xr-xr-x   1 root     root         8192 Mar 29  1994 gbut256/
-r-xr-xr-x   1 root     root     250134528 Dec  3  1993 groft.win*
-r-xr-xr-x   1 root     root       111635 Mar 28  1994 ngme.hlp*
-r-xr-xr-x   1 root     root          401 Mar 28  1994 ngme.ini*
-r-xr-xr-x   1 root     root       706048 Feb 19  1993 ngmecl.exe*
-r-xr-xr-x   1 root     root      4332486 Sep 21  1993 openanim.avi*
-r-xr-xr-x   1 root     root       202572 Apr 27  1993 setup.exe*
copley:~> cp /cdrom/commdlg.dll .
Feb 13 23:04:26 copley kernel: Number of blocks left: 28
Feb 13 23:04:26 copley kernel: Number of blocks left: 24
Feb 13 23:04:26 copley kernel: Number of blocks left: 20
Feb 13 23:04:26 copley kernel: Number of blocks left: 16
Feb 13 23:04:26 copley kernel: Number of blocks left: 12
Feb 13 23:04:26 copley kernel: Number of blocks left: 8
Feb 13 23:04:26 copley kernel: Number of blocks left: 4
Feb 13 23:04:26 copley kernel: Number of blocks left: 52
Feb 13 23:04:26 copley kernel: Number of blocks left: 48
Feb 13 23:04:26 copley kernel: Number of blocks left: 44
Feb 13 23:04:27 copley kernel: Number of blocks left: 40
Feb 13 23:04:27 copley kernel: Number of blocks left: 36
Feb 13 23:04:27 copley kernel: Number of blocks left: 32
Feb 13 23:04:27 copley kernel: Number of blocks left: 28
Feb 13 23:04:27 copley kernel: Number of blocks left: 24
Feb 13 23:04:27 copley kernel: Number of blocks left: 20
Feb 13 23:04:27 copley kernel: Number of blocks left: 16
Feb 13 23:04:27 copley kernel: Number of blocks left: 12
Feb 13 23:04:27 copley kernel: Number of blocks left: 8
Feb 13 23:04:27 copley kernel: Number of blocks left: 4
Feb 13 23:04:27 copley kernel: Number of blocks left: 76
Feb 13 23:04:27 copley kernel: Number of blocks left: 72
Feb 13 23:04:27 copley kernel: Number of blocks left: 68
Feb 13 23:04:27 copley kernel: Number of blocks left: 64
Feb 13 23:04:27 copley kernel: Number of blocks left: 60
Feb 13 23:04:27 copley kernel: Number of blocks left: 56
Feb 13 23:04:27 copley kernel: Number of blocks left: 52
Feb 13 23:04:27 copley kernel: Number of blocks left: 48
Feb 13 23:04:27 copley kernel: Number of blocks left: 44
Feb 13 23:04:27 copley kernel: Number of blocks left: 40
Feb 13 23:04:27 copley kernel: Number of blocks left: 36
Feb 13 23:04:27 copley kernel: Number of blocks left: 32
Feb 13 23:04:27 copley kernel: Number of blocks left: 28
Feb 13 23:04:27 copley kernel: Number of blocks left: 24
Feb 13 23:04:27 copley kernel: Number of blocks left: 20
Feb 13 23:04:27 copley kernel: Number of blocks left: 16
Feb 13 23:04:27 copley kernel: Number of blocks left: 12
Feb 13 23:04:27 copley kernel: Number of blocks left: 8
Feb 13 23:04:27 copley kernel: Number of blocks left: 4
Feb 13 23:04:28 copley kernel: Number of blocks left: 4
copley:~>