Re: [ndctl 3/5] papr: Add support to parse save_fail flag for dimm

Santosh Sivaraj
2021-02-25T05:53:17Z

Dan Williams <dan.j.williams@intel.com> writes:

> On Mon, Dec 21, 2020 at 8:26 PM Santosh Sivaraj <santosh@fossix.org> wrote:
>>
>> This will help in getting the dimm fail tests to run on papr family too.
>> Also add nvdimm_test compatibility string for recognizing the test module.
>>
>> Signed-off-by: Santosh Sivaraj <santosh@fossix.org>
>> ---
>> ndctl/lib/libndctl.c | 5 ++++-
>> 1 file changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/ndctl/lib/libndctl.c b/ndctl/lib/libndctl.c
>> index 5f09628..3fb3aed 100644
>> --- a/ndctl/lib/libndctl.c
>> +++ b/ndctl/lib/libndctl.c
>> @@ -815,6 +815,8 @@ 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)
>> @@ -1044,7 +1046,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);
>
> A bit unfortunate to leak test details into the production path,
> especially when nvdimm_test is meant to be generic. It seems what you
> really want is a generic way to determine if dimm supports the common
> error state flags, right? I'd add an api for that and say yes for nfit
> and papr.

Sorry to get to this late.

Though I would like it to be generic, there is some advantage of code reuse if
it's part of the PAPR_FAMILY.

So the idea in the above code is to determine which flag parsing code to call,
either parse_nfit_mem_flags or parse_papr_flags. Or I could write a new function
if the bus is of neither type. But that would again want me to duplicate error
inject related code, and may be other paths too.

Thanks,
Santosh