All of lore.kernel.org
 help / color / mirror / Atom feed
From: John Meneghini <jmeneghi@redhat.com>
To: Keith Busch <kbusch@kernel.org>, Christoph Hellwig <hch@lst.de>
Cc: Bryan Gurney <bgurney@redhat.com>,
	linux-kernel@vger.kernel.org, linux-nvme@lists.infradead.org,
	sagi@grimberg.me, axboe@kernel.dk, mpe@ellerman.id.au,
	naveen@kernel.org, maddy@linux.ibm.com, kernel@xen0n.name,
	bmarzins@redhat.com
Subject: Re: [PATCH 1/1] nvme: always enable multipath
Date: Fri, 22 Nov 2024 12:49:32 -0500	[thread overview]
Message-ID: <a6902168-0a8e-43ff-afef-b100f75d266c@redhat.com> (raw)
In-Reply-To: <Z0ClxgBJG_YBF-Ql@kbusch-mbp.dhcp.thefacebook.com>

On 11/22/24 10:39, Keith Busch wrote:
> On Fri, Nov 22, 2024 at 01:09:25PM +0100, Christoph Hellwig wrote:
>> On Thu, Nov 21, 2024 at 05:03:21PM -0500, Bryan Gurney wrote:
>>> Since device-mapper multipath will no longer be operating on NVMe
>>> devices, there is no longer a need to set CONFIG_NVME_MULTIPATH=n.
>>>
>>> Always enable NVMe multipath, remove CONFIG_NVME_MULTIPATH, and use
>>> the code paths that would be used if CONFIG_NVME_MULTIPATH=y.
>>
>> As mentioned last round not having to build the not tiny multipath
>> code for embedded systems and other small builds that never require
>> multipathing still seems like a sensible idea.
> 
> It's not just embedded either. I have plenty of single port datacenter
> PCIe NVMe's that claim multi-controller capabilities. I think it's some
> artifact of SRIOV that some versions of the firmware can bring.
> 
> Anyway, we only use the one physical function, so they're certainly not
> multipath devices here. We disable the CONFIG option because while the
> nvme multipath IO overhead is low, it's not zero.

So you're saying you want to keep CONFIG_NVME_MULTIPATH and simply remove the modparam nvme_core.multipath? (I know I said we 
were going to do that but that's before Bryan and I started testing his changes with blktests. I think we can fix that.)

The problem with this solution is: when you build a kernel with CONFIG_NVME_MULTIPATH=n you get exactly the same thing as 
CONFIG_NVME_MULTIPATH=y with nvme_core.multipath=n. You get a separate /dev/nvmeNN entry for every namespace/controller path, 
minus the multipath.c code.

So, I don't see the point. If you really want to stop supporting user space multi-path solutions like DMMP with nvme we need to 
stop creating multiple dev entries for multi-path controllers, no matter what.

Note that this multi-pathing stuff is a part of the confusion in UDEV, like I spoke about at ALPPS this year. One reason why the 
/dev/disk/by-path symlinks are so broken is because the kernel has at least three different ways to configure multi-pathing 
support for nvme devices.

We've been saying we're going to do this since since v5.18.

So how do we want to do this?

-
-		if (!multipath) {
-			dev_warn(ctrl->device,
-				"Found shared namespace %d, but multipathing not supported.\n",
-				info->nsid);
-			dev_warn_once(ctrl->device,
-				"Support for shared namespaces without CONFIG_NVME_MULTIPATH is deprecated and will be removed in Linux 6.0.\n");
-		}

commit ce8d78616a6b637d1b763eb18e32045687a84305
Author: Christoph Hellwig <hch@lst.de>
Date:   Tue Mar 15 13:27:07 2022 +0100

     nvme: warn about shared namespaces without CONFIG_NVME_MULTIPATH

     Start warning about exposing a namespace as multiple block devices,
     and set a fixed deprecation release.

     Signed-off-by: Christoph Hellwig <hch@lst.de>
     Reviewed-by: Keith Busch <kbusch@kernel.org>

(master) > git name-rev --tags --name-only ce8d78616a6b637d1b763eb18e32045687a84305
nvme-5.18-2022-03-17^0



  reply	other threads:[~2024-11-22 17:49 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-11-21 22:03 [PATCH 1/1] nvme: always enable multipath Bryan Gurney
2024-11-22  6:26 ` Nilay Shroff
2024-11-22 14:10   ` John Meneghini
2024-11-24 13:09     ` Nilay Shroff
2024-11-22 12:09 ` Christoph Hellwig
2024-11-22 15:39   ` Keith Busch
2024-11-22 17:49     ` John Meneghini [this message]
2024-11-22 18:15       ` Keith Busch
2024-11-22 18:29         ` John Meneghini
2024-11-22 17:52     ` John Meneghini
2024-11-22 18:16       ` Keith Busch

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=a6902168-0a8e-43ff-afef-b100f75d266c@redhat.com \
    --to=jmeneghi@redhat.com \
    --cc=axboe@kernel.dk \
    --cc=bgurney@redhat.com \
    --cc=bmarzins@redhat.com \
    --cc=hch@lst.de \
    --cc=kbusch@kernel.org \
    --cc=kernel@xen0n.name \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-nvme@lists.infradead.org \
    --cc=maddy@linux.ibm.com \
    --cc=mpe@ellerman.id.au \
    --cc=naveen@kernel.org \
    --cc=sagi@grimberg.me \
    /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.