Re: dm-mpath: Work with blk multi-queue drivers

[Mike Snitzer <snitzer@redhat.com>]

On Mon, Sep 29 2014 at  7:58pm -0400,
Junichi Nomura <j-nomura@ce.jp.nec.com> wrote:

> On 09/26/14 01:12, Mike Snitzer wrote:
> > On Thu, Sep 25 2014 at 11:57am -0400,
> > Keith Busch <keith.busch@intel.com> 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.