diff mbox series

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

Message ID 20201222042516.2984348-3-santosh@fossix.org (mailing list archive)
State New
Headers show
Series [ndctl,1/5] libndctl: test enablement for non-nfit devices | expand

Commit Message

Santosh Sivaraj Dec. 22, 2020, 4:25 a.m. UTC
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(-)

Comments

Dan Williams Jan. 28, 2021, 1:16 a.m. UTC | #1
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.
Santosh Sivaraj Feb. 25, 2021, 5:53 a.m. UTC | #2
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
diff mbox series

Patch

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);
 }
 
 /**