diff mbox series

[8/8] libmultipath: enforce queue_mode bio for nmve:tcp paths

Message ID 1665164144-6716-9-git-send-email-bmarzins@redhat.com (mailing list archive)
State Not Applicable, archived
Delegated to: christophe varoqui
Headers show
Series mutipath: handle nvme:tcp paths better | expand

Commit Message

Benjamin Marzinski Oct. 7, 2022, 5:35 p.m. UTC
nvme:tcp devices set BLK_MQ_F_BLOCKING (they are the only block devices
which multipath supports that do so), meaning that block_mq expects that
they can block at certain points while servicing a request.  However,
due to the way device-mapper sets up its queue, it is not able to set
BLK_MQ_F_BLOCKING when it includes paths that set this flag.  Patches
were written to address this issue but they were rejected upstream

https://lore.kernel.org/linux-block/YcH%2FE4JNag0QYYAa@infradead.org/T/#t

The proposed solution was to have multipath use the bio queue_mode for
multipath devices that include nvme:tcp paths.

Multipath devices now automatically add the "queue_mode bio" feature if
they include nvme:tcp paths.  If a multipath devices was created with
"queue_mode rq", it will disallow the addition of nvme:tcp paths.

Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
---
 libmultipath/configure.c   | 17 ++++++++++++++++-
 libmultipath/structs_vec.c |  7 +++++++
 multipath/multipath.conf.5 |  4 +++-
 3 files changed, 26 insertions(+), 2 deletions(-)

Comments

Martin Wilck Oct. 20, 2022, 2:58 p.m. UTC | #1
On Fri, 2022-10-07 at 12:35 -0500, Benjamin Marzinski wrote:
> nvme:tcp devices set BLK_MQ_F_BLOCKING (they are the only block
> devices
> which multipath supports that do so), meaning that block_mq expects
> that
> they can block at certain points while servicing a request.  However,
> due to the way device-mapper sets up its queue, it is not able to set
> BLK_MQ_F_BLOCKING when it includes paths that set this flag.  Patches
> were written to address this issue but they were rejected upstream
> 
> https://lore.kernel.org/linux-block/YcH%2FE4JNag0QYYAa@infradead.org/T/#t
> 
> The proposed solution was to have multipath use the bio queue_mode
> for
> multipath devices that include nvme:tcp paths.
> 
> Multipath devices now automatically add the "queue_mode bio" feature
> if
> they include nvme:tcp paths.  If a multipath devices was created with
> "queue_mode rq", it will disallow the addition of nvme:tcp paths.
> 
> Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>

Reviewed-by: Martin Wilck <mwilck@suse.com>


> ---
>  libmultipath/configure.c   | 17 ++++++++++++++++-
>  libmultipath/structs_vec.c |  7 +++++++
>  multipath/multipath.conf.5 |  4 +++-
>  3 files changed, 26 insertions(+), 2 deletions(-)
> 
> diff --git a/libmultipath/configure.c b/libmultipath/configure.c
> index 7c7babd9..e5249fc1 100644
> --- a/libmultipath/configure.c
> +++ b/libmultipath/configure.c
> @@ -260,6 +260,7 @@ int rr_optimize_path_order(struct pathgroup *pgp)
>  int setup_map(struct multipath *mpp, char **params, struct vectors
> *vecs)
>  {
>         struct pathgroup * pgp;
> +       struct path *pp;
>         struct config *conf;
>         int i, marginal_pathgroups;
>         char *save_attr;
> @@ -275,6 +276,14 @@ int setup_map(struct multipath *mpp, char
> **params, struct vectors *vecs)
>         if (mpp->disable_queueing && VECTOR_SIZE(mpp->paths) != 0)
>                 mpp->disable_queueing = 0;
>  
> +       /* Force QUEUE_MODE_BIO for maps with nvme:tcp paths */
> +       vector_foreach_slot(mpp->paths, pp, i) {
> +               if (pp->bus == SYSFS_BUS_NVME &&
> +                   pp->sg_id.proto_id == NVME_PROTOCOL_TCP) {
> +                       mpp->queue_mode = QUEUE_MODE_BIO;
> +                       break;
> +               }
> +       }
>         /*
>          * If this map was created with add_map_without_path(),
>          * mpp->hwe might not be set yet.
> @@ -1148,6 +1157,13 @@ int coalesce_paths (struct vectors *vecs,
> vector mpvec, char *refwwid,
>                         continue;
>                 }
>  
> +               cmpp = find_mp_by_wwid(curmp, pp1->wwid);
> +               if (cmpp && cmpp->queue_mode == QUEUE_MODE_RQ &&
> +                   pp1->bus == SYSFS_BUS_NVME && pp1->sg_id.proto_id
> ==
> +                   NVME_PROTOCOL_TCP) {
> +                       orphan_path(pp1, "nvme:tcp path not allowed
> with request queue_mode multipath device");
> +                       continue;
> +               }
>                 /*
>                  * at this point, we know we really got a new mp
>                  */
> @@ -1186,7 +1202,6 @@ int coalesce_paths (struct vectors *vecs,
> vector mpvec, char *refwwid,
>                 }
>                 verify_paths(mpp);
>  
> -               cmpp = find_mp_by_wwid(curmp, mpp->wwid);
>                 if (cmpp)
>                         mpp->queue_mode = cmpp->queue_mode;
>                 if (setup_map(mpp, &params, vecs)) {
> diff --git a/libmultipath/structs_vec.c b/libmultipath/structs_vec.c
> index 645896c6..5a618767 100644
> --- a/libmultipath/structs_vec.c
> +++ b/libmultipath/structs_vec.c
> @@ -264,6 +264,13 @@ int adopt_paths(vector pathvec, struct multipath
> *mpp)
>                         }
>                         if (pp->initialized == INIT_REMOVED)
>                                 continue;
> +                       if (mpp->queue_mode == QUEUE_MODE_RQ &&
> +                           pp->bus == SYSFS_BUS_NVME &&
> +                           pp->sg_id.proto_id == NVME_PROTOCOL_TCP)
> {
> +                               condlog(2, "%s: mulitpath device %s
> created with request queue_mode. Unable to add nvme:tcp paths",
> +                                       pp->dev, mpp->alias);
> +                               continue;
> +                       }
>                         if (!mpp->paths && !(mpp->paths =
> vector_alloc()))
>                                 goto err;
>  
> diff --git a/multipath/multipath.conf.5 b/multipath/multipath.conf.5
> index 07476497..1fea9d5a 100644
> --- a/multipath/multipath.conf.5
> +++ b/multipath/multipath.conf.5
> @@ -463,7 +463,9 @@ Before kernel 4.20 The default depends on the
> kernel parameter
>  \fBdm_mod.use_blk_mq\fR. It is \fImq\fR if the latter is set, and
> \fIrq\fR
>  otherwise. Since kernel 4.20, \fIrq\fR and \fImq\fR both correspond
> to
>  block-multiqueue. Once a multipath device has been created, its
> queue_mode
> -cannot be changed.
> +cannot be changed. \fInvme:tcp\fR paths are only supported in
> multipath
> +devices with queue_mode set to \fIbio\fR. multipath will
> automatically
> +set this when creating a device with \fInvme:tcp\fR paths.
>  .TP
>  The default is: \fB<unset>\fR
>  .RE

--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel
diff mbox series

Patch

diff --git a/libmultipath/configure.c b/libmultipath/configure.c
index 7c7babd9..e5249fc1 100644
--- a/libmultipath/configure.c
+++ b/libmultipath/configure.c
@@ -260,6 +260,7 @@  int rr_optimize_path_order(struct pathgroup *pgp)
 int setup_map(struct multipath *mpp, char **params, struct vectors *vecs)
 {
 	struct pathgroup * pgp;
+	struct path *pp;
 	struct config *conf;
 	int i, marginal_pathgroups;
 	char *save_attr;
@@ -275,6 +276,14 @@  int setup_map(struct multipath *mpp, char **params, struct vectors *vecs)
 	if (mpp->disable_queueing && VECTOR_SIZE(mpp->paths) != 0)
 		mpp->disable_queueing = 0;
 
+	/* Force QUEUE_MODE_BIO for maps with nvme:tcp paths */
+	vector_foreach_slot(mpp->paths, pp, i) {
+		if (pp->bus == SYSFS_BUS_NVME &&
+		    pp->sg_id.proto_id == NVME_PROTOCOL_TCP) {
+			mpp->queue_mode = QUEUE_MODE_BIO;
+			break;
+		}
+	}
 	/*
 	 * If this map was created with add_map_without_path(),
 	 * mpp->hwe might not be set yet.
@@ -1148,6 +1157,13 @@  int coalesce_paths (struct vectors *vecs, vector mpvec, char *refwwid,
 			continue;
 		}
 
+		cmpp = find_mp_by_wwid(curmp, pp1->wwid);
+		if (cmpp && cmpp->queue_mode == QUEUE_MODE_RQ &&
+		    pp1->bus == SYSFS_BUS_NVME && pp1->sg_id.proto_id ==
+		    NVME_PROTOCOL_TCP) {
+			orphan_path(pp1, "nvme:tcp path not allowed with request queue_mode multipath device");
+			continue;
+		}
 		/*
 		 * at this point, we know we really got a new mp
 		 */
@@ -1186,7 +1202,6 @@  int coalesce_paths (struct vectors *vecs, vector mpvec, char *refwwid,
 		}
 		verify_paths(mpp);
 
-		cmpp = find_mp_by_wwid(curmp, mpp->wwid);
 		if (cmpp)
 			mpp->queue_mode = cmpp->queue_mode;
 		if (setup_map(mpp, &params, vecs)) {
diff --git a/libmultipath/structs_vec.c b/libmultipath/structs_vec.c
index 645896c6..5a618767 100644
--- a/libmultipath/structs_vec.c
+++ b/libmultipath/structs_vec.c
@@ -264,6 +264,13 @@  int adopt_paths(vector pathvec, struct multipath *mpp)
 			}
 			if (pp->initialized == INIT_REMOVED)
 				continue;
+			if (mpp->queue_mode == QUEUE_MODE_RQ &&
+			    pp->bus == SYSFS_BUS_NVME &&
+			    pp->sg_id.proto_id == NVME_PROTOCOL_TCP) {
+				condlog(2, "%s: mulitpath device %s created with request queue_mode. Unable to add nvme:tcp paths",
+					pp->dev, mpp->alias);
+				continue;
+			}
 			if (!mpp->paths && !(mpp->paths = vector_alloc()))
 				goto err;
 
diff --git a/multipath/multipath.conf.5 b/multipath/multipath.conf.5
index 07476497..1fea9d5a 100644
--- a/multipath/multipath.conf.5
+++ b/multipath/multipath.conf.5
@@ -463,7 +463,9 @@  Before kernel 4.20 The default depends on the kernel parameter
 \fBdm_mod.use_blk_mq\fR. It is \fImq\fR if the latter is set, and \fIrq\fR
 otherwise. Since kernel 4.20, \fIrq\fR and \fImq\fR both correspond to
 block-multiqueue. Once a multipath device has been created, its queue_mode
-cannot be changed.
+cannot be changed. \fInvme:tcp\fR paths are only supported in multipath
+devices with queue_mode set to \fIbio\fR. multipath will automatically
+set this when creating a device with \fInvme:tcp\fR paths.
 .TP
 The default is: \fB<unset>\fR
 .RE