On Mon, Dec 05 2016 at 5:50pm -0500, Jens Axboe <axboe@xxxxxx> wrote: > On 12/05/2016 03:40 PM, Mike Snitzer wrote: > > On Mon, Dec 5, 2016 at 10:07 AM, Jens Axboe <axboe@xxxxxx> wrote: > >> > >> On 12/05/2016 06:05 AM, Christoph Hellwig wrote: > >>> On Fri, Dec 02, 2016 at 08:15:15PM -0700, Jens Axboe wrote: > >>>> No functional changes with this patch, it's just in preparation for > >>>> supporting legacy schedulers on blk-mq. > >>> > >>> Ewww. I think without refactoring to clear what 'use_mq_path' > >>> means here and better naming this is a total non-started. Even with > >>> that we'll now have yet another code path to worry about. Is there > >>> any chance to instead consolidate into a single path? > >> > >> It's not pretty at all. I should have prefaced this patchset with saying > >> that it's an experiment in seeing what it would take to simply use the > >> old IO schedulers, as a temporary measure, on blk/scsi-mq. I did clean > >> it up a bit after posting: > >> > >> http://git.kernel.dk/cgit/linux-block/log/?h=blk-mq-legacy-sched > >> > >> but I'm not going to claim this is anywhere near merge read, nor clean. > > > > Nice to see you've lowered your standards... > > > > Maybe now we can revisit this line of work? ;) > > http://git.kernel.org/cgit/linux/kernel/git/snitzer/linux.git/log/?h=wip > > I haven't lowered my standards. I thought this posting was pretty clear > - it's an experiment in what supporting legacy schedulers would look > like. As you quote me above, this is NOT proposed for merging, nor do I > consider it anywhere near clean. Where'd your sense of humor go? > > I'm not seeing anything in elevator_switch() that would prevent an > > elevator from attempting to be used on an mq device with > 1 hw queue. > > But I could easily be missing it. > > You missed it, it's in blk_mq_sched_init(), called from > elv_iosched_store() when trying to switch (or setup a new) schedulers. > > > That aside, this patchset has all the makings of a _serious_ problem > > for dm-mq multipath (e.g. if dm_mod.use_blk_mq=Y and > > dm_mod.dm_mq_nr_hw_queues=1). There is a bunch of code in > > drivers/dm/dm-rq.c that looks at q->mq_ops vs not to determine if mq > > is used vs old .request_fn. > > > > I think we really need a way to force an allocated mq request_queue > > (with a single hw_queue) to not support this technological terror > > you've constructed. (*cough* > > See BLK_MQ_F_NO_SCHED. Yeap, missed it, thanks. Reviewing patches via gmail _sucks_ I should've just looked at your git branch(es) from the start. > > I could be super dense on this line of work. But what is the point of > > all this? Seems like a really unfortunate distraction that makes the > > block code all the more encumbered with fiddley old vs new logic. So > > now we're opening old .request_fn users up to blk-mq-with-scheduler vs > > non-blk-mq bugs. > > See above, it's just an experiment in seeing what this would look like, > how transparent (or not) we could make that. OK, seems not very transparent so far. But that aside, I'm more curious on what the goal(s) and/or benefit(s) might be? I know that before you were hopeful to eventually eliminate the old .request_fn path in block core (in favor of blk-mq, once it grew IO scheduling capabilties). But by tieing blk-mq through to the old request path (and associated IO schedulers) it certainly complicates getting rid of all the legacy code. Selfishly, I'm looking forward to eliminating the old .request_fn request-based code in DM core. This advance to supporting the old IO schedulers make that less likely. > Don't overthink any of this, and don't start making plans or coming up > with problems on why X or Y would not work with whatever interface > variant of dm. That's jumping the gun. Not overthinking.. just thinking ;) But if this does happen then maybe I should look to invert the request-based DM core cleanup: remove all the old .request_fn support and impose the same (namely IO scheduler enabled DM multipath) via dm_mod.use_blk_mq=Y and dm_mod.dm_mod.dm_mq_nr_hw_queues=1 Mike -- To unsubscribe from this list: send the line "unsubscribe linux-block" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
