Re: [PATCH v3] powerpc/papr_scm: Make 'perf_stats' invisible if perf-stats unavailable

2021-06-26 Thread Michael Ellerman
On Thu, 13 May 2021 14:53:49 +0530, Vaibhav Jain wrote:
> In case performance stats for an nvdimm are not available, reading the
> 'perf_stats' sysfs file returns an -ENOENT error. A better approach is
> to make the 'perf_stats' file entirely invisible to indicate that
> performance stats for an nvdimm are unavailable.
> 
> So this patch updates 'papr_nd_attribute_group' to add a 'is_visible'
> callback implemented as newly introduced 'papr_nd_attribute_visible()'
> that returns an appropriate mode in case performance stats aren't
> supported in a given nvdimm.
> 
> [...]

Applied to powerpc/next.

[1/1] powerpc/papr_scm: Make 'perf_stats' invisible if perf-stats unavailable
  https://git.kernel.org/powerpc/c/ed78f56e1271f108e8af61baeba383dcd77adbec

cheers


[PATCH v3] powerpc/papr_scm: Make 'perf_stats' invisible if perf-stats unavailable

2021-05-13 Thread Vaibhav Jain
In case performance stats for an nvdimm are not available, reading the
'perf_stats' sysfs file returns an -ENOENT error. A better approach is
to make the 'perf_stats' file entirely invisible to indicate that
performance stats for an nvdimm are unavailable.

So this patch updates 'papr_nd_attribute_group' to add a 'is_visible'
callback implemented as newly introduced 'papr_nd_attribute_visible()'
that returns an appropriate mode in case performance stats aren't
supported in a given nvdimm.

Also the initialization of 'papr_scm_priv.stat_buffer_len' is moved
from papr_scm_nvdimm_init() to papr_scm_probe() so that it value is
available when 'papr_nd_attribute_visible()' is called during nvdimm
initialization.

Even though 'perf_stats' attribute is available since v5.9, there are
no known user-space tools/scripts that are dependent on presence of its
sysfs file. Hence I dont expect any user-space breakage with this
patch.

Fixes: 2d02bf835e57('powerpc/papr_scm: Fetch nvdimm performance stats from 
PHYP')
Reviewed-by: Dan Williams 
Signed-off-by: Vaibhav Jain 
---
Changelog:

v3:
* Minor spell corrections [ Dan Williams ]
* Switched to kobj_to_dev() helper in papr_nd_attribute_visible() [
  Dan Williams ]
* Updated ABI/../sysfs-bus-papr-pmem to indicate that the attribute is
  only available for devices that support performance stats.
* Updated patch description.

v2:
* Removed a redundant dev_info() from pap_scm_nvdimm_init() [ Aneesh ]
* Marked papr_nd_attribute_visible() as static which also fixed the
  build warning reported by kernel build robot
---
 Documentation/ABI/testing/sysfs-bus-papr-pmem |  8 +++--
 arch/powerpc/platforms/pseries/papr_scm.c | 35 +--
 2 files changed, 29 insertions(+), 14 deletions(-)

diff --git a/Documentation/ABI/testing/sysfs-bus-papr-pmem 
b/Documentation/ABI/testing/sysfs-bus-papr-pmem
index 8316c33862a0..0aa02bf2bde5 100644
--- a/Documentation/ABI/testing/sysfs-bus-papr-pmem
+++ b/Documentation/ABI/testing/sysfs-bus-papr-pmem
@@ -39,9 +39,11 @@ KernelVersion:   v5.9
 Contact:   linuxppc-dev , 
linux-nvd...@lists.01.org,
 Description:
(RO) Report various performance stats related to papr-scm NVDIMM
-   device.  Each stat is reported on a new line with each line
-   composed of a stat-identifier followed by it value. Below are
-   currently known dimm performance stats which are reported:
+   device. This attribute is only available for NVDIMM devices
+   that support reporting NVDIMM performance stats. Each stat is
+   reported on a new line with each line composed of a
+   stat-identifier followed by it value. Below are currently known
+   dimm performance stats which are reported:
 
* "CtlResCt" : Controller Reset Count
* "CtlResTm" : Controller Reset Elapsed Time
diff --git a/arch/powerpc/platforms/pseries/papr_scm.c 
b/arch/powerpc/platforms/pseries/papr_scm.c
index e2b69cc3beaf..e8577e7e49ab 100644
--- a/arch/powerpc/platforms/pseries/papr_scm.c
+++ b/arch/powerpc/platforms/pseries/papr_scm.c
@@ -907,6 +907,20 @@ static ssize_t flags_show(struct device *dev,
 }
 DEVICE_ATTR_RO(flags);
 
+static umode_t papr_nd_attribute_visible(struct kobject *kobj,
+struct attribute *attr, int n)
+{
+   struct device *dev = kobj_to_dev(kobj);
+   struct nvdimm *nvdimm = to_nvdimm(dev);
+   struct papr_scm_priv *p = nvdimm_provider_data(nvdimm);
+
+   /* For if perf-stats not available remove perf_stats sysfs */
+   if (attr == _attr_perf_stats.attr && p->stat_buffer_len == 0)
+   return 0;
+
+   return attr->mode;
+}
+
 /* papr_scm specific dimm attributes */
 static struct attribute *papr_nd_attributes[] = {
_attr_flags.attr,
@@ -916,6 +930,7 @@ static struct attribute *papr_nd_attributes[] = {
 
 static struct attribute_group papr_nd_attribute_group = {
.name = "papr",
+   .is_visible = papr_nd_attribute_visible,
.attrs = papr_nd_attributes,
 };
 
@@ -931,7 +946,6 @@ static int papr_scm_nvdimm_init(struct papr_scm_priv *p)
struct nd_region_desc ndr_desc;
unsigned long dimm_flags;
int target_nid, online_nid;
-   ssize_t stat_size;
 
p->bus_desc.ndctl = papr_scm_ndctl;
p->bus_desc.module = THIS_MODULE;
@@ -1016,16 +1030,6 @@ static int papr_scm_nvdimm_init(struct papr_scm_priv *p)
list_add_tail(>region_list, _nd_regions);
mutex_unlock(_ndr_lock);
 
-   /* Try retriving the stat buffer and see if its supported */
-   stat_size = drc_pmem_query_stats(p, NULL, 0);
-   if (stat_size > 0) {
-   p->stat_buffer_len = stat_size;
-   dev_dbg(>pdev->dev, "Max perf-stat size %lu-bytes\n",
-   p->stat_buffer_len);
-   } else {
-   dev_info(>pdev->dev, "Dimm performance stats unavailable\n");
-