* mcdx -- do_request(): non-read command to cd!! @ 2007-03-30 21:21 Rene Herman 2007-03-31 6:47 ` Jens Axboe 0 siblings, 1 reply; 29+ messages in thread From: Rene Herman @ 2007-03-30 21:21 UTC (permalink / raw) To: Al Viro; +Cc: Linux Kernel 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: root@5va2:~# modprobe mcdx root@5va2:~# 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) root@5va2:~# mount /dev/mcdx0 /mnt/cdrom mount: block device /dev/mcdx0 is write-protected, mounting read-only mount: /dev/mcdx0: can't read superblock root@5va2:~# 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 root@5va2:~# 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 ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: mcdx -- do_request(): non-read command to cd!! 2007-03-30 21:21 mcdx -- do_request(): non-read command to cd!! Rene Herman @ 2007-03-31 6:47 ` Jens Axboe 2007-03-31 18:23 ` Rene Herman 0 siblings, 1 reply; 29+ messages in thread From: Jens Axboe @ 2007-03-31 6:47 UTC (permalink / raw) To: Rene Herman; +Cc: Al Viro, Linux Kernel 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: > > root@5va2:~# modprobe mcdx > > root@5va2:~# 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) > > root@5va2:~# mount /dev/mcdx0 /mnt/cdrom > mount: block device /dev/mcdx0 is write-protected, mounting read-only > mount: /dev/mcdx0: can't read superblock > > root@5va2:~# 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 > root@5va2:~# > > 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 ^ permalink raw reply related [flat|nested] 29+ messages in thread
* Re: mcdx -- do_request(): non-read command to cd!! 2007-03-31 6:47 ` Jens Axboe @ 2007-03-31 18:23 ` Rene Herman 2007-04-01 10:06 ` Pekka Enberg 0 siblings, 1 reply; 29+ messages in thread From: Rene Herman @ 2007-03-31 18:23 UTC (permalink / raw) To: Jens Axboe; +Cc: Al Viro, Linux Kernel 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: root@5va2:~# mount -t iso9660 /dev/mcdx0 /mnt/cdrom mount: block device /dev/mcdx0 is write-protected, mounting read-only root@5va2:~# 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/0x00000001/1166 [<c1170bff>] __sched_text_start+0x57/0x574 [<c1171964>] schedule_timeout+0x70/0x8f [<c10199b2>] process_timeout+0x0/0x5 [<c11716a2>] interruptible_sleep_on_timeout+0x5d/0xa5 [<c100e695>] default_wake_function+0x0/0xc [<c482b8fb>] mcdx_xfer+0xae/0x2a5 [mcdx] [<c109bc78>] cfq_init_prio_data+0x57/0x12a [<c109bef2>] cfq_get_queue+0x119/0x16e [<c109c73b>] cfq_set_request+0x0/0x131 [<c1094f29>] elv_set_request+0x14/0x22 [<c10948c7>] elv_rb_del+0x23/0x31 [<c109b186>] cfq_remove_request+0x63/0xd9 [<c1094911>] elv_dispatch_sort+0x1c/0x67 [<c109b63c>] cfq_dispatch_insert+0x38/0x4c [<c109b726>] __cfq_dispatch_requests+0x72/0x1ad [<c109b95f>] cfq_dispatch_requests+0x50/0x77 [<c10622eb>] sync_buffer+0x0/0x2e [<c1094de9>] elv_next_request+0x5d/0x105 [<c10622eb>] sync_buffer+0x0/0x2e [<c482b473>] do_mcdx_request+0x9b/0xd2 [mcdx] [<c1096d06>] __generic_unplug_device+0x1d/0x1f [<c1096d19>] generic_unplug_device+0x11/0x29 [<c1096d3d>] blk_backing_dev_unplug+0xc/0xd [<c1062311>] sync_buffer+0x26/0x2e [<c11719d3>] __wait_on_bit+0x2c/0x51 [<c1171a67>] out_of_line_wait_on_bit+0x6f/0x77 [<c10622eb>] sync_buffer+0x0/0x2e [<c102218c>] wake_bit_function+0x0/0x3c [<c102218c>] wake_bit_function+0x0/0x3c [<c1062371>] __wait_on_buffer+0x22/0x25 [<c10631c1>] __bread_slow+0x4b/0x60 [<c10633ad>] __bread+0x18/0x1d [<c486f9cf>] isofs_fill_super+0xf0/0x5d7 [isofs] [<c10a016d>] radix_tree_delete+0x177/0x1a0 [<c104971f>] get_sb_bdev+0xc6/0x10f [<c1059af0>] mntput_no_expire+0x11/0x73 [<c10596bf>] alloc_vfsmnt+0x97/0xbe [<c4870863>] isofs_get_sb+0x20/0x25 [isofs] [<c486f8df>] isofs_fill_super+0x0/0x5d7 [isofs] [<c10498ea>] vfs_kern_mount+0x40/0x6f [<c1049943>] do_kern_mount+0x2a/0x3a [<c105a8e0>] do_new_mount+0x64/0x93 [<c105af29>] do_mount+0x185/0x19a [<c100e75c>] __wake_up_locked+0x1e/0x20 [<c100e695>] default_wake_function+0x0/0xc [<c105b1c4>] sys_mount+0x79/0xb5 [<c1002830>] syscall_call+0x7/0xb === Any access results in te above. A copy from the CD segfaulted: === root@5va2:~# 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. ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: mcdx -- do_request(): non-read command to cd!! 2007-03-31 18:23 ` Rene Herman @ 2007-04-01 10:06 ` Pekka Enberg 2007-04-01 10:16 ` Pekka Enberg 2007-04-02 0:02 ` Rene Herman 0 siblings, 2 replies; 29+ messages in thread From: Pekka Enberg @ 2007-04-01 10:06 UTC (permalink / raw) To: Rene Herman; +Cc: Jens Axboe, Al Viro, Linux Kernel [-- Attachment #1: Type: text/plain, Size: 729 bytes --] On 3/31/07, Rene Herman <rene.herman@gmail.com> wrote: > There's quite a bit of noise in dmesg though. Repeated 5 times: > > ===BUG: scheduling while atomic: mount/0x00000001/1166 > [<c1170bff>] __sched_text_start+0x57/0x574 > [<c1171964>] schedule_timeout+0x70/0x8f > [<c10199b2>] process_timeout+0x0/0x5 > [<c11716a2>] interruptible_sleep_on_timeout+0x5d/0xa5 > [<c100e695>] default_wake_function+0x0/0xc > [<c482b8fb>] mcdx_xfer+0xae/0x2a5 [mcdx] [snip] > [<c482b473>] do_mcdx_request+0x9b/0xd2 [mcdx] > [<c1096d06>] __generic_unplug_device+0x1d/0x1f > [<c1096d19>] generic_unplug_device+0x11/0x29 Looks like mcdx_xfer is sleeping while holding q->queue_lock. The attached (untested) patch should fix it. [-- Attachment #2: mcdx-drop-queue_lock-before-sleeping --] [-- Type: application/octet-stream, Size: 1588 bytes --] diff --git a/drivers/cdrom/mcdx.c b/drivers/cdrom/mcdx.c index f574962..e14dc03 100644 --- a/drivers/cdrom/mcdx.c +++ b/drivers/cdrom/mcdx.c @@ -577,11 +577,14 @@ static void do_mcdx_request(request_queue_t * q) if (!req) return; + 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 +592,7 @@ static void do_mcdx_request(request_queue_t * q) 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; } @@ -599,6 +603,7 @@ static void do_mcdx_request(request_queue_t * q) if (req->cmd != 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 +618,7 @@ static void do_mcdx_request(request_queue_t * q) req->nr_sectors); if (i == -1) { + spin_lock_irq(q->queue_lock); end_request(req, 0); goto again; } @@ -620,10 +626,12 @@ static void do_mcdx_request(request_queue_t * q) 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); } ^ permalink raw reply related [flat|nested] 29+ messages in thread
* Re: mcdx -- do_request(): non-read command to cd!! 2007-04-01 10:06 ` Pekka Enberg @ 2007-04-01 10:16 ` Pekka Enberg 2007-04-02 0:02 ` Rene Herman 1 sibling, 0 replies; 29+ messages in thread From: Pekka Enberg @ 2007-04-01 10:16 UTC (permalink / raw) To: Rene Herman; +Cc: Jens Axboe, Al Viro, Linux Kernel On 4/1/07, Pekka Enberg <penberg@cs.helsinki.fi> 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. ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: mcdx -- do_request(): non-read command to cd!! 2007-04-01 10:06 ` Pekka Enberg 2007-04-01 10:16 ` Pekka Enberg @ 2007-04-02 0:02 ` Rene Herman 2007-04-02 0:07 ` Rene Herman ` (2 more replies) 1 sibling, 3 replies; 29+ messages in thread From: Rene Herman @ 2007-04-02 0:02 UTC (permalink / raw) To: Pekka Enberg; +Cc: Jens Axboe, Al Viro, Linux Kernel 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: root@5va2:~# 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 root@5va2:~# This I know isn't: root@5va2:~# readcd dev=/dev/mcdx0 f=/dev/null Segmentation fault root@5va2:~# (leaves a "note: readcd[1174] exited with preempt_count 1" in the log) and after a "mount -t iso9660 /dev/mcdx0 /mnt/cdrom", a: root@5va2:~# 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. ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: mcdx -- do_request(): non-read command to cd!! 2007-04-02 0:02 ` Rene Herman @ 2007-04-02 0:07 ` Rene Herman 2007-04-02 6:50 ` Pekka J Enberg 2007-04-02 6:42 ` Jens Axboe 2007-04-02 7:07 ` Pekka Enberg 2 siblings, 1 reply; 29+ messages in thread From: Rene Herman @ 2007-04-02 0:07 UTC (permalink / raw) To: Pekka Enberg; +Cc: Jens Axboe, Al Viro, Linux Kernel [-- Attachment #1: Type: text/plain, Size: 1885 bytes --] 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: > > root@5va2:~# 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 > root@5va2:~# > > This I know isn't: > > root@5va2:~# readcd dev=/dev/mcdx0 f=/dev/null > Segmentation fault > root@5va2:~# > > (leaves a "note: readcd[1174] exited with preempt_count 1" in the log) > > and after a "mount -t iso9660 /dev/mcdx0 /mnt/cdrom", a: > > root@5va2:~# 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. [-- Attachment #2: mcdx.diff --] [-- Type: text/plain, Size: 1562 bytes --] --- drivers/cdrom/mcdx.c.orig 2007-04-02 00:25:09.000000000 +0200 +++ drivers/cdrom/mcdx.c 2007-04-02 00:37:53.000000000 +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); } ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: mcdx -- do_request(): non-read command to cd!! 2007-04-02 0:07 ` Rene Herman @ 2007-04-02 6:50 ` Pekka J Enberg 2007-04-02 7:10 ` Jens Axboe 0 siblings, 1 reply; 29+ messages in thread From: Pekka J Enberg @ 2007-04-02 6:50 UTC (permalink / raw) To: Rene Herman; +Cc: Jens Axboe, Al Viro, Linux Kernel 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 ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: mcdx -- do_request(): non-read command to cd!! 2007-04-02 6:50 ` Pekka J Enberg @ 2007-04-02 7:10 ` Jens Axboe 2007-04-02 7:37 ` Pekka J Enberg ` (2 more replies) 0 siblings, 3 replies; 29+ messages in thread From: Jens Axboe @ 2007-04-02 7:10 UTC (permalink / raw) To: Pekka J Enberg; +Cc: Rene Herman, Al Viro, Linux Kernel 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 <wim@iguana.be> --------------------------- + +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 <jens.axboe@oracle.com> + +--------------------------- -- Jens Axboe ^ permalink raw reply related [flat|nested] 29+ messages in thread
* Re: mcdx -- do_request(): non-read command to cd!! 2007-04-02 7:10 ` Jens Axboe @ 2007-04-02 7:37 ` Pekka J Enberg 2007-04-02 8:55 ` Pekka J Enberg 2007-04-02 15:39 ` Rene Herman 2 siblings, 0 replies; 29+ messages in thread From: Pekka J Enberg @ 2007-04-02 7:37 UTC (permalink / raw) To: Jens Axboe; +Cc: Rene Herman, Al Viro, Linux Kernel 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 <wim@iguana.be> > > --------------------------- > + > +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 <jens.axboe@oracle.com> > + > +--------------------------- 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; } ^ permalink raw reply related [flat|nested] 29+ messages in thread
* Re: mcdx -- do_request(): non-read command to cd!! 2007-04-02 7:10 ` Jens Axboe 2007-04-02 7:37 ` Pekka J Enberg @ 2007-04-02 8:55 ` Pekka J Enberg 2007-04-02 9:32 ` Boaz Harrosh 2007-04-02 15:18 ` Rene Herman 2007-04-02 15:39 ` Rene Herman 2 siblings, 2 replies; 29+ messages in thread From: Pekka J Enberg @ 2007-04-02 8:55 UTC (permalink / raw) To: Jens Axboe; +Cc: Rene Herman, Al Viro, Linux Kernel 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.000000000 +0300 +++ 2.6/drivers/cdrom/mcdx.c 2007-04-02 11:51:20.000000000 +0300 @@ -58,6 +58,7 @@ = "$Id: mcdx.c,v 1.21 1997/01/26 07: #include <linux/module.h> +#include <linux/delay.h> #include <linux/errno.h> #include <linux/interrupt.h> #include <linux/fs.h> @@ -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; + req->nr_sectors -= i; + req->buffer += (i * 512); + } + end_request(req, 1); goto again; } @@ -825,26 +825,6 @@ __setup("mcdx=", mcdx_setup); /* DIRTY PART ******************************************************/ -static void mcdx_delay(struct s_drive_stuff *stuff, long jifs) -/* This routine is used for sleeping. - * A jifs value <0 means NO sleeping, - * =0 means minimal sleeping (let the kernel - * run for other processes) - * >0 means at least sleep for that amount. - * May be we could use a simple count loop w/ jumps to itself, but - * I wanna make this independent of cpu speed. [1 jiffy is 1/HZ] sec */ -{ - if (jifs < 0) - return; - - xtrace(SLEEP, "*** delay: sleepq\n"); - interruptible_sleep_on_timeout(&stuff->sleepq, jifs); - xtrace(SLEEP, "delay awoken\n"); - if (signal_pending(current)) { - xtrace(SLEEP, "got signal\n"); - } -} - static irqreturn_t mcdx_intr(int irq, void *dev_id) { struct s_drive_stuff *stuffp = dev_id; @@ -902,13 +882,7 @@ static int mcdx_talk(struct s_drive_stuf if ((discard = (buffer == NULL))) buffer = &c; - while (stuffp->lock) { - xtrace(SLEEP, "*** talk: lockq\n"); - interruptible_sleep_on(&stuffp->lockq); - xtrace(SLEEP, "talk: awoken\n"); - } - - stuffp->lock = 1; + mutex_lock(&stuffp->mutex); /* An operation other then reading data destroys the * data already requested and remembered in stuffp->request, ... */ @@ -992,8 +966,7 @@ xtrace(TALK, "talk() got 0x%02x\n", * xinfo("talk() giving up\n"); #endif - stuffp->lock = 0; - wake_up_interruptible(&stuffp->lockq); + mutex_unlock(&stuffp->mutex); xtrace(TALK, "talk() done with 0x%02x\n", st); return st; @@ -1106,9 +1079,9 @@ stuffp->present = 0; /* this should be stuffp->wreg_hcon = stuffp->wreg_reset + 1; stuffp->wreg_chn = stuffp->wreg_hcon + 1; + mutex_init(&stuffp->mutex); + init_waitqueue_head(&stuffp->busyq); - init_waitqueue_head(&stuffp->lockq); - init_waitqueue_head(&stuffp->sleepq); /* check if i/o addresses are available */ if (!request_region(stuffp->wreg_data, MCDX_IO_SIZE, "mcdx")) { @@ -1203,7 +1176,7 @@ return 0; xtrace(INIT, "init() get garbage\n"); { int i; - mcdx_delay(stuffp, HZ / 2); + msleep_interruptible(500); for (i = 100; i; i--) (void) inb(stuffp->rreg_status); } @@ -1327,10 +1300,6 @@ int done = 0; return -1; } - while (stuffp->lock) { - interruptible_sleep_on(&stuffp->lockq); - } - if (stuffp->valid && (sector >= stuffp->pending) && (sector < stuffp->low_border)) { @@ -1346,7 +1315,7 @@ int done = 0; sector + nr_sectors) ? stuffp->high_border : border; - stuffp->lock = current->pid; + mutex_lock(&stuffp->mutex); do { @@ -1366,11 +1335,11 @@ int done = 0; } else continue; - stuffp->lock = 0; + mutex_unlock(&stuffp->mutex); + stuffp->busy = 0; stuffp->valid = 0; - wake_up_interruptible(&stuffp->lockq); xtrace(XFER, "transfer() done (-1)\n"); return -1; } @@ -1405,9 +1374,7 @@ insb(stuffp->rreg_data, &dummy[0], C } } while (++(stuffp->pending) < border); - stuffp->lock = 0; - wake_up_interruptible(&stuffp->lockq); - + mutex_unlock(&stuffp->mutex); } else { /* The requested sector(s) is/are out of the @@ -1654,7 +1621,7 @@ cmd[0], cmd[1], cmd[2], cmd[3], outsb(stuffp->wreg_data, cmd, sizeof cmd); - if (-1 == mcdx_getval(stuffp, 3 * HZ, 0, NULL)) { + if (-1 == mcdx_getval(stuffp, 3, 0, NULL)) { xwarn("playmsf() timeout\n"); return -1; } @@ -1907,7 +1874,7 @@ return mcdx_talk(stuffp, "\x40", 1, NUL } static int -mcdx_getval(struct s_drive_stuff *stuffp, int to, int delay, char *buf) +mcdx_getval(struct s_drive_stuff *stuffp, int to, int delay_sec, char *buf) { unsigned long timeout = to + jiffies; char c; @@ -1918,7 +1885,7 @@ mcdx_getval(struct s_drive_stuff *stuffp while (inb(stuffp->rreg_status) & MCDX_RBIT_STEN) { if (time_after(jiffies, timeout)) return -1; - mcdx_delay(stuffp, delay); + msleep_interruptible(delay_sec * 1000); } *buf = (unsigned char) inb(stuffp->rreg_data) & 0xff; ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: mcdx -- do_request(): non-read command to cd!! 2007-04-02 8:55 ` Pekka J Enberg @ 2007-04-02 9:32 ` Boaz Harrosh 2007-04-02 9:42 ` Pekka J Enberg 2007-04-02 9:42 ` Jens Axboe 2007-04-02 15:18 ` Rene Herman 1 sibling, 2 replies; 29+ messages in thread From: Boaz Harrosh @ 2007-04-02 9:32 UTC (permalink / raw) To: Pekka J Enberg; +Cc: Jens Axboe, Rene Herman, Al Viro, Linux Kernel 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.000000000 +0300 > +++ 2.6/drivers/cdrom/mcdx.c 2007-04-02 11:51:20.000000000 +0300 > @@ -58,6 +58,7 @@ = "$Id: mcdx.c,v 1.21 1997/01/26 07: > > #include <linux/module.h> > > +#include <linux/delay.h> > #include <linux/errno.h> > #include <linux/interrupt.h> > #include <linux/fs.h> > @@ -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; > + req->nr_sectors -= i; > + req->buffer += (i * 512); > + } > + end_request(req, 1); > goto again; > } > > @@ -825,26 +825,6 @@ __setup("mcdx=", mcdx_setup); > > /* DIRTY PART ******************************************************/ > > -static void mcdx_delay(struct s_drive_stuff *stuff, long jifs) > -/* This routine is used for sleeping. > - * A jifs value <0 means NO sleeping, > - * =0 means minimal sleeping (let the kernel > - * run for other processes) > - * >0 means at least sleep for that amount. > - * May be we could use a simple count loop w/ jumps to itself, but > - * I wanna make this independent of cpu speed. [1 jiffy is 1/HZ] sec */ > -{ > - if (jifs < 0) > - return; > - > - xtrace(SLEEP, "*** delay: sleepq\n"); > - interruptible_sleep_on_timeout(&stuff->sleepq, jifs); > - xtrace(SLEEP, "delay awoken\n"); > - if (signal_pending(current)) { > - xtrace(SLEEP, "got signal\n"); > - } > -} > - > static irqreturn_t mcdx_intr(int irq, void *dev_id) > { > struct s_drive_stuff *stuffp = dev_id; > @@ -902,13 +882,7 @@ static int mcdx_talk(struct s_drive_stuf > if ((discard = (buffer == NULL))) > buffer = &c; > > - while (stuffp->lock) { > - xtrace(SLEEP, "*** talk: lockq\n"); > - interruptible_sleep_on(&stuffp->lockq); > - xtrace(SLEEP, "talk: awoken\n"); > - } > - > - stuffp->lock = 1; > + mutex_lock(&stuffp->mutex); > > /* An operation other then reading data destroys the > * data already requested and remembered in stuffp->request, ... */ > @@ -992,8 +966,7 @@ xtrace(TALK, "talk() got 0x%02x\n", * > xinfo("talk() giving up\n"); > #endif > > - stuffp->lock = 0; > - wake_up_interruptible(&stuffp->lockq); > + mutex_unlock(&stuffp->mutex); > > xtrace(TALK, "talk() done with 0x%02x\n", st); > return st; > @@ -1106,9 +1079,9 @@ stuffp->present = 0; /* this should be > stuffp->wreg_hcon = stuffp->wreg_reset + 1; > stuffp->wreg_chn = stuffp->wreg_hcon + 1; > > + mutex_init(&stuffp->mutex); > + > init_waitqueue_head(&stuffp->busyq); > - init_waitqueue_head(&stuffp->lockq); > - init_waitqueue_head(&stuffp->sleepq); > > /* check if i/o addresses are available */ > if (!request_region(stuffp->wreg_data, MCDX_IO_SIZE, "mcdx")) { > @@ -1203,7 +1176,7 @@ return 0; > xtrace(INIT, "init() get garbage\n"); > { > int i; > - mcdx_delay(stuffp, HZ / 2); > + msleep_interruptible(500); > for (i = 100; i; i--) > (void) inb(stuffp->rreg_status); > } > @@ -1327,10 +1300,6 @@ int done = 0; > return -1; > } > > - while (stuffp->lock) { > - interruptible_sleep_on(&stuffp->lockq); > - } > - > if (stuffp->valid && (sector >= stuffp->pending) > && (sector < stuffp->low_border)) { > > @@ -1346,7 +1315,7 @@ int done = 0; > sector + nr_sectors) > ? stuffp->high_border : border; > > - stuffp->lock = current->pid; > + mutex_lock(&stuffp->mutex); > > do { > > @@ -1366,11 +1335,11 @@ int done = 0; > } else > continue; > > - stuffp->lock = 0; > + mutex_unlock(&stuffp->mutex); > + > stuffp->busy = 0; > stuffp->valid = 0; > > - wake_up_interruptible(&stuffp->lockq); > xtrace(XFER, "transfer() done (-1)\n"); > return -1; > } > @@ -1405,9 +1374,7 @@ insb(stuffp->rreg_data, &dummy[0], C > } > } while (++(stuffp->pending) < border); > > - stuffp->lock = 0; > - wake_up_interruptible(&stuffp->lockq); > - > + mutex_unlock(&stuffp->mutex); > } else { > > /* The requested sector(s) is/are out of the > @@ -1654,7 +1621,7 @@ cmd[0], cmd[1], cmd[2], cmd[3], > > outsb(stuffp->wreg_data, cmd, sizeof cmd); > > - if (-1 == mcdx_getval(stuffp, 3 * HZ, 0, NULL)) { > + if (-1 == mcdx_getval(stuffp, 3, 0, NULL)) { > xwarn("playmsf() timeout\n"); > return -1; > } > @@ -1907,7 +1874,7 @@ return mcdx_talk(stuffp, "\x40", 1, NUL > } > > static int > -mcdx_getval(struct s_drive_stuff *stuffp, int to, int delay, char *buf) > +mcdx_getval(struct s_drive_stuff *stuffp, int to, int delay_sec, char *buf) > { > unsigned long timeout = to + jiffies; > char c; > @@ -1918,7 +1885,7 @@ mcdx_getval(struct s_drive_stuff *stuffp > while (inb(stuffp->rreg_status) & MCDX_RBIT_STEN) { > if (time_after(jiffies, timeout)) > return -1; > - mcdx_delay(stuffp, delay); > + msleep_interruptible(delay_sec * 1000); > } > > *buf = (unsigned char) inb(stuffp->rreg_data) & 0xff; > - > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > Please read the FAQ at http://www.tux.org/lkml/ NACK Why are you using req->buffer in new code. req->buffer is a leftover from a very old block pc era and is supposed to be killed. Please do not use it in any new code. you should use bio_data(rq->bio) and if you must have a virtual memory pointer hanging around than keep it in private space. Boaz ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: mcdx -- do_request(): non-read command to cd!! 2007-04-02 9:32 ` Boaz Harrosh @ 2007-04-02 9:42 ` Pekka J Enberg 2007-04-02 9:42 ` Jens Axboe 1 sibling, 0 replies; 29+ messages in thread From: Pekka J Enberg @ 2007-04-02 9:42 UTC (permalink / raw) To: Boaz Harrosh; +Cc: Jens Axboe, Rene Herman, Al Viro, Linux Kernel 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 ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: mcdx -- do_request(): non-read command to cd!! 2007-04-02 9:32 ` Boaz Harrosh 2007-04-02 9:42 ` Pekka J Enberg @ 2007-04-02 9:42 ` Jens Axboe 2007-04-02 21:02 ` Rene Herman 1 sibling, 1 reply; 29+ messages in thread From: Jens Axboe @ 2007-04-02 9:42 UTC (permalink / raw) To: Boaz Harrosh; +Cc: Pekka J Enberg, Rene Herman, Al Viro, Linux Kernel 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.000000000 +0300 > > +++ 2.6/drivers/cdrom/mcdx.c 2007-04-02 11:51:20.000000000 +0300 > > @@ -58,6 +58,7 @@ = "$Id: mcdx.c,v 1.21 1997/01/26 07: > > > > #include <linux/module.h> > > > > +#include <linux/delay.h> > > #include <linux/errno.h> > > #include <linux/interrupt.h> > > #include <linux/fs.h> > > @@ -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; > > + req->nr_sectors -= i; > > + req->buffer += (i * 512); > > + } > > + end_request(req, 1); > > goto again; > > } > > > > @@ -825,26 +825,6 @@ __setup("mcdx=", mcdx_setup); > > > > /* DIRTY PART ******************************************************/ > > > > -static void mcdx_delay(struct s_drive_stuff *stuff, long jifs) > > -/* This routine is used for sleeping. > > - * A jifs value <0 means NO sleeping, > > - * =0 means minimal sleeping (let the kernel > > - * run for other processes) > > - * >0 means at least sleep for that amount. > > - * May be we could use a simple count loop w/ jumps to itself, but > > - * I wanna make this independent of cpu speed. [1 jiffy is 1/HZ] sec */ > > -{ > > - if (jifs < 0) > > - return; > > - > > - xtrace(SLEEP, "*** delay: sleepq\n"); > > - interruptible_sleep_on_timeout(&stuff->sleepq, jifs); > > - xtrace(SLEEP, "delay awoken\n"); > > - if (signal_pending(current)) { > > - xtrace(SLEEP, "got signal\n"); > > - } > > -} > > - > > static irqreturn_t mcdx_intr(int irq, void *dev_id) > > { > > struct s_drive_stuff *stuffp = dev_id; > > @@ -902,13 +882,7 @@ static int mcdx_talk(struct s_drive_stuf > > if ((discard = (buffer == NULL))) > > buffer = &c; > > > > - while (stuffp->lock) { > > - xtrace(SLEEP, "*** talk: lockq\n"); > > - interruptible_sleep_on(&stuffp->lockq); > > - xtrace(SLEEP, "talk: awoken\n"); > > - } > > - > > - stuffp->lock = 1; > > + mutex_lock(&stuffp->mutex); > > > > /* An operation other then reading data destroys the > > * data already requested and remembered in stuffp->request, ... */ > > @@ -992,8 +966,7 @@ xtrace(TALK, "talk() got 0x%02x\n", * > > xinfo("talk() giving up\n"); > > #endif > > > > - stuffp->lock = 0; > > - wake_up_interruptible(&stuffp->lockq); > > + mutex_unlock(&stuffp->mutex); > > > > xtrace(TALK, "talk() done with 0x%02x\n", st); > > return st; > > @@ -1106,9 +1079,9 @@ stuffp->present = 0; /* this should be > > stuffp->wreg_hcon = stuffp->wreg_reset + 1; > > stuffp->wreg_chn = stuffp->wreg_hcon + 1; > > > > + mutex_init(&stuffp->mutex); > > + > > init_waitqueue_head(&stuffp->busyq); > > - init_waitqueue_head(&stuffp->lockq); > > - init_waitqueue_head(&stuffp->sleepq); > > > > /* check if i/o addresses are available */ > > if (!request_region(stuffp->wreg_data, MCDX_IO_SIZE, "mcdx")) { > > @@ -1203,7 +1176,7 @@ return 0; > > xtrace(INIT, "init() get garbage\n"); > > { > > int i; > > - mcdx_delay(stuffp, HZ / 2); > > + msleep_interruptible(500); > > for (i = 100; i; i--) > > (void) inb(stuffp->rreg_status); > > } > > @@ -1327,10 +1300,6 @@ int done = 0; > > return -1; > > } > > > > - while (stuffp->lock) { > > - interruptible_sleep_on(&stuffp->lockq); > > - } > > - > > if (stuffp->valid && (sector >= stuffp->pending) > > && (sector < stuffp->low_border)) { > > > > @@ -1346,7 +1315,7 @@ int done = 0; > > sector + nr_sectors) > > ? stuffp->high_border : border; > > > > - stuffp->lock = current->pid; > > + mutex_lock(&stuffp->mutex); > > > > do { > > > > @@ -1366,11 +1335,11 @@ int done = 0; > > } else > > continue; > > > > - stuffp->lock = 0; > > + mutex_unlock(&stuffp->mutex); > > + > > stuffp->busy = 0; > > stuffp->valid = 0; > > > > - wake_up_interruptible(&stuffp->lockq); > > xtrace(XFER, "transfer() done (-1)\n"); > > return -1; > > } > > @@ -1405,9 +1374,7 @@ insb(stuffp->rreg_data, &dummy[0], C > > } > > } while (++(stuffp->pending) < border); > > > > - stuffp->lock = 0; > > - wake_up_interruptible(&stuffp->lockq); > > - > > + mutex_unlock(&stuffp->mutex); > > } else { > > > > /* The requested sector(s) is/are out of the > > @@ -1654,7 +1621,7 @@ cmd[0], cmd[1], cmd[2], cmd[3], > > > > outsb(stuffp->wreg_data, cmd, sizeof cmd); > > > > - if (-1 == mcdx_getval(stuffp, 3 * HZ, 0, NULL)) { > > + if (-1 == mcdx_getval(stuffp, 3, 0, NULL)) { > > xwarn("playmsf() timeout\n"); > > return -1; > > } > > @@ -1907,7 +1874,7 @@ return mcdx_talk(stuffp, "\x40", 1, NUL > > } > > > > static int > > -mcdx_getval(struct s_drive_stuff *stuffp, int to, int delay, char *buf) > > +mcdx_getval(struct s_drive_stuff *stuffp, int to, int delay_sec, char *buf) > > { > > unsigned long timeout = to + jiffies; > > char c; > > @@ -1918,7 +1885,7 @@ mcdx_getval(struct s_drive_stuff *stuffp > > while (inb(stuffp->rreg_status) & MCDX_RBIT_STEN) { > > if (time_after(jiffies, timeout)) > > return -1; > > - mcdx_delay(stuffp, delay); > > + msleep_interruptible(delay_sec * 1000); > > } > > > > *buf = (unsigned char) inb(stuffp->rreg_data) & 0xff; > > - > > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in > > the body of a message to majordomo@vger.kernel.org > > More majordomo info at http://vger.kernel.org/majordomo-info.html > > Please read the FAQ at http://www.tux.org/lkml/ > > NACK > > Why are you using req->buffer in new code. req->buffer is a leftover from a > very old block pc era and is supposed to be killed. Please do not use it in > any new code. > you should use bio_data(rq->bio) and if you must have a virtual memory pointer > hanging around than keep it in private space. While that is true, this is ancient crap code and there's not much point in rewriting that part as well. I somehow doubt that the cards are capable of highmem addressing in the first place :-) -- Jens Axboe ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: mcdx -- do_request(): non-read command to cd!! 2007-04-02 9:42 ` Jens Axboe @ 2007-04-02 21:02 ` Rene Herman 0 siblings, 0 replies; 29+ messages in thread From: Rene Herman @ 2007-04-02 21:02 UTC (permalink / raw) To: Jens Axboe; +Cc: Boaz Harrosh, Pekka J Enberg, Al Viro, Linux Kernel 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. ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: mcdx -- do_request(): non-read command to cd!! 2007-04-02 8:55 ` Pekka J Enberg 2007-04-02 9:32 ` Boaz Harrosh @ 2007-04-02 15:18 ` Rene Herman 2007-04-02 15:45 ` Rene Herman [not found] ` <Pine.LNX.4.64.0704021837480.6518@sbz-30.cs.Helsinki.FI> 1 sibling, 2 replies; 29+ messages in thread From: Rene Herman @ 2007-04-02 15:18 UTC (permalink / raw) To: Pekka J Enberg; +Cc: Jens Axboe, Al Viro, Linux Kernel [-- Attachment #1: Type: text/plain, Size: 1493 bytes --] 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: root@5va2:~# strace -o dd.strace dd if=/dev/mcdx0 of=foo.img with the same result: root@5va2:~# 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: root@5va2:~# strace -o readcd.strace readcd dev=/dev/mcdx0 f=foo.img root@5va2:~# 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. [-- Attachment #2: dd.strace --] [-- Type: text/plain, Size: 7103 bytes --] 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, ...}) = 0 mmap2(NULL, 378, PROT_READ, MAP_PRIVATE, 3, 0) = 0xb7f98000 close(3) = 0 open("/usr/lib/locale/en_US/LC_MEASUREMENT", O_RDONLY) = 3 fstat64(3, {st_mode=S_IFREG|0644, st_size=28, ...}) = 0 mmap2(NULL, 28, PROT_READ, MAP_PRIVATE, 3, 0) = 0xb7f97000 close(3) = 0 open("/usr/lib/locale/en_US/LC_TELEPHONE", O_RDONLY) = 3 fstat64(3, {st_mode=S_IFREG|0644, st_size=64, ...}) = 0 mmap2(NULL, 64, PROT_READ, MAP_PRIVATE, 3, 0) = 0xb7f96000 close(3) = 0 open("/usr/lib/locale/en_US/LC_ADDRESS", O_RDONLY) = 3 fstat64(3, {st_mode=S_IFREG|0644, st_size=160, ...}) = 0 mmap2(NULL, 160, PROT_READ, MAP_PRIVATE, 3, 0) = 0xb7f95000 close(3) = 0 open("/usr/lib/locale/en_US/LC_NAME", O_RDONLY) = 3 fstat64(3, {st_mode=S_IFREG|0644, st_size=82, ...}) = 0 mmap2(NULL, 82, PROT_READ, MAP_PRIVATE, 3, 0) = 0xb7f94000 close(3) = 0 open("/usr/lib/locale/en_US/LC_PAPER", O_RDONLY) = 3 fstat64(3, {st_mode=S_IFREG|0644, st_size=39, ...}) = 0 mmap2(NULL, 39, PROT_READ, MAP_PRIVATE, 3, 0) = 0xb7f93000 close(3) = 0 open("/usr/lib/locale/en_US/LC_MESSAGES", O_RDONLY) = 3 fstat64(3, {st_mode=S_IFDIR|0755, st_size=4096, ...}) = 0 close(3) = 0 open("/usr/lib/locale/en_US/LC_MESSAGES/SYS_LC_MESSAGES", O_RDONLY) = 3 fstat64(3, {st_mode=S_IFREG|0644, st_size=57, ...}) = 0 mmap2(NULL, 57, PROT_READ, MAP_PRIVATE, 3, 0) = 0xb7f92000 close(3) = 0 open("/usr/lib/locale/en_US/LC_MONETARY", O_RDONLY) = 3 fstat64(3, {st_mode=S_IFREG|0644, st_size=291, ...}) = 0 mmap2(NULL, 291, PROT_READ, MAP_PRIVATE, 3, 0) = 0xb7f91000 close(3) = 0 open("/usr/lib/locale/en_US/LC_TIME", O_RDONLY) = 3 fstat64(3, {st_mode=S_IFREG|0644, st_size=2456, ...}) = 0 mmap2(NULL, 2456, PROT_READ, MAP_PRIVATE, 3, 0) = 0xb7f90000 close(3) = 0 open("/usr/lib/locale/en_US/LC_NUMERIC", O_RDONLY) = 3 fstat64(3, {st_mode=S_IFREG|0644, st_size=59, ...}) = 0 mmap2(NULL, 59, PROT_READ, MAP_PRIVATE, 3, 0) = 0xb7f8f000 close(3) = 0 open("/usr/lib/locale/en_US/LC_CTYPE", O_RDONLY) = 3 fstat64(3, {st_mode=S_IFREG|0644, st_size=178476, ...}) = 0 mmap2(NULL, 178476, PROT_READ, MAP_PRIVATE, 3, 0) = 0xb7e16000 close(3) = 0 close(0) = 0 open("/dev/mcdx0", O_RDONLY|O_LARGEFILE) = 0 _llseek(0, 0, [0], SEEK_CUR) = 0 close(1) = 0 open("foo.img", O_WRONLY|O_CREAT|O_TRUNC|O_LARGEFILE, 0666) = 1 rt_sigaction(SIGUSR1, NULL, {SIG_DFL}, 8) = 0 rt_sigaction(SIGINT, NULL, {SIG_DFL}, 8) = 0 rt_sigaction(SIGUSR1, {0x8049910, [INT USR1], 0}, NULL, 8) = 0 rt_sigaction(SIGINT, {0x8049900, [INT USR1], SA_NOMASK|SA_ONESHOT}, NULL, 8) = 0 clock_gettime(CLOCK_MONOTONIC, {141, 786339816}) = 0 read(0, "", 512) = 0 close(0) = 0 close(1) = 0 clock_gettime(CLOCK_MONOTONIC, {141, 788366793}) = 0 open("/usr/share/locale/en_US/LC_MESSAGES/coreutils.mo", O_RDONLY) = -1 ENOENT (No such file or directory) open("/usr/share/locale/en/LC_MESSAGES/coreutils.mo", O_RDONLY) = -1 ENOENT (No such file or directory) write(2, "0+0 records in\n0+0 records out\n", 31) = 31 write(2, "0 bytes (0 B) copied", 20) = 20 write(2, ", 0.00202698 seconds, 0.0 kB/s\n", 31) = 31 close(1) = -1 EBADF (Bad file descriptor) exit_group(0) = ? ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: mcdx -- do_request(): non-read command to cd!! 2007-04-02 15:18 ` Rene Herman @ 2007-04-02 15:45 ` Rene Herman [not found] ` <Pine.LNX.4.64.0704021837480.6518@sbz-30.cs.Helsinki.FI> 1 sibling, 0 replies; 29+ messages in thread From: Rene Herman @ 2007-04-02 15:45 UTC (permalink / raw) To: Pekka J Enberg; +Cc: Jens Axboe, Al Viro, Linux Kernel 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. ^ permalink raw reply [flat|nested] 29+ messages in thread
[parent not found: <Pine.LNX.4.64.0704021837480.6518@sbz-30.cs.Helsinki.FI>]
[parent not found: <46112650.8080208@gmail.com>]
[parent not found: <Pine.LNX.4.64.0704021906040.7500@sbz-30.cs.Helsinki.FI>]
[parent not found: <461165FD.2010508@gmail.com>]
[parent not found: <Pine.LNX.4.64.0704030908420.20080@sbz-30.cs.Helsinki.FI>]
[parent not found: <Pine.LNX.4.64.0704030956330.20741@sbz-30.cs.Helsinki.FI>]
* Re: mcdx -- do_request(): non-read command to cd!! [not found] ` <Pine.LNX.4.64.0704030956330.20741@sbz-30.cs.Helsinki.FI> @ 2007-04-03 14:26 ` Rene Herman 2007-04-03 17:37 ` Pekka J Enberg [not found] ` <461256C1.4020906@gmail.com> 1 sibling, 1 reply; 29+ messages in thread From: Rene Herman @ 2007-04-03 14:26 UTC (permalink / raw) To: Pekka J Enberg; +Cc: Linux Kernel [-- Attachment #1: Type: text/plain, Size: 2264 bytes --] 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: root@5va2:~# dmesg -c >/dev/null root@5va2:~# 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 root@5va2:~# 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: root@5va2:~# dmesg -c >/dev/null root@5va2:~# 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. rene@7ixe4:~$ When I now switch the monitor to 5va2 directly, there is an oops on the screen, with a backtrace saying (manual transcription): Call Trace: [<c1047eb6>] 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: [<c1047787>] 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. [-- Attachment #2: dd.strace --] [-- Type: text/plain, Size: 7106 bytes --] 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 _sysctl({{CTL_KERN, KERN_VERSION}, 2, 0xbfba055c, 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) = 0xb7fbe000 read(3, "# Locale name alias data base.\n#"..., 4096) = 2586 read(3, "", 4096) = 0 close(3) = 0 munmap(0xb7fbe000, 4096) = 0 open("/usr/lib/locale/en_US/LC_IDENTIFICATION", O_RDONLY) = 3 fstat64(3, {st_mode=S_IFREG|0644, st_size=378, ...}) = 0 mmap2(NULL, 378, PROT_READ, MAP_PRIVATE, 3, 0) = 0xb7fbe000 close(3) = 0 open("/usr/lib/locale/en_US/LC_MEASUREMENT", O_RDONLY) = 3 fstat64(3, {st_mode=S_IFREG|0644, st_size=28, ...}) = 0 mmap2(NULL, 28, PROT_READ, MAP_PRIVATE, 3, 0) = 0xb7fbd000 close(3) = 0 open("/usr/lib/locale/en_US/LC_TELEPHONE", O_RDONLY) = 3 fstat64(3, {st_mode=S_IFREG|0644, st_size=64, ...}) = 0 mmap2(NULL, 64, PROT_READ, MAP_PRIVATE, 3, 0) = 0xb7fbc000 close(3) = 0 open("/usr/lib/locale/en_US/LC_ADDRESS", O_RDONLY) = 3 fstat64(3, {st_mode=S_IFREG|0644, st_size=160, ...}) = 0 mmap2(NULL, 160, PROT_READ, MAP_PRIVATE, 3, 0) = 0xb7fbb000 close(3) = 0 open("/usr/lib/locale/en_US/LC_NAME", O_RDONLY) = 3 fstat64(3, {st_mode=S_IFREG|0644, st_size=82, ...}) = 0 mmap2(NULL, 82, PROT_READ, MAP_PRIVATE, 3, 0) = 0xb7fba000 close(3) = 0 open("/usr/lib/locale/en_US/LC_PAPER", O_RDONLY) = 3 fstat64(3, {st_mode=S_IFREG|0644, st_size=39, ...}) = 0 mmap2(NULL, 39, PROT_READ, MAP_PRIVATE, 3, 0) = 0xb7fb9000 close(3) = 0 open("/usr/lib/locale/en_US/LC_MESSAGES", O_RDONLY) = 3 fstat64(3, {st_mode=S_IFDIR|0755, st_size=4096, ...}) = 0 close(3) = 0 open("/usr/lib/locale/en_US/LC_MESSAGES/SYS_LC_MESSAGES", O_RDONLY) = 3 fstat64(3, {st_mode=S_IFREG|0644, st_size=57, ...}) = 0 mmap2(NULL, 57, PROT_READ, MAP_PRIVATE, 3, 0) = 0xb7fb8000 close(3) = 0 open("/usr/lib/locale/en_US/LC_MONETARY", O_RDONLY) = 3 fstat64(3, {st_mode=S_IFREG|0644, st_size=291, ...}) = 0 mmap2(NULL, 291, PROT_READ, MAP_PRIVATE, 3, 0) = 0xb7fb7000 close(3) = 0 open("/usr/lib/locale/en_US/LC_TIME", O_RDONLY) = 3 fstat64(3, {st_mode=S_IFREG|0644, st_size=2456, ...}) = 0 mmap2(NULL, 2456, PROT_READ, MAP_PRIVATE, 3, 0) = 0xb7fb6000 close(3) = 0 open("/usr/lib/locale/en_US/LC_NUMERIC", O_RDONLY) = 3 fstat64(3, {st_mode=S_IFREG|0644, st_size=59, ...}) = 0 mmap2(NULL, 59, PROT_READ, MAP_PRIVATE, 3, 0) = 0xb7fb5000 close(3) = 0 open("/usr/lib/locale/en_US/LC_CTYPE", O_RDONLY) = 3 fstat64(3, {st_mode=S_IFREG|0644, st_size=178476, ...}) = 0 mmap2(NULL, 178476, PROT_READ, MAP_PRIVATE, 3, 0) = 0xb7e3c000 close(3) = 0 close(0) = 0 open("/dev/mcdx0", O_RDONLY|O_LARGEFILE) = 0 _llseek(0, 0, [0], SEEK_CUR) = 0 close(1) = 0 open("/dev/null", O_WRONLY|O_CREAT|O_TRUNC|O_LARGEFILE, 0666) = 1 rt_sigaction(SIGUSR1, NULL, {SIG_DFL}, 8) = 0 rt_sigaction(SIGINT, NULL, {SIG_DFL}, 8) = 0 rt_sigaction(SIGUSR1, {0x8049910, [INT USR1], 0}, NULL, 8) = 0 rt_sigaction(SIGINT, {0x8049900, [INT USR1], SA_NOMASK|SA_ONESHOT}, NULL, 8) = 0 clock_gettime(CLOCK_MONOTONIC, {203, 63871340}) = 0 read(0, "", 512) = 0 close(0) = 0 close(1) = 0 clock_gettime(CLOCK_MONOTONIC, {203, 68260899}) = 0 open("/usr/share/locale/en_US/LC_MESSAGES/coreutils.mo", O_RDONLY) = -1 ENOENT (No such file or directory) open("/usr/share/locale/en/LC_MESSAGES/coreutils.mo", O_RDONLY) = -1 ENOENT (No such file or directory) write(2, "0+0 records in\n0+0 records out\n", 31) = 31 write(2, "0 bytes (0 B) copied", 20) = 20 write(2, ", 0.00438956 seconds, 0.0 kB/s\n", 31) = 31 close(1) = -1 EBADF (Bad file descriptor) exit_group(0) = ? [-- Attachment #3: tar.strace --] [-- Type: text/plain, Size: 14882 bytes --] execve("/bin/tar", ["tar", "cvf", "foo.tar", "/mnt/cdrom"], [/* 28 vars */]) = 0 uname({sys="Linux", node="5va2", ...}) = 0 brk(0) = 0x8079000 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) = 0xb7f5b000 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) = 0xb7f53000 mmap2(0xb7f59000, 8192, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_FIXED|MAP_DENYWRITE, 3, 0x5) = 0xb7f59000 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) = 0xb7f52000 mmap2(NULL, 1240284, PROT_READ|PROT_EXEC, MAP_PRIVATE|MAP_DENYWRITE, 3, 0) = 0xb7e23000 mmap2(0xb7f4c000, 16384, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_FIXED|MAP_DENYWRITE, 3, 0x128) = 0xb7f4c000 mmap2(0xb7f50000, 7388, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_FIXED|MAP_ANONYMOUS, -1, 0) = 0xb7f50000 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) = 0xb7e11000 mmap2(0xb7e1f000, 8192, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_FIXED|MAP_DENYWRITE, 3, 0xd) = 0xb7e1f000 mmap2(0xb7e21000, 4568, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_FIXED|MAP_ANONYMOUS, -1, 0) = 0xb7e21000 close(3) = 0 mmap2(NULL, 4096, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) = 0xb7e10000 mprotect(0xb7f4c000, 4096, PROT_READ) = 0 set_thread_area({entry_number:-1 -> 6, base_addr:0xb7e106c0, limit:1048575, seg_32bit:1, contents:0, read_exec_only:0, limit_in_pages:1, seg_not_present:0, useable:1}) = 0 munmap(0xb7f5b000, 47774) = 0 set_tid_address(0xb7e10708) = 1179 rt_sigaction(SIGRTMIN, {0xb7e153a0, [], SA_SIGINFO}, NULL, 8) = 0 rt_sigaction(SIGRT_1, {0xb7e15420, [], 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, 0xbf8be31c, 32, (nil), 0}) = 0 clock_gettime(CLOCK_REALTIME, {1175605400, 150858645}) = 0 open("/usr/lib/locale/locale-archive", O_RDONLY|O_LARGEFILE) = -1 ENOENT (No such file or directory) brk(0) = 0x8079000 brk(0x809a000) = 0x809a000 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) = 0xb7f66000 read(3, "# Locale name alias data base.\n#"..., 4096) = 2586 read(3, "", 4096) = 0 close(3) = 0 munmap(0xb7f66000, 4096) = 0 open("/usr/lib/locale/en_US/LC_IDENTIFICATION", O_RDONLY) = 3 fstat64(3, {st_mode=S_IFREG|0644, st_size=378, ...}) = 0 mmap2(NULL, 378, PROT_READ, MAP_PRIVATE, 3, 0) = 0xb7f66000 close(3) = 0 open("/usr/lib/locale/en_US/LC_MEASUREMENT", O_RDONLY) = 3 fstat64(3, {st_mode=S_IFREG|0644, st_size=28, ...}) = 0 mmap2(NULL, 28, PROT_READ, MAP_PRIVATE, 3, 0) = 0xb7f65000 close(3) = 0 open("/usr/lib/locale/en_US/LC_TELEPHONE", O_RDONLY) = 3 fstat64(3, {st_mode=S_IFREG|0644, st_size=64, ...}) = 0 mmap2(NULL, 64, PROT_READ, MAP_PRIVATE, 3, 0) = 0xb7f64000 close(3) = 0 open("/usr/lib/locale/en_US/LC_ADDRESS", O_RDONLY) = 3 fstat64(3, {st_mode=S_IFREG|0644, st_size=160, ...}) = 0 mmap2(NULL, 160, PROT_READ, MAP_PRIVATE, 3, 0) = 0xb7f63000 close(3) = 0 open("/usr/lib/locale/en_US/LC_NAME", O_RDONLY) = 3 fstat64(3, {st_mode=S_IFREG|0644, st_size=82, ...}) = 0 mmap2(NULL, 82, PROT_READ, MAP_PRIVATE, 3, 0) = 0xb7f62000 close(3) = 0 open("/usr/lib/locale/en_US/LC_PAPER", O_RDONLY) = 3 fstat64(3, {st_mode=S_IFREG|0644, st_size=39, ...}) = 0 mmap2(NULL, 39, PROT_READ, MAP_PRIVATE, 3, 0) = 0xb7f61000 close(3) = 0 open("/usr/lib/locale/en_US/LC_MESSAGES", O_RDONLY) = 3 fstat64(3, {st_mode=S_IFDIR|0755, st_size=4096, ...}) = 0 close(3) = 0 open("/usr/lib/locale/en_US/LC_MESSAGES/SYS_LC_MESSAGES", O_RDONLY) = 3 fstat64(3, {st_mode=S_IFREG|0644, st_size=57, ...}) = 0 mmap2(NULL, 57, PROT_READ, MAP_PRIVATE, 3, 0) = 0xb7f60000 close(3) = 0 open("/usr/lib/locale/en_US/LC_MONETARY", O_RDONLY) = 3 fstat64(3, {st_mode=S_IFREG|0644, st_size=291, ...}) = 0 mmap2(NULL, 291, PROT_READ, MAP_PRIVATE, 3, 0) = 0xb7f5f000 close(3) = 0 open("/usr/lib/locale/en_US/LC_TIME", O_RDONLY) = 3 fstat64(3, {st_mode=S_IFREG|0644, st_size=2456, ...}) = 0 mmap2(NULL, 2456, PROT_READ, MAP_PRIVATE, 3, 0) = 0xb7f5e000 close(3) = 0 open("/usr/lib/locale/en_US/LC_NUMERIC", O_RDONLY) = 3 fstat64(3, {st_mode=S_IFREG|0644, st_size=59, ...}) = 0 mmap2(NULL, 59, PROT_READ, MAP_PRIVATE, 3, 0) = 0xb7f5d000 close(3) = 0 open("/usr/lib/locale/en_US/LC_CTYPE", O_RDONLY) = 3 fstat64(3, {st_mode=S_IFREG|0644, st_size=178476, ...}) = 0 mmap2(NULL, 178476, PROT_READ, MAP_PRIVATE, 3, 0) = 0xb7de4000 close(3) = 0 rt_sigaction(SIGCHLD, {SIG_DFL}, {SIG_DFL}, 8) = 0 open("foo.tar", O_WRONLY|O_CREAT|O_TRUNC|O_LARGEFILE, 0666) = 3 fstat64(3, {st_mode=S_IFREG|0644, st_size=0, ...}) = 0 open("/usr/share/locale/en_US/LC_MESSAGES/tar.mo", O_RDONLY) = -1 ENOENT (No such file or directory) open("/usr/share/locale/en/LC_MESSAGES/tar.mo", O_RDONLY) = -1 ENOENT (No such file or directory) write(2, "tar: ", 5) = 5 write(2, "Removing leading `/\' from member"..., 38) = 38 write(2, "\n", 1) = 1 lstat64("/mnt/cdrom", {st_mode=S_IFDIR|0555, st_size=2048, ...}) = 0 open("/mnt/cdrom", O_RDONLY|O_NONBLOCK|O_LARGEFILE|O_DIRECTORY) = 4 fstat64(4, {st_mode=S_IFDIR|0555, st_size=2048, ...}) = 0 fcntl64(4, F_SETFD, FD_CLOEXEC) = 0 getdents64(4, /* 8 entries */, 2048) = 232 getdents64(4, /* 0 entries */, 2048) = 0 close(4) = 0 socket(PF_FILE, SOCK_STREAM, 0) = 4 fcntl64(4, F_GETFL) = 0x2 (flags O_RDWR) fcntl64(4, F_SETFL, O_RDWR|O_NONBLOCK) = 0 connect(4, {sa_family=AF_FILE, path="/var/run/nscd/socket"}, 110) = -1 ENOENT (No such file or directory) close(4) = 0 socket(PF_FILE, SOCK_STREAM, 0) = 4 fcntl64(4, F_GETFL) = 0x2 (flags O_RDWR) fcntl64(4, F_SETFL, O_RDWR|O_NONBLOCK) = 0 connect(4, {sa_family=AF_FILE, path="/var/run/nscd/socket"}, 110) = -1 ENOENT (No such file or directory) close(4) = 0 open("/etc/nsswitch.conf", O_RDONLY) = 4 fstat64(4, {st_mode=S_IFREG|0644, st_size=1083, ...}) = 0 mmap2(NULL, 4096, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) = 0xb7f5c000 read(4, "#\n# /etc/nsswitch.conf\n#\n# An ex"..., 4096) = 1083 read(4, "", 4096) = 0 close(4) = 0 munmap(0xb7f5c000, 4096) = 0 open("/etc/ld.so.cache", O_RDONLY) = 4 fstat64(4, {st_mode=S_IFREG|0644, st_size=47774, ...}) = 0 mmap2(NULL, 47774, PROT_READ, MAP_PRIVATE, 4, 0) = 0xb7dd8000 close(4) = 0 open("/lib/tls/libnss_compat.so.2", O_RDONLY) = 4 read(4, "\177ELF\1\1\1\0\0\0\0\0\0\0\0\0\3\0\3\0\1\0\0\0\340\20"..., 512) = 512 fstat64(4, {st_mode=S_IFREG|0755, st_size=39243, ...}) = 0 mmap2(NULL, 37488, PROT_READ|PROT_EXEC, MAP_PRIVATE|MAP_DENYWRITE, 4, 0) = 0xb7dce000 mmap2(0xb7dd6000, 8192, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_FIXED|MAP_DENYWRITE, 4, 0x7) = 0xb7dd6000 close(4) = 0 open("/lib/tls/libnsl.so.1", O_RDONLY) = 4 read(4, "\177ELF\1\1\1\0\0\0\0\0\0\0\0\0\3\0\3\0\1\0\0\0 5\0\000"..., 512) = 512 fstat64(4, {st_mode=S_IFREG|0755, st_size=92279, ...}) = 0 mmap2(NULL, 88064, PROT_READ|PROT_EXEC, MAP_PRIVATE|MAP_DENYWRITE, 4, 0) = 0xb7db8000 mmap2(0xb7dca000, 8192, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_FIXED|MAP_DENYWRITE, 4, 0x11) = 0xb7dca000 mmap2(0xb7dcc000, 6144, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_FIXED|MAP_ANONYMOUS, -1, 0) = 0xb7dcc000 close(4) = 0 munmap(0xb7dd8000, 47774) = 0 open("/etc/ld.so.cache", O_RDONLY) = 4 fstat64(4, {st_mode=S_IFREG|0644, st_size=47774, ...}) = 0 mmap2(NULL, 47774, PROT_READ, MAP_PRIVATE, 4, 0) = 0xb7dd8000 close(4) = 0 open("/lib/tls/libnss_nis.so.2", O_RDONLY) = 4 read(4, "\177ELF\1\1\1\0\0\0\0\0\0\0\0\0\3\0\3\0\1\0\0\0P\34\0\000"..., 512) = 512 fstat64(4, {st_mode=S_IFREG|0755, st_size=41139, ...}) = 0 mmap2(NULL, 37416, PROT_READ|PROT_EXEC, MAP_PRIVATE|MAP_DENYWRITE, 4, 0) = 0xb7dae000 mmap2(0xb7db6000, 8192, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_FIXED|MAP_DENYWRITE, 4, 0x7) = 0xb7db6000 close(4) = 0 open("/lib/tls/libnss_files.so.2", O_RDONLY) = 4 read(4, "\177ELF\1\1\1\0\0\0\0\0\0\0\0\0\3\0\3\0\1\0\0\0P\33\0\000"..., 512) = 512 fstat64(4, {st_mode=S_IFREG|0755, st_size=45361, ...}) = 0 mmap2(NULL, 41612, PROT_READ|PROT_EXEC, MAP_PRIVATE|MAP_DENYWRITE, 4, 0) = 0xb7da3000 mmap2(0xb7dac000, 8192, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_FIXED|MAP_DENYWRITE, 4, 0x8) = 0xb7dac000 close(4) = 0 munmap(0xb7dd8000, 47774) = 0 open("/etc/passwd", O_RDONLY) = 4 fcntl64(4, F_GETFD) = 0 fcntl64(4, F_SETFD, FD_CLOEXEC) = 0 _llseek(4, 0, [0], SEEK_CUR) = 0 fstat64(4, {st_mode=S_IFREG|0644, st_size=707, ...}) = 0 mmap2(NULL, 707, PROT_READ, MAP_SHARED, 4, 0) = 0xb7f5c000 _llseek(4, 707, [707], SEEK_SET) = 0 munmap(0xb7f5c000, 707) = 0 close(4) = 0 socket(PF_FILE, SOCK_STREAM, 0) = 4 fcntl64(4, F_GETFL) = 0x2 (flags O_RDWR) fcntl64(4, F_SETFL, O_RDWR|O_NONBLOCK) = 0 connect(4, {sa_family=AF_FILE, path="/var/run/nscd/socket"}, 110) = -1 ENOENT (No such file or directory) close(4) = 0 socket(PF_FILE, SOCK_STREAM, 0) = 4 fcntl64(4, F_GETFL) = 0x2 (flags O_RDWR) fcntl64(4, F_SETFL, O_RDWR|O_NONBLOCK) = 0 connect(4, {sa_family=AF_FILE, path="/var/run/nscd/socket"}, 110) = -1 ENOENT (No such file or directory) close(4) = 0 open("/etc/group", O_RDONLY) = 4 fcntl64(4, F_GETFD) = 0 fcntl64(4, F_SETFD, FD_CLOEXEC) = 0 _llseek(4, 0, [0], SEEK_CUR) = 0 fstat64(4, {st_mode=S_IFREG|0644, st_size=467, ...}) = 0 mmap2(NULL, 467, PROT_READ, MAP_SHARED, 4, 0) = 0xb7f5c000 _llseek(4, 467, [467], SEEK_SET) = 0 munmap(0xb7f5c000, 467) = 0 close(4) = 0 fstat64(1, {st_mode=S_IFCHR|0620, st_rdev=makedev(136, 0), ...}) = 0 mmap2(NULL, 4096, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) = 0xb7f5c000 write(1, "/mnt/cdrom/\n", 12) = 12 lstat64("/mnt/cdrom/dott", {st_mode=S_IFDIR|0555, st_size=2048, ...}) = 0 open("/mnt/cdrom/dott", O_RDONLY|O_NONBLOCK|O_LARGEFILE|O_DIRECTORY) = 4 fstat64(4, {st_mode=S_IFDIR|0555, st_size=2048, ...}) = 0 fcntl64(4, F_SETFD, FD_CLOEXEC) = 0 getdents64(4, /* 14 entries */, 2048) = 432 getdents64(4, /* 0 entries */, 2048) = 0 close(4) = 0 write(1, "/mnt/cdrom/dott/\n", 17) = 17 lstat64("/mnt/cdrom/dott/adlib.ims", {st_mode=S_IFREG|0555, st_size=21896, ...}) = 0 open("/mnt/cdrom/dott/adlib.ims", O_RDONLY|O_LARGEFILE) = 4 write(1, "/mnt/cdrom/dott/adlib.ims\n", 26) = 26 read(4, "MZ\210\1+\0\2\0 \0>\2\377\377\0\0\0\0n\267\0\0\0\0\36\0"..., 8704) = 8704 write(3, "mnt/cdrom/\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0"..., 10240) = 10240 read(4, "\203\373\20uP\213V\10\213\302*\344\213\310\212\340*\300"..., 10240) = 10240 write(3, "\203\373\20uP\213V\10\213\302*\344\213\310\212\340*\300"..., 10240) = 10240 read(4, "\301\343\5+\330\212\207P\"*\344\367\330^_\311\303\220\v"..., 2952) = 2952 fstat64(4, {st_mode=S_IFREG|0555, st_size=21896, ...}) = 0 close(4) = 0 lstat64("/mnt/cdrom/dott/dott.bat", {st_mode=S_IFREG|0555, st_size=13, ...}) = 0 open("/mnt/cdrom/dott/dott.bat", O_RDONLY|O_LARGEFILE) = 4 write(1, "/mnt/cdrom/dott/dott.bat\n", 25) = 25 read(4, "cd ..\r\ndott\r\n", 13) = 13 fstat64(4, {st_mode=S_IFREG|0555, st_size=13, ...}) = 0 close(4) = 0 lstat64("/mnt/cdrom/dott/dotticon.ico", {st_mode=S_IFREG|0555, st_size=766, ...}) = 0 open("/mnt/cdrom/dott/dotticon.ico", O_RDONLY|O_LARGEFILE) = 4 write(1, "/mnt/cdrom/dott/dotticon.ico\n", 29) = 29 read(4, "\0\0\1\0\1\0 \20\0\0\0\0\0\350\2\0\0\26\0\0\0(\0\0\0 "..., 766) = 766 fstat64(4, {st_mode=S_IFREG|0555, st_size=766, ...}) = 0 close(4) = 0 lstat64("/mnt/cdrom/dott/gmidi.ims", {st_mode=S_IFREG|0555, st_size=19360, ...}) = 0 open("/mnt/cdrom/dott/gmidi.ims", O_RDONLY|O_LARGEFILE) = 4 write(1, "/mnt/cdrom/dott/gmidi.ims\n", 26) = 26 read(4, "MZ\240\1&\0\2\0 \0\272\1\377\377\0\0\0\0\361}\0\0\0\0\36"..., 4096) = 4096 write(3, "\301\343\5+\330\212\207P\"*\344\367\330^_\311\303\220\v"..., 10240) = 10240 read(4, "\2\213\360\v\366u\3\351\204\0\377v\n\377v\10V\377V\376"..., 10240) = 10240 write(3, "\2\213\360\v\366u\3\351\204\0\377v\n\377v\10V\377V\376"..., 10240) = 10240 read(4, "\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0"..., 5024) = 5024 fstat64(4, {st_mode=S_IFREG|0555, st_size=19360, ...}) = 0 close(4) = 0 lstat64("/mnt/cdrom/dott/maniac", {st_mode=S_IFDIR|0555, st_size=4096, ...}) = 0 open("/mnt/cdrom/dott/maniac", O_RDONLY|O_NONBLOCK|O_LARGEFILE|O_DIRECTORY) = 4 fstat64(4, {st_mode=S_IFDIR|0555, st_size=4096, ...}) = 0 fcntl64(4, F_SETFD, FD_CLOEXEC) = 0 getdents64(4, /* 56 entries */, 2048) = 1776 getdents64(4, /* 0 entries */, 2048) = 0 close(4) = 0 write(1, "/mnt/cdrom/dott/maniac/\n", 24) = 24 --- SIGSEGV (Segmentation fault) @ 0 (0) --- +++ killed by SIGSEGV +++ ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: mcdx -- do_request(): non-read command to cd!! 2007-04-03 14:26 ` Rene Herman @ 2007-04-03 17:37 ` Pekka J Enberg 0 siblings, 0 replies; 29+ messages in thread From: Pekka J Enberg @ 2007-04-03 17:37 UTC (permalink / raw) To: Rene Herman; +Cc: Linux Kernel On Tue, 3 Apr 2007, Rene Herman wrote: > I believe the "tar" runs has produced something useful now though: > > root@5va2:~# dmesg -c >/dev/null > root@5va2:~# 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. > rene@7ixe4:~$ > > When I now switch the monitor to 5va2 directly, there is an oops on the > screen, with a backtrace saying (manual transcription): > > Call Trace: > [<c1047eb6>] 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: [<c1047787>] 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 ^ permalink raw reply [flat|nested] 29+ messages in thread
[parent not found: <461256C1.4020906@gmail.com>]
* Re: mcdx -- do_request(): non-read command to cd!! [not found] ` <461256C1.4020906@gmail.com> @ 2007-04-03 14:33 ` Pekka J Enberg 2007-04-03 17:31 ` Pekka J Enberg 2007-04-04 6:19 ` Jens Axboe 0 siblings, 2 replies; 29+ messages in thread From: Pekka J Enberg @ 2007-04-03 14:33 UTC (permalink / raw) To: Rene Herman; +Cc: linux-kernel, Jens Axboe 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: > > root@5va2:~# dmesg -c >/dev/null > root@5va2:~# 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 > root@5va2:~# 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 ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: mcdx -- do_request(): non-read command to cd!! 2007-04-03 14:33 ` Pekka J Enberg @ 2007-04-03 17:31 ` Pekka J Enberg 2007-04-03 18:14 ` Rene Herman 2007-04-04 6:19 ` Jens Axboe 1 sibling, 1 reply; 29+ messages in thread From: Pekka J Enberg @ 2007-04-03 17:31 UTC (permalink / raw) To: Rene Herman; +Cc: linux-kernel, Jens Axboe 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) { ^ permalink raw reply related [flat|nested] 29+ messages in thread
* Re: mcdx -- do_request(): non-read command to cd!! 2007-04-03 17:31 ` Pekka J Enberg @ 2007-04-03 18:14 ` Rene Herman 2007-04-03 18:32 ` Pekka J Enberg 0 siblings, 1 reply; 29+ messages in thread From: Rene Herman @ 2007-04-03 18:14 UTC (permalink / raw) To: Pekka J Enberg; +Cc: linux-kernel, Jens Axboe 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! root@5va2:~# dd if=/dev/mcdx0 of=/dev/null Oops: 0002 [#1] CPU: 0 EIP: 0060:[<c1047787>] 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: 00000001 esp: c1575ef0 ds: 007b es: 007b ss: 0068 Process events/0 (pid: 4, ti=c1575000 task=c1577510 task.ti=c1575000) Stack: 00000000 00000002 c165fa38 c165fa38 00000002 c165fa00 00000000 c1047eb6 00000000 00000000 c16414e0 c160c960 c16414e0 c160c960 c14ab540 00000292 c1047f3a 00000000 00000000 00000292 c14ab544 c154ebc0 c101ec7a c154ec14 Call Trace: [<c1047eb6>] drain_array+0x94/0xc3 [<c1047f3a>] cache_reap+0x55/0xe9 [<c101ec7a>] run_workqueue+0x8f/0x14d [<c1047ee5>] cache_reap+0x0/0xe9 [<c101ee5a>] worker_thread+0x122/0x14b [<c100e2d8>] default_wake_function+0x0/0xc [<c100e2d8>] default_wake_function+0x0/0xc [<c101ed38>] worker_thread+0x0/0x14b [<c1021800>] kthread+0x72/0x97 [<c102178e>] kthread+0x0/0x97 [<c1002b47>] 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: [<c1047787>] 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. ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: mcdx -- do_request(): non-read command to cd!! 2007-04-03 18:14 ` Rene Herman @ 2007-04-03 18:32 ` Pekka J Enberg 2007-04-04 2:10 ` Rene Herman 0 siblings, 1 reply; 29+ messages in thread From: Pekka J Enberg @ 2007-04-03 18:32 UTC (permalink / raw) To: Rene Herman; +Cc: linux-kernel, Jens Axboe 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 ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: mcdx -- do_request(): non-read command to cd!! 2007-04-03 18:32 ` Pekka J Enberg @ 2007-04-04 2:10 ` Rene Herman 2007-04-04 6:30 ` Pekka Enberg 0 siblings, 1 reply; 29+ messages in thread From: Rene Herman @ 2007-04-04 2:10 UTC (permalink / raw) To: Pekka J Enberg; +Cc: linux-kernel, Jens Axboe 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. ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: mcdx -- do_request(): non-read command to cd!! 2007-04-04 2:10 ` Rene Herman @ 2007-04-04 6:30 ` Pekka Enberg 0 siblings, 0 replies; 29+ messages in thread From: Pekka Enberg @ 2007-04-04 6:30 UTC (permalink / raw) To: Rene Herman; +Cc: linux-kernel, Jens Axboe On 4/4/07, Rene Herman <rene.herman@gmail.com> 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 ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: mcdx -- do_request(): non-read command to cd!! 2007-04-03 14:33 ` Pekka J Enberg 2007-04-03 17:31 ` Pekka J Enberg @ 2007-04-04 6:19 ` Jens Axboe 1 sibling, 0 replies; 29+ messages in thread From: Jens Axboe @ 2007-04-04 6:19 UTC (permalink / raw) To: Pekka J Enberg; +Cc: Rene Herman, linux-kernel 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 ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: mcdx -- do_request(): non-read command to cd!! 2007-04-02 7:10 ` Jens Axboe 2007-04-02 7:37 ` Pekka J Enberg 2007-04-02 8:55 ` Pekka J Enberg @ 2007-04-02 15:39 ` Rene Herman 2 siblings, 0 replies; 29+ messages in thread From: Rene Herman @ 2007-04-02 15:39 UTC (permalink / raw) To: Jens Axboe; +Cc: Pekka J Enberg, Al Viro, Linux Kernel 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 <wim@iguana.be> > > --------------------------- > + > +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. ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: mcdx -- do_request(): non-read command to cd!! 2007-04-02 0:02 ` Rene Herman 2007-04-02 0:07 ` Rene Herman @ 2007-04-02 6:42 ` Jens Axboe 2007-04-02 7:07 ` Pekka Enberg 2 siblings, 0 replies; 29+ messages in thread From: Jens Axboe @ 2007-04-02 6:42 UTC (permalink / raw) To: Rene Herman; +Cc: Pekka Enberg, Al Viro, Linux Kernel 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 ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: mcdx -- do_request(): non-read command to cd!! 2007-04-02 0:02 ` Rene Herman 2007-04-02 0:07 ` Rene Herman 2007-04-02 6:42 ` Jens Axboe @ 2007-04-02 7:07 ` Pekka Enberg 2 siblings, 0 replies; 29+ messages in thread From: Pekka Enberg @ 2007-04-02 7:07 UTC (permalink / raw) To: Rene Herman; +Cc: Jens Axboe, Al Viro, Linux Kernel Hi Rene, On 4/2/07, Rene Herman <rene.herman@gmail.com> wrote: > root@5va2:~# 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 > root@5va2:~# > > This I know isn't: > > root@5va2:~# readcd dev=/dev/mcdx0 f=/dev/null > Segmentation fault > root@5va2:~# > > (leaves a "note: readcd[1174] exited with preempt_count 1" in the log) > > and after a "mount -t iso9660 /dev/mcdx0 /mnt/cdrom", a: > > root@5va2:~# 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? ^ permalink raw reply [flat|nested] 29+ messages in thread
end of thread, other threads:[~2007-04-04 6:30 UTC | newest] Thread overview: 29+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2007-03-30 21:21 mcdx -- do_request(): non-read command to cd!! Rene Herman 2007-03-31 6:47 ` Jens Axboe 2007-03-31 18:23 ` Rene Herman 2007-04-01 10:06 ` Pekka Enberg 2007-04-01 10:16 ` Pekka Enberg 2007-04-02 0:02 ` Rene Herman 2007-04-02 0:07 ` Rene Herman 2007-04-02 6:50 ` Pekka J Enberg 2007-04-02 7:10 ` Jens Axboe 2007-04-02 7:37 ` Pekka J Enberg 2007-04-02 8:55 ` Pekka J Enberg 2007-04-02 9:32 ` Boaz Harrosh 2007-04-02 9:42 ` Pekka J Enberg 2007-04-02 9:42 ` Jens Axboe 2007-04-02 21:02 ` Rene Herman 2007-04-02 15:18 ` Rene Herman 2007-04-02 15:45 ` Rene Herman [not found] ` <Pine.LNX.4.64.0704021837480.6518@sbz-30.cs.Helsinki.FI> [not found] ` <46112650.8080208@gmail.com> [not found] ` <Pine.LNX.4.64.0704021906040.7500@sbz-30.cs.Helsinki.FI> [not found] ` <461165FD.2010508@gmail.com> [not found] ` <Pine.LNX.4.64.0704030908420.20080@sbz-30.cs.Helsinki.FI> [not found] ` <Pine.LNX.4.64.0704030956330.20741@sbz-30.cs.Helsinki.FI> 2007-04-03 14:26 ` Rene Herman 2007-04-03 17:37 ` Pekka J Enberg [not found] ` <461256C1.4020906@gmail.com> 2007-04-03 14:33 ` Pekka J Enberg 2007-04-03 17:31 ` Pekka J Enberg 2007-04-03 18:14 ` Rene Herman 2007-04-03 18:32 ` Pekka J Enberg 2007-04-04 2:10 ` Rene Herman 2007-04-04 6:30 ` Pekka Enberg 2007-04-04 6:19 ` Jens Axboe 2007-04-02 15:39 ` Rene Herman 2007-04-02 6:42 ` Jens Axboe 2007-04-02 7:07 ` Pekka Enberg
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.