On Mon, Jan 23 2017 at 10:29am -0500, Christoph Hellwig <hch@xxxxxx> wrote: > DM already calls blk_mq_alloc_request on the request_queue of the > underlying device if it is a blk-mq device. But now that we allow drivers > to allocate additional data and initialize it ahead of time we need to do > the same for all drivers. Doing so and using the new cmd_size > infrastructure in the block layer greatly simplifies the dm-rq and mpath > code, and should also make arbitrary combinations of SQ and MQ devices > with SQ or MQ device mapper tables easily possible as a further step. Thanks for working (and suffering) through all of this request-based DM code. Nice to have someone else be painfully aware of the complexity in request-based DM's old request_fn support. The queue->cmd_size (per request data) definitely makes this more possible and is welcomed cleanup. The only concern I have is that using get_request() for the old request_fn request_queue eliminates the guaranteed availability of requests to allow for forward progress (on path failure or for the purposes of swap over mpath, etc). This isn't a concern for blk-mq because as you know we have a fixed set of tags (and associated preallocated requests). So I'm left unconvinced old request_fn request-based DM multipath isn't regressing in how request resubmission can be assured a request will be available when needed on retry in the face of path failure. dm_mod's 'reserved_rq_based_ios' module_param governs the minimum number of requests in the md->rq_pool (and defaults to 256 requests per request-based DM request_queue). Whereas blk_init_rl()'s mempool_create_node() uses BLKDEV_MIN_RQ (4) yet q->nr_requests = BLKDEV_MAX_RQ (128). Also, this patch eliminates the utility of 'reserved_rq_based_ios' module_param without actually removing it. Anyway, should blk-core evolve to allow drivers to specify a custom min_nr of requests in the old request_fn request_queue's mempool? Or is my concern overblown? Seems we're very close to making this request-based DM cleanup doable. Just would like some extra eyes and care/thought/guidance from yourself and likely Jens. Thanks, Mike p.s. dm.c:dm_alloc_md_mempools() could be cleaned up a bit more since only bio-based DM will have a pools->io_pool moving forward; but I can circle back to that cleanup after. -- 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
