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

[Chaitanya Kulkarni <chaitanyak@nvidia.com>]

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),