linux-nvdimm.lists.01.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2] powerpc/papr_scm: Reduce error severity if nvdimm stats inaccessible
@ 2021-05-05 19:16 Vaibhav Jain
  2021-05-06  4:58 ` Ira Weiny
  0 siblings, 1 reply; 3+ messages in thread
From: Vaibhav Jain @ 2021-05-05 19:16 UTC (permalink / raw)
  To: linuxppc-dev, linux-nvdimm
  Cc: Vaibhav Jain, Aneesh Kumar K . V, Michael Ellerman

Currently drc_pmem_qeury_stats() generates a dev_err in case
"Enable Performance Information Collection" feature is disabled from
HMC or performance stats are not available for an nvdimm. The error is
of the form below:

papr_scm ibm,persistent-memory:ibm,pmemory@44104001: Failed to query
	 performance stats, Err:-10

This error message confuses users as it implies a possible problem
with the nvdimm even though its due to a disabled/unavailable
feature. We fix this by explicitly handling the H_AUTHORITY and
H_UNSUPPORTED errors from the H_SCM_PERFORMANCE_STATS hcall.

In case of H_AUTHORITY error an info message is logged instead of an
error, saying that "Permission denied while accessing performance
stats". Also '-EACCES' error is return instead of -EPERM.

In case of H_UNSUPPORTED error we return a -EPERM error back from
drc_pmem_query_stats() indicating that performance stats-query
operation is not supported on this nvdimm.

Fixes: 2d02bf835e57('powerpc/papr_scm: Fetch nvdimm performance stats from PHYP')
Signed-off-by: Vaibhav Jain <vaibhav@linux.ibm.com>
---
Changelog

v2:
* Updated the message logged in case of H_AUTHORITY error [ Ira ]
* Switched from dev_warn to dev_info in case of H_AUTHORITY error.
* Instead of -EPERM return -EACCESS for H_AUTHORITY error.
* Added explicit handling of H_UNSUPPORTED error.
---
 arch/powerpc/platforms/pseries/papr_scm.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/arch/powerpc/platforms/pseries/papr_scm.c b/arch/powerpc/platforms/pseries/papr_scm.c
index ef26fe40efb0..12f1513f0fca 100644
--- a/arch/powerpc/platforms/pseries/papr_scm.c
+++ b/arch/powerpc/platforms/pseries/papr_scm.c
@@ -310,6 +310,13 @@ static ssize_t drc_pmem_query_stats(struct papr_scm_priv *p,
 		dev_err(&p->pdev->dev,
 			"Unknown performance stats, Err:0x%016lX\n", ret[0]);
 		return -ENOENT;
+	} else if (rc == H_AUTHORITY) {
+		dev_info(&p->pdev->dev,
+			 "Permission denied while accessing performance stats");
+		return -EACCES;
+	} else if (rc == H_UNSUPPORTED) {
+		dev_dbg(&p->pdev->dev, "Performance stats unsupported\n");
+		return -EPERM;
 	} else if (rc != H_SUCCESS) {
 		dev_err(&p->pdev->dev,
 			"Failed to query performance stats, Err:%lld\n", rc);
-- 
2.31.1
_______________________________________________
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-leave@lists.01.org

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH v2] powerpc/papr_scm: Reduce error severity if nvdimm stats inaccessible
  2021-05-05 19:16 [PATCH v2] powerpc/papr_scm: Reduce error severity if nvdimm stats inaccessible Vaibhav Jain
@ 2021-05-06  4:58 ` Ira Weiny
  2021-05-07 12:18   ` Vaibhav Jain
  0 siblings, 1 reply; 3+ messages in thread
From: Ira Weiny @ 2021-05-06  4:58 UTC (permalink / raw)
  To: Vaibhav Jain
  Cc: linuxppc-dev, linux-nvdimm, Aneesh Kumar K . V, Michael Ellerman

On Thu, May 06, 2021 at 12:46:06AM +0530, Vaibhav Jain wrote:
> Currently drc_pmem_qeury_stats() generates a dev_err in case
> "Enable Performance Information Collection" feature is disabled from
> HMC or performance stats are not available for an nvdimm. The error is
> of the form below:
> 
> papr_scm ibm,persistent-memory:ibm,pmemory@44104001: Failed to query
> 	 performance stats, Err:-10
> 
> This error message confuses users as it implies a possible problem
> with the nvdimm even though its due to a disabled/unavailable
> feature. We fix this by explicitly handling the H_AUTHORITY and
> H_UNSUPPORTED errors from the H_SCM_PERFORMANCE_STATS hcall.
> 
> In case of H_AUTHORITY error an info message is logged instead of an
> error, saying that "Permission denied while accessing performance
> stats". Also '-EACCES' error is return instead of -EPERM.

I thought you clarified before that this was a permission issue.  So why change
the error to EACCES?

> 
> In case of H_UNSUPPORTED error we return a -EPERM error back from
> drc_pmem_query_stats() indicating that performance stats-query
> operation is not supported on this nvdimm.

EPERM seems wrong here too...  ENOTSUP?

Ira
_______________________________________________
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-leave@lists.01.org

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH v2] powerpc/papr_scm: Reduce error severity if nvdimm stats inaccessible
  2021-05-06  4:58 ` Ira Weiny
@ 2021-05-07 12:18   ` Vaibhav Jain
  0 siblings, 0 replies; 3+ messages in thread
From: Vaibhav Jain @ 2021-05-07 12:18 UTC (permalink / raw)
  To: Ira Weiny
  Cc: linuxppc-dev, linux-nvdimm,
	Aneesh Kumar K . V  <aneesh.kumar@linux.ibm.com>,
	Michael Ellerman

Hi Ira,

Thanks for looking into this patch

Ira Weiny <ira.weiny@intel.com> writes:

> On Thu, May 06, 2021 at 12:46:06AM +0530, Vaibhav Jain wrote:
>> Currently drc_pmem_qeury_stats() generates a dev_err in case
>> "Enable Performance Information Collection" feature is disabled from
>> HMC or performance stats are not available for an nvdimm. The error is
>> of the form below:
>> 
>> papr_scm ibm,persistent-memory:ibm,pmemory@44104001: Failed to query
>> 	 performance stats, Err:-10
>> 
>> This error message confuses users as it implies a possible problem
>> with the nvdimm even though its due to a disabled/unavailable
>> feature. We fix this by explicitly handling the H_AUTHORITY and
>> H_UNSUPPORTED errors from the H_SCM_PERFORMANCE_STATS hcall.
>> 
>> In case of H_AUTHORITY error an info message is logged instead of an
>> error, saying that "Permission denied while accessing performance
>> stats". Also '-EACCES' error is return instead of -EPERM.
>
> I thought you clarified before that this was a permission issue.  So why change
> the error to EACCES?
>
EACCESS("Permission Denied") felt like a more accurate error code for
this case than EPERM("Operation not permitted"). So switched the usage
of EPERM error code to handle the case if this hcall is not supported
for an nvdimm.

>> 
>> In case of H_UNSUPPORTED error we return a -EPERM error back from
>> drc_pmem_query_stats() indicating that performance stats-query
>> operation is not supported on this nvdimm.
>
> EPERM seems wrong here too...  ENOTSUP?
Yes, will change it to EOPNOTSUPP in v3.

>
> Ira
> _______________________________________________
> Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
> To unsubscribe send an email to linux-nvdimm-leave@lists.01.org

-- 
Cheers
~ Vaibhav
_______________________________________________
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-leave@lists.01.org

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2021-05-07 12:18 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-05-05 19:16 [PATCH v2] powerpc/papr_scm: Reduce error severity if nvdimm stats inaccessible Vaibhav Jain
2021-05-06  4:58 ` Ira Weiny
2021-05-07 12:18   ` Vaibhav Jain

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).