* [PATCH] 2.4.20 drivers/cdrom/cdu31a.c @ 2003-02-07 13:43 Mauricio Martinez 2003-02-07 15:48 ` Corey Minyard 0 siblings, 1 reply; 5+ messages in thread From: Mauricio Martinez @ 2003-02-07 13:43 UTC (permalink / raw) To: linux-kernel 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. */ ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] 2.4.20 drivers/cdrom/cdu31a.c 2003-02-07 13:43 [PATCH] 2.4.20 drivers/cdrom/cdu31a.c Mauricio Martinez @ 2003-02-07 15:48 ` Corey Minyard 2003-02-11 18:10 ` Mauricio Martinez 0 siblings, 1 reply; 5+ messages in thread From: Corey Minyard @ 2003-02-07 15:48 UTC (permalink / raw) To: Mauricio Martinez; +Cc: linux-kernel [-- 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); ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] 2.4.20 drivers/cdrom/cdu31a.c 2003-02-07 15:48 ` Corey Minyard @ 2003-02-11 18:10 ` Mauricio Martinez 2003-02-11 18:52 ` Corey Minyard 0 siblings, 1 reply; 5+ messages in thread From: Mauricio Martinez @ 2003-02-11 18:10 UTC (permalink / raw) To: Corey Minyard; +Cc: linux-kernel 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. > > > > > > > ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] 2.4.20 drivers/cdrom/cdu31a.c 2003-02-11 18:10 ` Mauricio Martinez @ 2003-02-11 18:52 ` Corey Minyard 2003-02-14 13:53 ` Mauricio Martinez 0 siblings, 1 reply; 5+ messages in thread From: Corey Minyard @ 2003-02-11 18:52 UTC (permalink / raw) To: Mauricio Martinez; +Cc: linux-kernel -----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----- ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] 2.4.20 drivers/cdrom/cdu31a.c 2003-02-11 18:52 ` Corey Minyard @ 2003-02-14 13:53 ` Mauricio Martinez 0 siblings, 0 replies; 5+ messages in thread From: Mauricio Martinez @ 2003-02-14 13:53 UTC (permalink / raw) To: Corey Minyard; +Cc: linux-kernel [-- 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:~> ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2003-02-14 13:43 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2003-02-07 13:43 [PATCH] 2.4.20 drivers/cdrom/cdu31a.c Mauricio Martinez 2003-02-07 15:48 ` Corey Minyard 2003-02-11 18:10 ` Mauricio Martinez 2003-02-11 18:52 ` Corey Minyard 2003-02-14 13:53 ` Mauricio Martinez
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).