Re: [PATCH 7/8] powerpc/papr_scm: Re-implement 'papr_flags' using 'nd_papr_scm_dimm_health_stat'

["Aneesh Kumar K.V" <aneesh.kumar@linux.ibm.com>]

Vaibhav Jain <vaibhav@linux.ibm.com> writes:

> Previous commit [1] introduced 'struct nd_papr_scm_dimm_health_stat' for
> communicating health status of an nvdimm to libndctl. This struct
> however can also be used to cache the nvdimm health information in
> 'struct papr_scm_priv' instead of two '__be64' values. Benefit of this
> re-factoring will be apparent when support for libndctl being able to
> request nvdimm health stats is implemented where we can simply memcpy
> this struct over to the user-space provided payload envelope.
>
> Hence this patch introduces a new member 'struct papr_scm_priv.health'
> that caches the health information of a dimm. This member is populated
> inside drc_pmem_query_health() which checks for the various bit flags
> returned from H_SCM_HEALTH and sets them in this struct. We also
> re-factor 'papr_flags' sysfs attribute show function papr_flags_show()
> to use the flags in 'struct papr_scm_priv.health' to return
> appropriate status strings pertaining to dimm health.
>
> This patch shouldn't introduce any behavioral change.

This patch is undoing what is doing in PATCH 2. If you reorder things,
we could drop either this or PATCH2.?

>
> Signed-off-by: Vaibhav Jain <vaibhav@linux.ibm.com>
> ---
>  arch/powerpc/platforms/pseries/papr_scm.c | 61 ++++++++++++++++-------
>  1 file changed, 44 insertions(+), 17 deletions(-)
>
> diff --git a/arch/powerpc/platforms/pseries/papr_scm.c b/arch/powerpc/platforms/pseries/papr_scm.c
> index d5eea2f38fa6..29f38246c59f 100644
> --- a/arch/powerpc/platforms/pseries/papr_scm.c
> +++ b/arch/powerpc/platforms/pseries/papr_scm.c
> @@ -47,8 +47,7 @@ struct papr_scm_priv {
>  	struct mutex dimm_mutex;
>  
>  	/* Health information for the dimm */
> -	__be64 health_bitmap;
> -	__be64 health_bitmap_valid;
> +	struct nd_papr_scm_dimm_health_stat health;
>  
>  	/* length of the stat buffer as expected by phyp */
>  	size_t len_stat_buffer;
> @@ -205,6 +204,7 @@ static int drc_pmem_query_health(struct papr_scm_priv *p)
>  {
>  	unsigned long ret[PLPAR_HCALL_BUFSIZE];
>  	int64_t rc;
> +	__be64 health;
>  
>  	rc = plpar_hcall(H_SCM_HEALTH, ret, p->drc_index);
>  	if (rc != H_SUCCESS) {
> @@ -219,13 +219,41 @@ static int drc_pmem_query_health(struct papr_scm_priv *p)
>  		return rc;
>  
>  	/* Store the retrieved health information in dimm platform data */
> -	p->health_bitmap = ret[0];
> -	p->health_bitmap_valid = ret[1];
> +	health = ret[0] & ret[1];
>  
>  	dev_dbg(&p->pdev->dev,
>  		"Queried dimm health info. Bitmap:0x%016llx Mask:0x%016llx\n",
> -		be64_to_cpu(p->health_bitmap),
> -		be64_to_cpu(p->health_bitmap_valid));
> +		be64_to_cpu(ret[0]),
> +		be64_to_cpu(ret[1]));
> +
> +	memset(&p->health, 0, sizeof(p->health));
> +
> +	/* Check for various masks in bitmap and set the buffer */
> +	if (health & PAPR_SCM_DIMM_UNARMED_MASK)
> +		p->health.dimm_unarmed = true;
> +
> +	if (health & PAPR_SCM_DIMM_BAD_SHUTDOWN_MASK)
> +		p->health.dimm_bad_shutdown = true;
> +
> +	if (health & PAPR_SCM_DIMM_BAD_RESTORE_MASK)
> +		p->health.dimm_bad_restore = true;
> +
> +	if (health & PAPR_SCM_DIMM_ENCRYPTED)
> +		p->health.dimm_encrypted = true;
> +
> +	if (health & PAPR_SCM_DIMM_SCRUBBED_AND_LOCKED) {
> +		p->health.dimm_locked = true;
> +		p->health.dimm_scrubbed = true;
> +	}
> +
> +	if (health & PAPR_SCM_DIMM_HEALTH_UNHEALTHY)
> +		p->health.dimm_health = DSM_PAPR_SCM_DIMM_UNHEALTHY;
> +
> +	if (health & PAPR_SCM_DIMM_HEALTH_CRITICAL)
> +		p->health.dimm_health = DSM_PAPR_SCM_DIMM_CRITICAL;
> +
> +	if (health & PAPR_SCM_DIMM_HEALTH_FATAL)
> +		p->health.dimm_health = DSM_PAPR_SCM_DIMM_FATAL;
>  
>  	mutex_unlock(&p->dimm_mutex);
>  	return 0;
> @@ -513,7 +541,6 @@ static ssize_t papr_flags_show(struct device *dev,
>  {
>  	struct nvdimm *dimm = to_nvdimm(dev);
>  	struct papr_scm_priv *p = nvdimm_provider_data(dimm);
> -	__be64 health;
>  	int rc;
>  
>  	rc = drc_pmem_query_health(p);
> @@ -525,26 +552,26 @@ static ssize_t papr_flags_show(struct device *dev,
>  	if (rc)
>  		return rc;
>  
> -	health = p->health_bitmap & p->health_bitmap_valid;
> -
> -	/* Check for various masks in bitmap and set the buffer */
> -	if (health & PAPR_SCM_DIMM_UNARMED_MASK)
> +	if (p->health.dimm_unarmed)
>  		rc += sprintf(buf, "not_armed ");
>  
> -	if (health & PAPR_SCM_DIMM_BAD_SHUTDOWN_MASK)
> +	if (p->health.dimm_bad_shutdown)
>  		rc += sprintf(buf + rc, "save_fail ");
>  
> -	if (health & PAPR_SCM_DIMM_BAD_RESTORE_MASK)
> +	if (p->health.dimm_bad_restore)
>  		rc += sprintf(buf + rc, "restore_fail ");
>  
> -	if (health & PAPR_SCM_DIMM_ENCRYPTED)
> +	if (p->health.dimm_encrypted)
>  		rc += sprintf(buf + rc, "encrypted ");
>  
> -	if (health & PAPR_SCM_DIMM_SMART_EVENT_MASK)
> +	if (p->health.dimm_health)
>  		rc += sprintf(buf + rc, "smart_notify ");
>  
> -	if (health & PAPR_SCM_DIMM_SCRUBBED_AND_LOCKED)
> -		rc += sprintf(buf + rc, "scrubbed locked ");
> +	if (p->health.dimm_scrubbed)
> +		rc += sprintf(buf + rc, "scrubbed ");
> +
> +	if (p->health.dimm_locked)
> +		rc += sprintf(buf + rc, "locked ");
>  
>  	if (rc > 0)
>  		rc += sprintf(buf + rc, "\n");
> -- 
> 2.24.1