On Fri, Apr 24 2015 at 3:33pm -0400,
Christoph Hellwig <hch@xxxxxx> wrote:
> Currently dm-multipath has to clone the bios for every request sent
> to the lower devices, which wastes cpu cycles and ties down memory.
>
> This patch instead adds a new REQ_CLONE flag that instructs req_bio_endio
> to not complete bios attached to a request, which we set on clone
> requests similar to bios in a flush sequence. With this change I/O
> errors on a path failure only get propagated to dm-multipath, which
> can then either resubmit the I/O or complete the bios on the original
> request.
>
> I've done some basic testing of this on a Linux target with ALUA support,
> and it survives path failures during I/O nicely.
Thanks for working on this (it slipped in priority on my TODO list).
Will be great to get this reviewed/tested/staged for 4.2.
But your patch needs rebasing against latest upstream code. I'd imagine
this means you also haven't tested with "use_blk_mq" enabled (from
commit 17e149b8f).
I'll do a much more careful review once you rebase but I noticed one
thing worth mentioning now:
> diff --git a/drivers/md/dm-table.c b/drivers/md/dm-table.c
> index 6554d91..4ac0a47 100644
> --- a/drivers/md/dm-table.c
> +++ b/drivers/md/dm-table.c
> @@ -941,23 +941,30 @@ bool dm_table_mq_request_based(struct dm_table *t)
>
> static int dm_table_alloc_md_mempools(struct dm_table *t)
> {
> - unsigned type = dm_table_get_type(t);
> + int type = dm_table_get_type(t);
> unsigned per_bio_data_size = 0;
> - struct dm_target *tgt;
> unsigned i;
>
> - if (unlikely(type == DM_TYPE_NONE)) {
> + switch (type) {
You're missing a case statement here for DM_TYPE_BIO_BASED:
> + for (i = 0; i < t->num_targets; i++) {
> + struct dm_target *tgt = t->targets + i;
> +
> + per_bio_data_size = max(per_bio_data_size,
> + tgt->per_bio_data_size);
> + }
> +
> + t->mempools = dm_alloc_bio_mempools(t->integrity_supported,
> + per_bio_data_size);
> + break;
> + case DM_TYPE_REQUEST_BASED:
> + case DM_TYPE_MQ_REQUEST_BASED:
> + t->mempools = dm_alloc_rq_mempools(type);
> + break;
> + default:
> DMWARN("no table type is set, can't allocate mempools");
> return -EINVAL;
> }
>
> - if (type == DM_TYPE_BIO_BASED)
> - for (i = 0; i < t->num_targets; i++) {
> - tgt = t->targets + i;
> - per_bio_data_size = max(per_bio_data_size, tgt->per_bio_data_size);
> - }
> -
> - t->mempools = dm_alloc_md_mempools(type, t->integrity_supported, per_bio_data_size);
> if (!t->mempools)
> return -ENOMEM;
>
--
dm-devel mailing list
dm-devel@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/dm-devel
