Re: mcdx -- do_request(): non-read command to cd!!
On 4/4/07, Rene Herman <[EMAIL PROTECTED]> wrote:
Taking forever to reproduce in as far as getting the oops. The thing is
now just locking hard each time. Will keep on trying...
Can you get anything out with sysrq-t? The original oops would be
enough to conclude it's a double-free if it weren't for this:
if (stuffp->toc) {
kfree(stuffp->toc);
stuffp->toc = NULL;
}
While the code is obviously unsafe, we would have to be interrupted
between the read and the assignment, but you don't even have preempt
enabled! So I don't quite yet see where the concurrency is coming
from.
What you can do here is protect the above sequence with a spinlock,
for example, which might paper-over the double-free enough to get you
running again...
Pekka
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: mcdx -- do_request(): non-read command to cd!!
On Tue, Apr 03 2007, Pekka J Enberg wrote: > > mcdx:: close() > > However, we never hit do_mcdx_request(). Jens, do we need to do > set_capacity() somewhere? I don't see other cdrom drivers doing it but > they could be broken too... Yeah, that would be appropriate. -- Jens Axboe - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: mcdx -- do_request(): non-read command to cd!!
On 04/03/2007 08:32 PM, Pekka J Enberg wrote: Please enable CONFIG_DEBUG_SLAB now and reproduce. It should tell us what's going wrong there. Taking forever to reproduce in as far as getting the oops. The thing is now just locking hard each time. Will keep on trying... Rene. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: mcdx -- do_request(): non-read command to cd!!
On Tue, 3 Apr 2007, Rene Herman wrote: > That is, the exact same oops the tar test showed which I expect is progress -- > the dd is now in fact doing something :-) Yes, that's expected. I think we fixed dd now. On Tue, 3 Apr 2007, Rene Herman wrote: > When I now switched the monitor to 5va2 itself it was happily generating more > oopses and supposedly "fixing up recursive faults", while advicing me to > reboot. 2 seconds later it didn't leave me a choice as it dropped dead :-) Please enable CONFIG_DEBUG_SLAB now and reproduce. It should tell us what's going wrong there. On Tue, 3 Apr 2007, Rene Herman wrote: > Just to keep track, I'm now using the patches (minus 03) at: > > http://members.home.nl/rene.herman/mcdx/ > > (for lkml: with 03, the machine locks itself hard each and every time without > information). Yes, because it is totally broken. So please just drop the patch. AFAICT the driver makes the process sleep in mcdex_transfer() until an interrupt comes to acknowledge that the I/O has completed but the patch makes the code sleep while holding the mutex and once the interrupt comes and tries to grab it, the machine will deadlock. We probably need to fix it somehow but lets see what CONFIG_DEBUG_SLAB tells us first. Btw, if anyone has a datasheet for this beast, I would like a copy. Pekka - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: mcdx -- do_request(): non-read command to cd!!
On 04/03/2007 07:31 PM, Pekka J Enberg wrote:
Oh, well, here goes. Rene, would you be so kind and spin the wheel once
more? =)
Absolutely!
[EMAIL PROTECTED]:~# dd if=/dev/mcdx0 of=/dev/null
Oops: 0002 [#1]
CPU:0
EIP:0060:[]Not tainted VLI
EFLAGS: 00010082 (2.6.20.4 #11)
EIP is at free_block+0x58/0xcf
eax: 07ff46c6 ebx: c1d5f100 ecx: c1d5f12c edx: 160010b9
esi: c16414e0 edi: c160c960 ebp: 0001 esp: c1575ef0
ds: 007b es: 007b ss: 0068
Process events/0 (pid: 4, ti=c1575000 task=c1577510 task.ti=c1575000)
Stack: 0002 c165fa38 c165fa38 0002 c165fa00
c1047eb6
c16414e0 c160c960 c16414e0 c160c960 c14ab540
0292
c1047f3a 0292 c14ab544 c154ebc0 c101ec7a
c154ec14
Call Trace:
[] drain_array+0x94/0xc3
[] cache_reap+0x55/0xe9
[] run_workqueue+0x8f/0x14d
[] cache_reap+0x0/0xe9
[] worker_thread+0x122/0x14b
[] default_wake_function+0x0/0xc
[] default_wake_function+0x0/0xc
[] worker_thread+0x0/0x14b
[] kthread+0x72/0x97
[] kthread+0x0/0x97
[] kernel_thread_helper+0x7/0x10
===
Code: 05 03 15 4c ab 4a c1 8b 02 f6 c4 40 74 03 8b 52 0c 8b 02 a8 80 75
04 0f 0b eb fe 8b 5a 1c 8b 44 24 20 8b 53 04 8b 74 87 18 8b 03 <89> 02
c7 03 00 01 10 00 89 50 04 c7 43 04 00 02 20 00 8b 44 24
EIP: [] free_block+0x58/0xcf SS:ESP 0068:c1575ef0
That is, the exact same oops the tar test showed which I expect is
progress -- the dd is now in fact doing something :-)
When I now switched the monitor to 5va2 itself it was happily generating
more oopses and supposedly "fixing up recursive faults", while advicing
me to reboot. 2 seconds later it didn't leave me a choice as it dropped
dead :-)
Just to keep track, I'm now using the patches (minus 03) at:
http://members.home.nl/rene.herman/mcdx/
(for lkml: with 03, the machine locks itself hard each and every time
without information).
Thanks again :)
Rene.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: mcdx -- do_request(): non-read command to cd!!
On Tue, 3 Apr 2007, Rene Herman wrote: > I believe the "tar" runs has produced something useful now though: > > [EMAIL PROTECTED]:~# dmesg -c >/dev/null > [EMAIL PROTECTED]:~# strace -o tar.strace tar cvf foo.tar /mnt/cdrom > tar: Removing leading `/' from member names > /mnt/cdrom/ > /mnt/cdrom/dott/ > /mnt/cdrom/dott/adlib.ims > /mnt/cdrom/dott/dott.bat > /mnt/cdrom/dott/dotticon.ico > /mnt/cdrom/dott/gmidi.ims > /mnt/cdrom/dott/maniac/ > rlogin: connection closed. > [EMAIL PROTECTED]:~$ > > When I now switch the monitor to 5va2 directly, there is an oops on the > screen, with a backtrace saying (manual transcription): > > Call Trace: > [] drain_array+0x94/0xc3 > [ ...] cache_reap+0x55/0xe9 > [ ...] run_workqueue+0x8f/0x14d > [ ...] cache_reap+0x0//0xe9 > [ ...] worker_thread+0x122/0x14b > [ ...] default_wake_function+0x0/0xc > [ ...] default_wake_function+0x0/0xc > [ ...] worker_thread+0x0/0x14b > [ ...] kthread+0x72/0x97 > [ ...] kthread+0x0/0x97 > [ ...] kernel_thread_helper+0x7/0x10 > === > Code: [ ... ] > EIP: [] free_block+0x58/0xcf SS:ESP [ ... ] > > Couple of functions mentioned twice in that backtrace. That looks a > little off no? It looks like we are passing a bad pointer to slab. Please turn on CONFIG_DEBUG_SLAB and reproduce. It's probably a double-free as the TOC reading bits look racy. Pekka - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: mcdx -- do_request(): non-read command to cd!!
On Tue, 3 Apr 2007, Pekka J Enberg wrote:
> However, we never hit do_mcdx_request(). Jens, do we need to do
> set_capacity() somewhere? I don't see other cdrom drivers doing it but
> they could be broken too...
Oh, well, here goes. Rene, would you be so kind and spin the wheel once
more? =)
Pekka
diff --git a/drivers/cdrom/mcdx.c b/drivers/cdrom/mcdx.c
index f574962..8ed61c9 100644
--- a/drivers/cdrom/mcdx.c
+++ b/drivers/cdrom/mcdx.c
@@ -672,6 +672,7 @@ static int mcdx_open(struct cdrom_device
if (-1 == mcdx_requesttocdata(stuffp, &stuffp->di, 1)) {
stuffp->lastsector = -1;
+ set_capacity(stuffp->disk, 0);
} else {
@@ -692,6 +693,7 @@ static int mcdx_open(struct cdrom_device
stuffp->di.msf_leadout.second,
stuffp->di.msf_leadout.frame,
msf2log(&stuffp->di.msf_leadout));
+ set_capacity(stuffp->disk, stuffp->lastsector + 1);
}
if (stuffp->toc) {
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: mcdx -- do_request(): non-read command to cd!!
On 04/03/2007 08:57 AM, Pekka J Enberg wrote:
> > Does this change the dd case?
> >
> > diff --git a/drivers/cdrom/mcdx.c b/drivers/cdrom/mcdx.c
> > index f574962..6c613d0 100644
> > --- a/drivers/cdrom/mcdx.c
> > +++ b/drivers/cdrom/mcdx.c
> > @@ -1248,6 +1248,7 @@ #endif
> > disk->private_data = stuffp;
> > disk->queue = mcdx_queue;
> > add_disk(disk);
> > + blk_queue_hardsect_size(mcdx_queue, MCDX_CDBLK);
> > printk(msg);
> > return 0;
> > }
On Tue, 3 Apr 2007, Rene Herman wrote:
> No, I'm afraid not. It's still the same effect:
>
> [EMAIL PROTECTED]:~# dmesg -c >/dev/null
> [EMAIL PROTECTED]:~# strace -o dd.strace dd if=/dev/mcdx0 of=/dev/null
> 0+0 records in
> 0+0 records out
> 0 bytes (0 B) copied, 0.00438956 seconds, 0.0 kB/s
> [EMAIL PROTECTED]:~# dmesg >dd.xtrace
Looking at the dd strace:
> close(0)= 0
> open("/dev/mcdx0", O_RDONLY|O_LARGEFILE) = 0
> _llseek(0, 0, [0], SEEK_CUR)= 0
The block device is open, dd has the current offset.
> close(1)= 0
> open("/dev/null", O_WRONLY|O_CREAT|O_TRUNC|O_LARGEFILE, 0666) = 1
[snip]
> read(0, "", 512)= 0
We start to read but get nothing!
> close(0)= 0
> close(1)= 0
Now looking at the driver debug trace:
> mcdx open() Single Session disk found
> mcdx mcdx_media_changed called for device mcdx0
> mcdx:: talk() 1 / 5000 tries, res.size 1, command 0x40<7>mcdx::
We do mcdx_getstatus() here...
> mcdx:: talk() command sent
> mcdx:: *** delay: sleepq
> mcdx:: delay awoken
> mcdx:: *** delay: sleepq
> mcdx:: delay awoken
> mcdx:: talk() got status 0x50
> mcdx:: talk() done with 0x50
...and the status bits seem to be MCDX_RBIT_CHECK and MCDX_RBIT_CHECK.
> mcdx:: close()
However, we never hit do_mcdx_request(). Jens, do we need to do
set_capacity() somewhere? I don't see other cdrom drivers doing it but
they could be broken too...
Pekka
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: mcdx -- do_request(): non-read command to cd!!
On 04/03/2007 08:57 AM, Pekka J Enberg wrote:
[ re-including linux-kernel ]
Does this change the dd case?
diff --git a/drivers/cdrom/mcdx.c b/drivers/cdrom/mcdx.c
index f574962..6c613d0 100644
--- a/drivers/cdrom/mcdx.c
+++ b/drivers/cdrom/mcdx.c
@@ -1248,6 +1248,7 @@ #endif
disk->private_data = stuffp;
disk->queue = mcdx_queue;
add_disk(disk);
+ blk_queue_hardsect_size(mcdx_queue, MCDX_CDBLK);
printk(msg);
return 0;
}
No, I'm afraid not. It's still the same effect:
[EMAIL PROTECTED]:~# dmesg -c >/dev/null
[EMAIL PROTECTED]:~# strace -o dd.strace dd if=/dev/mcdx0 of=/dev/null
0+0 records in
0+0 records out
0 bytes (0 B) copied, 0.00438956 seconds, 0.0 kB/s
[EMAIL PROTECTED]:~# dmesg >dd.xtrace
dd.strace again attached. I've dropped the mutex patch for now and now
the xtrace is littered with waitqueue debugging. I'll see if I can
disable that particular debugging trace I guess. dd.xtrace sent privately...
"readcd dev=/dev/mcdx0 f=foo.img" is still an immediate hard lockup also
without the mutex. I believe the "tar" runs has produced something
useful now though:
[EMAIL PROTECTED]:~# dmesg -c >/dev/null
[EMAIL PROTECTED]:~# strace -o tar.strace tar cvf foo.tar /mnt/cdrom
tar: Removing leading `/' from member names
/mnt/cdrom/
/mnt/cdrom/dott/
/mnt/cdrom/dott/adlib.ims
/mnt/cdrom/dott/dott.bat
/mnt/cdrom/dott/dotticon.ico
/mnt/cdrom/dott/gmidi.ims
/mnt/cdrom/dott/maniac/
rlogin: connection closed.
[EMAIL PROTECTED]:~$
When I now switch the monitor to 5va2 directly, there is an oops on the
screen, with a backtrace saying (manual transcription):
Call Trace:
[] drain_array+0x94/0xc3
[ ...] cache_reap+0x55/0xe9
[ ...] run_workqueue+0x8f/0x14d
[ ...] cache_reap+0x0//0xe9
[ ...] worker_thread+0x122/0x14b
[ ...] default_wake_function+0x0/0xc
[ ...] default_wake_function+0x0/0xc
[ ...] worker_thread+0x0/0x14b
[ ...] kthread+0x72/0x97
[ ...] kthread+0x0/0x97
[ ...] kernel_thread_helper+0x7/0x10
===
Code: [ ... ]
EIP: [] free_block+0x58/0xcf SS:ESP [ ... ]
Couple of functions mentioned twice in that backtrace. That looks a
little off no?
"tar.strace" made it to disk, so it's attached as well.
Rene.
execve("/bin/dd", ["dd", "if=/dev/mcdx0", "of=/dev/null"], [/* 28 vars */]) = 0
uname({sys="Linux", node="5va2", ...}) = 0
brk(0) = 0x8052000
access("/etc/ld.so.preload", R_OK) = -1 ENOENT (No such file or directory)
open("/etc/ld.so.cache", O_RDONLY) = 3
fstat64(3, {st_mode=S_IFREG|0644, st_size=47774, ...}) = 0
mmap2(NULL, 47774, PROT_READ, MAP_PRIVATE, 3, 0) = 0xb7fb3000
close(3)= 0
open("/lib/tls/librt.so.1", O_RDONLY) = 3
read(3, "\177ELF\1\1\1\0\0\0\0\0\0\0\0\0\3\0\3\0\1\0\0\0`\35\0\000"..., 512) =
512
fstat64(3, {st_mode=S_IFREG|0755, st_size=34763, ...}) = 0
mmap2(NULL, 29260, PROT_READ|PROT_EXEC, MAP_PRIVATE|MAP_DENYWRITE, 3, 0) =
0xb7fab000
mmap2(0xb7fb1000, 8192, PROT_READ|PROT_WRITE,
MAP_PRIVATE|MAP_FIXED|MAP_DENYWRITE, 3, 0x5) = 0xb7fb1000
close(3)= 0
open("/lib/tls/libc.so.6", O_RDONLY)= 3
read(3, "\177ELF\1\1\1\0\0\0\0\0\0\0\0\0\3\0\3\0\1\0\0\0\20O\1\000"..., 512) =
512
fstat64(3, {st_mode=S_IFREG|0755, st_size=1441201, ...}) = 0
mmap2(NULL, 4096, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) =
0xb7faa000
mmap2(NULL, 1240284, PROT_READ|PROT_EXEC, MAP_PRIVATE|MAP_DENYWRITE, 3, 0) =
0xb7e7b000
mmap2(0xb7fa4000, 16384, PROT_READ|PROT_WRITE,
MAP_PRIVATE|MAP_FIXED|MAP_DENYWRITE, 3, 0x128) = 0xb7fa4000
mmap2(0xb7fa8000, 7388, PROT_READ|PROT_WRITE,
MAP_PRIVATE|MAP_FIXED|MAP_ANONYMOUS, -1, 0) = 0xb7fa8000
close(3)= 0
open("/lib/tls/libpthread.so.0", O_RDONLY) = 3
read(3, "\177ELF\1\1\1\0\0\0\0\0\0\0\0\0\3\0\3\0\1\0\0\0\20H\0\000"..., 512) =
512
fstat64(3, {st_mode=S_IFREG|0755, st_size=88005, ...}) = 0
mmap2(NULL, 70104, PROT_READ|PROT_EXEC, MAP_PRIVATE|MAP_DENYWRITE, 3, 0) =
0xb7e69000
mmap2(0xb7e77000, 8192, PROT_READ|PROT_WRITE,
MAP_PRIVATE|MAP_FIXED|MAP_DENYWRITE, 3, 0xd) = 0xb7e77000
mmap2(0xb7e79000, 4568, PROT_READ|PROT_WRITE,
MAP_PRIVATE|MAP_FIXED|MAP_ANONYMOUS, -1, 0) = 0xb7e79000
close(3)= 0
mmap2(NULL, 4096, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) =
0xb7e68000
mprotect(0xb7fa4000, 4096, PROT_READ) = 0
set_thread_area({entry_number:-1 -> 6, base_addr:0xb7e686c0, limit:1048575,
seg_32bit:1, contents:0, read_exec_only:0, limit_in_pages:1, seg_not_present:0,
useable:1}) = 0
munmap(0xb7fb3000, 47774) = 0
set_tid_address(0xb7e68708) = 1173
rt_sigaction(SIGRTMIN, {0xb7e6d3a0, [], SA_SIGINFO}, NULL, 8) = 0
rt_sigaction(SIGRT_1, {0xb7e6d420, [], SA_RESTART|SA_SIGINFO}, NULL, 8) = 0
rt_sigprocmask(SIG_UNBLOCK, [RTMIN RT_1], NULL, 8) = 0
getrlimit(RLIMIT_STACK, {rlim_cur=8192*1024, rlim_max=RLIM_INFINITY}) = 0
_
Re: mcdx -- do_request(): non-read command to cd!!
On 04/02/2007 11:42 AM, Jens Axboe wrote: I somehow doubt that the cards are capable of highmem addressing in the first place :-) Well, we could pretend we're using 286s and define the 15-16 MB hole as "highmem". But other than that, these things plug into an ISA bus, so no. Most of them are strictly PIO anyway in fact... Rene. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: mcdx -- do_request(): non-read command to cd!!
On 04/02/2007 05:18 PM, Rene Herman wrote: Thanks again, spinning! Oh, forgot to mention. Yes, earlier PREEMPT was on and it's now disabled. Rene. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: mcdx -- do_request(): non-read command to cd!!
On 04/02/2007 09:10 AM, Jens Axboe wrote: Updated patch attached :-) diff --git a/Documentation/feature-removal-schedule.txt b/Documentation/feature-removal-schedule.txt index 0bc8b0b..cff761a 100644 --- a/Documentation/feature-removal-schedule.txt +++ b/Documentation/feature-removal-schedule.txt @@ -324,3 +324,10 @@ Why: the i8xx_tco watchdog driver has been replaced by the iTCO_wdt Who: Wim Van Sebroeck <[EMAIL PROTECTED]> --- + +What: The legacy CDROM drivers (drivers/cdrom/, except cdrom.c) +When: In 2.6.23 Not going to complain about it loudly. I like playing with these old drives, but the drivers are so immensely broken that they don't seem to serve a very useful purpose anymore; even if I, or someone else, would ever get around to try and revive this hardware under Linux; the current state of these drivers seems to imply we'd be better of starting from scratch. And even more so since I expect that any new attempts should do the "no really, I'm a scsi drive" thing that most anything else is doing these days. Using a different infrastructure like that would, I presume, mean that really only the hardware details would remain useful from the old drivers, and those will be present inside linux-2.6.22.tar.gz same as before. Having said that, it would still be good to have mcdx functional so please don't let me keep you, Pekka, or anyone else from looking :-) With a bit of luck sbpcd and cdu31a (and cm206 as the other controller that I have) will be similarly broken and having an example in mcdx will be useful whatever happens to the drivers in the upstream tree... +Why: They are all terminally broken (most don't even compile) That's not true though... I just checked and they all compile on 2.6.20.4. sbpcd and cm206 with cli/sti warnings, the others without even. Rene. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: mcdx -- do_request(): non-read command to cd!!
On 04/02/2007 10:55 AM, Pekka J Enberg wrote:
On Mon, 2 Apr 2007, Jens Axboe wrote:
But as I wrote earlier, it'll take lots more to make this driver close
to functional.
Heh, looking at the driver, that's quite an understatement =). Rene,
here's an untested attempt to use a mutex instead of the horribly broken
waitqueue "synchronization" in mcdx. It may or may not help so give it a
spin if you want.
Thanks again, spinning! I've split up the patch into its history and am
now using the three available at:
http://members.home.nl/rene.herman/mcdx/
against 2.6.20.4. The mutex does change things but not so much for the
better unfortunately. With this, I'm still able to do:
[EMAIL PROTECTED]:~# strace -o dd.strace dd if=/dev/mcdx0 of=foo.img
with the same result:
[EMAIL PROTECTED]:~# strace -o dd.strace dd if=/dev/mcdx0 of=foo.img
0+0 records in
0+0 records out
0 bytes (0 B) copied, 0.00202698 seconds, 0.0 kB/s
(dd.strace attached) but the readcd and tar tests:
[EMAIL PROTECTED]:~# strace -o readcd.strace readcd dev=/dev/mcdx0 f=foo.img
[EMAIL PROTECTED]:~# strace -o tar.strace tar cvf foo.tar /mnt/cdrom
now lock up the machine hard each time; no pings replies, no nothing.
Using f=/dev/null with readcd does not change things. Using /dev/null
with tar _does_ but tar seems to notice it's writing to /dev/null and
"optimizes" the actual reading away so that's no surprise.
Thanks again! Still love inserting CDs in this thing, so I'll happily
keep on testing... :-)
Rene.
execve("/bin/dd", ["dd", "if=/dev/mcdx0", "of=foo.img"], [/* 28 vars */]) = 0
uname({sys="Linux", node="5va2", ...}) = 0
brk(0) = 0x8052000
access("/etc/ld.so.preload", R_OK) = -1 ENOENT (No such file or directory)
open("/etc/ld.so.cache", O_RDONLY) = 3
fstat64(3, {st_mode=S_IFREG|0644, st_size=47774, ...}) = 0
mmap2(NULL, 47774, PROT_READ, MAP_PRIVATE, 3, 0) = 0xb7f8d000
close(3)= 0
open("/lib/tls/librt.so.1", O_RDONLY) = 3
read(3, "\177ELF\1\1\1\0\0\0\0\0\0\0\0\0\3\0\3\0\1\0\0\0`\35\0\000"..., 512) =
512
fstat64(3, {st_mode=S_IFREG|0755, st_size=34763, ...}) = 0
mmap2(NULL, 29260, PROT_READ|PROT_EXEC, MAP_PRIVATE|MAP_DENYWRITE, 3, 0) =
0xb7f85000
mmap2(0xb7f8b000, 8192, PROT_READ|PROT_WRITE,
MAP_PRIVATE|MAP_FIXED|MAP_DENYWRITE, 3, 0x5) = 0xb7f8b000
close(3)= 0
open("/lib/tls/libc.so.6", O_RDONLY)= 3
read(3, "\177ELF\1\1\1\0\0\0\0\0\0\0\0\0\3\0\3\0\1\0\0\0\20O\1\000"..., 512) =
512
fstat64(3, {st_mode=S_IFREG|0755, st_size=1441201, ...}) = 0
mmap2(NULL, 4096, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) =
0xb7f84000
mmap2(NULL, 1240284, PROT_READ|PROT_EXEC, MAP_PRIVATE|MAP_DENYWRITE, 3, 0) =
0xb7e55000
mmap2(0xb7f7e000, 16384, PROT_READ|PROT_WRITE,
MAP_PRIVATE|MAP_FIXED|MAP_DENYWRITE, 3, 0x128) = 0xb7f7e000
mmap2(0xb7f82000, 7388, PROT_READ|PROT_WRITE,
MAP_PRIVATE|MAP_FIXED|MAP_ANONYMOUS, -1, 0) = 0xb7f82000
close(3)= 0
open("/lib/tls/libpthread.so.0", O_RDONLY) = 3
read(3, "\177ELF\1\1\1\0\0\0\0\0\0\0\0\0\3\0\3\0\1\0\0\0\20H\0\000"..., 512) =
512
fstat64(3, {st_mode=S_IFREG|0755, st_size=88005, ...}) = 0
mmap2(NULL, 70104, PROT_READ|PROT_EXEC, MAP_PRIVATE|MAP_DENYWRITE, 3, 0) =
0xb7e43000
mmap2(0xb7e51000, 8192, PROT_READ|PROT_WRITE,
MAP_PRIVATE|MAP_FIXED|MAP_DENYWRITE, 3, 0xd) = 0xb7e51000
mmap2(0xb7e53000, 4568, PROT_READ|PROT_WRITE,
MAP_PRIVATE|MAP_FIXED|MAP_ANONYMOUS, -1, 0) = 0xb7e53000
close(3)= 0
mmap2(NULL, 4096, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) =
0xb7e42000
mprotect(0xb7f7e000, 4096, PROT_READ) = 0
set_thread_area({entry_number:-1 -> 6, base_addr:0xb7e426c0, limit:1048575,
seg_32bit:1, contents:0, read_exec_only:0, limit_in_pages:1, seg_not_present:0,
useable:1}) = 0
munmap(0xb7f8d000, 47774) = 0
set_tid_address(0xb7e42708) = 1177
rt_sigaction(SIGRTMIN, {0xb7e473a0, [], SA_SIGINFO}, NULL, 8) = 0
rt_sigaction(SIGRT_1, {0xb7e47420, [], SA_RESTART|SA_SIGINFO}, NULL, 8) = 0
rt_sigprocmask(SIG_UNBLOCK, [RTMIN RT_1], NULL, 8) = 0
getrlimit(RLIMIT_STACK, {rlim_cur=8192*1024, rlim_max=RLIM_INFINITY}) = 0
_sysctl({{CTL_KERN, KERN_VERSION}, 2, 0xbf93cd5c, 32, (nil), 0}) = 0
open("/usr/lib/locale/locale-archive", O_RDONLY|O_LARGEFILE) = -1 ENOENT (No
such file or directory)
brk(0) = 0x8052000
brk(0x8073000) = 0x8073000
open("/usr/share/locale/locale.alias", O_RDONLY) = 3
fstat64(3, {st_mode=S_IFREG|0644, st_size=2586, ...}) = 0
mmap2(NULL, 4096, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) =
0xb7f98000
read(3, "# Locale name alias data base.\n#"..., 4096) = 2586
read(3, "", 4096) = 0
close(3)= 0
munmap(0xb7f98000, 4096)= 0
open("/usr/lib/locale/en_US/LC_IDENTIFICATION", O_RDONLY) = 3
fstat64(3, {st_mode=S_IFREG|0644, st_size=378, .
Re: mcdx -- do_request(): non-read command to cd!!
Pekka J Enberg wrote:
> On Mon, 2 Apr 2007, Jens Axboe wrote:
>> But as I wrote earlier, it'll take lots more to make this driver close
>> to functional.
>
> Heh, looking at the driver, that's quite an understatement =). Rene,
> here's an untested attempt to use a mutex instead of the horribly broken
> waitqueue "synchronization" in mcdx. It may or may not help so give it a
> spin if you want.
>
> Pekka
>
> ---
> drivers/cdrom/mcdx.c | 121
> ++-
> 1 file changed, 44 insertions(+), 77 deletions(-)
>
> Index: 2.6/drivers/cdrom/mcdx.c
> ===
> --- 2.6.orig/drivers/cdrom/mcdx.c 2007-04-02 11:50:40.0 +0300
> +++ 2.6/drivers/cdrom/mcdx.c 2007-04-02 11:51:20.0 +0300
> @@ -58,6 +58,7 @@ = "$Id: mcdx.c,v 1.21 1997/01/26 07:
>
> #include
>
> +#include
> #include
> #include
> #include
> @@ -151,15 +152,14 @@ struct s_version {
> /* Per drive/controller stuff **/
>
> struct s_drive_stuff {
> + struct mutex mutex;
> +
> /* waitqueues */
> wait_queue_head_t busyq;
> - wait_queue_head_t lockq;
> - wait_queue_head_t sleepq;
>
> /* flags */
> volatile int introk;/* status of last irq operation */
> volatile int busy; /* drive performs an operation */
> - volatile int lock; /* exclusive usage */
>
> /* cd infos */
> struct s_diskinfo di;
> @@ -266,7 +266,6 @@ static unsigned int uint2bcd(unsigned in
> static unsigned int bcd2uint(unsigned char);
> static unsigned port(int *);
> static int irq(int *);
> -static void mcdx_delay(struct s_drive_stuff *, long jifs);
> static int mcdx_transfer(struct s_drive_stuff *, char *buf, int sector,
>int nr_sectors);
> static int mcdx_xfer(struct s_drive_stuff *, char *buf, int sector,
> @@ -287,7 +286,7 @@ static int mcdx_requestmultidiskinfo(str
> static int mcdx_requesttocdata(struct s_drive_stuff *, struct s_diskinfo *,
> int);
> static int mcdx_getstatus(struct s_drive_stuff *, int);
> -static int mcdx_getval(struct s_drive_stuff *, int to, int delay, char *);
> +static int mcdx_getval(struct s_drive_stuff *, int to, int delay_sec, char
> *);
> static int mcdx_talk(struct s_drive_stuff *,
>const unsigned char *cmd, size_t,
>void *buffer, size_t size, unsigned int timeout, int);
> @@ -577,56 +576,57 @@ static void do_mcdx_request(request_queu
> if (!req)
> return;
>
> + if (!blk_fs_request(req)) {
> + end_request(req, 0);
> + goto again;
> + }
> +
> stuffp = req->rq_disk->private_data;
>
> if (!stuffp->present) {
> xwarn("do_request(): bad device: %s\n",req->rq_disk->disk_name);
> xtrace(REQUEST, "end_request(0): bad device\n");
> end_request(req, 0);
> - return;
> + goto again;
> }
>
> if (stuffp->audio) {
> xwarn("do_request() attempt to read from audio cd\n");
> xtrace(REQUEST, "end_request(0): read from audio\n");
> end_request(req, 0);
> - return;
> + goto again;
> }
>
> xtrace(REQUEST, "do_request() (%lu + %lu)\n",
> req->sector, req->nr_sectors);
>
> - if (req->cmd != READ) {
> + if (rq_data_dir(req) != READ) {
> xwarn("do_request(): non-read command to cd!!\n");
> xtrace(REQUEST, "end_request(0): write\n");
> end_request(req, 0);
> - return;
> - }
> - else {
> - stuffp->status = 0;
> - while (req->nr_sectors) {
> - int i;
> -
> - i = mcdx_transfer(stuffp,
> - req->buffer,
> - req->sector,
> - req->nr_sectors);
> -
> - if (i == -1) {
> - end_request(req, 0);
> - goto again;
> - }
> - req->sector += i;
> - req->nr_sectors -= i;
> - req->buffer += (i * 512);
> - }
> - end_request(req, 1);
> goto again;
> -
> - xtrace(REQUEST, "end_request(1)\n");
> - end_request(req, 1);
> }
>
> + stuffp->status = 0;
> + while (req->nr_sectors) {
> + int i;
> +
> + spin_unlock_irq(q->queue_lock);
> + i = mcdx_transfer(stuffp,
> + req->buffer,
> + req->sector,
> + req->nr_sectors);
> + spin_lock_irq(q->queue_lock);
> +
> + if (i == -1) {
> +
Re: mcdx -- do_request(): non-read command to cd!!
On Mon, Apr 02 2007, Boaz Harrosh wrote:
> Pekka J Enberg wrote:
> > On Mon, 2 Apr 2007, Jens Axboe wrote:
> >> But as I wrote earlier, it'll take lots more to make this driver close
> >> to functional.
> >
> > Heh, looking at the driver, that's quite an understatement =). Rene,
> > here's an untested attempt to use a mutex instead of the horribly broken
> > waitqueue "synchronization" in mcdx. It may or may not help so give it a
> > spin if you want.
> >
> > Pekka
> >
> > ---
> > drivers/cdrom/mcdx.c | 121
> > ++-
> > 1 file changed, 44 insertions(+), 77 deletions(-)
> >
> > Index: 2.6/drivers/cdrom/mcdx.c
> > ===
> > --- 2.6.orig/drivers/cdrom/mcdx.c 2007-04-02 11:50:40.0 +0300
> > +++ 2.6/drivers/cdrom/mcdx.c2007-04-02 11:51:20.0 +0300
> > @@ -58,6 +58,7 @@ = "$Id: mcdx.c,v 1.21 1997/01/26 07:
> >
> > #include
> >
> > +#include
> > #include
> > #include
> > #include
> > @@ -151,15 +152,14 @@ struct s_version {
> > /* Per drive/controller stuff **/
> >
> > struct s_drive_stuff {
> > + struct mutex mutex;
> > +
> > /* waitqueues */
> > wait_queue_head_t busyq;
> > - wait_queue_head_t lockq;
> > - wait_queue_head_t sleepq;
> >
> > /* flags */
> > volatile int introk;/* status of last irq operation */
> > volatile int busy; /* drive performs an operation */
> > - volatile int lock; /* exclusive usage */
> >
> > /* cd infos */
> > struct s_diskinfo di;
> > @@ -266,7 +266,6 @@ static unsigned int uint2bcd(unsigned in
> > static unsigned int bcd2uint(unsigned char);
> > static unsigned port(int *);
> > static int irq(int *);
> > -static void mcdx_delay(struct s_drive_stuff *, long jifs);
> > static int mcdx_transfer(struct s_drive_stuff *, char *buf, int sector,
> > int nr_sectors);
> > static int mcdx_xfer(struct s_drive_stuff *, char *buf, int sector,
> > @@ -287,7 +286,7 @@ static int mcdx_requestmultidiskinfo(str
> > static int mcdx_requesttocdata(struct s_drive_stuff *, struct s_diskinfo *,
> >int);
> > static int mcdx_getstatus(struct s_drive_stuff *, int);
> > -static int mcdx_getval(struct s_drive_stuff *, int to, int delay, char *);
> > +static int mcdx_getval(struct s_drive_stuff *, int to, int delay_sec, char
> > *);
> > static int mcdx_talk(struct s_drive_stuff *,
> > const unsigned char *cmd, size_t,
> > void *buffer, size_t size, unsigned int timeout, int);
> > @@ -577,56 +576,57 @@ static void do_mcdx_request(request_queu
> > if (!req)
> > return;
> >
> > + if (!blk_fs_request(req)) {
> > + end_request(req, 0);
> > + goto again;
> > + }
> > +
> > stuffp = req->rq_disk->private_data;
> >
> > if (!stuffp->present) {
> > xwarn("do_request(): bad device: %s\n",req->rq_disk->disk_name);
> > xtrace(REQUEST, "end_request(0): bad device\n");
> > end_request(req, 0);
> > - return;
> > + goto again;
> > }
> >
> > if (stuffp->audio) {
> > xwarn("do_request() attempt to read from audio cd\n");
> > xtrace(REQUEST, "end_request(0): read from audio\n");
> > end_request(req, 0);
> > - return;
> > + goto again;
> > }
> >
> > xtrace(REQUEST, "do_request() (%lu + %lu)\n",
> >req->sector, req->nr_sectors);
> >
> > - if (req->cmd != READ) {
> > + if (rq_data_dir(req) != READ) {
> > xwarn("do_request(): non-read command to cd!!\n");
> > xtrace(REQUEST, "end_request(0): write\n");
> > end_request(req, 0);
> > - return;
> > - }
> > - else {
> > - stuffp->status = 0;
> > - while (req->nr_sectors) {
> > - int i;
> > -
> > - i = mcdx_transfer(stuffp,
> > - req->buffer,
> > - req->sector,
> > - req->nr_sectors);
> > -
> > - if (i == -1) {
> > - end_request(req, 0);
> > - goto again;
> > - }
> > - req->sector += i;
> > - req->nr_sectors -= i;
> > - req->buffer += (i * 512);
> > - }
> > - end_request(req, 1);
> > goto again;
> > -
> > - xtrace(REQUEST, "end_request(1)\n");
> > - end_request(req, 1);
> > }
> >
> > + stuffp->status = 0;
> > + while (req->nr_sectors) {
> > + int i;
> > +
> > + spin_unlock_irq(q->queue_lock);
> > + i = mcdx_transfer(stuffp,
> > + req->buffer,
> > +
Re: mcdx -- do_request(): non-read command to cd!!
On Mon, 2 Apr 2007, Boaz Harrosh wrote: > Why are you using req->buffer in new code. You did read the patch, right? It's not new code. Feel free to send a patch, though. Thanks. Pekka - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: mcdx -- do_request(): non-read command to cd!!
On Mon, 2 Apr 2007, Jens Axboe wrote:
> But as I wrote earlier, it'll take lots more to make this driver close
> to functional.
Heh, looking at the driver, that's quite an understatement =). Rene,
here's an untested attempt to use a mutex instead of the horribly broken
waitqueue "synchronization" in mcdx. It may or may not help so give it a
spin if you want.
Pekka
---
drivers/cdrom/mcdx.c | 121 ++-
1 file changed, 44 insertions(+), 77 deletions(-)
Index: 2.6/drivers/cdrom/mcdx.c
===
--- 2.6.orig/drivers/cdrom/mcdx.c 2007-04-02 11:50:40.0 +0300
+++ 2.6/drivers/cdrom/mcdx.c2007-04-02 11:51:20.0 +0300
@@ -58,6 +58,7 @@ = "$Id: mcdx.c,v 1.21 1997/01/26 07:
#include
+#include
#include
#include
#include
@@ -151,15 +152,14 @@ struct s_version {
/* Per drive/controller stuff **/
struct s_drive_stuff {
+ struct mutex mutex;
+
/* waitqueues */
wait_queue_head_t busyq;
- wait_queue_head_t lockq;
- wait_queue_head_t sleepq;
/* flags */
volatile int introk;/* status of last irq operation */
volatile int busy; /* drive performs an operation */
- volatile int lock; /* exclusive usage */
/* cd infos */
struct s_diskinfo di;
@@ -266,7 +266,6 @@ static unsigned int uint2bcd(unsigned in
static unsigned int bcd2uint(unsigned char);
static unsigned port(int *);
static int irq(int *);
-static void mcdx_delay(struct s_drive_stuff *, long jifs);
static int mcdx_transfer(struct s_drive_stuff *, char *buf, int sector,
int nr_sectors);
static int mcdx_xfer(struct s_drive_stuff *, char *buf, int sector,
@@ -287,7 +286,7 @@ static int mcdx_requestmultidiskinfo(str
static int mcdx_requesttocdata(struct s_drive_stuff *, struct s_diskinfo *,
int);
static int mcdx_getstatus(struct s_drive_stuff *, int);
-static int mcdx_getval(struct s_drive_stuff *, int to, int delay, char *);
+static int mcdx_getval(struct s_drive_stuff *, int to, int delay_sec, char *);
static int mcdx_talk(struct s_drive_stuff *,
const unsigned char *cmd, size_t,
void *buffer, size_t size, unsigned int timeout, int);
@@ -577,56 +576,57 @@ static void do_mcdx_request(request_queu
if (!req)
return;
+ if (!blk_fs_request(req)) {
+ end_request(req, 0);
+ goto again;
+ }
+
stuffp = req->rq_disk->private_data;
if (!stuffp->present) {
xwarn("do_request(): bad device: %s\n",req->rq_disk->disk_name);
xtrace(REQUEST, "end_request(0): bad device\n");
end_request(req, 0);
- return;
+ goto again;
}
if (stuffp->audio) {
xwarn("do_request() attempt to read from audio cd\n");
xtrace(REQUEST, "end_request(0): read from audio\n");
end_request(req, 0);
- return;
+ goto again;
}
xtrace(REQUEST, "do_request() (%lu + %lu)\n",
req->sector, req->nr_sectors);
- if (req->cmd != READ) {
+ if (rq_data_dir(req) != READ) {
xwarn("do_request(): non-read command to cd!!\n");
xtrace(REQUEST, "end_request(0): write\n");
end_request(req, 0);
- return;
- }
- else {
- stuffp->status = 0;
- while (req->nr_sectors) {
- int i;
-
- i = mcdx_transfer(stuffp,
- req->buffer,
- req->sector,
- req->nr_sectors);
-
- if (i == -1) {
- end_request(req, 0);
- goto again;
- }
- req->sector += i;
- req->nr_sectors -= i;
- req->buffer += (i * 512);
- }
- end_request(req, 1);
goto again;
-
- xtrace(REQUEST, "end_request(1)\n");
- end_request(req, 1);
}
+ stuffp->status = 0;
+ while (req->nr_sectors) {
+ int i;
+
+ spin_unlock_irq(q->queue_lock);
+ i = mcdx_transfer(stuffp,
+ req->buffer,
+ req->sector,
+ req->nr_sectors);
+ spin_lock_irq(q->queue_lock);
+
+ if (i == -1) {
+ end_request(req, 0);
+ goto again;
+ }
+ req->sector += i;
+
Re: mcdx -- do_request(): non-read command to cd!!
On Mon, 2 Apr 2007, Jens Axboe wrote:
> Updated patch attached :-)
>
> diff --git a/Documentation/feature-removal-schedule.txt
> b/Documentation/feature-removal-schedule.txt
> index 0bc8b0b..cff761a 100644
> --- a/Documentation/feature-removal-schedule.txt
> +++ b/Documentation/feature-removal-schedule.txt
> @@ -324,3 +324,10 @@ Why: the i8xx_tco watchdog driver has been replaced
> by the iTCO_wdt
> Who: Wim Van Sebroeck <[EMAIL PROTECTED]>
>
> ---
> +
> +What:The legacy CDROM drivers (drivers/cdrom/, except cdrom.c)
> +When:In 2.6.23
> +Why: They are all terminally broken (most don't even compile)
> +Who: Jens Axboe <[EMAIL PROTECTED]>
> +
> +---
I am certainly ok with that. But until then, update patch below =)
Pekka
diff --git a/drivers/cdrom/mcdx.c b/drivers/cdrom/mcdx.c
index f574962..a1e4297 100644
--- a/drivers/cdrom/mcdx.c
+++ b/drivers/cdrom/mcdx.c
@@ -577,56 +577,57 @@ static void do_mcdx_request(request_queu
if (!req)
return;
+ if (!blk_fs_request(req)) {
+ end_request(req, 0);
+ goto again;
+ }
+
stuffp = req->rq_disk->private_data;
if (!stuffp->present) {
xwarn("do_request(): bad device: %s\n",req->rq_disk->disk_name);
xtrace(REQUEST, "end_request(0): bad device\n");
end_request(req, 0);
- return;
+ goto again;
}
if (stuffp->audio) {
xwarn("do_request() attempt to read from audio cd\n");
xtrace(REQUEST, "end_request(0): read from audio\n");
end_request(req, 0);
- return;
+ goto again;
}
xtrace(REQUEST, "do_request() (%lu + %lu)\n",
req->sector, req->nr_sectors);
- if (req->cmd != READ) {
+ if (rq_data_dir(req) != READ) {
xwarn("do_request(): non-read command to cd!!\n");
xtrace(REQUEST, "end_request(0): write\n");
end_request(req, 0);
- return;
- }
- else {
- stuffp->status = 0;
- while (req->nr_sectors) {
- int i;
-
- i = mcdx_transfer(stuffp,
- req->buffer,
- req->sector,
- req->nr_sectors);
-
- if (i == -1) {
- end_request(req, 0);
- goto again;
- }
- req->sector += i;
- req->nr_sectors -= i;
- req->buffer += (i * 512);
- }
- end_request(req, 1);
goto again;
-
- xtrace(REQUEST, "end_request(1)\n");
- end_request(req, 1);
}
+ stuffp->status = 0;
+ while (req->nr_sectors) {
+ int i;
+
+ spin_unlock_irq(q->queue_lock);
+ i = mcdx_transfer(stuffp,
+ req->buffer,
+ req->sector,
+ req->nr_sectors);
+ spin_lock_irq(q->queue_lock);
+
+ if (i == -1) {
+ end_request(req, 0);
+ goto again;
+ }
+ req->sector += i;
+ req->nr_sectors -= i;
+ req->buffer += (i * 512);
+ }
+ end_request(req, 1);
goto again;
}
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: mcdx -- do_request(): non-read command to cd!!
On Mon, Apr 02 2007, Pekka J Enberg wrote:
> On Mon, 2 Apr 2007, Rene Herman wrote:
> > + if (!blk_fs_request(req)) {
> > + spin_lock_irq(q->queue_lock);
>
> Contrary to what I said, this spin_lock_irq() is wrong...
>
> > + end_request(req, 0);
> > + goto again;
> > + }
> > +
> > + spin_unlock_irq(q->queue_lock);
>
> ...because we drop the lock here.
well you did make it overly complex, you should just have dropped the
lock around
spin_unlock_irq(q->queue_lock);
i = mcdx_transfer(...);
spin_lock_irq(q->queue_lock);
and be done with it. But as I wrote earlier, it'll take lots more to
make this driver close to functional. For one, most of the return
statements in do_mcdx_request() really want to be goto again's.
Updated patch attached :-)
diff --git a/Documentation/feature-removal-schedule.txt
b/Documentation/feature-removal-schedule.txt
index 0bc8b0b..cff761a 100644
--- a/Documentation/feature-removal-schedule.txt
+++ b/Documentation/feature-removal-schedule.txt
@@ -324,3 +324,10 @@ Why: the i8xx_tco watchdog driver has been replaced
by the iTCO_wdt
Who: Wim Van Sebroeck <[EMAIL PROTECTED]>
---
+
+What: The legacy CDROM drivers (drivers/cdrom/, except cdrom.c)
+When: In 2.6.23
+Why: They are all terminally broken (most don't even compile)
+Who: Jens Axboe <[EMAIL PROTECTED]>
+
+---
--
Jens Axboe
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: mcdx -- do_request(): non-read command to cd!!
Hi Rene, On 4/2/07, Rene Herman <[EMAIL PROTECTED]> wrote: [EMAIL PROTECTED]:~# dd if=/dev/mcdx0 of=/dev/null bs=2048 0+0 records in 0+0 records out 0 bytes (0 B) copied, 0.000221955 seconds, 0.0 kB/s [EMAIL PROTECTED]:~# This I know isn't: [EMAIL PROTECTED]:~# readcd dev=/dev/mcdx0 f=/dev/null Segmentation fault [EMAIL PROTECTED]:~# (leaves a "note: readcd[1174] exited with preempt_count 1" in the log) and after a "mount -t iso9660 /dev/mcdx0 /mnt/cdrom", a: [EMAIL PROTECTED]:~# tar cv /mnt/cdrom >/dev/null has upto now done all of: [snip] Try with CONFIG_PREEMPT_NONE as suggested by Jens. Can we see strace for the dd, readcd, and tar runs, please? - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: mcdx -- do_request(): non-read command to cd!!
On Mon, 2 Apr 2007, Rene Herman wrote:
> + if (!blk_fs_request(req)) {
> + spin_lock_irq(q->queue_lock);
Contrary to what I said, this spin_lock_irq() is wrong...
> + end_request(req, 0);
> + goto again;
> + }
> +
> + spin_unlock_irq(q->queue_lock);
...because we drop the lock here.
Pekka
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: mcdx -- do_request(): non-read command to cd!!
On Mon, Apr 02 2007, Rene Herman wrote: > On 04/01/2007 12:06 PM, Pekka Enberg wrote: > > >Looks like mcdx_xfer is sleeping while holding q->queue_lock. The > >attached (untested) patch should fix it. > > This (including your followup) does indeed avoid the traces in the > kernel log, but unfortunately, the driver seems to need a bit more. I took a quick look at the driver, and it's pretty much utter crap so I would not expect much. If you want to use it, make sure you at least disable any form of SMP and preempt. > Last time Al Viro suggested ripping them out, I slightly objected: > > http://lkml.org/lkml/2004/5/2/123 > > but well, although I like playing with this stuff, I still don't know > the first thing about the block layer and given the shape these things > are in... The questionable quality of the ancient CDROM drivers itself makes it appealing to just dump it. -- Jens Axboe - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: mcdx -- do_request(): non-read command to cd!!
On 04/02/2007 02:02 AM, Rene Herman wrote:
On 04/01/2007 12:06 PM, Pekka Enberg wrote:
Looks like mcdx_xfer is sleeping while holding q->queue_lock. The
attached (untested) patch should fix it.
This (including your followup) does indeed avoid the traces in the
kernel log, but unfortunately, the driver seems to need a bit more.
This may be expected, I'm not sure:
[EMAIL PROTECTED]:~# dd if=/dev/mcdx0 of=/dev/null bs=2048
0+0 records in
0+0 records out
0 bytes (0 B) copied, 0.000221955 seconds, 0.0 kB/s
[EMAIL PROTECTED]:~#
This I know isn't:
[EMAIL PROTECTED]:~# readcd dev=/dev/mcdx0 f=/dev/null
Segmentation fault
[EMAIL PROTECTED]:~#
(leaves a "note: readcd[1174] exited with preempt_count 1" in the log)
and after a "mount -t iso9660 /dev/mcdx0 /mnt/cdrom", a:
[EMAIL PROTECTED]:~# tar cv /mnt/cdrom >/dev/null
has upto now done all of:
1) segfault
2) make the kernel oops
3) reset the machine
This thing is just so badly broken... ;-(
I've attached the current patches from Jens and yourself (against
2.6.20.4) as a double check, but the driver's still totally unuseable
even with them...
No, I didn't.
I own two other legacy drives; a Panasonic CR562 (sbpcd.c) and a Sony
CDU33A (cdu31a.c) which together with this Mitsumi LU005S (mcdx.c) make
up also all but one of the controllers I have; a few standalone, but
most on old ISA soundcards. When I last tested, cdu31a somewhat worked
and sbpcd didn't. Had high hopes for mcdx.c upon seeing it compile
without warnings, but alas.
Last time Al Viro suggested ripping them out, I slightly objected:
http://lkml.org/lkml/2004/5/2/123
but well, although I like playing with this stuff, I still don't know
the first thing about the block layer and given the shape these things
are in...
Many thanks for looking though!
Rene.
--- drivers/cdrom/mcdx.c.orig 2007-04-02 00:25:09.0 +0200
+++ drivers/cdrom/mcdx.c2007-04-02 00:37:53.0 +0200
@@ -577,11 +577,20 @@
if (!req)
return;
+ if (!blk_fs_request(req)) {
+ spin_lock_irq(q->queue_lock);
+ end_request(req, 0);
+ goto again;
+ }
+
+ spin_unlock_irq(q->queue_lock);
+
stuffp = req->rq_disk->private_data;
if (!stuffp->present) {
xwarn("do_request(): bad device: %s\n",req->rq_disk->disk_name);
xtrace(REQUEST, "end_request(0): bad device\n");
+ spin_lock_irq(q->queue_lock);
end_request(req, 0);
return;
}
@@ -589,6 +598,7 @@
if (stuffp->audio) {
xwarn("do_request() attempt to read from audio cd\n");
xtrace(REQUEST, "end_request(0): read from audio\n");
+ spin_lock_irq(q->queue_lock);
end_request(req, 0);
return;
}
@@ -596,9 +606,10 @@
xtrace(REQUEST, "do_request() (%lu + %lu)\n",
req->sector, req->nr_sectors);
- if (req->cmd != READ) {
+ if (rq_data_dir(req) != READ) {
xwarn("do_request(): non-read command to cd!!\n");
xtrace(REQUEST, "end_request(0): write\n");
+ spin_lock_irq(q->queue_lock);
end_request(req, 0);
return;
}
@@ -613,6 +624,7 @@
req->nr_sectors);
if (i == -1) {
+ spin_lock_irq(q->queue_lock);
end_request(req, 0);
goto again;
}
@@ -620,10 +632,12 @@
req->nr_sectors -= i;
req->buffer += (i * 512);
}
+ spin_lock_irq(q->queue_lock);
end_request(req, 1);
goto again;
xtrace(REQUEST, "end_request(1)\n");
+ spin_lock_irq(q->queue_lock);
end_request(req, 1);
}
Re: mcdx -- do_request(): non-read command to cd!!
On 04/01/2007 12:06 PM, Pekka Enberg wrote: Looks like mcdx_xfer is sleeping while holding q->queue_lock. The attached (untested) patch should fix it. This (including your followup) does indeed avoid the traces in the kernel log, but unfortunately, the driver seems to need a bit more. This may be expected, I'm not sure: [EMAIL PROTECTED]:~# dd if=/dev/mcdx0 of=/dev/null bs=2048 0+0 records in 0+0 records out 0 bytes (0 B) copied, 0.000221955 seconds, 0.0 kB/s [EMAIL PROTECTED]:~# This I know isn't: [EMAIL PROTECTED]:~# readcd dev=/dev/mcdx0 f=/dev/null Segmentation fault [EMAIL PROTECTED]:~# (leaves a "note: readcd[1174] exited with preempt_count 1" in the log) and after a "mount -t iso9660 /dev/mcdx0 /mnt/cdrom", a: [EMAIL PROTECTED]:~# tar cv /mnt/cdrom >/dev/null has upto now done all of: 1) segfault 2) make the kernel oops 3) reset the machine This thing is just so badly broken... ;-( I've attached the current patches from Jens and yourself (against 2.6.20.4) as a double check, but the driver's still totally unuseable even with them... I own two other legacy drives; a Panasonic CR562 (sbpcd.c) and a Sony CDU33A (cdu31a.c) which together with this Mitsumi LU005S (mcdx.c) make up also all but one of the controllers I have; a few standalone, but most on old ISA soundcards. When I last tested, cdu31a somewhat worked and sbpcd didn't. Had high hopes for mcdx.c upon seeing it compile without warnings, but alas. Last time Al Viro suggested ripping them out, I slightly objected: http://lkml.org/lkml/2004/5/2/123 but well, although I like playing with this stuff, I still don't know the first thing about the block layer and given the shape these things are in... Many thanks for looking though! Rene. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: mcdx -- do_request(): non-read command to cd!!
On 4/1/07, Pekka Enberg <[EMAIL PROTECTED]> wrote: Looks like mcdx_xfer is sleeping while holding q->queue_lock. The attached (untested) patch should fix it. You also need to add a spin_lock_irq() before the call to end_request() to Jens' patch. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: mcdx -- do_request(): non-read command to cd!!
On 3/31/07, Rene Herman <[EMAIL PROTECTED]> wrote: There's quite a bit of noise in dmesg though. Repeated 5 times: ===BUG: scheduling while atomic: mount/0x0001/1166 [] __sched_text_start+0x57/0x574 [] schedule_timeout+0x70/0x8f [] process_timeout+0x0/0x5 [] interruptible_sleep_on_timeout+0x5d/0xa5 [] default_wake_function+0x0/0xc [] mcdx_xfer+0xae/0x2a5 [mcdx] [snip] [] do_mcdx_request+0x9b/0xd2 [mcdx] [] __generic_unplug_device+0x1d/0x1f [] generic_unplug_device+0x11/0x29 Looks like mcdx_xfer is sleeping while holding q->queue_lock. The attached (untested) patch should fix it. mcdx-drop-queue_lock-before-sleeping Description: Binary data
Re: mcdx -- do_request(): non-read command to cd!!
On 03/31/2007 08:47 AM, Jens Axboe wrote:
Try this.
diff --git a/drivers/cdrom/mcdx.c b/drivers/cdrom/mcdx.c
index f574962..7086313 100644
--- a/drivers/cdrom/mcdx.c
+++ b/drivers/cdrom/mcdx.c
@@ -577,6 +577,11 @@ static void do_mcdx_request(request_queue_t * q)
if (!req)
return;
+ if (!blk_fs_request(req)) {
+ end_request(req, 0);
+ goto again;
+ }
+
stuffp = req->rq_disk->private_data;
if (!stuffp->present) {
@@ -596,7 +601,7 @@ static void do_mcdx_request(request_queue_t * q)
xtrace(REQUEST, "do_request() (%lu + %lu)\n",
req->sector, req->nr_sectors);
- if (req->cmd != READ) {
+ if (rq_data_dir(req) != READ) {
xwarn("do_request(): non-read command to cd!!\n");
xtrace(REQUEST, "end_request(0): write\n");
end_request(req, 0);
Thank you! Yes, that works in so far that it now indeed does actually
mount the CD:
[EMAIL PROTECTED]:~# mount -t iso9660 /dev/mcdx0 /mnt/cdrom
mount: block device /dev/mcdx0 is write-protected, mounting read-only
[EMAIL PROTECTED]:~# ls /mnt/cdrom/
dott dott.exe dottdemo indydemo rebel samdemo
There's quite a bit of noise in dmesg though. Repeated 5 times:
===BUG: scheduling while atomic: mount/0x0001/1166
[] __sched_text_start+0x57/0x574
[] schedule_timeout+0x70/0x8f
[] process_timeout+0x0/0x5
[] interruptible_sleep_on_timeout+0x5d/0xa5
[] default_wake_function+0x0/0xc
[] mcdx_xfer+0xae/0x2a5 [mcdx]
[] cfq_init_prio_data+0x57/0x12a
[] cfq_get_queue+0x119/0x16e
[] cfq_set_request+0x0/0x131
[] elv_set_request+0x14/0x22
[] elv_rb_del+0x23/0x31
[] cfq_remove_request+0x63/0xd9
[] elv_dispatch_sort+0x1c/0x67
[] cfq_dispatch_insert+0x38/0x4c
[] __cfq_dispatch_requests+0x72/0x1ad
[] cfq_dispatch_requests+0x50/0x77
[] sync_buffer+0x0/0x2e
[] elv_next_request+0x5d/0x105
[] sync_buffer+0x0/0x2e
[] do_mcdx_request+0x9b/0xd2 [mcdx]
[] __generic_unplug_device+0x1d/0x1f
[] generic_unplug_device+0x11/0x29
[] blk_backing_dev_unplug+0xc/0xd
[] sync_buffer+0x26/0x2e
[] __wait_on_bit+0x2c/0x51
[] out_of_line_wait_on_bit+0x6f/0x77
[] sync_buffer+0x0/0x2e
[] wake_bit_function+0x0/0x3c
[] wake_bit_function+0x0/0x3c
[] __wait_on_buffer+0x22/0x25
[] __bread_slow+0x4b/0x60
[] __bread+0x18/0x1d
[] isofs_fill_super+0xf0/0x5d7 [isofs]
[] radix_tree_delete+0x177/0x1a0
[] get_sb_bdev+0xc6/0x10f
[] mntput_no_expire+0x11/0x73
[] alloc_vfsmnt+0x97/0xbe
[] isofs_get_sb+0x20/0x25 [isofs]
[] isofs_fill_super+0x0/0x5d7 [isofs]
[] vfs_kern_mount+0x40/0x6f
[] do_kern_mount+0x2a/0x3a
[] do_new_mount+0x64/0x93
[] do_mount+0x185/0x19a
[] __wake_up_locked+0x1e/0x20
[] default_wake_function+0x0/0xc
[] sys_mount+0x79/0xb5
[] syscall_call+0x7/0xb
===
Any access results in te above. A copy from the CD segfaulted:
===
[EMAIL PROTECTED]:~# cp /mnt/cdrom/dott.exe .
malloc: unwind_prot.c:247: assertion botched
free: called with unallocated block argument
Segmentation fault
===
This sounds like a userspace problem but I sure haven't seen it before.
There's a few "blk: request botched" followed by similar backtraces in
the log after that.
It looks like mcdx_xfer() is seriously broken. If you don't particularly
care for spending time on this thing -- feel free. To top it off, if you
unload the module, it doesn't clean up after itself and the IRQ is kept
in use (and a cat /proc/interrupts faults).
I'm only using the thing for the heck of it...
Rene.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: mcdx -- do_request(): non-read command to cd!!
On Fri, Mar 30 2007, Rene Herman wrote:
> Hi Al.
>
> GIT doesn't remember, it's been too long, but IIRC you were the last one
> to do some work on mcdx (the old proprietary mitsumi cd-rom driver). The
> thing builds without warnings on 2.6.20.4, unlike most other proprietary
> CD-ROM drivers, so someone did...
>
> In any case, I just bet you're positively thrilled receiving bug-reports
> for the thing right? Mmm?
>
> I dug up a 1-speed Mitsumi CRMC-LU005S today. Brilliant drive! You push
> on the front, after which it comes loose and you then yank the entire
> drive, mechanism and all, out of its casing over some kind of magnetic
> resistance it seems and then open a _second_ top-loading door, put in
> the CD and follow the procedure backwards again. I've done that at least
> 20 times now and I'm not by any means done yet. Brilliant.
>
> The drive works fine under DOS (*), with both IRQ-less and IRQ-enabled
> controllers. The linux driver does not work though:
>
> [EMAIL PROTECTED]:~# modprobe mcdx
>
> [EMAIL PROTECTED]:~# dmesg | tail -4
> mcdx Version 2.14(hs)
> mcdx $Id: mcdx.c,v 1.21 1997/01/26 07:12:59 davem Exp $
> Uniform CD-ROM driver Revision: 3.20
> mcdx: Mitsumi CD-ROM installed at 0x300, irq 15. (Firmware version M 4)
>
> [EMAIL PROTECTED]:~# mount /dev/mcdx0 /mnt/cdrom
> mount: block device /dev/mcdx0 is write-protected, mounting read-only
> mount: /dev/mcdx0: can't read superblock
>
> [EMAIL PROTECTED]:~# dmesg | tail -4
> mcdx: Mitsumi CD-ROM installed at 0x300, irq 15. (Firmware version M 4)
> mcdx do_request(): non-read command to cd!!
> end_request: I/O error, dev mcdx0, sector 0
> FAT: unable to read boot sector
> [EMAIL PROTECTED]:~#
>
> This same 300/15 pair works under DOS in the same machine and IRQ15 is
> firing. The error sounds very block-ish. Would you happen to know?
>
> I'll happily test patches :-)
Try this.
diff --git a/drivers/cdrom/mcdx.c b/drivers/cdrom/mcdx.c
index f574962..7086313 100644
--- a/drivers/cdrom/mcdx.c
+++ b/drivers/cdrom/mcdx.c
@@ -577,6 +577,11 @@ static void do_mcdx_request(request_queue_t * q)
if (!req)
return;
+ if (!blk_fs_request(req)) {
+ end_request(req, 0);
+ goto again;
+ }
+
stuffp = req->rq_disk->private_data;
if (!stuffp->present) {
@@ -596,7 +601,7 @@ static void do_mcdx_request(request_queue_t * q)
xtrace(REQUEST, "do_request() (%lu + %lu)\n",
req->sector, req->nr_sectors);
- if (req->cmd != READ) {
+ if (rq_data_dir(req) != READ) {
xwarn("do_request(): non-read command to cd!!\n");
xtrace(REQUEST, "end_request(0): write\n");
end_request(req, 0);
--
Jens Axboe
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
mcdx -- do_request(): non-read command to cd!!
Hi Al. GIT doesn't remember, it's been too long, but IIRC you were the last one to do some work on mcdx (the old proprietary mitsumi cd-rom driver). The thing builds without warnings on 2.6.20.4, unlike most other proprietary CD-ROM drivers, so someone did... In any case, I just bet you're positively thrilled receiving bug-reports for the thing right? Mmm? I dug up a 1-speed Mitsumi CRMC-LU005S today. Brilliant drive! You push on the front, after which it comes loose and you then yank the entire drive, mechanism and all, out of its casing over some kind of magnetic resistance it seems and then open a _second_ top-loading door, put in the CD and follow the procedure backwards again. I've done that at least 20 times now and I'm not by any means done yet. Brilliant. The drive works fine under DOS (*), with both IRQ-less and IRQ-enabled controllers. The linux driver does not work though: [EMAIL PROTECTED]:~# modprobe mcdx [EMAIL PROTECTED]:~# dmesg | tail -4 mcdx Version 2.14(hs) mcdx $Id: mcdx.c,v 1.21 1997/01/26 07:12:59 davem Exp $ Uniform CD-ROM driver Revision: 3.20 mcdx: Mitsumi CD-ROM installed at 0x300, irq 15. (Firmware version M 4) [EMAIL PROTECTED]:~# mount /dev/mcdx0 /mnt/cdrom mount: block device /dev/mcdx0 is write-protected, mounting read-only mount: /dev/mcdx0: can't read superblock [EMAIL PROTECTED]:~# dmesg | tail -4 mcdx: Mitsumi CD-ROM installed at 0x300, irq 15. (Firmware version M 4) mcdx do_request(): non-read command to cd!! end_request: I/O error, dev mcdx0, sector 0 FAT: unable to read boot sector [EMAIL PROTECTED]:~# This same 300/15 pair works under DOS in the same machine and IRQ15 is firing. The error sounds very block-ish. Would you happen to know? I'll happily test patches :-) Rene. (*) Mitsumi not only has the old DOS driver online, it even keeps a scanned manual available: http://www.mitsumi.com/enduser/1_drivers.html#Manuals - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
