linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [RFT PATCH] amiga, atari floppy: Use one request queue per disk
@ 2010-09-23 19:54 Vivek Goyal
  2010-09-23 19:54 ` [PATCH 1/2] amiga floppy: Stop sharing request queue across multiple gendisks Vivek Goyal
                   ` (3 more replies)
  0 siblings, 4 replies; 15+ messages in thread
From: Vivek Goyal @ 2010-09-23 19:54 UTC (permalink / raw)
  To: linux-kernel, axboe; +Cc: vgoyal, hch


Hi,

We seem to have deprecated the notion of sharing request queue across gendisks. Now we need to instanciate one request queue per disk.

There see to be still some drivers sharing request queue across disks. Arch
specific floppy drivers like amiga and atari are doing so.

These are two patches which should fix the issue. But these patches are
completely untested. Not even compilte tested. Don't have hardware to test
them. 

Would be great if somebody who has the hardware can lend a hand here to
see if these patches work.

Thanks
Vivek

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

* [PATCH 1/2] amiga floppy: Stop sharing request queue across multiple gendisks
  2010-09-23 19:54 [RFT PATCH] amiga, atari floppy: Use one request queue per disk Vivek Goyal
@ 2010-09-23 19:54 ` Vivek Goyal
  2010-10-28 17:38   ` Geert Uytterhoeven
  2010-09-23 19:54 ` [PATCH 2/2] atari " Vivek Goyal
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 15+ messages in thread
From: Vivek Goyal @ 2010-09-23 19:54 UTC (permalink / raw)
  To: linux-kernel, axboe; +Cc: vgoyal, hch

o Use one request queue per gendisk instead of sharing request queue

o Don't have hardware. No compile testing or run time testing done. Completely
  untested.

Signed-off-by: Vivek Goyal <vgoyal@redhat.com>
---
 drivers/block/amiflop.c |   59 ++++++++++++++++++++++++++++++++++++++--------
 1 files changed, 48 insertions(+), 11 deletions(-)

diff --git a/drivers/block/amiflop.c b/drivers/block/amiflop.c
index 76f114f..ead8b77 100644
--- a/drivers/block/amiflop.c
+++ b/drivers/block/amiflop.c
@@ -114,8 +114,6 @@ static unsigned long int fd_def_df0 = FD_DD_3;     /* default for df0 if it does
 module_param(fd_def_df0, ulong, 0);
 MODULE_LICENSE("GPL");
 
-static struct request_queue *floppy_queue;
-
 /*
  *  Macros
  */
@@ -164,6 +162,7 @@ static volatile int selected = -1;	/* currently selected drive */
 static int writepending;
 static int writefromint;
 static char *raw_buf;
+static int fdc_queue;
 
 static DEFINE_SPINLOCK(amiflop_lock);
 
@@ -1334,6 +1333,42 @@ static int get_track(int drive, int track)
 	return -1;
 }
 
+/*
+ * Round-robin between our available drives, doing one request from each
+ */
+static struct request *set_next_request(void)
+{
+	struct request_queue *q;
+	int cnt = FD_MAX_UNITS;
+	struct request *rq;
+
+	/* Find next queue we can dispatch from */
+	fdc_queue = fdc_queue + 1;
+	if (fdc_queue == FD_MAX_UNITS)
+		fdc_queue = 0;
+
+	for(cnt = FD_MAX_UNITS; cnt > 0, cnt--) {
+
+		if (unit[fdc_queue].type->code == FD_NODRIVE) {
+			if (++fdc_queue == FD_MAX_UNITS)
+				fdc_queue = 0;
+			cotinue;
+		}
+
+		q = unit[fdc_queue].gendisk->queue;
+		if (q) {
+			rq = blk_fetch_request(q);
+			if (rq)
+				break;
+		}
+
+		if (++fdc_queue == FD_MAX_UNITS)
+			fdc_queue = 0;
+	}
+
+	return rq;
+}
+
 static void redo_fd_request(void)
 {
 	struct request *rq;
@@ -1345,7 +1380,7 @@ static void redo_fd_request(void)
 	int err;
 
 next_req:
-	rq = blk_fetch_request(floppy_queue);
+	rq = set_next_request();
 	if (!rq) {
 		/* Nothing left to do */
 		return;
@@ -1682,6 +1717,13 @@ static int __init fd_probe_drives(void)
 			continue;
 		}
 		unit[drive].gendisk = disk;
+
+		disk->queue = blk_init_queue(do_fd_request, &amiflop_lock);
+		if (!disk->queue) {
+			unit[drive].type->code = FD_NODRIVE;
+			continue;
+		}
+
 		drives++;
 		if ((unit[drive].trackbuf = kmalloc(FLOPPY_MAX_SECTORS * 512, GFP_KERNEL)) == NULL) {
 			printk("no mem for ");
@@ -1695,7 +1737,6 @@ static int __init fd_probe_drives(void)
 		disk->fops = &floppy_fops;
 		sprintf(disk->disk_name, "fd%d", drive);
 		disk->private_data = &unit[drive];
-		disk->queue = floppy_queue;
 		set_capacity(disk, 880*2);
 		add_disk(disk);
 	}
@@ -1743,11 +1784,6 @@ static int __init amiga_floppy_probe(struct platform_device *pdev)
 		goto out_irq2;
 	}
 
-	ret = -ENOMEM;
-	floppy_queue = blk_init_queue(do_fd_request, &amiflop_lock);
-	if (!floppy_queue)
-		goto out_queue;
-
 	ret = -ENODEV;
 	if (fd_probe_drives() < 1) /* No usable drives */
 		goto out_probe;
@@ -1791,7 +1827,6 @@ static int __init amiga_floppy_probe(struct platform_device *pdev)
 	return 0;
 
 out_probe:
-	blk_cleanup_queue(floppy_queue);
 out_queue:
 	free_irq(IRQ_AMIGA_CIAA_TB, NULL);
 out_irq2:
@@ -1810,9 +1845,12 @@ static int __exit amiga_floppy_remove(struct platform_device *pdev)
 
 	for( i = 0; i < FD_MAX_UNITS; i++) {
 		if (unit[i].type->code != FD_NODRIVE) {
+			struct request_queue *q = unit[i].gendisk->queue;
 			del_gendisk(unit[i].gendisk);
 			put_disk(unit[i].gendisk);
 			kfree(unit[i].trackbuf);
+			if (q)
+				blk_cleanup_queue(q);
 		}
 	}
 	blk_unregister_region(MKDEV(FLOPPY_MAJOR, 0), 256);
@@ -1820,7 +1858,6 @@ static int __exit amiga_floppy_remove(struct platform_device *pdev)
 	free_irq(IRQ_AMIGA_DSKBLK, NULL);
 	custom.dmacon = DMAF_DISK; /* disable DMA */
 	amiga_chip_free(raw_buf);
-	blk_cleanup_queue(floppy_queue);
 	unregister_blkdev(FLOPPY_MAJOR, "fd");
 }
 #endif
-- 
1.7.2.3


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

* [PATCH 2/2] atari floppy: Stop sharing request queue across multiple gendisks
  2010-09-23 19:54 [RFT PATCH] amiga, atari floppy: Use one request queue per disk Vivek Goyal
  2010-09-23 19:54 ` [PATCH 1/2] amiga floppy: Stop sharing request queue across multiple gendisks Vivek Goyal
@ 2010-09-23 19:54 ` Vivek Goyal
  2010-09-23 20:15   ` Vivek Goyal
  2010-09-23 22:35 ` [RFT PATCH] amiga, atari floppy: Use one request queue per disk Vivek Goyal
  2010-09-24 18:37 ` Jens Axboe
  3 siblings, 1 reply; 15+ messages in thread
From: Vivek Goyal @ 2010-09-23 19:54 UTC (permalink / raw)
  To: linux-kernel, axboe; +Cc: vgoyal, hch

o Use one request queue per gendisk instead of sharing the queue.

o Don't have hardware. No compile testing or run time testing done. Completely
  untested.

Signed-off-by: Vivek Goyal <vgoyal@redhat.com>
---
 drivers/block/ataflop.c |   50 ++++++++++++++++++++++++++++++++++++----------
 1 files changed, 39 insertions(+), 11 deletions(-)

diff --git a/drivers/block/ataflop.c b/drivers/block/ataflop.c
index aceb964..bad514cb 100644
--- a/drivers/block/ataflop.c
+++ b/drivers/block/ataflop.c
@@ -79,8 +79,8 @@
 
 #undef DEBUG
 
-static struct request_queue *floppy_queue;
 static struct request *fd_request;
+static int fdc_queue;
 
 /* Disk types: DD, HD, ED */
 static struct atari_disk_type {
@@ -1391,6 +1391,29 @@ static void setup_req_params( int drive )
 			ReqTrack, ReqSector, (unsigned long)ReqData ));
 }
 
+/*
+ * Round-robin between our available drives, doing one request from each
+ */
+static int set_next_request(void)
+{
+	struct request_queue *q;
+	int old_pos = fdc_queue;
+	struct request *rq;
+
+	do {
+		q = unit[fdc_queue].disk->queue;
+		if (++fdc_queue == FD_MAX_UNITS)
+			fdc_queue = 0;
+		if (q) {
+			rq = blk_fetch_request(q);
+			if (rq)
+				break;
+		}
+	} while (fdc_queue != old_pos);
+
+	return rq;
+}
+
 
 static void redo_fd_request(void)
 {
@@ -1405,7 +1428,7 @@ static void redo_fd_request(void)
 
 repeat:
 	if (!fd_request) {
-		fd_request = blk_fetch_request(floppy_queue);
+		fd_request = set_next_request();
 		if (!fd_request)
 			goto the_end;
 	}
@@ -1932,10 +1955,6 @@ static int __init atari_floppy_init (void)
 	PhysTrackBuffer = virt_to_phys(TrackBuffer);
 	BufferDrive = BufferSide = BufferTrack = -1;
 
-	floppy_queue = blk_init_queue(do_fd_request, &ataflop_lock);
-	if (!floppy_queue)
-		goto Enomem;
-
 	for (i = 0; i < FD_MAX_UNITS; i++) {
 		unit[i].track = -1;
 		unit[i].flags = 0;
@@ -1944,7 +1963,10 @@ static int __init atari_floppy_init (void)
 		sprintf(unit[i].disk->disk_name, "fd%d", i);
 		unit[i].disk->fops = &floppy_fops;
 		unit[i].disk->private_data = &unit[i];
-		unit[i].disk->queue = floppy_queue;
+		unit[i].disk->queue = blk_init_queue(do_fd_request,
+					&ataflop_lock);
+		if (!unit[i].disk->queue)
+			goto Enomem;
 		set_capacity(unit[i].disk, MAX_DISK_SIZE * 2);
 		add_disk(unit[i].disk);
 	}
@@ -1959,10 +1981,14 @@ static int __init atari_floppy_init (void)
 
 	return 0;
 Enomem:
-	while (i--)
+	while (i--) {
+		struct request_queue *q = unit[i].disk->queue;
+
 		put_disk(unit[i].disk);
-	if (floppy_queue)
-		blk_cleanup_queue(floppy_queue);
+		if (q)
+			blk_cleanup_queue(q);
+	}
+
 	unregister_blkdev(FLOPPY_MAJOR, "fd");
 	return -ENOMEM;
 }
@@ -2011,12 +2037,14 @@ static void __exit atari_floppy_exit(void)
 	int i;
 	blk_unregister_region(MKDEV(FLOPPY_MAJOR, 0), 256);
 	for (i = 0; i < FD_MAX_UNITS; i++) {
+		struct request_queue *q = unit[i].disk->queue;
+
 		del_gendisk(unit[i].disk);
 		put_disk(unit[i].disk);
+		blk_cleanup_queue(q);
 	}
 	unregister_blkdev(FLOPPY_MAJOR, "fd");
 
-	blk_cleanup_queue(floppy_queue);
 	del_timer_sync(&fd_timer);
 	atari_stram_free( DMABuffer );
 }
-- 
1.7.2.3


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

* Re: [PATCH 2/2] atari floppy: Stop sharing request queue across multiple gendisks
  2010-09-23 19:54 ` [PATCH 2/2] atari " Vivek Goyal
@ 2010-09-23 20:15   ` Vivek Goyal
  0 siblings, 0 replies; 15+ messages in thread
From: Vivek Goyal @ 2010-09-23 20:15 UTC (permalink / raw)
  To: linux-kernel, axboe; +Cc: hch

On Thu, Sep 23, 2010 at 03:54:06PM -0400, Vivek Goyal wrote:
> o Use one request queue per gendisk instead of sharing the queue.
> 
> o Don't have hardware. No compile testing or run time testing done. Completely
>   untested.
> 
> Signed-off-by: Vivek Goyal <vgoyal@redhat.com>
> ---
>  drivers/block/ataflop.c |   50 ++++++++++++++++++++++++++++++++++++----------
>  1 files changed, 39 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/block/ataflop.c b/drivers/block/ataflop.c
> index aceb964..bad514cb 100644
> --- a/drivers/block/ataflop.c
> +++ b/drivers/block/ataflop.c
> @@ -79,8 +79,8 @@
>  
>  #undef DEBUG
>  
> -static struct request_queue *floppy_queue;
>  static struct request *fd_request;
> +static int fdc_queue;
>  
>  /* Disk types: DD, HD, ED */
>  static struct atari_disk_type {
> @@ -1391,6 +1391,29 @@ static void setup_req_params( int drive )
>  			ReqTrack, ReqSector, (unsigned long)ReqData ));
>  }
>  
> +/*
> + * Round-robin between our available drives, doing one request from each
> + */
> +static int set_next_request(void)

While scanning the patch found one bug. set_next_request() should be 
returning "struct request *". Second version of patch is attached.


o Use one request queue per gendisk instead of sharing the queue.

o Don't have hardware. No compile testing or run time testing done. Completely
  untested.

Signed-off-by: Vivek Goyal <vgoyal@redhat.com>
---
 drivers/block/ataflop.c |   50 +++++++++++++++++++++++++++++++++++++-----------
 1 file changed, 39 insertions(+), 11 deletions(-)

Index: linux-2.6-block/drivers/block/ataflop.c
===================================================================
--- linux-2.6-block.orig/drivers/block/ataflop.c	2010-09-21 15:56:35.000000000 -0400
+++ linux-2.6-block/drivers/block/ataflop.c	2010-09-23 16:12:35.826001335 -0400
@@ -79,8 +79,8 @@
 
 #undef DEBUG
 
-static struct request_queue *floppy_queue;
 static struct request *fd_request;
+static int fdc_queue;
 
 /* Disk types: DD, HD, ED */
 static struct atari_disk_type {
@@ -1391,6 +1391,29 @@ static void setup_req_params( int drive 
 			ReqTrack, ReqSector, (unsigned long)ReqData ));
 }
 
+/*
+ * Round-robin between our available drives, doing one request from each
+ */
+static struct request *set_next_request(void)
+{
+	struct request_queue *q;
+	int old_pos = fdc_queue;
+	struct request *rq;
+
+	do {
+		q = unit[fdc_queue].disk->queue;
+		if (++fdc_queue == FD_MAX_UNITS)
+			fdc_queue = 0;
+		if (q) {
+			rq = blk_fetch_request(q);
+			if (rq)
+				break;
+		}
+	} while (fdc_queue != old_pos);
+
+	return rq;
+}
+
 
 static void redo_fd_request(void)
 {
@@ -1405,7 +1428,7 @@ static void redo_fd_request(void)
 
 repeat:
 	if (!fd_request) {
-		fd_request = blk_fetch_request(floppy_queue);
+		fd_request = set_next_request();
 		if (!fd_request)
 			goto the_end;
 	}
@@ -1932,10 +1955,6 @@ static int __init atari_floppy_init (voi
 	PhysTrackBuffer = virt_to_phys(TrackBuffer);
 	BufferDrive = BufferSide = BufferTrack = -1;
 
-	floppy_queue = blk_init_queue(do_fd_request, &ataflop_lock);
-	if (!floppy_queue)
-		goto Enomem;
-
 	for (i = 0; i < FD_MAX_UNITS; i++) {
 		unit[i].track = -1;
 		unit[i].flags = 0;
@@ -1944,7 +1963,10 @@ static int __init atari_floppy_init (voi
 		sprintf(unit[i].disk->disk_name, "fd%d", i);
 		unit[i].disk->fops = &floppy_fops;
 		unit[i].disk->private_data = &unit[i];
-		unit[i].disk->queue = floppy_queue;
+		unit[i].disk->queue = blk_init_queue(do_fd_request,
+					&ataflop_lock);
+		if (!unit[i].disk->queue)
+			goto Enomem;
 		set_capacity(unit[i].disk, MAX_DISK_SIZE * 2);
 		add_disk(unit[i].disk);
 	}
@@ -1959,10 +1981,14 @@ static int __init atari_floppy_init (voi
 
 	return 0;
 Enomem:
-	while (i--)
+	while (i--) {
+		struct request_queue *q = unit[i].disk->queue;
+
 		put_disk(unit[i].disk);
-	if (floppy_queue)
-		blk_cleanup_queue(floppy_queue);
+		if (q)
+			blk_cleanup_queue(q);
+	}
+
 	unregister_blkdev(FLOPPY_MAJOR, "fd");
 	return -ENOMEM;
 }
@@ -2011,12 +2037,14 @@ static void __exit atari_floppy_exit(voi
 	int i;
 	blk_unregister_region(MKDEV(FLOPPY_MAJOR, 0), 256);
 	for (i = 0; i < FD_MAX_UNITS; i++) {
+		struct request_queue *q = unit[i].disk->queue;
+
 		del_gendisk(unit[i].disk);
 		put_disk(unit[i].disk);
+		blk_cleanup_queue(q);
 	}
 	unregister_blkdev(FLOPPY_MAJOR, "fd");
 
-	blk_cleanup_queue(floppy_queue);
 	del_timer_sync(&fd_timer);
 	atari_stram_free( DMABuffer );
 }

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

* Re: [RFT PATCH] amiga, atari floppy: Use one request queue per disk
  2010-09-23 19:54 [RFT PATCH] amiga, atari floppy: Use one request queue per disk Vivek Goyal
  2010-09-23 19:54 ` [PATCH 1/2] amiga floppy: Stop sharing request queue across multiple gendisks Vivek Goyal
  2010-09-23 19:54 ` [PATCH 2/2] atari " Vivek Goyal
@ 2010-09-23 22:35 ` Vivek Goyal
  2010-09-24  0:43   ` Andreas Bombe
  2010-09-24  8:57   ` Jens Axboe
  2010-09-24 18:37 ` Jens Axboe
  3 siblings, 2 replies; 15+ messages in thread
From: Vivek Goyal @ 2010-09-23 22:35 UTC (permalink / raw)
  To: linux-kernel, axboe, linux-m68k; +Cc: hch

On Thu, Sep 23, 2010 at 03:54:04PM -0400, Vivek Goyal wrote:
> 
> Hi,
> 
> We seem to have deprecated the notion of sharing request queue across gendisks. Now we need to instanciate one request queue per disk.
> 
> There see to be still some drivers sharing request queue across disks. Arch
> specific floppy drivers like amiga and atari are doing so.
> 
> These are two patches which should fix the issue. But these patches are
> completely untested. Not even compilte tested. Don't have hardware to test
> them. 
> 
> Would be great if somebody who has the hardware can lend a hand here to
> see if these patches work.

Sending this mail to linux-m68k@lists.linux-m68k.org list, incase somebody
can help there with testing.

Vivek

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

* Re: [RFT PATCH] amiga, atari floppy: Use one request queue per disk
  2010-09-23 22:35 ` [RFT PATCH] amiga, atari floppy: Use one request queue per disk Vivek Goyal
@ 2010-09-24  0:43   ` Andreas Bombe
  2010-09-24  8:57   ` Jens Axboe
  1 sibling, 0 replies; 15+ messages in thread
From: Andreas Bombe @ 2010-09-24  0:43 UTC (permalink / raw)
  To: Vivek Goyal; +Cc: linux-kernel, axboe, linux-m68k, hch

On Thu, Sep 23, 2010 at 06:35:30PM -0400, Vivek Goyal wrote:
> On Thu, Sep 23, 2010 at 03:54:04PM -0400, Vivek Goyal wrote:
> > Would be great if somebody who has the hardware can lend a hand here to
> > see if these patches work.

Last time I looked at it (well over a year ago), amiflop was seriously
bitrotted from disuse. Just a warning, it may not work properly even
before applying these patches.


Andreas Bombe

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

* Re: [RFT PATCH] amiga, atari floppy: Use one request queue per disk
  2010-09-23 22:35 ` [RFT PATCH] amiga, atari floppy: Use one request queue per disk Vivek Goyal
  2010-09-24  0:43   ` Andreas Bombe
@ 2010-09-24  8:57   ` Jens Axboe
  1 sibling, 0 replies; 15+ messages in thread
From: Jens Axboe @ 2010-09-24  8:57 UTC (permalink / raw)
  To: Vivek Goyal; +Cc: linux-kernel, linux-m68k, hch

On 2010-09-24 00:35, Vivek Goyal wrote:
> On Thu, Sep 23, 2010 at 03:54:04PM -0400, Vivek Goyal wrote:
>>
>> Hi,
>>
>> We seem to have deprecated the notion of sharing request queue across gendisks. Now we need to instanciate one request queue per disk.
>>
>> There see to be still some drivers sharing request queue across disks. Arch
>> specific floppy drivers like amiga and atari are doing so.
>>
>> These are two patches which should fix the issue. But these patches are
>> completely untested. Not even compilte tested. Don't have hardware to test
>> them. 
>>
>> Would be great if somebody who has the hardware can lend a hand here to
>> see if these patches work.
> 
> Sending this mail to linux-m68k@lists.linux-m68k.org list, incase somebody
> can help there with testing.

I think for these drivers, the best we can probably due is ensure that it
cross compiles. As Andreas mentions, getting test coverage is hard and
it's not even given that they work as-is right now.

And since getting this done serves the greater higher purpose of getting
rid of some nasty hacks in the core kernel, we can't let some ancient
and bit rotted (and most likely completely unused) drivers stop this.

-- 
Jens Axboe


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

* Re: [RFT PATCH] amiga, atari floppy: Use one request queue per disk
  2010-09-23 19:54 [RFT PATCH] amiga, atari floppy: Use one request queue per disk Vivek Goyal
                   ` (2 preceding siblings ...)
  2010-09-23 22:35 ` [RFT PATCH] amiga, atari floppy: Use one request queue per disk Vivek Goyal
@ 2010-09-24 18:37 ` Jens Axboe
  2010-09-24 20:17   ` Vivek Goyal
  2010-09-24 20:21   ` Vivek Goyal
  3 siblings, 2 replies; 15+ messages in thread
From: Jens Axboe @ 2010-09-24 18:37 UTC (permalink / raw)
  To: Vivek Goyal; +Cc: linux-kernel, hch

On 2010-09-23 21:54, Vivek Goyal wrote:
> Hi,
> 
> We seem to have deprecated the notion of sharing request queue across gendisks. Now we need to instanciate one request queue per disk.
> 
> There see to be still some drivers sharing request queue across disks. Arch
> specific floppy drivers like amiga and atari are doing so.
> 
> These are two patches which should fix the issue. But these patches are
> completely untested. Not even compilte tested. Don't have hardware to test
> them. 
> 
> Would be great if somebody who has the hardware can lend a hand here to
> see if these patches work.

I have (tentatively) added both patches to for-2.6.37/drivers. I used the
latter version of 2/2.

-- 
Jens Axboe


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

* Re: [RFT PATCH] amiga, atari floppy: Use one request queue per disk
  2010-09-24 18:37 ` Jens Axboe
@ 2010-09-24 20:17   ` Vivek Goyal
  2010-09-24 20:21   ` Vivek Goyal
  1 sibling, 0 replies; 15+ messages in thread
From: Vivek Goyal @ 2010-09-24 20:17 UTC (permalink / raw)
  To: Jens Axboe; +Cc: linux-kernel, hch

On Fri, Sep 24, 2010 at 08:37:10PM +0200, Jens Axboe wrote:
> On 2010-09-23 21:54, Vivek Goyal wrote:
> > Hi,
> > 
> > We seem to have deprecated the notion of sharing request queue across gendisks. Now we need to instanciate one request queue per disk.
> > 
> > There see to be still some drivers sharing request queue across disks. Arch
> > specific floppy drivers like amiga and atari are doing so.
> > 
> > These are two patches which should fix the issue. But these patches are
> > completely untested. Not even compilte tested. Don't have hardware to test
> > them. 
> > 
> > Would be great if somebody who has the hardware can lend a hand here to
> > see if these patches work.
> 
> I have (tentatively) added both patches to for-2.6.37/drivers. I used the
> latter version of 2/2.

Thanks Jens.

Finally I could setup and do cross compile for m68k. amiga floppy driver
fails to compile. Please find attached the fix.

Thanks
Vivek

o Compile fixes for amiga floppy driver.

Signed-off-by: Vivek Goyal <vgoyal@redhat.com>
---
 drivers/block/amiflop.c |    5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

Index: linux-2.6-block/drivers/block/amiflop.c
===================================================================
--- linux-2.6-block.orig/drivers/block/amiflop.c	2010-09-24 16:12:19.000000000 -0400
+++ linux-2.6-block/drivers/block/amiflop.c	2010-09-24 16:13:38.000000000 -0400
@@ -1347,12 +1347,12 @@ static struct request *set_next_request(
 	if (fdc_queue == FD_MAX_UNITS)
 		fdc_queue = 0;
 
-	for(cnt = FD_MAX_UNITS; cnt > 0, cnt--) {
+	for(cnt = FD_MAX_UNITS; cnt > 0; cnt--) {
 
 		if (unit[fdc_queue].type->code == FD_NODRIVE) {
 			if (++fdc_queue == FD_MAX_UNITS)
 				fdc_queue = 0;
-			cotinue;
+			continue;
 		}
 
 		q = unit[fdc_queue].gendisk->queue;
@@ -1827,7 +1827,6 @@ static int __init amiga_floppy_probe(str
 	return 0;
 
 out_probe:
-out_queue:
 	free_irq(IRQ_AMIGA_CIAA_TB, NULL);
 out_irq2:
 	free_irq(IRQ_AMIGA_DSKBLK, NULL);

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

* Re: [RFT PATCH] amiga, atari floppy: Use one request queue per disk
  2010-09-24 18:37 ` Jens Axboe
  2010-09-24 20:17   ` Vivek Goyal
@ 2010-09-24 20:21   ` Vivek Goyal
  2010-09-24 21:41     ` Mark Lord
  2010-09-24 21:46     ` [PATCH] fix blk-exec.c compile error: always define 'sysctl_hung_task_timeout_secs' (resend) Mark Lord
  1 sibling, 2 replies; 15+ messages in thread
From: Vivek Goyal @ 2010-09-24 20:21 UTC (permalink / raw)
  To: Jens Axboe; +Cc: linux-kernel, hch, Mark Lord

On Fri, Sep 24, 2010 at 08:37:10PM +0200, Jens Axboe wrote:
> On 2010-09-23 21:54, Vivek Goyal wrote:
> > Hi,
> > 
> > We seem to have deprecated the notion of sharing request queue across gendisks. Now we need to instanciate one request queue per disk.
> > 
> > There see to be still some drivers sharing request queue across disks. Arch
> > specific floppy drivers like amiga and atari are doing so.
> > 
> > These are two patches which should fix the issue. But these patches are
> > completely untested. Not even compilte tested. Don't have hardware to test
> > them. 
> > 
> > Would be great if somebody who has the hardware can lend a hand here to
> > see if these patches work.
> 
> I have (tentatively) added both patches to for-2.6.37/drivers. I used the
> latter version of 2/2.

Also which cross compiling for m68k, I noticed following commit fails
compilation. I had to reset git to older commit to test amiga and atari
flopply driver compilation.

*******************************************************************
block/blk-exec.c: In function 'blk_execute_rq':
block/blk-exec.c:101: error: 'sysctl_hung_task_timeout_secs' undeclared
(first use in this function)
block/blk-exec.c:101: error: (Each undeclared identifier is reported only
once
block/blk-exec.c:101: error: for each function it appears in.)
make[1]: *** [block/blk-exec.o] Error 1
make: *** [block] Error 2
***********************************************************************

commit 4b1977698ceb4c4caa800d475127139da49966f9
Author: Mark Lord <kernel@teksavvy.com>
Date:   Fri Sep 24 09:51:13 2010 -0400

    block: Prevent hang_check firing during long I/O
    
    During long I/O operations, the hang_check timer may fire,
    trigger stack dumps that unnecessarily alarm the user.
    
    Eg.  hdparm --security-erase NULL /dev/sdb  ## can take *hours* to
complete
    
    So, if hang_check is armed, we should wake up periodically
    to prevent it from triggering.  This patch uses a wake-up interval
    equal to half the hang_check timer period, which keeps overhead low
enough.
    
    Signed-off-by: Mark Lord <mlord@pobox.com>
    Signed-off-by: Jens Axboe <jaxboe@fusionio.com>

Thanks
Vivek

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

* Re: [RFT PATCH] amiga, atari floppy: Use one request queue per disk
  2010-09-24 20:21   ` Vivek Goyal
@ 2010-09-24 21:41     ` Mark Lord
  2010-09-24 21:46     ` [PATCH] fix blk-exec.c compile error: always define 'sysctl_hung_task_timeout_secs' (resend) Mark Lord
  1 sibling, 0 replies; 15+ messages in thread
From: Mark Lord @ 2010-09-24 21:41 UTC (permalink / raw)
  To: Jens Axboe; +Cc: Vivek Goyal, linux-kernel, hch, IDE/ATA development list

[-- Attachment #1: Type: text/plain, Size: 803 bytes --]

Ensure that 'sysctl_hung_task_timeout_secs' is defined
even when CONFIG_DETECT_HUNG_TASK is not set.
This way we can safely reference it without need for
#ifdefs elsewhere in the code.  eg. in block/blk-exec.c

Signed-off-by: Mark Lord <mlord@pobox.com>

Jens: use the attachment.  My email client is having a fit today.

--- a/include/linux/sched.h	2010-09-20 19:56:53.000000000 -0400
+++ b/include/linux/sched.h	2010-09-24 17:22:01.707291995 -0400
@@ -336,6 +336,9 @@
  extern int proc_dohung_task_timeout_secs(struct ctl_table *table, int write,
  					 void __user *buffer,
  					 size_t *lenp, loff_t *ppos);
+#else
+/* Avoid need for ifdefs elsewhere in the code */
+enum { sysctl_hung_task_timeout_secs = 0 };
  #endif
  
  /* Attach to any functions which should be ignored in wchan output. */

[-- Attachment #2: fix_blk_exec_compile_error.patch --]
[-- Type: text/x-patch, Size: 475 bytes --]

--- a/include/linux/sched.h	2010-09-20 19:56:53.000000000 -0400
+++ b/include/linux/sched.h	2010-09-24 17:22:01.707291995 -0400
@@ -336,6 +336,9 @@
 extern int proc_dohung_task_timeout_secs(struct ctl_table *table, int write,
 					 void __user *buffer,
 					 size_t *lenp, loff_t *ppos);
+#else
+/* Avoid need for ifdefs elsewhere in the code */
+enum { sysctl_hung_task_timeout_secs = 0 };
 #endif
 
 /* Attach to any functions which should be ignored in wchan output. */

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

* [PATCH] fix blk-exec.c compile error: always define 'sysctl_hung_task_timeout_secs' (resend)
  2010-09-24 20:21   ` Vivek Goyal
  2010-09-24 21:41     ` Mark Lord
@ 2010-09-24 21:46     ` Mark Lord
  2010-09-25  9:18       ` Jens Axboe
  1 sibling, 1 reply; 15+ messages in thread
From: Mark Lord @ 2010-09-24 21:46 UTC (permalink / raw)
  To: Vivek Goyal, Jens Axboe; +Cc: linux-kernel, hch, IDE/ATA development list

Ensure that 'sysctl_hung_task_timeout_secs' is defined
even when CONFIG_DETECT_HUNG_TASK is not set.
This way we can safely reference it without need for
ifdefs in the code elsewhere.  eg. in block/blk-exec.c

Signed-off-by: Mark Lord <mlord@pobox.com>

(Resending from a repaired email client.)
Jens: my apologies for not catching this this earlier.

--- a/include/linux/sched.h	2010-09-20 19:56:53.000000000 -0400
+++ b/include/linux/sched.h	2010-09-24 17:22:01.707291995 -0400
@@ -336,6 +336,9 @@
 extern int proc_dohung_task_timeout_secs(struct ctl_table *table, int write,
 					 void __user *buffer,
 					 size_t *lenp, loff_t *ppos);
+#else
+/* Avoid need for ifdefs elsewhere in the code */
+enum { sysctl_hung_task_timeout_secs = 0 };
 #endif
 
 /* Attach to any functions which should be ignored in wchan output. */




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

* Re: [PATCH] fix blk-exec.c compile error: always define  'sysctl_hung_task_timeout_secs' (resend)
  2010-09-24 21:46     ` [PATCH] fix blk-exec.c compile error: always define 'sysctl_hung_task_timeout_secs' (resend) Mark Lord
@ 2010-09-25  9:18       ` Jens Axboe
  0 siblings, 0 replies; 15+ messages in thread
From: Jens Axboe @ 2010-09-25  9:18 UTC (permalink / raw)
  To: Mark Lord; +Cc: Vivek Goyal, linux-kernel, hch, IDE/ATA development list

On 2010-09-24 23:46, Mark Lord wrote:
> Ensure that 'sysctl_hung_task_timeout_secs' is defined
> even when CONFIG_DETECT_HUNG_TASK is not set.
> This way we can safely reference it without need for
> ifdefs in the code elsewhere.  eg. in block/blk-exec.c

Thanks Mark, applied.

-- 
Jens Axboe


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

* Re: [PATCH 1/2] amiga floppy: Stop sharing request queue across multiple gendisks
  2010-09-23 19:54 ` [PATCH 1/2] amiga floppy: Stop sharing request queue across multiple gendisks Vivek Goyal
@ 2010-10-28 17:38   ` Geert Uytterhoeven
  2010-10-28 18:08     ` Vivek Goyal
  0 siblings, 1 reply; 15+ messages in thread
From: Geert Uytterhoeven @ 2010-10-28 17:38 UTC (permalink / raw)
  To: Vivek Goyal; +Cc: linux-kernel, axboe, hch, Linux/m68k

On Thu, Sep 23, 2010 at 21:54, Vivek Goyal <vgoyal@redhat.com> wrote:
> o Use one request queue per gendisk instead of sharing request queue
>
> o Don't have hardware. No compile testing or run time testing done. Completely
>  untested.
>
> Signed-off-by: Vivek Goyal <vgoyal@redhat.com>
> ---
>  drivers/block/amiflop.c |   59 ++++++++++++++++++++++++++++++++++++++--------
>  1 files changed, 48 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/block/amiflop.c b/drivers/block/amiflop.c
> index 76f114f..ead8b77 100644
> --- a/drivers/block/amiflop.c
> +++ b/drivers/block/amiflop.c
> @@ -114,8 +114,6 @@ static unsigned long int fd_def_df0 = FD_DD_3;     /* default for df0 if it does
>  module_param(fd_def_df0, ulong, 0);
>  MODULE_LICENSE("GPL");
>
> -static struct request_queue *floppy_queue;
> -
>  /*
>  *  Macros
>  */
> @@ -164,6 +162,7 @@ static volatile int selected = -1;  /* currently selected drive */
>  static int writepending;
>  static int writefromint;
>  static char *raw_buf;
> +static int fdc_queue;
>
>  static DEFINE_SPINLOCK(amiflop_lock);
>
> @@ -1334,6 +1333,42 @@ static int get_track(int drive, int track)
>        return -1;
>  }
>
> +/*
> + * Round-robin between our available drives, doing one request from each
> + */
> +static struct request *set_next_request(void)
> +{
> +       struct request_queue *q;
> +       int cnt = FD_MAX_UNITS;
> +       struct request *rq;
> +
> +       /* Find next queue we can dispatch from */
> +       fdc_queue = fdc_queue + 1;
> +       if (fdc_queue == FD_MAX_UNITS)
> +               fdc_queue = 0;
> +
> +       for(cnt = FD_MAX_UNITS; cnt > 0, cnt--) {
> +
> +               if (unit[fdc_queue].type->code == FD_NODRIVE) {
> +                       if (++fdc_queue == FD_MAX_UNITS)
> +                               fdc_queue = 0;
> +                       cotinue;
> +               }
> +
> +               q = unit[fdc_queue].gendisk->queue;
> +               if (q) {
> +                       rq = blk_fetch_request(q);
> +                       if (rq)
> +                               break;
> +               }
> +
> +               if (++fdc_queue == FD_MAX_UNITS)
> +                       fdc_queue = 0;
> +       }
> +
> +       return rq;
> +}

drivers/block/amiflop.c:1344: warning: ‘rq’ may be used uninitialized
in this function
drivers/block/ataflop.c:1402: warning: ‘rq’ may be used uninitialized
in this function

Should `rq' just be initialized to NULL? I looked at
floppy.c:set_next_request(), but it's
completely different.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

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

* Re: [PATCH 1/2] amiga floppy: Stop sharing request queue across multiple gendisks
  2010-10-28 17:38   ` Geert Uytterhoeven
@ 2010-10-28 18:08     ` Vivek Goyal
  0 siblings, 0 replies; 15+ messages in thread
From: Vivek Goyal @ 2010-10-28 18:08 UTC (permalink / raw)
  To: Geert Uytterhoeven; +Cc: linux-kernel, axboe, hch, Linux/m68k

On Thu, Oct 28, 2010 at 07:38:33PM +0200, Geert Uytterhoeven wrote:
> On Thu, Sep 23, 2010 at 21:54, Vivek Goyal <vgoyal@redhat.com> wrote:
> > o Use one request queue per gendisk instead of sharing request queue
> >
> > o Don't have hardware. No compile testing or run time testing done. Completely
> >  untested.
> >
> > Signed-off-by: Vivek Goyal <vgoyal@redhat.com>
> > ---
> >  drivers/block/amiflop.c |   59 ++++++++++++++++++++++++++++++++++++++--------
> >  1 files changed, 48 insertions(+), 11 deletions(-)
> >
> > diff --git a/drivers/block/amiflop.c b/drivers/block/amiflop.c
> > index 76f114f..ead8b77 100644
> > --- a/drivers/block/amiflop.c
> > +++ b/drivers/block/amiflop.c
> > @@ -114,8 +114,6 @@ static unsigned long int fd_def_df0 = FD_DD_3;     /* default for df0 if it does
> >  module_param(fd_def_df0, ulong, 0);
> >  MODULE_LICENSE("GPL");
> >
> > -static struct request_queue *floppy_queue;
> > -
> >  /*
> >  *  Macros
> >  */
> > @@ -164,6 +162,7 @@ static volatile int selected = -1;  /* currently selected drive */
> >  static int writepending;
> >  static int writefromint;
> >  static char *raw_buf;
> > +static int fdc_queue;
> >
> >  static DEFINE_SPINLOCK(amiflop_lock);
> >
> > @@ -1334,6 +1333,42 @@ static int get_track(int drive, int track)
> >        return -1;
> >  }
> >
> > +/*
> > + * Round-robin between our available drives, doing one request from each
> > + */
> > +static struct request *set_next_request(void)
> > +{
> > +       struct request_queue *q;
> > +       int cnt = FD_MAX_UNITS;
> > +       struct request *rq;
> > +
> > +       /* Find next queue we can dispatch from */
> > +       fdc_queue = fdc_queue + 1;
> > +       if (fdc_queue == FD_MAX_UNITS)
> > +               fdc_queue = 0;
> > +
> > +       for(cnt = FD_MAX_UNITS; cnt > 0, cnt--) {
> > +
> > +               if (unit[fdc_queue].type->code == FD_NODRIVE) {
> > +                       if (++fdc_queue == FD_MAX_UNITS)
> > +                               fdc_queue = 0;
> > +                       cotinue;
> > +               }
> > +
> > +               q = unit[fdc_queue].gendisk->queue;
> > +               if (q) {
> > +                       rq = blk_fetch_request(q);
> > +                       if (rq)
> > +                               break;
> > +               }
> > +
> > +               if (++fdc_queue == FD_MAX_UNITS)
> > +                       fdc_queue = 0;
> > +       }
> > +
> > +       return rq;
> > +}
> 
> drivers/block/amiflop.c:1344: warning: ‘rq’ may be used uninitialized
> in this function
> drivers/block/ataflop.c:1402: warning: ‘rq’ may be used uninitialized
> in this function
> 
> Should `rq' just be initialized to NULL? I looked at
> floppy.c:set_next_request(), but it's
> completely different.
> 

I think we should explicitly initialize rq=NULL so that we don't return
a garbage value if we can't find any rq to dispatch.

I will send a patch to fix that. 

Thanks
Vivek

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

end of thread, other threads:[~2010-10-28 18:09 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-09-23 19:54 [RFT PATCH] amiga, atari floppy: Use one request queue per disk Vivek Goyal
2010-09-23 19:54 ` [PATCH 1/2] amiga floppy: Stop sharing request queue across multiple gendisks Vivek Goyal
2010-10-28 17:38   ` Geert Uytterhoeven
2010-10-28 18:08     ` Vivek Goyal
2010-09-23 19:54 ` [PATCH 2/2] atari " Vivek Goyal
2010-09-23 20:15   ` Vivek Goyal
2010-09-23 22:35 ` [RFT PATCH] amiga, atari floppy: Use one request queue per disk Vivek Goyal
2010-09-24  0:43   ` Andreas Bombe
2010-09-24  8:57   ` Jens Axboe
2010-09-24 18:37 ` Jens Axboe
2010-09-24 20:17   ` Vivek Goyal
2010-09-24 20:21   ` Vivek Goyal
2010-09-24 21:41     ` Mark Lord
2010-09-24 21:46     ` [PATCH] fix blk-exec.c compile error: always define 'sysctl_hung_task_timeout_secs' (resend) Mark Lord
2010-09-25  9:18       ` 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).