[PATCH V4 1/1] nvme: Add verbose error logging

Sagi Grimberg sagi at grimberg.me
Tue Jan 25 14:54:35 PST 2022


> +#endif
> + if (err_str == NULL)
> + err_str =  "I/O Error";
> +
> + if (ns) {
> +#ifdef CONFIG_NVME_VERBOSE_ERRORS
> + for (i = 0, entry = nvme_ops ; i < ARRAY_SIZE(nvme_ops) ; i++)
> + if (entry[i].code == nr->cmd->common.opcode)
> + op_str = entry[i].string;
> 
> Same here, the loop looks redundant to me:
> if (nr->cmd->common.opcode <= ARRAY_SIZE(nvme_ops) &&
>     nvme_ops[nr->cmd->common.opcode] != NULL)
> op_str = nvme_ops[nr->cmd->common.opcode].string;
> else
> op_str = "Unknown"
> 
> 
> Currently the nvme_ops and nvme_admin_ops arrays are not built by index so I need to walk the array to find the
> appropriate opcode/name.  Are you asking to change these arrays to be build by index?
> 
> static const char * const nvme_admin_ops[] = {
>          [nvme_admin_delete_sq] = "Delete SQ",
>          [nvme_admin_create_sq] = "Create SQ",
>          [nvme_admin_get_log_page] = "Get Log Page”,
> 
> Rather than:
> 
> static const struct nvme_string_table nvme_admin_ops[] = {
>          { nvme_admin_delete_sq,         "Delete SQ" },
>          { nvme_admin_create_sq,         "Create SQ" },
>          { nvme_admin_get_log_page,      "Get Log Page" },

Yes, is that a problem?



More information about the Linux-nvme mailing list