[PATCH V10 2/8] nvmet: add NVM Command Set Identifier support

Christoph Hellwig hch at lst.de
Tue Mar 9 11:37:46 GMT 2021


> +static void nvmet_set_csi_nvm_effects(struct nvme_effects_log *log)
>  {
>  	log->acs[nvme_admin_get_log_page]	= cpu_to_le32(1 << 0);
>  	log->acs[nvme_admin_identify]		= cpu_to_le32(1 << 0);
>  	log->acs[nvme_admin_abort_cmd]		= cpu_to_le32(1 << 0);
> @@ -184,8 +177,45 @@ static void nvmet_execute_get_log_cmd_effects_ns(struct nvmet_req *req)
>  	log->iocs[nvme_cmd_flush]		= cpu_to_le32(1 << 0);
>  	log->iocs[nvme_cmd_dsm]			= cpu_to_le32(1 << 0);
>  	log->iocs[nvme_cmd_write_zeroes]	= cpu_to_le32(1 << 0);
> +}
>  
> +static u16 nvmet_set_csi_zns_effects(struct nvme_effects_log *log)
> +{
> +	if (!IS_ENABLED(CONFIG_BLK_DEV_ZONED))
> +		return NVME_SC_INVALID_IO_CMD_SET;

I'd move this check into the caller.

> +	switch (req->cmd->get_log_page.csi) {
> +	case NVME_CSI_NVM:
> +		nvmet_set_csi_nvm_effects(log);
> +		break;
> +	case NVME_CSI_ZNS:
> +		status = nvmet_set_csi_zns_effects(log);
> +		break;

ZNS also needs a call to nvmet_set_csi_nvm_effects as it also supports
the the NVM commands.  But more importantly the ZNS case does not
belong into this patch at all, but into one that actually adds ZNS
support.  CSI support is just infrastructure not tied to a new command
set.  Same for all the other places referencing ZNS in this patch.


> +	default:
> +		status = NVME_SC_INVALID_LOG_PAGE;
> +		break;
> +	}
> +
> +	if (status == NVME_SC_SUCCESS)
> +		status = nvmet_copy_to_sgl(req, 0, log, sizeof(*log));

Also I'd use a goto for the failure case instead of the check here.
the 

> +	switch (req->ns->csi) {
> +	case NVME_CSI_NVM:
> +		status = nvmet_copy_ns_identifier(req, NVME_NIDT_CSI, o);
> +		break;
> +	case NVME_CSI_ZNS:
> +		if (!IS_ENABLED(CONFIG_BLK_DEV_ZONED))
> +			return NVME_SC_INVALID_IO_CMD_SET;
> +
> +		status = nvmet_copy_ns_identifier(req, NVME_NIDT_CSI, o);
> +		break;
> +	default:
> +		status = NVME_SC_INVALID_IO_CMD_SET;
> +	}
> +
> +	return status;

Just return directly from inside the switch statement to simplify this
a bit.



More information about the Linux-nvme mailing list