[dm-devel] dm-mpath: Work with blk multi-queue drivers
Junichi Nomura
j-nomura at ce.jp.nec.com
Tue Sep 30 23:43:47 UTC 2014
On 09/30/14 23:18, Mike Snitzer wrote:
> On Mon, Sep 29 2014 at 7:58pm -0400,
> Junichi Nomura <j-nomura at 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 at 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()?
Yes.
>> 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.
Actually, early implementation of request-based DM had tried to
use blk_get_request() by converting them to irqsave/irqrestore
variants. However, since (old, non-mq version of) blk_get_request
is designed to be called in process context, such a change
could have confused the interface.
As a result, current DM code implements pre-allocation of memory
and mapping separately.
I think blk-mq already does pre-allocation of requests internally
and mq version of blk_get_request is actually a mapping function
in this case.
So, I suspect DM functions for pre-allocation (clone_rq) and mapping
(map_request) are good place for containing the duality inside.
--
Jun'ichi Nomura, NEC Corporation
More information about the dm-devel
mailing list