| Message ID | 20210505191606.51666-1-vaibhav@linux.ibm.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v2] powerpc/papr_scm: Reduce error severity if nvdimm stats inaccessible | expand |
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
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
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);
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(+)