On Sun, Nov 8, 2020 at 4:20 AM Santosh Sivaraj <sant...@fossix.org> wrote:
>
-ENOCOMMITMESSAGE
Also this patch is doing more than just skipping SMART tests, which
would have been noticed by writing a commit message, and which
probably would have prompted those changes to move to their own
patch... with their own commit message of course.
> Signed-off-by: Santosh Sivaraj <sant...@fossix.org>
> ---
> ndctl/lib/libndctl.c | 34 ++++++++++++++++++++++++++--------
> test/libndctl.c | 3 ++-
> 2 files changed, 28 insertions(+), 9 deletions(-)
>
> diff --git a/ndctl/lib/libndctl.c b/ndctl/lib/libndctl.c
> index d1f8e4e..26fc14c 100644
> --- a/ndctl/lib/libndctl.c
> +++ b/ndctl/lib/libndctl.c
> @@ -815,8 +815,11 @@ static void parse_papr_flags(struct ndctl_dimm *dimm,
> char *flags)
> dimm->flags.f_restore = 1;
> else if (strcmp(start, "smart_notify") == 0)
> dimm->flags.f_smart = 1;
> + else if (strcmp(start, "save_fail") == 0)
> + dimm->flags.f_save = 1;
> start = end + 1;
> }
> +
> if (end != start)
> dbg(ctx, "%s: Flags:%s\n", ndctl_dimm_get_devname(dimm),
> flags);
> }
> @@ -1044,7 +1047,8 @@ NDCTL_EXPORT int ndctl_bus_is_papr_scm(struct ndctl_bus
> *bus)
> if (sysfs_read_attr(bus->ctx, bus->bus_buf, buf) < 0)
> return 0;
>
> - return (strcmp(buf, "ibm,pmemory") == 0);
> + return (strcmp(buf, "ibm,pmemory") == 0 ||
> + strcmp(buf, "nvdimm_test") == 0);
> }
>
> /**
> @@ -1661,6 +1665,7 @@ static void populate_dimm_attributes(struct ndctl_dimm
> *dimm,
> char buf[SYSFS_ATTR_SIZE];
> struct ndctl_ctx *ctx = dimm->bus->ctx;
> char *path = calloc(1, strlen(dimm_base) + 100);
> + int i;
>
> sprintf(path, "%s/phys_id", dimm_base);
> if (sysfs_read_attr(ctx, path, buf) < 0)
> @@ -1690,6 +1695,16 @@ static void populate_dimm_attributes(struct ndctl_dimm
> *dimm,
> dimm->manufacturing_location = b[2];
> }
> }
> +
> + sprintf(path, "%s/format", dimm_base);
> + if (sysfs_read_attr(ctx, path, buf) == 0)
> + dimm->format[0] = strtoul(buf, NULL, 0);
> + for (i = 1; i < dimm->formats; i++) {
> + sprintf(path, "%s/format%d", dimm_base, i);
> + if (sysfs_read_attr(ctx, path, buf) == 0)
> + dimm->format[i] = strtoul(buf, NULL, 0);
> + }
The "format" attribute is this strange artifact of the ACPI NFIT
definition. You've already done the work to emulate it, but I wish I
could have saved you from that diversion because I don't see the point
for papr, especially just to make a unit test happy.
The "format" was related to a JEDEC designation IIRC, but it has no
practical implication on the kernel or ndctl. It was only 3rd party
management software that thought they needed this, but even then I
think the justification was dubious.
> +
> sprintf(path, "%s/subsystem_vendor", dimm_base);
> if (sysfs_read_attr(ctx, path, buf) == 0)
> dimm->subsystem_vendor_id = strtoul(buf, NULL, 0);
> @@ -1853,7 +1868,8 @@ static void *add_dimm(void *parent, int id, const char
> *dimm_base)
> if (!path)
> return NULL;
>
> - sprintf(path, "%s/nfit/formats", dimm_base);
> + sprintf(path, "%s%s/formats", dimm_base,
> + ndctl_bus_has_nfit(bus) ? "/nfit" : "");
> if (sysfs_read_attr(ctx, path, buf) < 0)
> formats = 1;
> else
> @@ -1927,9 +1943,9 @@ static void *add_dimm(void *parent, int id, const char
> *dimm_base)
> else
> dimm->fwa_result = fwa_result_to_result(buf);
>
> + dimm->formats = formats;
> /* Check if the given dimm supports nfit */
> if (ndctl_bus_has_nfit(bus)) {
> - dimm->formats = formats;
> rc = add_nfit_dimm(dimm, dimm_base);
> } else if (ndctl_bus_has_of_node(bus)) {
> rc = add_papr_dimm(dimm, dimm_base);
> @@ -2592,13 +2608,15 @@ static void *add_region(void *parent, int id, const
> char *region_base)
> goto err_read;
> region->num_mappings = strtoul(buf, NULL, 0);
>
> - sprintf(path, "%s/nfit/range_index", region_base);
> - if (ndctl_bus_has_nfit(bus)) {
> - if (sysfs_read_attr(ctx, path, buf) < 0)
> + sprintf(path, "%s%s/range_index", region_base,
> + ndctl_bus_has_nfit(bus) ? "/nfit" : "");
> + if (sysfs_read_attr(ctx, path, buf) < 0) {
> + if (ndctl_bus_has_nfit(bus))
> goto err_read;
> - region->range_index = strtoul(buf, NULL, 0);
> + else
> + region->range_index = -1;
> } else
> - region->range_index = -1;
> + region->range_index = strtoul(buf, NULL, 0);
>
> sprintf(path, "%s/read_only", region_base);
> if (sysfs_read_attr(ctx, path, buf) < 0)
> diff --git a/test/libndctl.c b/test/libndctl.c
> index 994e0fa..b7e7b68 100644
> --- a/test/libndctl.c
> +++ b/test/libndctl.c
> @@ -2427,7 +2427,8 @@ static int check_commands(struct ndctl_bus *bus, struct
> ndctl_dimm *dimm,
> * The kernel did not start emulating v1.2 namespace spec smart data
> * until 4.9.
> */
> - if (!ndctl_test_attempt(test, KERNEL_VERSION(4, 9, 0)))
> + if (!ndctl_test_attempt(test, KERNEL_VERSION(4, 9, 0))
> + || !ndctl_bus_has_nfit(bus))
> dimm_commands &= ~((1 << ND_CMD_SMART)
> | (1 << ND_CMD_SMART_THRESHOLD));
I was going to say split test/libndctl.c into a generic test and and
nfit-specific test/libndctl-nfit.c, but if the vast majority of the
tests runs with just small tweaks like this I think I can do that
refactoring later.
_______________________________________________
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-le...@lists.01.org
