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

[AlanCui4080 <me@alancui.cc>]

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.