linux-scsi.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] fix req->cmd == INT cases
@ 2007-06-19 15:59 Boaz Harrosh
  2007-06-19 17:16 ` Jens Axboe
  0 siblings, 1 reply; 10+ messages in thread
From: Boaz Harrosh @ 2007-06-19 15:59 UTC (permalink / raw)
  To: Jens Axboe, Andrew Morton
  Cc: Paul Bristow, Dave Gilbert, Russell King, Dave Alan Gilbert,
	Paul Clements, Pavel Machek, Steven Whitehouse,
	Eberhard Moenkeberg, David van Leeuwen, Oliver Raupach,
	Heiko Schlittermann, Leo Spiekman, Vadim V. Model, linux-scsi

>From 6f4641911701cbdbe01516305b682012d8c6982a Mon Sep 17 00:00:00 2001
Date: Tue, 19 Jun 2007 18:54:54 +0300

 I have unearthed very old bugs in stale drivers that still
 used request->cmd as a READ|WRITE int.
 This patch is maybe a proof that these drivers have not been
 used for a long time. Should they be removed completely?

 If you are CCed it means your name was in one of these drivers
 source files, or in the MAINTAINERS file. Please acknowledge
 the possible removal of this old driver.

Signed-off-by: Boaz Harrosh <bharrosh@panasas.com>

drivers/acorn/block/fd1772.c
Jens Axboe
Paul Bristow <paul@paulbristow.net>
Dave Gilbert (linux@treblig.org)

drivers/acorn/block/mfmhd.c
TO: Russell King <rmk@arm.linux.org.uk>
TO: Dave Alan Gilbert <gilbertd@cs.man.ac.uk>

drivers/block/amiflop.c
Greg Harp
Joerg Dorchain

drivers/block/nbd.c
Paul Clements <Paul.Clements@steeleye.com>
Pavel Machek <pavel@suse.cz>
Steven Whitehouse <swhiteho@redhat.com>

drivers/cdrom/aztcd.c
Eberhard Moenkeberg <emoenke@gwdg.de>
Werner Zimmermann(Werner.Zimmermann@fht-esslingen.de)

drivers/cdrom/cm206.c
David van Leeuwen <avid@tm.tno.nl>

drivers/cdrom/gscd.c
Oliver Raupach <raupach@nwfs1.rz.fh-hannover.de>

drivers/cdrom/mcdx.c
Heiko Schlittermann <heiko@lotte.sax.de>

drivers/cdrom/optcd.c
Leo Spiekman <spiekman@dutette.et.tudelft.nl>

drivers/cdrom/sjcd.c
Vadim V. Model <model@cecmow.enet.dec.com>
vadim@rbrf.ru
vadim@ipsun.ras.ru

drivers/ide/legacy/hd.c
Linus Trovalds
---
 drivers/acorn/block/fd1772.c |    2 +-
 drivers/acorn/block/mfmhd.c  |    9 +++++----
 drivers/block/amiflop.c      |    2 +-
 drivers/block/nbd.c          |    2 +-
 drivers/cdrom/aztcd.c        |    2 +-
 drivers/cdrom/cm206.c        |    2 +-
 drivers/cdrom/gscd.c         |    2 +-
 drivers/cdrom/mcdx.c         |    2 +-
 drivers/cdrom/optcd.c        |    2 +-
 drivers/cdrom/sjcd.c         |    2 +-
 drivers/ide/legacy/hd.c      |    4 ++--
 11 files changed, 16 insertions(+), 15 deletions(-)

diff --git a/drivers/acorn/block/fd1772.c b/drivers/acorn/block/fd1772.c
index 674bf81..423ed08 100644
--- a/drivers/acorn/block/fd1772.c
+++ b/drivers/acorn/block/fd1772.c
@@ -1246,7 +1246,7 @@ repeat:
     del_timer(&motor_off_timer);
 
     ReqCnt = 0;
-    ReqCmd = CURRENT->cmd;
+    ReqCmd = rq_data_dir(CURRENT);
     ReqBlock = CURRENT->sector;
     ReqBuffer = CURRENT->buffer;
     setup_req_params(drive);
diff --git a/drivers/acorn/block/mfmhd.c b/drivers/acorn/block/mfmhd.c
index 689a4c3..7a4e12b 100644
--- a/drivers/acorn/block/mfmhd.c
+++ b/drivers/acorn/block/mfmhd.c
@@ -439,7 +439,7 @@ static void mfm_rw_intr(void)
        a choice of command end or some data which is ready to be
collected */
     /* I think we have to transfer data while the interrupt line is on
and its
        not any other type of interrupt */
-    if (CURRENT->cmd == WRITE) {
+    if (rq_data_dir(CURRENT) == WRITE) {
         extern void hdc63463_writedma(void);
         if ((hdc63463_dataleft <= 0) && (!(mfm_status & STAT_CED))) {
             printk("mfm_rw_intr: Apparent DMA write request when no
more to DMA\n");
@@ -799,7 +799,7 @@ static void issue_request(unsigned int block,
unsigned int nsect,
     raw_cmd.head = start_head;
     raw_cmd.cylinder = track / p->heads;
     raw_cmd.cmdtype = CURRENT->cmd;
-    raw_cmd.cmdcode = CURRENT->cmd == WRITE ? CMD_WD : CMD_RD;
+    raw_cmd.cmdcode = rq_data_dir(CURRENT) == WRITE ? CMD_WD : CMD_RD;
     raw_cmd.cmddata[0] = dev + 1;    /* DAG: +1 to get US */
     raw_cmd.cmddata[1] = raw_cmd.head;
     raw_cmd.cmddata[2] = raw_cmd.cylinder >> 8;
@@ -830,7 +830,7 @@ static void issue_request(unsigned int block,
unsigned int nsect,
     hdc63463_dataleft = nsect * 256;    /* Better way? */
 
     DBG("mfm%c: %sing: CHS=%d/%d/%d, sectors=%d, buffer=0x%08lx (%p)\n",
-         raw_cmd.dev + 'a', (CURRENT->cmd == READ) ? "read" : "writ",
+         raw_cmd.dev + 'a', rq_data_dir(CURRENT) == READ ? "read" : "writ",
                raw_cmd.cylinder,
                raw_cmd.head,
         raw_cmd.sector, nsect, (unsigned long) Copy_buffer, CURRENT);
@@ -917,7 +917,8 @@ static void mfm_request(void)
 
         DBG("mfm_request: block after offset=%d\n", block);
 
-        if (CURRENT->cmd != READ && CURRENT->cmd != WRITE) {
+        if (rq_data_dir(CURRENT) != READ &&
+                                    rq_data_dir(CURRENT) != WRITE) {
             printk("unknown mfm-command %d\n", CURRENT->cmd);
             end_request(CURRENT, 0);
             Busy = 0;
diff --git a/drivers/block/amiflop.c b/drivers/block/amiflop.c
index 27a1390..6ce8b89 100644
--- a/drivers/block/amiflop.c
+++ b/drivers/block/amiflop.c
@@ -1363,7 +1363,7 @@ static void redo_fd_request(void)
 #ifdef DEBUG
         printk("fd: sector %ld + %d requested for %s\n",
                CURRENT->sector,cnt,
-               (CURRENT->cmd==READ)?"read":"write");
+               (rq_data_dir(CURRENT) == READ) ? "read" : "write");
 #endif
         block = CURRENT->sector + cnt;
         if ((int)block > floppy->blocks) {
diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c
index 069ae39..c575fb1 100644
--- a/drivers/block/nbd.c
+++ b/drivers/block/nbd.c
@@ -416,7 +416,7 @@ static void nbd_clear_que(struct nbd_device *lo)
 /*
  * We always wait for result of write, for now. It would be nice to
make it optional
  * in future
- * if ((req->cmd == WRITE) && (lo->flags & NBD_WRITE_NOCHK))
+ * if ((rq_data_dir(req) == WRITE) && (lo->flags & NBD_WRITE_NOCHK))
  *   { printk( "Warning: Ignoring result!\n"); nbd_end_request( req ); }
  */
 
diff --git a/drivers/cdrom/aztcd.c b/drivers/cdrom/aztcd.c
index 1f9fb7a..d7fbfaa 100644
--- a/drivers/cdrom/aztcd.c
+++ b/drivers/cdrom/aztcd.c
@@ -229,7 +229,7 @@ static struct request_queue *azt_queue;
 static int current_valid(void)
 {
         return CURRENT &&
-        CURRENT->cmd == READ &&
+        rq_data_dir(CURRENT) == READ &&
         CURRENT->sector != -1;
 }
 
diff --git a/drivers/cdrom/cm206.c b/drivers/cdrom/cm206.c
index 2301311..2f8fe3b 100644
--- a/drivers/cdrom/cm206.c
+++ b/drivers/cdrom/cm206.c
@@ -851,7 +851,7 @@ static void do_cm206_request(request_queue_t * q)
         if (!req)
             return;
 
-        if (req->cmd != READ) {
+        if (rq_data_dir(req) != READ) {
             debug(("Non-read command %d on cdrom\n", req->cmd));
             end_request(req, 0);
             continue;
diff --git a/drivers/cdrom/gscd.c b/drivers/cdrom/gscd.c
index b3ab6e9..176742e 100644
--- a/drivers/cdrom/gscd.c
+++ b/drivers/cdrom/gscd.c
@@ -264,7 +264,7 @@ repeat:
     if (req->sector == -1)
         goto out;
 
-    if (req->cmd != READ) {
+    if (rq_data_dir(req) != READ) {
         printk("GSCD: bad cmd %u\n", rq_data_dir(req));
         end_request(req, 0);
         goto repeat;
diff --git a/drivers/cdrom/mcdx.c b/drivers/cdrom/mcdx.c
index 4310cc8..972ee9c 100644
--- a/drivers/cdrom/mcdx.c
+++ b/drivers/cdrom/mcdx.c
@@ -596,7 +596,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);
diff --git a/drivers/cdrom/optcd.c b/drivers/cdrom/optcd.c
index 3541690..efd619c 100644
--- a/drivers/cdrom/optcd.c
+++ b/drivers/cdrom/optcd.c
@@ -977,7 +977,7 @@ static int update_toc(void)
 static int current_valid(void)
 {
         return CURRENT &&
-        CURRENT->cmd == READ &&
+        rq_data_dir(CURRENT) == READ &&
         CURRENT->sector != -1;
 }
 
diff --git a/drivers/cdrom/sjcd.c b/drivers/cdrom/sjcd.c
index 5409fca..76c24e6 100644
--- a/drivers/cdrom/sjcd.c
+++ b/drivers/cdrom/sjcd.c
@@ -1064,7 +1064,7 @@ static void sjcd_invalidate_buffers(void)
 static int current_valid(void)
 {
         return CURRENT &&
-        CURRENT->cmd == READ &&
+        rq_data_dir(CURRENT) == READ &&
         CURRENT->sector != -1;
 }
 
diff --git a/drivers/ide/legacy/hd.c b/drivers/ide/legacy/hd.c
index 45ed035..6004b7d 100644
--- a/drivers/ide/legacy/hd.c
+++ b/drivers/ide/legacy/hd.c
@@ -622,8 +622,8 @@ repeat:
     head  = track % disk->head;
     cyl   = track / disk->head;
 #ifdef DEBUG
-    printk("%s: %sing: CHS=%d/%d/%d, sectors=%d, buffer=%p\n",
-        req->rq_disk->disk_name, (req->cmd == READ)?"read":"writ",
+    printk("%s: %s: CHS=%d/%d/%d, sectors=%d, buffer=%p\n",
+        req->rq_disk->disk_name, dma_dir_to_string(rq_dma_dir(req)),
         cyl, head, sec, nsect, req->buffer);
 #endif
     if (blk_fs_request(req)) {
-- 
1.5.0.4.402.g8035



^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH] fix req->cmd == INT cases
  2007-06-19 15:59 [PATCH] fix req->cmd == INT cases Boaz Harrosh
@ 2007-06-19 17:16 ` Jens Axboe
  2007-06-20 10:53   ` Boaz Harrosh
  0 siblings, 1 reply; 10+ messages in thread
From: Jens Axboe @ 2007-06-19 17:16 UTC (permalink / raw)
  To: Boaz Harrosh
  Cc: Andrew Morton, Paul Bristow, Dave Gilbert, Russell King,
	Dave Alan Gilbert, Paul Clements, Pavel Machek, Steven Whitehouse,
	Eberhard Moenkeberg, David van Leeuwen, Oliver Raupach,
	Heiko Schlittermann, Leo Spiekman, Vadim V. Model, linux-scsi

On Tue, Jun 19 2007, Boaz Harrosh wrote:
> From 6f4641911701cbdbe01516305b682012d8c6982a Mon Sep 17 00:00:00 2001
> Date: Tue, 19 Jun 2007 18:54:54 +0300
> 
>  I have unearthed very old bugs in stale drivers that still
>  used request->cmd as a READ|WRITE int.
>  This patch is maybe a proof that these drivers have not been
>  used for a long time. Should they be removed completely?

If you look at most of the code, it's inside ifdef debug statements or
comments most of them. So I don't think you can base any removal
suggestion on that.

The patch itself looks fine though, if you send one that isn't mangled
I'll apply it to the 2.6.23 branch.

-- 
Jens Axboe


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] fix req->cmd == INT cases
  2007-06-19 17:16 ` Jens Axboe
@ 2007-06-20 10:53   ` Boaz Harrosh
  2007-06-20 10:59     ` Jens Axboe
  2007-06-20 12:22     ` Christoph Hellwig
  0 siblings, 2 replies; 10+ messages in thread
From: Boaz Harrosh @ 2007-06-20 10:53 UTC (permalink / raw)
  To: Jens Axboe; +Cc: linux-scsi

Jens Axboe wrote:
> 
> If you look at most of the code, it's inside ifdef debug statements or
> comments most of them. So I don't think you can base any removal
> suggestion on that.
> 
> The patch itself looks fine though, if you send one that isn't mangled
> I'll apply it to the 2.6.23 branch.
> 
Sorry new Thunderbird Installation. I forgot it does that.

 - I have unearthed very old bugs in stale drivers that still
   used request->cmd as a READ|WRITE int
 - This patch is maybe a proof that these drivers have not been
   used for a long time. Should they be removed completely?

Drivers that currently do not work for sure:
 drivers/acorn/block/fd1772.c
 drivers/acorn/block/mfmhd.c
 drivers/cdrom/aztcd.c
 drivers/cdrom/cm206.c
 drivers/cdrom/gscd.c
 drivers/cdrom/mcdx.c
 drivers/cdrom/optcd.c
 drivers/cdrom/sjcd.c

Drivers with cosmetic fixes only:
  drivers/block/amiflop.c
  drivers/block/nbd.c
  drivers/ide/legacy/hd.c

Signed-off-by: Boaz Harrosh <bharrosh@panasas.com>
---
 drivers/acorn/block/fd1772.c |    2 +-
 drivers/acorn/block/mfmhd.c  |   13 +++----------
 drivers/block/amiflop.c      |    2 +-
 drivers/block/nbd.c          |    2 +-
 drivers/cdrom/aztcd.c        |    2 +-
 drivers/cdrom/cm206.c        |    2 +-
 drivers/cdrom/gscd.c         |    2 +-
 drivers/cdrom/mcdx.c         |    2 +-
 drivers/cdrom/optcd.c        |    2 +-
 drivers/cdrom/sjcd.c         |    2 +-
 drivers/ide/legacy/hd.c      |    3 ++-
 11 files changed, 14 insertions(+), 20 deletions(-)

diff --git a/drivers/acorn/block/fd1772.c b/drivers/acorn/block/fd1772.c
index 674bf81..423ed08 100644
--- a/drivers/acorn/block/fd1772.c
+++ b/drivers/acorn/block/fd1772.c
@@ -1246,7 +1246,7 @@ repeat:
 	del_timer(&motor_off_timer);

 	ReqCnt = 0;
-	ReqCmd = CURRENT->cmd;
+	ReqCmd = rq_data_dir(CURRENT);
 	ReqBlock = CURRENT->sector;
 	ReqBuffer = CURRENT->buffer;
 	setup_req_params(drive);
diff --git a/drivers/acorn/block/mfmhd.c b/drivers/acorn/block/mfmhd.c
index 689a4c3..d85520f 100644
--- a/drivers/acorn/block/mfmhd.c
+++ b/drivers/acorn/block/mfmhd.c
@@ -439,7 +439,7 @@ static void mfm_rw_intr(void)
 	   a choice of command end or some data which is ready to be collected */
 	/* I think we have to transfer data while the interrupt line is on and its
 	   not any other type of interrupt */
-	if (CURRENT->cmd == WRITE) {
+	if (rq_data_dir(CURRENT) == WRITE) {
 		extern void hdc63463_writedma(void);
 		if ((hdc63463_dataleft <= 0) && (!(mfm_status & STAT_CED))) {
 			printk("mfm_rw_intr: Apparent DMA write request when no more to DMA\n");
@@ -799,7 +799,7 @@ static void issue_request(unsigned int block, unsigned int nsect,
 	raw_cmd.head = start_head;
 	raw_cmd.cylinder = track / p->heads;
 	raw_cmd.cmdtype = CURRENT->cmd;
-	raw_cmd.cmdcode = CURRENT->cmd == WRITE ? CMD_WD : CMD_RD;
+	raw_cmd.cmdcode = rq_data_dir(CURRENT) == WRITE ? CMD_WD : CMD_RD;
 	raw_cmd.cmddata[0] = dev + 1;	/* DAG: +1 to get US */
 	raw_cmd.cmddata[1] = raw_cmd.head;
 	raw_cmd.cmddata[2] = raw_cmd.cylinder >> 8;
@@ -830,7 +830,7 @@ static void issue_request(unsigned int block, unsigned int nsect,
 	hdc63463_dataleft = nsect * 256;	/* Better way? */

 	DBG("mfm%c: %sing: CHS=%d/%d/%d, sectors=%d, buffer=0x%08lx (%p)\n",
-	     raw_cmd.dev + 'a', (CURRENT->cmd == READ) ? "read" : "writ",
+	     raw_cmd.dev + 'a', rq_data_dir(CURRENT) == READ ? "read" : "writ",
 		       raw_cmd.cylinder,
 		       raw_cmd.head,
 	    raw_cmd.sector, nsect, (unsigned long) Copy_buffer, CURRENT);
@@ -917,13 +917,6 @@ static void mfm_request(void)

 		DBG("mfm_request: block after offset=%d\n", block);

-		if (CURRENT->cmd != READ && CURRENT->cmd != WRITE) {
-			printk("unknown mfm-command %d\n", CURRENT->cmd);
-			end_request(CURRENT, 0);
-			Busy = 0;
-			printk("mfm: continue 4\n");
-			continue;
-		}
 		issue_request(block, nsect, CURRENT);

 		break;
diff --git a/drivers/block/amiflop.c b/drivers/block/amiflop.c
index 27a1390..6ce8b89 100644
--- a/drivers/block/amiflop.c
+++ b/drivers/block/amiflop.c
@@ -1363,7 +1363,7 @@ static void redo_fd_request(void)
 #ifdef DEBUG
 		printk("fd: sector %ld + %d requested for %s\n",
 		       CURRENT->sector,cnt,
-		       (CURRENT->cmd==READ)?"read":"write");
+		       (rq_data_dir(CURRENT) == READ) ? "read" : "write");
 #endif
 		block = CURRENT->sector + cnt;
 		if ((int)block > floppy->blocks) {
diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c
index 069ae39..c575fb1 100644
--- a/drivers/block/nbd.c
+++ b/drivers/block/nbd.c
@@ -416,7 +416,7 @@ static void nbd_clear_que(struct nbd_device *lo)
 /*
  * We always wait for result of write, for now. It would be nice to make it optional
  * in future
- * if ((req->cmd == WRITE) && (lo->flags & NBD_WRITE_NOCHK))
+ * if ((rq_data_dir(req) == WRITE) && (lo->flags & NBD_WRITE_NOCHK))
  *   { printk( "Warning: Ignoring result!\n"); nbd_end_request( req ); }
  */

diff --git a/drivers/cdrom/aztcd.c b/drivers/cdrom/aztcd.c
index 1f9fb7a..d7fbfaa 100644
--- a/drivers/cdrom/aztcd.c
+++ b/drivers/cdrom/aztcd.c
@@ -229,7 +229,7 @@ static struct request_queue *azt_queue;
 static int current_valid(void)
 {
         return CURRENT &&
-		CURRENT->cmd == READ &&
+		rq_data_dir(CURRENT) == READ &&
 		CURRENT->sector != -1;
 }

diff --git a/drivers/cdrom/cm206.c b/drivers/cdrom/cm206.c
index 2301311..2f8fe3b 100644
--- a/drivers/cdrom/cm206.c
+++ b/drivers/cdrom/cm206.c
@@ -851,7 +851,7 @@ static void do_cm206_request(request_queue_t * q)
 		if (!req)
 			return;

-		if (req->cmd != READ) {
+		if (rq_data_dir(req) != READ) {
 			debug(("Non-read command %d on cdrom\n", req->cmd));
 			end_request(req, 0);
 			continue;
diff --git a/drivers/cdrom/gscd.c b/drivers/cdrom/gscd.c
index b3ab6e9..176742e 100644
--- a/drivers/cdrom/gscd.c
+++ b/drivers/cdrom/gscd.c
@@ -264,7 +264,7 @@ repeat:
 	if (req->sector == -1)
 		goto out;

-	if (req->cmd != READ) {
+	if (rq_data_dir(req) != READ) {
 		printk("GSCD: bad cmd %u\n", rq_data_dir(req));
 		end_request(req, 0);
 		goto repeat;
diff --git a/drivers/cdrom/mcdx.c b/drivers/cdrom/mcdx.c
index 4310cc8..972ee9c 100644
--- a/drivers/cdrom/mcdx.c
+++ b/drivers/cdrom/mcdx.c
@@ -596,7 +596,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);
diff --git a/drivers/cdrom/optcd.c b/drivers/cdrom/optcd.c
index 3541690..efd619c 100644
--- a/drivers/cdrom/optcd.c
+++ b/drivers/cdrom/optcd.c
@@ -977,7 +977,7 @@ static int update_toc(void)
 static int current_valid(void)
 {
         return CURRENT &&
-		CURRENT->cmd == READ &&
+		rq_data_dir(CURRENT) == READ &&
 		CURRENT->sector != -1;
 }

diff --git a/drivers/cdrom/sjcd.c b/drivers/cdrom/sjcd.c
index 5409fca..76c24e6 100644
--- a/drivers/cdrom/sjcd.c
+++ b/drivers/cdrom/sjcd.c
@@ -1064,7 +1064,7 @@ static void sjcd_invalidate_buffers(void)
 static int current_valid(void)
 {
         return CURRENT &&
-		CURRENT->cmd == READ &&
+		rq_data_dir(CURRENT) == READ &&
 		CURRENT->sector != -1;
 }

diff --git a/drivers/ide/legacy/hd.c b/drivers/ide/legacy/hd.c
index 45ed035..a954142 100644
--- a/drivers/ide/legacy/hd.c
+++ b/drivers/ide/legacy/hd.c
@@ -623,7 +623,8 @@ repeat:
 	cyl   = track / disk->head;
 #ifdef DEBUG
 	printk("%s: %sing: CHS=%d/%d/%d, sectors=%d, buffer=%p\n",
-		req->rq_disk->disk_name, (req->cmd == READ)?"read":"writ",
+		req->rq_disk->disk_name,
+		req_data_dir(req) == READ ? "read" : "writ",
 		cyl, head, sec, nsect, req->buffer);
 #endif
 	if (blk_fs_request(req)) {
-- 
1.5.0.4.402.g8035



^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH] fix req->cmd == INT cases
  2007-06-20 10:53   ` Boaz Harrosh
@ 2007-06-20 10:59     ` Jens Axboe
  2007-06-20 11:31       ` Boaz Harrosh
  2007-06-20 12:22     ` Christoph Hellwig
  1 sibling, 1 reply; 10+ messages in thread
From: Jens Axboe @ 2007-06-20 10:59 UTC (permalink / raw)
  To: Boaz Harrosh; +Cc: linux-scsi

On Wed, Jun 20 2007, Boaz Harrosh wrote:
> Jens Axboe wrote:
> > 
> > If you look at most of the code, it's inside ifdef debug statements or
> > comments most of them. So I don't think you can base any removal
> > suggestion on that.
> > 
> > The patch itself looks fine though, if you send one that isn't mangled
> > I'll apply it to the 2.6.23 branch.
> > 
> Sorry new Thunderbird Installation. I forgot it does that.

The new patch applies cleanly, except on nbd where it fails. It suceeds
with -l to ignore white space, so perhaps your mail setup isn't 100% up
to standard yet :-)

-- 
Jens Axboe


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] fix req->cmd == INT cases
  2007-06-20 10:59     ` Jens Axboe
@ 2007-06-20 11:31       ` Boaz Harrosh
  0 siblings, 0 replies; 10+ messages in thread
From: Boaz Harrosh @ 2007-06-20 11:31 UTC (permalink / raw)
  To: Jens Axboe; +Cc: linux-scsi

Jens Axboe wrote:
> On Wed, Jun 20 2007, Boaz Harrosh wrote:
>> Jens Axboe wrote:
>>> If you look at most of the code, it's inside ifdef debug statements or
>>> comments most of them. So I don't think you can base any removal
>>> suggestion on that.
>>>
>>> The patch itself looks fine though, if you send one that isn't mangled
>>> I'll apply it to the 2.6.23 branch.
>>>
>> Sorry new Thunderbird Installation. I forgot it does that.
> 
> The new patch applies cleanly, except on nbd where it fails. It suceeds
> with -l to ignore white space, so perhaps your mail setup isn't 100% up
> to standard yet :-)
> 

Thanks, you are right again, it was set to wrap text at 85 chars and when
so, it will eat spaces at end of lines. Setting wrap to 0 fixes it.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] fix req->cmd == INT cases
  2007-06-20 10:53   ` Boaz Harrosh
  2007-06-20 10:59     ` Jens Axboe
@ 2007-06-20 12:22     ` Christoph Hellwig
  2007-06-20 12:24       ` Jens Axboe
  1 sibling, 1 reply; 10+ messages in thread
From: Christoph Hellwig @ 2007-06-20 12:22 UTC (permalink / raw)
  To: Boaz Harrosh; +Cc: Jens Axboe, linux-scsi, linux-kernel

On Wed, Jun 20, 2007 at 01:53:32PM +0300, Boaz Harrosh wrote:
> Jens Axboe wrote:
> > 
> > If you look at most of the code, it's inside ifdef debug statements or
> > comments most of them. So I don't think you can base any removal
> > suggestion on that.
> > 
> > The patch itself looks fine though, if you send one that isn't mangled
> > I'll apply it to the 2.6.23 branch.
> > 
> Sorry new Thunderbird Installation. I forgot it does that.
> 
>  - I have unearthed very old bugs in stale drivers that still
>    used request->cmd as a READ|WRITE int
>  - This patch is maybe a proof that these drivers have not been
>    used for a long time. Should they be removed completely?
> 
> Drivers that currently do not work for sure:
>  drivers/acorn/block/fd1772.c
>  drivers/acorn/block/mfmhd.c

Afaik these are old arm26 driver, and that port is dead and should
probably be removed from the tree entirely.

>  drivers/cdrom/aztcd.c
>  drivers/cdrom/cm206.c
>  drivers/cdrom/gscd.c
>  drivers/cdrom/mcdx.c
>  drivers/cdrom/optcd.c
>  drivers/cdrom/sjcd.c

These are old cdrom drivers that are broken in various ways and probably
should be killed off aswell.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] fix req->cmd == INT cases
  2007-06-20 12:22     ` Christoph Hellwig
@ 2007-06-20 12:24       ` Jens Axboe
  2007-06-20 12:26         ` Christoph Hellwig
  2007-06-20 21:16         ` Rene Herman
  0 siblings, 2 replies; 10+ messages in thread
From: Jens Axboe @ 2007-06-20 12:24 UTC (permalink / raw)
  To: Christoph Hellwig, Boaz Harrosh, linux-scsi, linux-kernel

On Wed, Jun 20 2007, Christoph Hellwig wrote:
> On Wed, Jun 20, 2007 at 01:53:32PM +0300, Boaz Harrosh wrote:
> > Jens Axboe wrote:
> > > 
> > > If you look at most of the code, it's inside ifdef debug statements or
> > > comments most of them. So I don't think you can base any removal
> > > suggestion on that.
> > > 
> > > The patch itself looks fine though, if you send one that isn't mangled
> > > I'll apply it to the 2.6.23 branch.
> > > 
> > Sorry new Thunderbird Installation. I forgot it does that.
> > 
> >  - I have unearthed very old bugs in stale drivers that still
> >    used request->cmd as a READ|WRITE int
> >  - This patch is maybe a proof that these drivers have not been
> >    used for a long time. Should they be removed completely?
> > 
> > Drivers that currently do not work for sure:
> >  drivers/acorn/block/fd1772.c
> >  drivers/acorn/block/mfmhd.c
> 
> Afaik these are old arm26 driver, and that port is dead and should
> probably be removed from the tree entirely.
> 
> >  drivers/cdrom/aztcd.c
> >  drivers/cdrom/cm206.c
> >  drivers/cdrom/gscd.c
> >  drivers/cdrom/mcdx.c
> >  drivers/cdrom/optcd.c
> >  drivers/cdrom/sjcd.c
> 
> These are old cdrom drivers that are broken in various ways and probably
> should be killed off aswell.

I agree (with both sentiments), would anyone mind if I just killed them
off? mitsumi support is being reworked, that can get reintroduced once
the driver is in a stable state.

-- 
Jens Axboe


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] fix req->cmd == INT cases
  2007-06-20 12:24       ` Jens Axboe
@ 2007-06-20 12:26         ` Christoph Hellwig
  2007-06-20 21:16         ` Rene Herman
  1 sibling, 0 replies; 10+ messages in thread
From: Christoph Hellwig @ 2007-06-20 12:26 UTC (permalink / raw)
  To: Jens Axboe; +Cc: Christoph Hellwig, Boaz Harrosh, linux-scsi, linux-kernel

On Wed, Jun 20, 2007 at 02:24:09PM +0200, Jens Axboe wrote:
> > These are old cdrom drivers that are broken in various ways and probably
> > should be killed off aswell.
> 
> I agree (with both sentiments), would anyone mind if I just killed them
> off? mitsumi support is being reworked, that can get reintroduced once
> the driver is in a stable state.

Yeah - the new mitsumi doesn't share anything with the old one anyway.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] fix req->cmd == INT cases
  2007-06-20 12:24       ` Jens Axboe
  2007-06-20 12:26         ` Christoph Hellwig
@ 2007-06-20 21:16         ` Rene Herman
  2007-06-21  6:32           ` Jens Axboe
  1 sibling, 1 reply; 10+ messages in thread
From: Rene Herman @ 2007-06-20 21:16 UTC (permalink / raw)
  To: Jens Axboe; +Cc: Christoph Hellwig, Boaz Harrosh, linux-scsi, linux-kernel

On 06/20/2007 02:24 PM, Jens Axboe wrote:

> On Wed, Jun 20 2007, Christoph Hellwig wrote:
>> On Wed, Jun 20, 2007 at 01:53:32PM +0300, Boaz Harrosh wrote:

>>>  drivers/cdrom/aztcd.c
>>>  drivers/cdrom/cm206.c
>>>  drivers/cdrom/gscd.c
>>>  drivers/cdrom/mcdx.c
>>>  drivers/cdrom/optcd.c
>>>  drivers/cdrom/sjcd.c
>>> 
>> These are old cdrom drivers that are broken in various ways and
>> probably should be killed off aswell.
> 
> I agree (with both sentiments), would anyone mind if I just killed them
> off?

I wouldn't. As far as I'm concerned everything in drivers/cdrom except 
viocd.c and cdrom.c itself can go, assuming I'll be allowed to bring back 
support for the legacy cdrom types I'd still like to have supported -- in 
the first place mitsumi (mcdx), panasonic (sbpcd) and sony (cdu31a) and 
perhaps at some point other types if/when I happen across the hardware.

The old drivers serve as a source of hardware information but at least mcdx 
was broken in so many ways that it only did so at the source level. When I 
wanted to check throughput with the old driver I actually had to go back as 
far as 2.0.34 to find a working driver (the old mcd.c, already removed since 
2.6.10). 2.0.34 sources are available from kernel.org, and 2.6.22 sources 
even from my local machine...

> mitsumi support is being reworked, that can get reintroduced once the
> driver is in a stable state.

... which, by the way, is still waiting on comment from anyone with a clue 
as to why it makes the machine go boom (easily repeatable when using CFQ, 
not or not easily when using AS):

http://lkml.org/lkml/2007/6/4/50

Rene.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] fix req->cmd == INT cases
  2007-06-20 21:16         ` Rene Herman
@ 2007-06-21  6:32           ` Jens Axboe
  0 siblings, 0 replies; 10+ messages in thread
From: Jens Axboe @ 2007-06-21  6:32 UTC (permalink / raw)
  To: Rene Herman; +Cc: Christoph Hellwig, Boaz Harrosh, linux-scsi, linux-kernel

On Wed, Jun 20 2007, Rene Herman wrote:
> On 06/20/2007 02:24 PM, Jens Axboe wrote:
> 
> >On Wed, Jun 20 2007, Christoph Hellwig wrote:
> >>On Wed, Jun 20, 2007 at 01:53:32PM +0300, Boaz Harrosh wrote:
> 
> >>> drivers/cdrom/aztcd.c
> >>> drivers/cdrom/cm206.c
> >>> drivers/cdrom/gscd.c
> >>> drivers/cdrom/mcdx.c
> >>> drivers/cdrom/optcd.c
> >>> drivers/cdrom/sjcd.c
> >>>
> >>These are old cdrom drivers that are broken in various ways and
> >>probably should be killed off aswell.
> >
> >I agree (with both sentiments), would anyone mind if I just killed them
> >off?
> 
> I wouldn't. As far as I'm concerned everything in drivers/cdrom except 
> viocd.c and cdrom.c itself can go, assuming I'll be allowed to bring back 
> support for the legacy cdrom types I'd still like to have supported -- in 
> the first place mitsumi (mcdx), panasonic (sbpcd) and sony (cdu31a) and 
> perhaps at some point other types if/when I happen across the hardware.

Sure, a working clean driver would always be allowed in. I'll remove the
legacy drivers in 2.6.23.

> The old drivers serve as a source of hardware information but at least
> mcdx was broken in so many ways that it only did so at the source
> level. When I wanted to check throughput with the old driver I
> actually had to go back as far as 2.0.34 to find a working driver (the
> old mcd.c, already removed since 2.6.10). 2.0.34 sources are available
> from kernel.org, and 2.6.22 sources even from my local machine...

Heh, that's pretty scary!

> >mitsumi support is being reworked, that can get reintroduced once the
> >driver is in a stable state.
> 
> ... which, by the way, is still waiting on comment from anyone with a clue 
> as to why it makes the machine go boom (easily repeatable when using CFQ, 
> not or not easily when using AS):
> 
> http://lkml.org/lkml/2007/6/4/50

Yeah I know, I will get around to it eventually.

-- 
Jens Axboe


^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2007-06-21  6:33 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2007-06-19 15:59 [PATCH] fix req->cmd == INT cases Boaz Harrosh
2007-06-19 17:16 ` Jens Axboe
2007-06-20 10:53   ` Boaz Harrosh
2007-06-20 10:59     ` Jens Axboe
2007-06-20 11:31       ` Boaz Harrosh
2007-06-20 12:22     ` Christoph Hellwig
2007-06-20 12:24       ` Jens Axboe
2007-06-20 12:26         ` Christoph Hellwig
2007-06-20 21:16         ` Rene Herman
2007-06-21  6:32           ` Jens Axboe

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).