Re: mcdx -- do_request(): non-read command to cd!!
From: Pekka J Enberg
Date: Mon Apr 02 2007 - 04:56:14 EST
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@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/