On Mon, Sep 29 2014 at 7:58pm -0400, Junichi Nomura <j-nomura@xxxxxxxxxxxxx> wrote: > On 09/26/14 01:12, Mike Snitzer wrote: > > On Thu, Sep 25 2014 at 11:57am -0400, > > Keith Busch <keith.busch@xxxxxxxxx> wrote: > >> For my part, the goal was to change as little as possible to get basic > >> blk-mq support working safely without regressing, and performance is > >> not even on my radar yet. I purposefully did not try to understand the > >> existing design well enough to propose re-arching. If we can address the > >> 'request' life cycle management duality issue, would this be acceptable > >> as a stopgap for blk-mq support? > > > > We can ignore my desire to cleanup existing request-based DM's bio > > cloning for now. And yes, resolving the duality issue would need to > > happen. But your proposed change still has the issue of no longer using > > a dedicated mempool per rq-based DM device to allocate requests from. > > If we were to do that I'm pretty sure this new dm.c:dm_get_request() > > wrapper would need to call blk_get_request() with GFP_ATOMIC. > > > > Either GFP_ATOMIC or I think we _could_ relax to GFP_NOWAIT if and only > > if we were willing to explicitly disallow stacking request-based DM > > devices (which nothing uses at this point). So I'd like to get Junichi > > and Alasdair's feedback on the implications. Junichi and/or Alasdair? > > The problem with "stacking request-based DM devices" is > caused by shared mempool (i.e. the pool gets emptied by > upper layer and we can't make forward progress). > So it should be ok if request has per-device mempool > (I think it does.) Current request-based DM provides a per-device mempool that all cloned requests are allocated from. But Keith's approach to have map_rq call blk_get_request will no longer make use of that DM provided mempool. But are you referring to the request_queue's use of a mempool that is initialized with blk_init_rl() in blk_init_allocated_queue()? > However, using blk_get_request() in map function will > require more changes in the code as blk_get_request() > assumes interrupt-enabled context. Ah yes, blk_get_request will unconditionally disable interrupts using spin_lock_irq. Not yet looked at the implications though. -- dm-devel mailing list dm-devel@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/dm-devel