Messages in this thread | | | Date | Wed, 22 Sep 2010 00:17:54 +0200 | From | Jens Axboe <> | Subject | 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") |
| |
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
| |