Vaibhav Jain <[email protected]> writes:

> Presently add_dimm() only probes dimms that support NFIT/ACPI. Hence
> this patch refactors this functionality into two functions namely
> add_dimm() and add_nfit_dimm(). Function add_dimm() performs
> allocation and common 'struct ndctl_dimm' initialization and depending
> on whether the dimm-bus supports NIFT, calls add_nfit_dimm(). Once
> the probe is completed based on the value of 'ndctl_dimm.cmd_family'
> appropriate dimm-ops are assigned to the dimm.
>
> In case dimm-bus is of unknown type or doesn't support NFIT the
> initialization still continues, with no dimm-ops assigned to the
> 'struct ndctl_dimm' there-by limiting the functionality available.
>
> This patch shouldn't introduce any behavioral change.
>
> Signed-off-by: Vaibhav Jain <[email protected]>
> ---
> Changelog:
>
> v1..v2:
> * None


Looks good to me. For the series,

Reviewed-by: Santosh S <[email protected]>


Thanks,
Santosh

> ---
>  ndctl/lib/libndctl.c | 193 +++++++++++++++++++++++++------------------
>  1 file changed, 112 insertions(+), 81 deletions(-)
>
> diff --git a/ndctl/lib/libndctl.c b/ndctl/lib/libndctl.c
> index ee737cbbfe3e..d76dbf7e17de 100644
> --- a/ndctl/lib/libndctl.c
> +++ b/ndctl/lib/libndctl.c
> @@ -1441,82 +1441,15 @@ static int ndctl_bind(struct ndctl_ctx *ctx, struct 
> kmod_module *module,
>  static int ndctl_unbind(struct ndctl_ctx *ctx, const char *devpath);
>  static struct kmod_module *to_module(struct ndctl_ctx *ctx, const char 
> *alias);
>  
> -static void *add_dimm(void *parent, int id, const char *dimm_base)
> +static int add_nfit_dimm(struct ndctl_dimm *dimm, const char *dimm_base)
>  {
> -     int formats, i;
> -     struct ndctl_dimm *dimm;
> +     int i, rc = -1;
>       char buf[SYSFS_ATTR_SIZE];
> -     struct ndctl_bus *bus = parent;
> -     struct ndctl_ctx *ctx = bus->ctx;
> +     struct ndctl_ctx *ctx = dimm->bus->ctx;
>       char *path = calloc(1, strlen(dimm_base) + 100);
>  
>       if (!path)
> -             return NULL;
> -
> -     sprintf(path, "%s/nfit/formats", dimm_base);
> -     if (sysfs_read_attr(ctx, path, buf) < 0)
> -             formats = 1;
> -     else
> -             formats = clamp(strtoul(buf, NULL, 0), 1UL, 2UL);
> -
> -     dimm = calloc(1, sizeof(*dimm) + sizeof(int) * formats);
> -     if (!dimm)
> -             goto err_dimm;
> -     dimm->bus = bus;
> -     dimm->id = id;
> -
> -     sprintf(path, "%s/dev", dimm_base);
> -     if (sysfs_read_attr(ctx, path, buf) < 0)
> -             goto err_read;
> -     if (sscanf(buf, "%d:%d", &dimm->major, &dimm->minor) != 2)
> -             goto err_read;
> -
> -     sprintf(path, "%s/commands", dimm_base);
> -     if (sysfs_read_attr(ctx, path, buf) < 0)
> -             goto err_read;
> -     dimm->cmd_mask = parse_commands(buf, 1);
> -
> -     dimm->dimm_buf = calloc(1, strlen(dimm_base) + 50);
> -     if (!dimm->dimm_buf)
> -             goto err_read;
> -     dimm->buf_len = strlen(dimm_base) + 50;
> -
> -     dimm->dimm_path = strdup(dimm_base);
> -     if (!dimm->dimm_path)
> -             goto err_read;
> -
> -     sprintf(path, "%s/modalias", dimm_base);
> -     if (sysfs_read_attr(ctx, path, buf) < 0)
> -             goto err_read;
> -     dimm->module = to_module(ctx, buf);
> -
> -     dimm->handle = -1;
> -     dimm->phys_id = -1;
> -     dimm->serial = -1;
> -     dimm->vendor_id = -1;
> -     dimm->device_id = -1;
> -     dimm->revision_id = -1;
> -     dimm->health_eventfd = -1;
> -     dimm->dirty_shutdown = -ENOENT;
> -     dimm->subsystem_vendor_id = -1;
> -     dimm->subsystem_device_id = -1;
> -     dimm->subsystem_revision_id = -1;
> -     dimm->manufacturing_date = -1;
> -     dimm->manufacturing_location = -1;
> -     dimm->cmd_family = -1;
> -     dimm->nfit_dsm_mask = ULONG_MAX;
> -     for (i = 0; i < formats; i++)
> -             dimm->format[i] = -1;
> -
> -     sprintf(path, "%s/flags", dimm_base);
> -     if (sysfs_read_attr(ctx, path, buf) < 0) {
> -             dimm->locked = -1;
> -             dimm->aliased = -1;
> -     } else
> -             parse_dimm_flags(dimm, buf);
> -
> -     if (!ndctl_bus_has_nfit(bus))
> -             goto out;
> +             return -1;
>  
>       /*
>        * 'unique_id' may not be available on older kernels, so don't
> @@ -1582,24 +1515,15 @@ static void *add_dimm(void *parent, int id, const 
> char *dimm_base)
>       sprintf(path, "%s/nfit/family", dimm_base);
>       if (sysfs_read_attr(ctx, path, buf) == 0)
>               dimm->cmd_family = strtoul(buf, NULL, 0);
> -     if (dimm->cmd_family == NVDIMM_FAMILY_INTEL)
> -             dimm->ops = intel_dimm_ops;
> -     if (dimm->cmd_family == NVDIMM_FAMILY_HPE1)
> -             dimm->ops = hpe1_dimm_ops;
> -     if (dimm->cmd_family == NVDIMM_FAMILY_MSFT)
> -             dimm->ops = msft_dimm_ops;
> -     if (dimm->cmd_family == NVDIMM_FAMILY_HYPERV)
> -             dimm->ops = hyperv_dimm_ops;
>  
>       sprintf(path, "%s/nfit/dsm_mask", dimm_base);
>       if (sysfs_read_attr(ctx, path, buf) == 0)
>               dimm->nfit_dsm_mask = strtoul(buf, NULL, 0);
>  
> -     dimm->formats = formats;
>       sprintf(path, "%s/nfit/format", dimm_base);
>       if (sysfs_read_attr(ctx, path, buf) == 0)
>               dimm->format[0] = strtoul(buf, NULL, 0);
> -     for (i = 1; i < formats; i++) {
> +     for (i = 1; i < dimm->formats; i++) {
>               sprintf(path, "%s/nfit/format%d", dimm_base, i);
>               if (sysfs_read_attr(ctx, path, buf) == 0)
>                       dimm->format[i] = strtoul(buf, NULL, 0);
> @@ -1610,7 +1534,114 @@ static void *add_dimm(void *parent, int id, const 
> char *dimm_base)
>               parse_nfit_mem_flags(dimm, buf);
>  
>       dimm->health_eventfd = open(path, O_RDONLY|O_CLOEXEC);
> +     rc = 0;
> + err_read:
> +
> +     free(path);
> +     return rc;
> +}
> +
> +static void *add_dimm(void *parent, int id, const char *dimm_base)
> +{
> +     int formats, i, rc = -ENODEV;
> +     struct ndctl_dimm *dimm = NULL;
> +     char buf[SYSFS_ATTR_SIZE];
> +     struct ndctl_bus *bus = parent;
> +     struct ndctl_ctx *ctx = bus->ctx;
> +     char *path = calloc(1, strlen(dimm_base) + 100);
> +
> +     if (!path)
> +             return NULL;
> +
> +     sprintf(path, "%s/nfit/formats", dimm_base);
> +     if (sysfs_read_attr(ctx, path, buf) < 0)
> +             formats = 1;
> +     else
> +             formats = clamp(strtoul(buf, NULL, 0), 1UL, 2UL);
> +
> +     dimm = calloc(1, sizeof(*dimm) + sizeof(int) * formats);
> +     if (!dimm)
> +             goto err_dimm;
> +     dimm->bus = bus;
> +     dimm->id = id;
> +
> +     sprintf(path, "%s/dev", dimm_base);
> +     if (sysfs_read_attr(ctx, path, buf) < 0)
> +             goto err_read;
> +     if (sscanf(buf, "%d:%d", &dimm->major, &dimm->minor) != 2)
> +             goto err_read;
> +
> +     sprintf(path, "%s/commands", dimm_base);
> +     if (sysfs_read_attr(ctx, path, buf) < 0)
> +             goto err_read;
> +     dimm->cmd_mask = parse_commands(buf, 1);
> +
> +     dimm->dimm_buf = calloc(1, strlen(dimm_base) + 50);
> +     if (!dimm->dimm_buf)
> +             goto err_read;
> +     dimm->buf_len = strlen(dimm_base) + 50;
> +
> +     dimm->dimm_path = strdup(dimm_base);
> +     if (!dimm->dimm_path)
> +             goto err_read;
> +
> +     sprintf(path, "%s/modalias", dimm_base);
> +     if (sysfs_read_attr(ctx, path, buf) < 0)
> +             goto err_read;
> +     dimm->module = to_module(ctx, buf);
> +
> +     dimm->handle = -1;
> +     dimm->phys_id = -1;
> +     dimm->serial = -1;
> +     dimm->vendor_id = -1;
> +     dimm->device_id = -1;
> +     dimm->revision_id = -1;
> +     dimm->health_eventfd = -1;
> +     dimm->dirty_shutdown = -ENOENT;
> +     dimm->subsystem_vendor_id = -1;
> +     dimm->subsystem_device_id = -1;
> +     dimm->subsystem_revision_id = -1;
> +     dimm->manufacturing_date = -1;
> +     dimm->manufacturing_location = -1;
> +     dimm->cmd_family = -1;
> +     dimm->nfit_dsm_mask = ULONG_MAX;
> +     for (i = 0; i < formats; i++)
> +             dimm->format[i] = -1;
> +
> +     sprintf(path, "%s/flags", dimm_base);
> +     if (sysfs_read_attr(ctx, path, buf) < 0) {
> +             dimm->locked = -1;
> +             dimm->aliased = -1;
> +     } else
> +             parse_dimm_flags(dimm, buf);
> +
> +     /* Check if the given dimm supports nfit */
> +     if (ndctl_bus_has_nfit(bus)) {
> +             dimm->formats = formats;
> +             rc = add_nfit_dimm(dimm, dimm_base);
> +     }
> +
> +     if (rc == -ENODEV) {
> +             /* Unprobed dimm with no family */
> +             rc = 0;
> +             goto out;
> +     }
> +
> +     /* Assign dimm-ops based on command family */
> +     if (dimm->cmd_family == NVDIMM_FAMILY_INTEL)
> +             dimm->ops = intel_dimm_ops;
> +     if (dimm->cmd_family == NVDIMM_FAMILY_HPE1)
> +             dimm->ops = hpe1_dimm_ops;
> +     if (dimm->cmd_family == NVDIMM_FAMILY_MSFT)
> +             dimm->ops = msft_dimm_ops;
> +     if (dimm->cmd_family == NVDIMM_FAMILY_HYPERV)
> +             dimm->ops = hyperv_dimm_ops;
>   out:
> +     if (rc) {
> +             err(ctx, "Unable to probe dimm:%d. Err:%d\n", id, rc);
> +             goto err_read;
> +     }
> +
>       list_add(&bus->dimms, &dimm->list);
>       free(path);
>  
> -- 
> 2.25.3
_______________________________________________
Linux-nvdimm mailing list -- [email protected]
To unsubscribe send an email to [email protected]

Reply via email to