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. */
--- 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);
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.
> >
> >
> >
>
-----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-----
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