On Tue, Nov 19, 2024 at 01:16:28PM +0100, Christoph Hellwig wrote:
> +static int nvme_read_fdp_config(struct nvme_ns *ns, struct nvme_ns_info *info)
> +{
> + struct nvme_fdp_config result;
> + struct nvme_fdp_config_log *log;
> + struct nvme_fdp_config_desc *configs;
> + size_t log_size;
> + int error;
> +
> + error = nvme_get_features(ns->ctrl, NVME_FEAT_FDP, info->endgid, NULL,
> + 0, &result);
> + if (error)
> + return error;
> +
> + if (!(result.flags & FDPCFG_FDPE)) {
> + dev_warn(ns->ctrl->device, "FDP not enable in current config\n");
> + return -EINVAL;
> + }
> +
> + log_size = sizeof(*log) + (result.fdpcidx + 1) * sizeof(*configs);
> + log = kmalloc(log_size, GFP_KERNEL);
> + if (!log)
> + return -ENOMEM;
> +
> + error = nvme_get_log_lsi(ns->ctrl, info->nsid, NVME_LOG_FDP_CONFIGS,
> + 0, 0, log, log_size, 0, info->endgid);
> + if (error) {
> + dev_warn(ns->ctrl->device,
> + "failed to read FDP config log: 0x%x\n", error);
> + goto out_free_log;
> + }
> +
> + if (le32_to_cpu(log->size) < log_size) {
> + dev_warn(ns->ctrl->device, "FDP log too small: %d vs %zd\n",
> + le32_to_cpu(log->size), log_size);
> + error = -EINVAL;
> + goto out_free_log;
> + }
> +
> + configs = (struct nvme_fdp_config_desc *)(log + 1);
> + if (le32_to_cpu(configs[result.fdpcidx].nrg) > 1) {
> + dev_warn(ns->ctrl->device, "FDP NRG > 1 not supported\n");
Why not support multiple reclaim groups?
> + return -EINVAL;
> + }
> + ns->head->runs = le64_to_cpu(configs[result.fdpcidx].runs);
The config descriptors are variable length, so you can't just index into
it. You have to read each index individually to get the next index's offset.
Something like:
struct nvme_fdp_config_desc *configs;
void *l;
int i;
...
l = log + 1;
for (i = 0; i < result.fdpcidx; i++) {
configs = l;
l += le16_to_cpu(configs->size);
}
ns->head->runs = le64_to_cpu(configs->runs);
