All of lore.kernel.org
 help / color / mirror / Atom feed
* cgq vs bdi names, was "cfq-iosched: fix a kernel OOPs when usb key is inserted"
@ 2010-09-20 13:03 Christoph Hellwig
  2010-09-20 13:12 ` Jens Axboe
  2010-09-20 13:32 ` cgq vs bdi names, was "cfq-iosched: fix a kernel OOPs when usb key is inserted" Vivek Goyal
  0 siblings, 2 replies; 10+ messages in thread
From: Christoph Hellwig @ 2010-09-20 13:03 UTC (permalink / raw)
  To: vgoyal; +Cc: jaxboe, linux-kernel

Hi Vivek, hi Jens,

where was http://git.kernel.dk/?p=linux-2.6-block.git;a=commitdiff_plain;h=44c74d6292e97f8bd9adfa6b0df3cb4f3c42a6dc posted  on the mailinglist?

I can't find it in my lkml or fsdevel inboxes.  Either way I don't think
just papering over the underlying issue like this is a good idea.  

The big issue is that cfq tries to scanf the textual representation of
the dev_t from the request_queue by abusing the bdi.  But the reason why
we don't have a dev_t in the request_queue is that it's still not
unique.  If it was we could easily add a dev_t into the request_queue
and be done with it.

So the fix is either to get rid of the last remaining users of shared
request_queues (IIRC the various floppy drivers) and just add a dev_t
in the request_queue for the bdi, tracing and cfq, or add a dev_t into
the request_queue and add a flag for shared request queues that the
floppy driver and whoever needs it set and let the bdi sysfs code, cfq
and blocktrace ignore theis request_queue.  This will also allow to
get rid of the crap about ignoring failures due to already register
or prematurely unregistered bdis and actually add real error handling
to that code.

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

* Re: cgq vs bdi names, was "cfq-iosched: fix a kernel OOPs when usb key  is inserted"
  2010-09-20 13:03 cgq vs bdi names, was "cfq-iosched: fix a kernel OOPs when usb key is inserted" Christoph Hellwig
@ 2010-09-20 13:12 ` Jens Axboe
  2010-09-20 22:30   ` [PATCH] floppy: switch to one queue per drive instead of sharing a queue (Was: Re: cgq vs bdi names, was "cfq-iosched: fix a kernel OOPs when usb key is inserted") Vivek Goyal
  2010-09-20 13:32 ` cgq vs bdi names, was "cfq-iosched: fix a kernel OOPs when usb key is inserted" Vivek Goyal
  1 sibling, 1 reply; 10+ messages in thread
From: Jens Axboe @ 2010-09-20 13:12 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: vgoyal, linux-kernel

On 2010-09-20 15:03, Christoph Hellwig wrote:
> Hi Vivek, hi Jens,
> 
> where was http://git.kernel.dk/?p=linux-2.6-block.git;a=commitdiff_plain;h=44c74d6292e97f8bd9adfa6b0df3cb4f3c42a6dc posted  on the mailinglist?
> 
> I can't find it in my lkml or fsdevel inboxes.  Either way I don't think
> just papering over the underlying issue like this is a good idea.  
> 
> The big issue is that cfq tries to scanf the textual representation of
> the dev_t from the request_queue by abusing the bdi.  But the reason why
> we don't have a dev_t in the request_queue is that it's still not
> unique.  If it was we could easily add a dev_t into the request_queue
> and be done with it.
> 
> So the fix is either to get rid of the last remaining users of shared
> request_queues (IIRC the various floppy drivers) and just add a dev_t
> in the request_queue for the bdi, tracing and cfq, or add a dev_t into
> the request_queue and add a flag for shared request queues that the
> floppy driver and whoever needs it set and let the bdi sysfs code, cfq
> and blocktrace ignore theis request_queue.  This will also allow to
> get rid of the crap about ignoring failures due to already register
> or prematurely unregistered bdis and actually add real error handling
> to that code.

I did this one 15 months ago (according to git), but I never got it
tested:

http://git.kernel.dk/?p=linux-2.6-block.git;a=commitdiff;h=7a0ebc7ea1db42a71841df6a15be9fd420fae980

IIRC, the mtd stuff also uses a shared queue. But I think that is it.
Would indeed be VERY nice to finally get rid of that crap, it has
technically been outlawed since 2.5.1.

-- 
Jens Axboe


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

* Re: cgq vs bdi names, was "cfq-iosched: fix a kernel OOPs when usb key is inserted"
  2010-09-20 13:03 cgq vs bdi names, was "cfq-iosched: fix a kernel OOPs when usb key is inserted" Christoph Hellwig
  2010-09-20 13:12 ` Jens Axboe
@ 2010-09-20 13:32 ` Vivek Goyal
  1 sibling, 0 replies; 10+ messages in thread
From: Vivek Goyal @ 2010-09-20 13:32 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: jaxboe, linux-kernel

On Mon, Sep 20, 2010 at 03:03:18PM +0200, Christoph Hellwig wrote:
> Hi Vivek, hi Jens,
> 
> where was http://git.kernel.dk/?p=linux-2.6-block.git;a=commitdiff_plain;h=44c74d6292e97f8bd9adfa6b0df3cb4f3c42a6dc posted  on the mailinglist?
> 
> I can't find it in my lkml or fsdevel inboxes.  Either way I don't think
> just papering over the underlying issue like this is a good idea.  

Hi Christoph,

https://patchwork.kernel.org/patch/163661/

> 
> The big issue is that cfq tries to scanf the textual representation of
> the dev_t from the request_queue by abusing the bdi.  But the reason why
> we don't have a dev_t in the request_queue is that it's still not
> unique.  If it was we could easily add a dev_t into the request_queue
> and be done with it.
> 
> So the fix is either to get rid of the last remaining users of shared
> request_queues (IIRC the various floppy drivers) and just add a dev_t
> in the request_queue for the bdi, tracing and cfq, or add a dev_t into
> the request_queue and add a flag for shared request queues that the
> floppy driver and whoever needs it set and let the bdi sysfs code, cfq
> and blocktrace ignore theis request_queue.  This will also allow to
> get rid of the crap about ignoring failures due to already register
> or prematurely unregistered bdis and actually add real error handling
> to that code.

Agreed that multiple disks backed by single reuqest queue does not fit
CFQ's + cgroup model. I am also using same model for throttling block
devices. So it is important to fix it. 

I will test the patch Jens wrote long back and mentioned in other mail for
fixing floppy drivers. Will also try to scan for other drivers doing this.
If you have some hints for me regarding what drivers to look at, that will
help.

Thanks
Vivek 

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

* [PATCH] floppy: switch to one queue per drive instead of sharing a queue (Was: Re: cgq vs bdi names, was "cfq-iosched: fix a kernel OOPs when usb key  is inserted")
  2010-09-20 13:12 ` Jens Axboe
@ 2010-09-20 22:30   ` Vivek Goyal
  2010-09-21 18:25     ` Vivek Goyal
  0 siblings, 1 reply; 10+ messages in thread
From: Vivek Goyal @ 2010-09-20 22:30 UTC (permalink / raw)
  To: Jens Axboe; +Cc: Christoph Hellwig, linux-kernel

On Mon, Sep 20, 2010 at 03:12:44PM +0200, Jens Axboe wrote:
> On 2010-09-20 15:03, Christoph Hellwig wrote:
> > Hi Vivek, hi Jens,
> > 
> > where was http://git.kernel.dk/?p=linux-2.6-block.git;a=commitdiff_plain;h=44c74d6292e97f8bd9adfa6b0df3cb4f3c42a6dc posted  on the mailinglist?
> > 
> > I can't find it in my lkml or fsdevel inboxes.  Either way I don't think
> > just papering over the underlying issue like this is a good idea.  
> > 
> > The big issue is that cfq tries to scanf the textual representation of
> > the dev_t from the request_queue by abusing the bdi.  But the reason why
> > we don't have a dev_t in the request_queue is that it's still not
> > unique.  If it was we could easily add a dev_t into the request_queue
> > and be done with it.
> > 
> > So the fix is either to get rid of the last remaining users of shared
> > request_queues (IIRC the various floppy drivers) and just add a dev_t
> > in the request_queue for the bdi, tracing and cfq, or add a dev_t into
> > the request_queue and add a flag for shared request queues that the
> > floppy driver and whoever needs it set and let the bdi sysfs code, cfq
> > and blocktrace ignore theis request_queue.  This will also allow to
> > get rid of the crap about ignoring failures due to already register
> > or prematurely unregistered bdis and actually add real error handling
> > to that code.
> 
> I did this one 15 months ago (according to git), but I never got it
> tested:
> 
> http://git.kernel.dk/?p=linux-2.6-block.git;a=commitdiff;h=7a0ebc7ea1db42a71841df6a15be9fd420fae980
> 
> IIRC, the mtd stuff also uses a shared queue. But I think that is it.
> Would indeed be VERY nice to finally get rid of that crap, it has
> technically been outlawed since 2.5.1.

Hi Jens,

I have refreshed your patch to apply on latest tree. I did testing with
one floppy controller and it was hanging because we were taking
floppy_lock both inside and outside of function set_next_request(). I got
rid of that and now atleast with one flopply controller this patch is
working fine.

I tried connecting another floppy controller to see if I see two request
queues being registered, but somehow my BIOS does not recognize two 
floppy controllers and only makes one of these operational.

Thanks
Vivek


Pretty straight forward conversion. Note that we do round-robin
between the drives that have available requests, before we simply
used the drive that the IO scheduler told us to. Since the IO
scheduler doesn't care about multiple devices per queue, the resulting
sort would not have made sense.

Signed-off-by: Jens Axboe <jaxboe@fusionio.com>
Signed-off-by: Vivek Goyal <vgoyal@redhat.com>
---
 drivers/block/floppy.c |   66 +++++++++++++++++++++++++++++++++----------------
 1 file changed, 45 insertions(+), 21 deletions(-)

Index: linux-2.6/drivers/block/floppy.c
===================================================================
--- linux-2.6.orig/drivers/block/floppy.c	2010-09-20 13:20:18.000000000 -0400
+++ linux-2.6/drivers/block/floppy.c	2010-09-20 15:39:46.406861152 -0400
@@ -258,8 +258,8 @@ static int irqdma_allocated;
 #include <linux/completion.h>
 
 static struct request *current_req;
-static struct request_queue *floppy_queue;
 static void do_fd_request(struct request_queue *q);
+static int set_next_request(void);
 
 #ifndef fd_get_dma_residue
 #define fd_get_dma_residue() get_dma_residue(FLOPPY_DMA)
@@ -413,6 +413,7 @@ static struct gendisk *disks[N_DRIVE];
 static struct block_device *opened_bdev[N_DRIVE];
 static DEFINE_MUTEX(open_lock);
 static struct floppy_raw_cmd *raw_cmd, default_raw_cmd;
+static int fdc_queue;
 
 /*
  * This struct defines the different floppy types.
@@ -890,8 +891,8 @@ static void unlock_fdc(void)
 	del_timer(&fd_timeout);
 	cont = NULL;
 	clear_bit(0, &fdc_busy);
-	if (current_req || blk_peek_request(floppy_queue))
-		do_fd_request(floppy_queue);
+	if (current_req || set_next_request())
+		do_fd_request(current_req->q);
 	spin_unlock_irqrestore(&floppy_lock, flags);
 	wake_up(&fdc_wait);
 }
@@ -2243,8 +2244,8 @@ static void floppy_end_request(struct re
  * logical buffer */
 static void request_done(int uptodate)
 {
-	struct request_queue *q = floppy_queue;
 	struct request *req = current_req;
+	struct request_queue *q;
 	unsigned long flags;
 	int block;
 	char msg[sizeof("request done ") + sizeof(int) * 3];
@@ -2258,6 +2259,8 @@ static void request_done(int uptodate)
 		return;
 	}
 
+	q = req->q;
+
 	if (uptodate) {
 		/* maintain values for invalidation on geometry
 		 * change */
@@ -2811,6 +2814,28 @@ static int make_raw_rw_request(void)
 	return 2;
 }
 
+/*
+ * 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;
+
+	do {
+		q = disks[fdc_queue]->queue;
+		if (++fdc_queue == N_DRIVE)
+			fdc_queue = 0;
+		if (q) {
+			current_req = blk_fetch_request(q);
+			if (current_req)
+				break;
+		}
+	} while (fdc_queue != old_pos);
+
+	return current_req != NULL;
+}
+
 static void redo_fd_request(void)
 {
 	int drive;
@@ -2822,17 +2847,17 @@ static void redo_fd_request(void)
 
 do_request:
 	if (!current_req) {
-		struct request *req;
+		int pending;
+
+		spin_lock_irq(&floppy_lock);
+		pending = set_next_request();
+		spin_unlock_irq(&floppy_lock);
 
-		spin_lock_irq(floppy_queue->queue_lock);
-		req = blk_fetch_request(floppy_queue);
-		spin_unlock_irq(floppy_queue->queue_lock);
-		if (!req) {
+		if (!pending) {
 			do_floppy = NULL;
 			unlock_fdc();
 			return;
 		}
-		current_req = req;
 	}
 	drive = (long)current_req->rq_disk->private_data;
 	set_fdc(drive);
@@ -4165,6 +4190,13 @@ static int __init floppy_init(void)
 			goto out_put_disk;
 		}
 
+		disks[dr]->queue = blk_init_queue(do_fd_request, &floppy_lock);
+		if (!disks[dr]->queue) {
+			err = -ENOMEM;
+			goto out_put_disk;
+		}
+
+		blk_queue_max_hw_sectors(disks[dr]->queue, 64);
 		disks[dr]->major = FLOPPY_MAJOR;
 		disks[dr]->first_minor = TOMINOR(dr);
 		disks[dr]->fops = &floppy_fops;
@@ -4183,13 +4215,6 @@ static int __init floppy_init(void)
 	if (err)
 		goto out_unreg_blkdev;
 
-	floppy_queue = blk_init_queue(do_fd_request, &floppy_lock);
-	if (!floppy_queue) {
-		err = -ENOMEM;
-		goto out_unreg_driver;
-	}
-	blk_queue_max_hw_sectors(floppy_queue, 64);
-
 	blk_register_region(MKDEV(FLOPPY_MAJOR, 0), 256, THIS_MODULE,
 			    floppy_find, NULL, NULL);
 
@@ -4317,7 +4342,6 @@ static int __init floppy_init(void)
 
 		/* to be cleaned up... */
 		disks[drive]->private_data = (void *)(long)drive;
-		disks[drive]->queue = floppy_queue;
 		disks[drive]->flags |= GENHD_FL_REMOVABLE;
 		disks[drive]->driverfs_dev = &floppy_device[drive].dev;
 		add_disk(disks[drive]);
@@ -4333,8 +4357,6 @@ out_flush_work:
 		floppy_release_irq_and_dma();
 out_unreg_region:
 	blk_unregister_region(MKDEV(FLOPPY_MAJOR, 0), 256);
-	blk_cleanup_queue(floppy_queue);
-out_unreg_driver:
 	platform_driver_unregister(&floppy_driver);
 out_unreg_blkdev:
 	unregister_blkdev(FLOPPY_MAJOR, "fd");
@@ -4342,6 +4364,8 @@ out_put_disk:
 	while (dr--) {
 		del_timer(&motor_off_timer[dr]);
 		put_disk(disks[dr]);
+		if (disks[dr]->queue)
+			blk_cleanup_queue(disks[dr]->queue);
 	}
 	return err;
 }
@@ -4550,11 +4574,11 @@ static void __exit floppy_module_exit(vo
 			platform_device_unregister(&floppy_device[drive]);
 		}
 		put_disk(disks[drive]);
+		blk_cleanup_queue(disks[drive]->queue);
 	}
 
 	del_timer_sync(&fd_timeout);
 	del_timer_sync(&fd_timer);
-	blk_cleanup_queue(floppy_queue);
 
 	if (atomic_read(&usage_count))
 		floppy_release_irq_and_dma();

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

* Re: [PATCH] floppy: switch to one queue per drive instead of sharing a queue (Was: Re: cgq vs bdi names, was "cfq-iosched: fix a kernel OOPs when usb key  is inserted")
  2010-09-20 22:30   ` [PATCH] floppy: switch to one queue per drive instead of sharing a queue (Was: Re: cgq vs bdi names, was "cfq-iosched: fix a kernel OOPs when usb key is inserted") Vivek Goyal
@ 2010-09-21 18:25     ` Vivek Goyal
  2010-09-21 22:17       ` Jens Axboe
  0 siblings, 1 reply; 10+ messages in thread
From: Vivek Goyal @ 2010-09-21 18:25 UTC (permalink / raw)
  To: Jens Axboe; +Cc: Christoph Hellwig, linux-kernel

On Mon, Sep 20, 2010 at 06:30:01PM -0400, Vivek Goyal wrote:
> On Mon, Sep 20, 2010 at 03:12:44PM +0200, Jens Axboe wrote:
> > On 2010-09-20 15:03, Christoph Hellwig wrote:
> > > Hi Vivek, hi Jens,
> > > 
> > > where was http://git.kernel.dk/?p=linux-2.6-block.git;a=commitdiff_plain;h=44c74d6292e97f8bd9adfa6b0df3cb4f3c42a6dc posted  on the mailinglist?
> > > 
> > > I can't find it in my lkml or fsdevel inboxes.  Either way I don't think
> > > just papering over the underlying issue like this is a good idea.  
> > > 
> > > The big issue is that cfq tries to scanf the textual representation of
> > > the dev_t from the request_queue by abusing the bdi.  But the reason why
> > > we don't have a dev_t in the request_queue is that it's still not
> > > unique.  If it was we could easily add a dev_t into the request_queue
> > > and be done with it.
> > > 
> > > So the fix is either to get rid of the last remaining users of shared
> > > request_queues (IIRC the various floppy drivers) and just add a dev_t
> > > in the request_queue for the bdi, tracing and cfq, or add a dev_t into
> > > the request_queue and add a flag for shared request queues that the
> > > floppy driver and whoever needs it set and let the bdi sysfs code, cfq
> > > and blocktrace ignore theis request_queue.  This will also allow to
> > > get rid of the crap about ignoring failures due to already register
> > > or prematurely unregistered bdis and actually add real error handling
> > > to that code.
> > 
> > I did this one 15 months ago (according to git), but I never got it
> > tested:
> > 
> > http://git.kernel.dk/?p=linux-2.6-block.git;a=commitdiff;h=7a0ebc7ea1db42a71841df6a15be9fd420fae980
> > 
> > IIRC, the mtd stuff also uses a shared queue. But I think that is it.
> > Would indeed be VERY nice to finally get rid of that crap, it has
> > technically been outlawed since 2.5.1.
> 
> Hi Jens,
> 
> I have refreshed your patch to apply on latest tree. I did testing with
> one floppy controller and it was hanging because we were taking
> floppy_lock both inside and outside of function set_next_request(). I got
> rid of that and now atleast with one flopply controller this patch is
> working fine.
> 
> I tried connecting another floppy controller to see if I see two request
> queues being registered, but somehow my BIOS does not recognize two 
> floppy controllers and only makes one of these operational.
> 

Christoph suggested to use qemu and test with multiple flopply disk
controller and it worked. Thanks Christoph.

I exported two floppy disks and they show up as /dev/fd0 and /dev/fd1 in
the guest.

With the patch I also verified that changing IO scheduler on fd0 does
not change it automatically for fd1. That means they both are using 
separate requests queues after the patch. Without the patch changing
IO scheduler on one changed it automatically for the other controller
meaning they were sharing same request queue.

I did basic testing of being able to mount the floppy disks and being
able to read/write simple text files.

Jens, do let me know if you are curious to know about test results in
some other configuration.

Thanks
Vivek

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

* Re: [PATCH] floppy: switch to one queue per drive instead of  sharing a queue (Was: Re: cgq vs bdi names, was "cfq-iosched: fix a kernel  OOPs when usb key  is inserted")
  2010-09-21 18:25     ` Vivek Goyal
@ 2010-09-21 22:17       ` Jens Axboe
  2010-09-22 20:42         ` Vivek Goyal
  0 siblings, 1 reply; 10+ messages in thread
From: Jens Axboe @ 2010-09-21 22:17 UTC (permalink / raw)
  To: Vivek Goyal; +Cc: Christoph Hellwig, linux-kernel

On 2010-09-21 20:25, Vivek Goyal wrote:
> On Mon, Sep 20, 2010 at 06:30:01PM -0400, Vivek Goyal wrote:
>> On Mon, Sep 20, 2010 at 03:12:44PM +0200, Jens Axboe wrote:
>>> On 2010-09-20 15:03, Christoph Hellwig wrote:
>>>> Hi Vivek, hi Jens,
>>>>
>>>> where was http://git.kernel.dk/?p=linux-2.6-block.git;a=commitdiff_plain;h=44c74d6292e97f8bd9adfa6b0df3cb4f3c42a6dc posted  on the mailinglist?
>>>>
>>>> I can't find it in my lkml or fsdevel inboxes.  Either way I don't think
>>>> just papering over the underlying issue like this is a good idea.  
>>>>
>>>> The big issue is that cfq tries to scanf the textual representation of
>>>> the dev_t from the request_queue by abusing the bdi.  But the reason why
>>>> we don't have a dev_t in the request_queue is that it's still not
>>>> unique.  If it was we could easily add a dev_t into the request_queue
>>>> and be done with it.
>>>>
>>>> So the fix is either to get rid of the last remaining users of shared
>>>> request_queues (IIRC the various floppy drivers) and just add a dev_t
>>>> in the request_queue for the bdi, tracing and cfq, or add a dev_t into
>>>> the request_queue and add a flag for shared request queues that the
>>>> floppy driver and whoever needs it set and let the bdi sysfs code, cfq
>>>> and blocktrace ignore theis request_queue.  This will also allow to
>>>> get rid of the crap about ignoring failures due to already register
>>>> or prematurely unregistered bdis and actually add real error handling
>>>> to that code.
>>>
>>> I did this one 15 months ago (according to git), but I never got it
>>> tested:
>>>
>>> http://git.kernel.dk/?p=linux-2.6-block.git;a=commitdiff;h=7a0ebc7ea1db42a71841df6a15be9fd420fae980
>>>
>>> IIRC, the mtd stuff also uses a shared queue. But I think that is it.
>>> Would indeed be VERY nice to finally get rid of that crap, it has
>>> technically been outlawed since 2.5.1.
>>
>> Hi Jens,
>>
>> I have refreshed your patch to apply on latest tree. I did testing with
>> one floppy controller and it was hanging because we were taking
>> floppy_lock both inside and outside of function set_next_request(). I got
>> rid of that and now atleast with one flopply controller this patch is
>> working fine.
>>
>> I tried connecting another floppy controller to see if I see two request
>> queues being registered, but somehow my BIOS does not recognize two 
>> floppy controllers and only makes one of these operational.
>>
> 
> Christoph suggested to use qemu and test with multiple flopply disk
> controller and it worked. Thanks Christoph.
> 
> I exported two floppy disks and they show up as /dev/fd0 and /dev/fd1 in
> the guest.
> 
> With the patch I also verified that changing IO scheduler on fd0 does
> not change it automatically for fd1. That means they both are using 
> separate requests queues after the patch. Without the patch changing
> IO scheduler on one changed it automatically for the other controller
> meaning they were sharing same request queue.
> 
> I did basic testing of being able to mount the floppy disks and being
> able to read/write simple text files.
> 
> Jens, do let me know if you are curious to know about test results in
> some other configuration.

Super, thanks a lot! I will queue this up. If you want to continue
this (very noble) crusade, the next target is
drivers/mtd/mtd_blkdevs.c. I never got that one started, but it was
next on my list.

I will queue up the floppy patch for .37.

-- 
Jens Axboe


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

* Re: [PATCH] floppy: switch to one queue per drive instead of sharing a queue (Was: Re: cgq vs bdi names, was "cfq-iosched: fix a kernel OOPs when usb key  is inserted")
  2010-09-21 22:17       ` Jens Axboe
@ 2010-09-22 20:42         ` Vivek Goyal
  2010-09-23  7:59           ` Jens Axboe
  0 siblings, 1 reply; 10+ messages in thread
From: Vivek Goyal @ 2010-09-22 20:42 UTC (permalink / raw)
  To: Jens Axboe; +Cc: Christoph Hellwig, linux-kernel, Maxim Levitsky

On Wed, Sep 22, 2010 at 12:17:54AM +0200, Jens Axboe wrote:

[..]
> Super, thanks a lot! I will queue this up. If you want to continue
> this (very noble) crusade, the next target is
> drivers/mtd/mtd_blkdevs.c. I never got that one started, but it was
> next on my list.

Hi Jens,

IIUC, for MTD this conversion is already done and we seem to be using separate
request queue for each gendisk instead of some disks sharing a single
request queue. I think by following is relevant commit. Maxim should be
able to confirm though.

-----------------------------------------------------------------------
commit a863862257b7dd08d855bafcb0aedd9ad848ed91
Author: Maxim Levitsky <maximlevitsky@gmail.com>
Date:   Mon Feb 22 20:39:29 2010 +0200

    mtd: blktrans: remove mtd_blkcore_priv, switch to per device queue and
threa
    
    This is the biggest change. To make hotplug possible, and this layer
    clean, the mtd_blktrans_dev now contains everything for a single mtd
    block translation device. Also removed some very old leftovers.
    
    Signed-off-by: Maxim Levitsky <maximlevitsky@gmail.com>
    Signed-off-by: David Woodhouse <David.Woodhouse@intel.com>
-----------------------------------------------------------------------

I did create two mtd devices (from block devices, block2mtd), then then
created two block devices on top of those mtd devices (using mtdblock)
and these two devices mtdblock0 and mtdblock1 seems to be using their
own separate request queues. (Change in ioscheduler on one does not change
it on other queue).

So to me it looks like MTD is all set, until and unless I am missing
something.

Vivek

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

* Re: [PATCH] floppy: switch to one queue per drive instead of  sharing a queue (Was: Re: cgq vs bdi names, was "cfq-iosched: fix a kernel  OOPs when usb key  is inserted")
  2010-09-22 20:42         ` Vivek Goyal
@ 2010-09-23  7:59           ` Jens Axboe
  2010-09-23  8:05             ` Christoph Hellwig
  2010-09-23 21:40             ` Vivek Goyal
  0 siblings, 2 replies; 10+ messages in thread
From: Jens Axboe @ 2010-09-23  7:59 UTC (permalink / raw)
  To: Vivek Goyal; +Cc: Christoph Hellwig, linux-kernel, Maxim Levitsky

On 2010-09-22 22:42, Vivek Goyal wrote:
> On Wed, Sep 22, 2010 at 12:17:54AM +0200, Jens Axboe wrote:
> 
> [..]
>> Super, thanks a lot! I will queue this up. If you want to continue
>> this (very noble) crusade, the next target is
>> drivers/mtd/mtd_blkdevs.c. I never got that one started, but it was
>> next on my list.
> 
> Hi Jens,
> 
> IIUC, for MTD this conversion is already done and we seem to be using separate
> request queue for each gendisk instead of some disks sharing a single
> request queue. I think by following is relevant commit. Maxim should be
> able to confirm though.

Great! So the question is if other drivers with shared queues remain.

-- 
Jens Axboe


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

* Re: [PATCH] floppy: switch to one queue per drive instead of  sharing a queue (Was: Re: cgq vs bdi names, was "cfq-iosched: fix a kernel  OOPs when usb key  is inserted")
  2010-09-23  7:59           ` Jens Axboe
@ 2010-09-23  8:05             ` Christoph Hellwig
  2010-09-23 21:40             ` Vivek Goyal
  1 sibling, 0 replies; 10+ messages in thread
From: Christoph Hellwig @ 2010-09-23  8:05 UTC (permalink / raw)
  To: Jens Axboe; +Cc: Vivek Goyal, Christoph Hellwig, linux-kernel, Maxim Levitsky

On Thu, Sep 23, 2010 at 09:59:11AM +0200, Jens Axboe wrote:
> Great! So the question is if other drivers with shared queues remain.

A lot of the other floppy drivers are copy & paste job from floppy.c, so
I'd start looking there.


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

* Re: [PATCH] floppy: switch to one queue per drive instead of sharing a queue (Was: Re: cgq vs bdi names, was "cfq-iosched: fix a kernel OOPs when usb key  is inserted")
  2010-09-23  7:59           ` Jens Axboe
  2010-09-23  8:05             ` Christoph Hellwig
@ 2010-09-23 21:40             ` Vivek Goyal
  1 sibling, 0 replies; 10+ messages in thread
From: Vivek Goyal @ 2010-09-23 21:40 UTC (permalink / raw)
  To: Jens Axboe; +Cc: Christoph Hellwig, linux-kernel, Maxim Levitsky

On Thu, Sep 23, 2010 at 09:59:11AM +0200, Jens Axboe wrote:
> On 2010-09-22 22:42, Vivek Goyal wrote:
> > On Wed, Sep 22, 2010 at 12:17:54AM +0200, Jens Axboe wrote:
> > 
> > [..]
> >> Super, thanks a lot! I will queue this up. If you want to continue
> >> this (very noble) crusade, the next target is
> >> drivers/mtd/mtd_blkdevs.c. I never got that one started, but it was
> >> next on my list.
> > 
> > Hi Jens,
> > 
> > IIUC, for MTD this conversion is already done and we seem to be using separate
> > request queue for each gendisk instead of some disks sharing a single
> > request queue. I think by following is relevant commit. Maxim should be
> > able to confirm though.
> 
> Great! So the question is if other drivers with shared queues remain.
> 

I was browsing through drivers/blcok/ dir and apart from amiga and atari
floppy drivers, following also seems to be sharing the request queue across
gendisks.

drivers/block/cpqarray.c
drivers/block/hd.c
drivers/block/swim3.c
drivers/block/swim.c
drivers/block/xd.c

I will look into fixing those, the biggest challenge in front of me is
testing the changes. Especially arch specific drivers.

Thanks
Vivek

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

end of thread, other threads:[~2010-09-23 21:40 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-09-20 13:03 cgq vs bdi names, was "cfq-iosched: fix a kernel OOPs when usb key is inserted" Christoph Hellwig
2010-09-20 13:12 ` Jens Axboe
2010-09-20 22:30   ` [PATCH] floppy: switch to one queue per drive instead of sharing a queue (Was: Re: cgq vs bdi names, was "cfq-iosched: fix a kernel OOPs when usb key is inserted") Vivek Goyal
2010-09-21 18:25     ` Vivek Goyal
2010-09-21 22:17       ` Jens Axboe
2010-09-22 20:42         ` Vivek Goyal
2010-09-23  7:59           ` Jens Axboe
2010-09-23  8:05             ` Christoph Hellwig
2010-09-23 21:40             ` Vivek Goyal
2010-09-20 13:32 ` cgq vs bdi names, was "cfq-iosched: fix a kernel OOPs when usb key is inserted" Vivek Goyal

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.