diff mbox series

[REPOST,v2,1/2] libndctl, intel: Indicate supported smart-inject types

Message ID 163491461724.1641479.6370717053054036222.stgit@lep8c.aus.stglabs.ibm.com (mailing list archive)
State Superseded
Headers show
Series papr: Implement initial support for injecting smart errors | expand

Commit Message

Shivaprasad G Bhat Oct. 22, 2021, 2:57 p.m. UTC 
From: Vaibhav Jain <vaibhav@linux.ibm.com>

Presently the inject-smart code assumes support for injecting all
smart-errors namely media-temperature, controller-temperature,
spares-remaining, fatal-health and unsafe-shutdown. This assumption
may break in case of other non-Intel NVDIMM types namely PAPR NVDIMMs
which presently only have support for injecting unsafe-shutdown and
fatal health events.

Trying to inject-smart errors on PAPR NVDIMMs causes problems as
smart_inject() prematurely exits when trying to inject
media-temperature smart-error errors out.

To fix this issue the patch proposes extending the definition of
dimm_op 'smart_inject_supported' to return bitmap of flags indicating
the type of smart-error injections supported by an NVDIMM. These types
are indicated by the newly introduced defines ND_SMART_INJECT_* . A
dimm-ops provide can return an bitmap composed of these flags back
from its implementation of 'smart_inject_supported' to indicate to
dimm_inject_smart() which type of smart-error injection it
supports. In case of an error the dimm-op is still expected to return
a negative error code back to the caller.

The patch updates intel_dimm_smart_inject_supported() to return a
bitmap composed of all ND_SMART_INJECT_* flags to indicate support for
all smart-error types.

Finally the patch also updates smart_inject() to test for specific
ND_START_INJECT_* flags before sending a smart-inject command via
dimm-provider.

Signed-off-by: Vaibhav Jain <vaibhav@linux.ibm.com>
---
Changelog:

Since v1:
Link: https://lore.kernel.org/nvdimm/20210712173132.1205192-2-vaibhav@linux.ibm.com
* Minor update to patch description

 ndctl/inject-smart.c |   33 ++++++++++++++++++++++++++-------
 ndctl/lib/intel.c    |    7 ++++++-
 ndctl/libndctl.h     |    8 ++++++++
 3 files changed, 40 insertions(+), 8 deletions(-)

Comments

Ira Weiny Oct. 28, 2021, 4:45 a.m. UTC | #1 
On Fri, Oct 22, 2021 at 09:57:07AM -0500, Shivaprasad G Bhat wrote:
> From: Vaibhav Jain <vaibhav@linux.ibm.com>
> 

[snip]

>  
> -static int smart_inject(struct ndctl_dimm *dimm)
> +static int smart_inject(struct ndctl_dimm *dimm, unsigned int inject_types)
>  {
>  	const char *name = ndctl_dimm_get_devname(dimm);
>  	struct ndctl_cmd *si_cmd = NULL;
>  	int rc = -EOPNOTSUPP;
>  
> -	send_inject_val(media_temperature)
> -	send_inject_val(ctrl_temperature)
> -	send_inject_val(spares)
> -	send_inject_bool(fatal)
> -	send_inject_bool(unsafe_shutdown)
> +	if (inject_types & ND_SMART_INJECT_MEDIA_TEMPERATURE)
> +		send_inject_val(media_temperature);
>  
> +	if (inject_types & ND_SMART_INJECT_CTRL_TEMPERATURE)
> +		send_inject_val(ctrl_temperature);
> +
> +	if (inject_types & ND_SMART_INJECT_SPARES_REMAINING)
> +		send_inject_val(spares);
> +
> +	if (inject_types & ND_SMART_INJECT_HEALTH_STATE)
> +		send_inject_bool(fatal);
> +
> +	if (inject_types & ND_SMART_INJECT_UNCLEAN_SHUTDOWN)
> +		send_inject_bool(unsafe_shutdown);
>  out:
>  	ndctl_cmd_unref(si_cmd);
>  	return rc;
> @@ -415,8 +423,10 @@ static int dimm_inject_smart(struct ndctl_dimm *dimm)
>  	struct json_object *jhealth;
>  	struct json_object *jdimms;
>  	struct json_object *jdimm;
> +	unsigned int supported_types;
>  	int rc;
>  
> +	/* Get supported smart injection types */

NIT: this comment is probably unnecessary.

>  	rc = ndctl_dimm_smart_inject_supported(dimm);
>  	switch (rc) {
>  	case -ENOTTY:
> @@ -431,6 +441,15 @@ static int dimm_inject_smart(struct ndctl_dimm *dimm)
>  		error("%s: smart injection not supported by either platform firmware or the kernel.",
>  			ndctl_dimm_get_devname(dimm));
>  		return rc;
> +	default:
> +		if (rc < 0) {
> +			error("%s: Unknown error %d while checking for smart injection support",
> +			      ndctl_dimm_get_devname(dimm), rc);
> +			return rc;
> +		}
> +		/* Assigning to an unsigned type since rc < 0 */

Comment wrong?

> +		supported_types = rc;
> +		break;
>  	}
>  
>  	if (sctx.op_mask & (1 << OP_SET)) {

[snip]

Other than the comment it looks fine.

Reviewed-by: Ira Weiny <ira.weiny@intel.com>
Ira Weiny Nov. 3, 2021, 4:45 p.m. UTC | #2 
On Wed, Nov 03, 2021 at 02:33:37PM +0530, Shivaprasad G Bhat wrote:
>    Thanks for the comments Ira. Replies inline..
> 
>    On 10/28/21 10:15, Ira Weiny wrote:
> 
>  On Fri, Oct 22, 2021 at 09:57:07AM -0500, Shivaprasad G Bhat wrote:
> 
>  From: Vaibhav Jain [1]<vaibhav@linux.ibm.com>
> 
> 
>  [snip]
> 
>  < snip>
> 
>          ndctl_cmd_unref(si_cmd);
>          return rc;
>  @@ -415,8 +423,10 @@ static int dimm_inject_smart(struct ndctl_dimm *dimm)
>          struct json_object *jhealth;
>          struct json_object *jdimms;
>          struct json_object *jdimm;
>  +   unsigned int supported_types;
>          int rc;
> 
>  +   /* Get supported smart injection types */
> 
>  NIT: this comment is probably unnecessary.
> 
>    The code changes definition of the return value hence wanted to
> 
>    clarify that, it indicates a set of flags instead of a boolean.
> 
>    From that pov, the comment helps avoiding the confusion.
> 
>    Please let me know If you think the otherway.

But that change is documented in the change log.  When reading the code after
the change the function is pretty self documenting.

And if you want to document the call, that should be done with the definition
in the *.c file.  Not at the call sites.

Ira

> 
>          rc = ndctl_dimm_smart_inject_supported(dimm);
>          switch (rc) {
>          case -ENOTTY:
>  @@ -431,6 +441,15 @@ static int dimm_inject_smart(struct ndctl_dimm *dimm)
>                  error("%s: smart injection not supported by either platform firmware or the kernel.",
>                          ndctl_dimm_get_devname(dimm));
>                  return rc;
>  +   default:
>  +           if (rc < 0) {
>  +                   error("%s: Unknown error %d while checking for smart injection support",
>  +                         ndctl_dimm_get_devname(dimm), rc);
>  +                   return rc;
>  +           }
>  +           /* Assigning to an unsigned type since rc < 0 */
> 
>  Comment wrong?
> 
>    Will get rid of it.
> 
>  +           supported_types = rc;
>  +           break;
>          }
> 
>          if (sctx.op_mask & (1 << OP_SET)) {
> 
>  [snip]
> 
>  Other than the comment it looks fine.
> 
>  Reviewed-by: Ira Weiny [2]<ira.weiny@intel.com>
> 
> 
>    Thanks!
> 
>    -Shivaprasad
> 
> References
> 
>    Visible links
>    1. mailto:vaibhav@linux.ibm.com
>    2. mailto:ira.weiny@intel.com
diff mbox series

Patch

diff --git a/ndctl/inject-smart.c b/ndctl/inject-smart.c
index 9077bca2..ef0620f5 100644
--- a/ndctl/inject-smart.c
+++ b/ndctl/inject-smart.c
@@ -393,18 +393,26 @@  out:
 	} \
 }
 
-static int smart_inject(struct ndctl_dimm *dimm)
+static int smart_inject(struct ndctl_dimm *dimm, unsigned int inject_types)
 {
 	const char *name = ndctl_dimm_get_devname(dimm);
 	struct ndctl_cmd *si_cmd = NULL;
 	int rc = -EOPNOTSUPP;
 
-	send_inject_val(media_temperature)
-	send_inject_val(ctrl_temperature)
-	send_inject_val(spares)
-	send_inject_bool(fatal)
-	send_inject_bool(unsafe_shutdown)
+	if (inject_types & ND_SMART_INJECT_MEDIA_TEMPERATURE)
+		send_inject_val(media_temperature);
 
+	if (inject_types & ND_SMART_INJECT_CTRL_TEMPERATURE)
+		send_inject_val(ctrl_temperature);
+
+	if (inject_types & ND_SMART_INJECT_SPARES_REMAINING)
+		send_inject_val(spares);
+
+	if (inject_types & ND_SMART_INJECT_HEALTH_STATE)
+		send_inject_bool(fatal);
+
+	if (inject_types & ND_SMART_INJECT_UNCLEAN_SHUTDOWN)
+		send_inject_bool(unsafe_shutdown);
 out:
 	ndctl_cmd_unref(si_cmd);
 	return rc;
@@ -415,8 +423,10 @@  static int dimm_inject_smart(struct ndctl_dimm *dimm)
 	struct json_object *jhealth;
 	struct json_object *jdimms;
 	struct json_object *jdimm;
+	unsigned int supported_types;
 	int rc;
 
+	/* Get supported smart injection types */
 	rc = ndctl_dimm_smart_inject_supported(dimm);
 	switch (rc) {
 	case -ENOTTY:
@@ -431,6 +441,15 @@  static int dimm_inject_smart(struct ndctl_dimm *dimm)
 		error("%s: smart injection not supported by either platform firmware or the kernel.",
 			ndctl_dimm_get_devname(dimm));
 		return rc;
+	default:
+		if (rc < 0) {
+			error("%s: Unknown error %d while checking for smart injection support",
+			      ndctl_dimm_get_devname(dimm), rc);
+			return rc;
+		}
+		/* Assigning to an unsigned type since rc < 0 */
+		supported_types = rc;
+		break;
 	}
 
 	if (sctx.op_mask & (1 << OP_SET)) {
@@ -439,7 +458,7 @@  static int dimm_inject_smart(struct ndctl_dimm *dimm)
 			goto out;
 	}
 	if (sctx.op_mask & (1 << OP_INJECT)) {
-		rc = smart_inject(dimm);
+		rc = smart_inject(dimm, supported_types);
 		if (rc)
 			goto out;
 	}
diff --git a/ndctl/lib/intel.c b/ndctl/lib/intel.c
index a3df26e6..13148545 100644
--- a/ndctl/lib/intel.c
+++ b/ndctl/lib/intel.c
@@ -455,7 +455,12 @@  static int intel_dimm_smart_inject_supported(struct ndctl_dimm *dimm)
 		return -EIO;
 	}
 
-	return 0;
+	/* Indicate all smart injection types are supported */
+	return ND_SMART_INJECT_SPARES_REMAINING |
+		ND_SMART_INJECT_MEDIA_TEMPERATURE |
+		ND_SMART_INJECT_CTRL_TEMPERATURE |
+		ND_SMART_INJECT_HEALTH_STATE |
+		ND_SMART_INJECT_UNCLEAN_SHUTDOWN;
 }
 
 static const char *intel_cmd_desc(int fn)
diff --git a/ndctl/libndctl.h b/ndctl/libndctl.h
index cdadd5fd..54539ac7 100644
--- a/ndctl/libndctl.h
+++ b/ndctl/libndctl.h
@@ -69,6 +69,13 @@  extern "C" {
 #define ND_EVENT_HEALTH_STATE		(1 << 3)
 #define ND_EVENT_UNCLEAN_SHUTDOWN	(1 << 4)
 
+/* Flags indicating support for various smart injection types */
+#define ND_SMART_INJECT_SPARES_REMAINING	(1 << 0)
+#define ND_SMART_INJECT_MEDIA_TEMPERATURE	(1 << 1)
+#define ND_SMART_INJECT_CTRL_TEMPERATURE	(1 << 2)
+#define ND_SMART_INJECT_HEALTH_STATE		(1 << 3)
+#define ND_SMART_INJECT_UNCLEAN_SHUTDOWN	(1 << 4)
+
 size_t ndctl_min_namespace_size(void);
 size_t ndctl_sizeof_namespace_index(void);
 size_t ndctl_sizeof_namespace_label(void);
@@ -310,6 +317,7 @@  int ndctl_cmd_smart_inject_spares(struct ndctl_cmd *cmd, bool enable,
 		unsigned int spares);
 int ndctl_cmd_smart_inject_fatal(struct ndctl_cmd *cmd, bool enable);
 int ndctl_cmd_smart_inject_unsafe_shutdown(struct ndctl_cmd *cmd, bool enable);
+/* Returns a bitmap of ND_SMART_INJECT_* supported */
 int ndctl_dimm_smart_inject_supported(struct ndctl_dimm *dimm);
 
 struct ndctl_cmd *ndctl_dimm_cmd_new_vendor_specific(struct ndctl_dimm *dimm,