[PATCH] nvme: make prp passthrough usage less scary

[PATCH] nvme: make prp passthrough usage less scary

[Keith Busch]

From: Keith Busch <kbusch@kernel.org>

The warning is a bit alarming, and it only prints for the very first
non-sgl capable device that receives a passthrough command. Just log an
informational message on initial discovery for every device.

Signed-off-by: Keith Busch <kbusch@kernel.org>
---
 drivers/nvme/host/core.c  |  4 ++++
 drivers/nvme/host/ioctl.c | 12 ++----------
 2 files changed, 6 insertions(+), 10 deletions(-)

diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index dc388e24caade..1e7e42b43aa3f 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -3749,6 +3749,10 @@ int nvme_init_ctrl_finish(struct nvme_ctrl *ctrl, bool was_suspended)
 		ret = nvme_hwmon_init(ctrl);
 		if (ret == -EINTR)
 			return ret;
+
+		if (!nvme_ctrl_sgl_supported(ctrl))
+			dev_info(ctrl->device,
+				"passthrough uses implicit buffer lengths\n");
 	}
 
 	clear_bit(NVME_CTRL_DIRTY_CAPABILITY, &ctrl->flags);
diff --git a/drivers/nvme/host/ioctl.c b/drivers/nvme/host/ioctl.c
index 9597a87cf05dc..39d6e69cbbe72 100644
--- a/drivers/nvme/host/ioctl.c
+++ b/drivers/nvme/host/ioctl.c
@@ -125,16 +125,8 @@ static int nvme_map_user_request(struct request *req, u64 ubuffer,
 	struct bio *bio = NULL;
 	int ret;
 
-	if (!nvme_ctrl_sgl_supported(ctrl))
-		dev_warn_once(ctrl->device, "using unchecked data buffer\n");
-	if (has_metadata) {
-		if (!supports_metadata)
-			return -EINVAL;
-
-		if (!nvme_ctrl_meta_sgl_supported(ctrl))
-			dev_warn_once(ctrl->device,
-				      "using unchecked metadata buffer\n");
-	}
+	if (has_metadata && !supports_metadata)
+		return -EINVAL;
 
 	if (iter)
 		ret = blk_rq_map_user_iov(q, req, NULL, iter, GFP_KERNEL);
-- 
2.52.0

Re: [PATCH] nvme: make prp passthrough usage less scary

[Christoph Hellwig]

Looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>

Re: [PATCH] nvme: make prp passthrough usage less scary

[Jens Axboe]

On 5/6/26 7:18 AM, Keith Busch wrote:
> From: Keith Busch <kbusch@kernel.org>
> 
> The warning is a bit alarming, and it only prints for the very first
> non-sgl capable device that receives a passthrough command. Just log an
> informational message on initial discovery for every device.

Looks good to me, just ran some testing and hit this too. Neither
are super clear, but hey, this is an improvement:

Reviewed-by: Jens Axboe <axboe@kernel.dk>

-- 
Jens Axboe

Re: [PATCH] nvme: make prp passthrough usage less scary

[AlanCui4080]

On Wednesday, 6 May 2026 21:18，you wrote：
> From: Keith Busch <kbusch@kernel.org>
> 
> The warning is a bit alarming, and it only prints for the very first
> non-sgl capable device that receives a passthrough command. Just log an
> informational message on initial discovery for every device.

Since we decide to show this warning at device initial discovery stage,
here are some similar warnings that I think are essentially the same.

Those are not really "a problem", but as I said:
> The asymmetry can be misleading to users. If all devices in the system report 
> the same issue, it might not be a problem, but if only one device reports it, 
> it might (especially since the user has two identical drives). 
(Yes I did have two identical drives) and here is the warnings complained by
the kernel nvme driver.

May 07 23:01:34 archlinux kernel: nvme nvme0: missing or invalid SUBNQN field.
May 07 23:01:34 archlinux kernel: nvme nvme1: missing or invalid SUBNQN field.
May 07 23:06:34 AlanArchDesktop kernel: nvme nvme0: using unchecked data buffer
May 07 23:06:34 AlanArchDesktop kernel: block nvme0n1: No UUID available providing old NGUID

The first two warnings is not misleading, since they are both reported for the both
two drives, but the next two messages is not, i checked the firmware revision and
the hardware revision, but they are identically the same, makes me really confused.

So,

In the result of "grep -C3 -rn "dev_warn_once" ./drivers/nvme/*":
./drivers/nvme/host/core.c-1210-        if (ns) {
./drivers/nvme/host/core.c-1211-                effects = le32_to_cpu(ns->head->effects->iocs[opcode]);
./drivers/nvme/host/core.c-1212-                if (effects & ~(NVME_CMD_EFFECTS_CSUPP | NVME_CMD_EFFECTS_LBCC))
./drivers/nvme/host/core.c:1213:                        dev_warn_once(ctrl->device,
./drivers/nvme/host/core.c-1214-                                "IO command:%02x has unusual effects:%08x\n",
./drivers/nvme/host/core.c-1215-                                opcode, effects);
./drivers/nvme/host/core.c-1216-

Because there is only nvme_passthru_start will call nvme_command_effects,
so I think make it dev_warn should be fine and will not produce too much
warning messages on console, or use dev_warn_ratelimited considering future using.

./drivers/nvme/host/core.c-2736-                ready_timeout = NVME_CRTO_CRWMT(crto);
./drivers/nvme/host/core.c-2737-
./drivers/nvme/host/core.c-2738-                if (ready_timeout < timeout)
./drivers/nvme/host/core.c:2739:                        dev_warn_once(ctrl->device, "bad crto:%x cap:%llx\n",
./drivers/nvme/host/core.c-2740-                                      crto, ctrl->cap);
./drivers/nvme/host/core.c-2741-                else
./drivers/nvme/host/core.c-2742-                        timeout = ready_timeout;

./drivers/nvme/host/core.c-2763-        ret = nvme_set_features(ctrl, NVME_FEAT_TIMESTAMP, 0, &ts, sizeof(ts),
./drivers/nvme/host/core.c-2764-                        NULL);
./drivers/nvme/host/core.c-2765-        if (ret)
./drivers/nvme/host/core.c:2766:                dev_warn_once(ctrl->device,
./drivers/nvme/host/core.c-2767-                        "could not set timestamp (%d)\n", ret);
./drivers/nvme/host/core.c-2768-        return ret;
./drivers/nvme/host/core.c-2769-}

Because the first dev_warn_once is in the nvme_enable_ctrl will be unlikely called
multiple times and the second one(nvme_configure_timestamp) is called only by
nvme_init_ctrl_finish, so I think make it dev_warn should be fine also.

./drivers/nvme/host/core.c-4045-                        dev_warn(ctrl->device,
./drivers/nvme/host/core.c-4046-                                "Found shared namespace %d, but multipathing not supported.\n",
./drivers/nvme/host/core.c-4047-                                info->nsid);
./drivers/nvme/host/core.c:4048:                        dev_warn_once(ctrl->device,
./drivers/nvme/host/core.c-4049-                                "Shared namespace support requires core_nvme.multipath=Y.\n");
./drivers/nvme/host/core.c-4050-                }
./drivers/nvme/host/core.c-4051-        }

Should be OK to dev_warn_once as only hint about how to deal with it.

./drivers/nvme/host/ioctl.c-126-        int ret;
./drivers/nvme/host/ioctl.c-127-
./drivers/nvme/host/ioctl.c-128-        if (!nvme_ctrl_sgl_supported(ctrl))
./drivers/nvme/host/ioctl.c:129:                dev_warn_once(ctrl->device, "using unchecked data buffer\n");
./drivers/nvme/host/ioctl.c-130-        if (has_metadata) {
./drivers/nvme/host/ioctl.c-131-                if (!supports_metadata)
./drivers/nvme/host/ioctl.c-132-                        return -EINVAL;
./drivers/nvme/host/ioctl.c-133-
./drivers/nvme/host/ioctl.c-134-                if (!nvme_ctrl_meta_sgl_supported(ctrl))
./drivers/nvme/host/ioctl.c:135:                        dev_warn_once(ctrl->device,
./drivers/nvme/host/ioctl.c-136-                                      "using unchecked metadata buffer\n");
./drivers/nvme/host/ioctl.c-137-        }
./drivers/nvme/host/ioctl.c-138-

Resolved in your last patch.

./drivers/nvme/host/sysfs.c-148-         */
./drivers/nvme/host/sysfs.c-149-        if (uuid_is_null(&ids->uuid)) {
./drivers/nvme/host/sysfs.c-150-                // moved to core.c 
./drivers/nvme/host/sysfs.c:151:                // dev_warn_once(dev,
./drivers/nvme/host/sysfs.c-152-                //      "No UUID available providing old NGUID\n");
./drivers/nvme/host/sysfs.c-153-                return sysfs_emit(buf, "%pU\n", ids->nguid);
./drivers/nvme/host/sysfs.c-154-        }

Essentially the same as the "prp passthrough usage warning", the patch was sent
before this email.


Thank you for your attention to this matter.

Alan.

Re: [PATCH] nvme: make prp passthrough usage less scary

[Sagi Grimberg]

Reviewed-by: Sagi Grimberg <sagi@grimberg.me>