> +static int nvme_check_fdp(struct nvme_ns *ns, struct nvme_ns_info *info,
> + u8 fdp_idx)
Maybe nvme_query_fdp_runs or something else that makes it clear this
is trying to find the runs field might make sense to name this a little
bit more descriptively.
> +{
> + struct nvme_fdp_config_log hdr, *h;
> + struct nvme_fdp_config_desc *desc;
> + size_t size = sizeof(hdr);
> + int i, n, ret;
> + void *log;
> +
> + info->runs = 0;
> + ret = nvme_get_log_lsi(ns->ctrl, 0, NVME_LOG_FDP_CONFIGS, 0, NVME_CSI_NVM,
Overly long line here, and same for the second call below.
> + (void *)&hdr, size, 0, info->endgid);
And this cast isn't actually needed.
> + n = le16_to_cpu(h->numfdpc) + 1;
> + if (fdp_idx > n)
> + goto out;
> +
> + log = h + 1;
> + do {
> + desc = log;
> + log += le16_to_cpu(desc->dsze);
> + } while (i++ < fdp_idx);
Maybe a for loop makes it easier to avoid the uninitialized variable,
e.g.
for (i = 0; i < fdp_index; i++) {
..
> + if (ns->ctrl->ctratt & NVME_CTRL_ATTR_FDPS) {
> + ret = nvme_query_fdp_info(ns, info);
> + if (ret)
> + dev_warn(ns->ctrl->device,
> + "FDP failure status:0x%x\n", ret);
> + if (ret < 0)
> + goto out;
> + }
Looking at the full series with the next patch applied I'm a bit
confused about the handling when rescanning. AFAIK the code now always
goes into nvme_query_fdp_info when NVME_CTRL_ATTR_FDPS even if
head->plids/head->nr_plids is already set, and that will then simply
override them, even if they were already set.
Also the old freeing of head->plids in nvme_free_ns_head seems gone in
this version.
Last not but least "FDP failure" is probably not a very helpful message
when it could come from about half a dozen different commands sent to
the device.
