From mboxrd@z Thu Jan 1 00:00:00 1970 From: Junichi Nomura Subject: Re: dm-mpath: Work with blk multi-queue drivers Date: Tue, 30 Sep 2014 23:43:47 +0000 Message-ID: <542B4033.30001@ce.jp.nec.com> References: <1411491802-7356-1-git-send-email-keith.busch@intel.com> <20140924145215.GA26398@infradead.org> <20140924183453.GA23052@redhat.com> <20140924184834.GB23052@redhat.com> <20140925001341.GA25145@redhat.com> <20140925161215.GA29645@redhat.com> <5429F227.4080604@ce.jp.nec.com> <20140930141828.GA5161@redhat.com> Reply-To: device-mapper development Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <20140930141828.GA5161@redhat.com> Content-Language: ja-JP Content-ID: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: dm-devel-bounces@redhat.com Errors-To: dm-devel-bounces@redhat.com To: Mike Snitzer , Keith Busch , Christoph Hellwig Cc: "dm-devel@redhat.com" List-Id: dm-devel.ids On 09/30/14 23:18, Mike Snitzer wrote: > On Mon, Sep 29 2014 at 7:58pm -0400, > Junichi Nomura wrote: > >> On 09/26/14 01:12, Mike Snitzer wrote: >>> On Thu, Sep 25 2014 at 11:57am -0400, >>> Keith Busch 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