[dm-devel] [PATCH 05/16] dm: always defer request allocation to the owner of the request_queue
Mike Snitzer
snitzer at redhat.com
Tue Jan 24 10:05:39 UTC 2017
On Mon, Jan 23 2017 at 10:29am -0500,
Christoph Hellwig <hch at lst.de> 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.
More information about the dm-devel
mailing list