[PATCH V6 1/1] nvme: Add verbose error logging
Christoph Hellwig
hch at lst.de
Wed Feb 2 05:55:56 PST 2022
On Tue, Feb 01, 2022 at 04:50:50PM -0800, Alan Adamson wrote:
> +nvme-core-y := core.o ioctl.o errors.o
I still don't like the errors.c/o name. This isn't really about
errors per se, but about pretty printing protocol constants. I think
I suggested pretty_print.c before, but maybe just following drivers/scsi/
and calling this constants.c might be best.
> + const unsigned char *op_str = "I/O Cmd";
> + const unsigned char *admin_op_str = "Admin Cmd";
No nee for these constants, just put this into the main format string.
> + const unsigned char *err_str = NULL;
> +
> + err_str = nvme_get_error_status_str(nr->status);
> +
> + if (ns) {
> + op_str = nvme_get_opcode_str(nr->cmd->common.opcode);
And not real need for err_str and op_str either, just put the calls
into the argument list of the pr_err_ratelimited calls.
Also please mark nvme_log_err static and move it so that there is no
need for a forward declaration.
> +const unsigned char *nvme_get_error_status_str(u16 status)
> +{
> + const unsigned char *err_str;
> +
> + if ((status & 0x7ff) < ARRAY_SIZE(nvme_statuses) &&
> + nvme_statuses[status & 0x7ff] != NULL)
> + err_str = nvme_statuses[status & 0x7ff];
> + else
> + err_str = "Unknown";
This can be simplified to:
status &= 0x7ff;
if (status < ARRAY_SIZE(nvme_statuses) nvme_statuses[status])
return nvme_statuses[status];
return "Unknown";
Similar for the other helpers.
> +EXPORT_SYMBOL_GPL(nvme_get_error_status_str);
No need to export any of the function added in this patch, as they are
all used only inside of nvme-core.ko
More information about the Linux-nvme
mailing list