All of lore.kernel.org
 help / color / mirror / Atom feed
From: Keith Busch <keith.busch@intel.com>
To: Mike Snitzer <snitzer@redhat.com>
Cc: axboe@kernel.dk, hch@lst.de, emilne@redhat.com,
	james.smart@broadcom.com, hare@suse.de, Bart.VanAssche@wdc.com,
	linux-block@vger.kernel.org, linux-nvme@lists.infradead.org,
	dm-devel@redhat.com
Subject: Re: [for-4.16 PATCH 0/5] block, nvme, dm: allow DM multipath to use NVMe's error handler
Date: Tue, 26 Dec 2017 13:51:10 -0700	[thread overview]
Message-ID: <20171226205109.GB13188@localhost.localdomain> (raw)
In-Reply-To: <20171219210546.65928-1-snitzer@redhat.com>

On Tue, Dec 19, 2017 at 04:05:41PM -0500, Mike Snitzer wrote:
> These patches enable DM multipath to work well on NVMe over Fabrics
> devices.  Currently that implies CONFIG_NVME_MULTIPATH is _not_ set.
> 
> But follow-on work will be to make it so that native NVMe multipath
> and DM multipath can be made to co-exist (e.g. blacklisting certain
> NVMe devices from being consumed by native NVMe multipath?)

Hi Mike,

I've reviewed the series and I support with the goal. I'm not a big fan,
though, of having yet-another-field to set in bio and req on each IO.

Unless I'm missing something, I think we can make this simpler if you add
the new 'failover_req_fn' as an attribute of the struct request_queue
instead of threading it through bio and request. Native nvme multipath
can set the field directly in the nvme driver, and dm-mpath can set it
in each path when not using the nvme mpath. What do you think?

WARNING: multiple messages have this Message-ID (diff)
From: keith.busch@intel.com (Keith Busch)
Subject: [for-4.16 PATCH 0/5] block, nvme, dm: allow DM multipath to use NVMe's error handler
Date: Tue, 26 Dec 2017 13:51:10 -0700	[thread overview]
Message-ID: <20171226205109.GB13188@localhost.localdomain> (raw)
In-Reply-To: <20171219210546.65928-1-snitzer@redhat.com>

On Tue, Dec 19, 2017@04:05:41PM -0500, Mike Snitzer wrote:
> These patches enable DM multipath to work well on NVMe over Fabrics
> devices.  Currently that implies CONFIG_NVME_MULTIPATH is _not_ set.
> 
> But follow-on work will be to make it so that native NVMe multipath
> and DM multipath can be made to co-exist (e.g. blacklisting certain
> NVMe devices from being consumed by native NVMe multipath?)

Hi Mike,

I've reviewed the series and I support with the goal. I'm not a big fan,
though, of having yet-another-field to set in bio and req on each IO.

Unless I'm missing something, I think we can make this simpler if you add
the new 'failover_req_fn' as an attribute of the struct request_queue
instead of threading it through bio and request. Native nvme multipath
can set the field directly in the nvme driver, and dm-mpath can set it
in each path when not using the nvme mpath. What do you think?

  parent reply	other threads:[~2017-12-26 20:51 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-12-19 21:05 [for-4.16 PATCH 0/5] block, nvme, dm: allow DM multipath to use NVMe's error handler Mike Snitzer
2017-12-19 21:05 ` Mike Snitzer
2017-12-19 21:05 ` [for-4.16 PATCH 1/5] block: establish request failover callback infrastructure Mike Snitzer
2017-12-19 21:05   ` Mike Snitzer
2017-12-19 21:05 ` [for-4.16 PATCH 2/5] nvme: use request's failover callback for multipath failover Mike Snitzer
2017-12-19 21:05   ` Mike Snitzer
2017-12-19 21:05 ` [for-4.16 PATCH 3/5] nvme: move nvme_req_needs_failover() from multipath to core Mike Snitzer
2017-12-19 21:05   ` Mike Snitzer
2017-12-19 21:05 ` [for-4.16 PATCH 4/5] dm mpath: use NVMe error handling to know when an error is retryable Mike Snitzer
2017-12-19 21:05   ` Mike Snitzer
2017-12-20 16:58   ` Mike Snitzer
2017-12-20 16:58     ` Mike Snitzer
2017-12-20 20:33     ` [dm-devel] " Sagi Grimberg
2017-12-20 20:33       ` Sagi Grimberg
2017-12-19 21:05 ` [for-4.16 PATCH 5/5] dm mpath: skip calls to end_io_bio if using NVMe bio-based and round-robin Mike Snitzer
2017-12-19 21:05   ` Mike Snitzer
2017-12-22 18:02 ` [for-4.16 PATCH 0/5] block, nvme, dm: allow DM multipath to use NVMe's error handler Mike Snitzer
2017-12-22 18:02   ` Mike Snitzer
2017-12-26 20:51 ` Keith Busch [this message]
2017-12-26 20:51   ` Keith Busch
2017-12-27  2:42   ` Mike Snitzer
2017-12-27  2:42     ` Mike Snitzer

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20171226205109.GB13188@localhost.localdomain \
    --to=keith.busch@intel.com \
    --cc=Bart.VanAssche@wdc.com \
    --cc=axboe@kernel.dk \
    --cc=dm-devel@redhat.com \
    --cc=emilne@redhat.com \
    --cc=hare@suse.de \
    --cc=hch@lst.de \
    --cc=james.smart@broadcom.com \
    --cc=linux-block@vger.kernel.org \
    --cc=linux-nvme@lists.infradead.org \
    --cc=snitzer@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.