[v2,0/2] nvme-mpath: Add IO stats support

Message

Sagi Grimberg Oct. 3, 2022, 9:43 a.m. UTC

I've been hearing complaints about the fact that the nvme mpath stack
device does not expose IO stats just like any normal block device,
instead people need to check the bottom namespaces hidden devices,
mapping back to the mpath device node.

This really sucks, especially for observability hooks/plugins that
I've seen people do.

This series make the nvme mpath device expose normal IO stats. Given
that nvme-mpath doesn't have any context after submitting the bio, we
use the core completion path to start/end stats accounting on its
behalf, a similar practice that we use for other multipath related stuff.

While we are "double" accounting every request, this is a preferable
approach as opposed to try and locally collect/combine accurate stats
from multiple bottom hidden devices in the driver, or teach the block
layer to do so.

Local tests with null-blk + nvme-loop did not show any noticeable
performance degradation (within 1%).

Feedback is welcome.

Changes from v1:
- split into 2 patches, one prep and second is nvme-mpath stats
- fix possible use-after-free when ending request and accounting mpath io
  stats
- mark REQ_NVME_MPATH_IO_STATS to allow user to disable stats on the mpath
  device (also in the middle of a request).

Sagi Grimberg (2):
  nvme: introduce nvme_start_request
  nvme: support io stats on the mpath device

 drivers/nvme/host/apple.c     |  2 +-
 drivers/nvme/host/core.c      | 10 ++++++++++
 drivers/nvme/host/fc.c        |  2 +-
 drivers/nvme/host/multipath.c | 25 +++++++++++++++++++++++++
 drivers/nvme/host/nvme.h      | 13 +++++++++++++
 drivers/nvme/host/pci.c       |  2 +-
 drivers/nvme/host/rdma.c      |  2 +-
 drivers/nvme/host/tcp.c       |  2 +-
 drivers/nvme/target/loop.c    |  2 +-
 9 files changed, 54 insertions(+), 6 deletions(-)

Comments

Christoph Hellwig Nov. 29, 2022, 2:45 p.m. UTC | #1

I've applied this with nvme_start_request changed to an inline
funtion.

Keith Busch Nov. 29, 2022, 4:43 p.m. UTC | #2

On Tue, Nov 29, 2022 at 03:45:10PM +0100, Christoph Hellwig wrote:
> I've applied this with nvme_start_request changed to an inline
> funtion.

You'll also need to change patch 2 with this inlined. That patch needs
to either inline nvme_mpath_start_request(), or add an
EXPORT_SYMBOL_GPL.