Re: [PATCH] nvme: implement support for nvme relaxed effects

Chaitanya Kulkarni
2024-02-06T07:59:00Z

Keith,

On 2/5/24 11:17, Keith Busch wrote:
> From: Keith Busch <kbusch@kernel.org>
>
> NVM Express TP4167 provides a way for controllers to report a relaxed
> execution constraint. Specifically, it notifies of exclusivity for IO
> vs. admin commands instead of grouping these together. If set, then we
> don't need to freeze IO in order to execute that admin command. The
> freezing distrupts IO processes, so it's nice to avoid that if the
> controller tells us we don't need to do that.
>
> Signed-off-by: Keith Busch <kbusch@kernel.org>
> ---
> drivers/nvme/host/core.c | 4 ++++
> include/linux/nvme.h | 1 +
> 2 files changed, 5 insertions(+)
>
> diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
> index 975245527c1fc..75927e8579857 100644
> --- a/drivers/nvme/host/core.c
> +++ b/drivers/nvme/host/core.c
> @@ -1153,6 +1153,10 @@ u32 nvme_command_effects(struct nvme_ctrl *ctrl, struct nvme_ns *ns, u8 opcode)
> effects &= ~NVME_CMD_EFFECTS_CSE_MASK;
> } else {
> effects = le32_to_cpu(ctrl->effects->acs[opcode]);
> +
> + /* Ignore execution restrictions if any relaxation bits are set */

not a blocker, but I found following much cleaner and easy to search in
spec :-

+               /*
+                *  Command Submission and Execution Relaxation (CSER) takes
+                *  priority over Command Submission and Execution (CSE).
+                */

but if others are okay with original comment, looks good.

Reviewed-by: Chaitanya Kulkarni <kch@nvidia.com>

-ck

> + if (effects & NVME_CMD_EFFECTS_CSER_MASK)
> + effects &= ~NVME_CMD_EFFECTS_CSE_MASK;
> }
>
> return effects;
> diff --git a/include/linux/nvme.h b/include/linux/nvme.h
> index bc605ec4a3fd0..3ef4053ea9500 100644
> --- a/include/linux/nvme.h
> +++ b/include/linux/nvme.h
> @@ -646,6 +646,7 @@ enum {
> NVME_CMD_EFFECTS_NCC = 1 << 2,
> NVME_CMD_EFFECTS_NIC = 1 << 3,
> NVME_CMD_EFFECTS_CCC = 1 << 4,
> + NVME_CMD_EFFECTS_CSER_MASK = GENMASK(15, 14),
> NVME_CMD_EFFECTS_CSE_MASK = GENMASK(18, 16),
> NVME_CMD_EFFECTS_UUID_SEL = 1 << 19,
> NVME_CMD_EFFECTS_SCOPE_MASK = GENMASK(31, 20),