mcdx -- do_request(): non-read command to cd!!

mcdx -- do_request(): non-read command to cd!!

[Rene Herman]

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

Re: mcdx -- do_request(): non-read command to cd!!

[Jens Axboe]

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

Re: mcdx -- do_request(): non-read command to cd!!

[Rene Herman]

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.

Re: mcdx -- do_request(): non-read command to cd!!

[Pekka Enberg]

[-- 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);
 	}
 

Re: mcdx -- do_request(): non-read command to cd!!

[Pekka Enberg]

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.

Re: mcdx -- do_request(): non-read command to cd!!

[Rene Herman]

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.

Re: mcdx -- do_request(): non-read command to cd!!

[Rene Herman]

[-- 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);
 	}
 

Re: mcdx -- do_request(): non-read command to cd!!

[Pekka J Enberg]

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

Re: mcdx -- do_request(): non-read command to cd!!

[Jens Axboe]

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

Re: mcdx -- do_request(): non-read command to cd!!

[Pekka J Enberg]

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;
 }
 
 

Re: mcdx -- do_request(): non-read command to cd!!

[Pekka J Enberg]

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;

Re: mcdx -- do_request(): non-read command to cd!!

[Boaz Harrosh]

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

Re: mcdx -- do_request(): non-read command to cd!!

[Pekka J Enberg]

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

Re: mcdx -- do_request(): non-read command to cd!!

[Jens Axboe]

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

Re: mcdx -- do_request(): non-read command to cd!!

[Rene Herman]

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.

Re: mcdx -- do_request(): non-read command to cd!!

[Rene Herman]

[-- 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)                           = ?

Re: mcdx -- do_request(): non-read command to cd!!

[Rene Herman]

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.

Re: mcdx -- do_request(): non-read command to cd!!

[Rene Herman]

[-- 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 +++

Re: mcdx -- do_request(): non-read command to cd!!

[Pekka J Enberg]

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

Re: mcdx -- do_request(): non-read command to cd!!

[Pekka J Enberg]

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

Re: mcdx -- do_request(): non-read command to cd!!

[Pekka J Enberg]

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) {

Re: mcdx -- do_request(): non-read command to cd!!

[Rene Herman]

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.

Re: mcdx -- do_request(): non-read command to cd!!

[Pekka J Enberg]

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

Re: mcdx -- do_request(): non-read command to cd!!

[Rene Herman]

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.

Re: mcdx -- do_request(): non-read command to cd!!

[Pekka Enberg]

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

Re: mcdx -- do_request(): non-read command to cd!!

[Jens Axboe]

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

Re: mcdx -- do_request(): non-read command to cd!!

[Rene Herman]

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.

Re: mcdx -- do_request(): non-read command to cd!!

[Jens Axboe]

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

Re: mcdx -- do_request(): non-read command to cd!!

[Pekka Enberg]

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?