[PATCH v2] nvme-fc: FPIN link integrity handling

Christoph Hellwig hch at lst.de
Tue Apr 2 05:55:34 PDT 2024


> +static struct nvme_fc_rport *nvme_fc_rport_from_wwpn(struct nvme_fc_lport *lport,
> +						     u64 rport_wwpn)

Pleae fix all this whacky formatting.  Avoid the overly long lines
(in this case by just having the return value on it's own line)
and use two tab continuations instead of wasting most of the continuing
line.

> +/*
> + * nvme_fc_fpin_li_lport_update - routine to update Link Integrity
> + * event statistics.
> + * @lport:		local port the FPIN was received on
> + * @tlv:		pointer to link integrity descriptor
> + *
> + */

What's the point of this half-kernel doc comment?  It doesn't really
add much valye for a local function.

> +static void
> +nvme_fc_fpin_li_lport_update(struct nvme_fc_lport *lport, struct fc_tlv_desc *tlv)
> +{
> +	unsigned int i, pname_count;
> +	struct nvme_fc_rport *attached_rport;
> +	struct fc_fn_li_desc *li_desc = (struct fc_fn_li_desc *)tlv;

The casting here is pretty gross.  Sounds like the FC code should use
a union or better fix fc_fn_li_desc to not duplicate the tag/len
fields?

> +	u64 wwpn;
> +
> +	wwpn = be64_to_cpu(li_desc->attached_wwpn);
> +	attached_rport = nvme_fc_rport_from_wwpn(lport, wwpn);
> +	pname_count = be32_to_cpu(li_desc->pname_count);

... and if these were initialized at declartion time thing would
get a lot easier to read.

> +EXPORT_SYMBOL(nvme_fc_fpin_rcv);

All nvme exports are EXPORT_SYMBOL_GPL

Also please split the patch up into nvme core, nvme-fc and
driver changes.



More information about the Linux-nvme mailing list