| Message ID | 20210505191708.51939-1-vaibhav@linux.ibm.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | powerpc/papr_scm: Make 'perf_stats' invisible if perf-stats unavailable | expand |
| Related | show |
| Context | Check | Description |
|---|---|---|
| snowpatch_ozlabs/apply_patch | success | Successfully applied on branch powerpc/merge (7619d98e5041d5c25aba5428704dba6121237a9a) |
| snowpatch_ozlabs/build-ppc64le | success | Build succeeded |
| snowpatch_ozlabs/build-ppc64be | success | Build succeeded |
| snowpatch_ozlabs/build-ppc64e | success | Build succeeded |
| snowpatch_ozlabs/build-pmac32 | success | Build succeeded |
| snowpatch_ozlabs/checkpatch | success | total: 0 errors, 0 warnings, 0 checks, 73 lines checked |
| snowpatch_ozlabs/needsstable | warning | Please consider tagging this patch for stable! |
Hi Vaibhav,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on powerpc/next]
[also build test WARNING on v5.12 next-20210505]
[cannot apply to scottwood/next mpe/next]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Vaibhav-Jain/powerpc-papr_scm-Make-perf_stats-invisible-if-perf-stats-unavailable/20210506-031853
base: https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git next
config: powerpc-allyesconfig (attached as .config)
compiler: powerpc64-linux-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/0day-ci/linux/commit/8e7ce04ec36dea95fad178585b697a9c5b5c259d
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Vaibhav-Jain/powerpc-papr_scm-Make-perf_stats-invisible-if-perf-stats-unavailable/20210506-031853
git checkout 8e7ce04ec36dea95fad178585b697a9c5b5c259d
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross W=1 ARCH=powerpc
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All warnings (new ones prefixed by >>):
>> arch/powerpc/platforms/pseries/papr_scm.c:903:9: warning: no previous prototype for 'papr_nd_attribute_visible' [-Wmissing-prototypes]
903 | umode_t papr_nd_attribute_visible(struct kobject *kobj, struct attribute *attr,
| ^~~~~~~~~~~~~~~~~~~~~~~~~
vim +/papr_nd_attribute_visible +903 arch/powerpc/platforms/pseries/papr_scm.c
902
> 903 umode_t papr_nd_attribute_visible(struct kobject *kobj, struct attribute *attr,
904 int n)
905 {
906 struct device *dev = container_of(kobj, typeof(*dev), kobj);
907 struct nvdimm *nvdimm = to_nvdimm(dev);
908 struct papr_scm_priv *p = nvdimm_provider_data(nvdimm);
909
910 /* For if perf-stats not available remove perf_stats sysfs */
911 if (attr == &dev_attr_perf_stats.attr && p->stat_buffer_len == 0)
912 return 0;
913
914 return attr->mode;
915 }
916
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
Vaibhav Jain <vaibhav@linux.ibm.com> writes: > 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. > > Fixes: 2d02bf835e57('powerpc/papr_scm: Fetch nvdimm performance stats from PHYP') > Signed-off-by: Vaibhav Jain <vaibhav@linux.ibm.com> > --- > arch/powerpc/platforms/pseries/papr_scm.c | 37 ++++++++++++++++------- > 1 file changed, 26 insertions(+), 11 deletions(-) > > diff --git a/arch/powerpc/platforms/pseries/papr_scm.c b/arch/powerpc/platforms/pseries/papr_scm.c > index 12f1513f0fca..90f0af8fefe8 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); > > +umode_t papr_nd_attribute_visible(struct kobject *kobj, struct attribute *attr, > + int n) > +{ > + struct device *dev = container_of(kobj, typeof(*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 == &dev_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[] = { > &dev_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(&p->region_list, &papr_nd_regions); > mutex_unlock(&papr_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(&p->pdev->dev, "Max perf-stat size %lu-bytes\n", > - p->stat_buffer_len); > - } else { > - dev_info(&p->pdev->dev, "Dimm performance stats unavailable\n"); > - } > - > return 0; > > err: nvdimm_bus_unregister(p->bus); > @@ -1102,6 +1106,7 @@ static int papr_scm_probe(struct platform_device *pdev) > u64 blocks, block_size; > struct papr_scm_priv *p; > const char *uuid_str; > + ssize_t stat_size; > u64 uuid[2]; > int rc; > > @@ -1179,6 +1184,16 @@ static int papr_scm_probe(struct platform_device *pdev) > p->res.name = pdev->name; > p->res.flags = IORESOURCE_MEM; > > + /* 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(&p->pdev->dev, "Max perf-stat size %lu-bytes\n", > + p->stat_buffer_len); > + } else { > + dev_info(&p->pdev->dev, "Dimm performance stats unavailable\n"); > + } With this patch https://lore.kernel.org/linuxppc-dev/20210505191606.51666-1-vaibhav@linux.ibm.com We are adding details of whyy performance stat query hcall failed. Do we need to print again here? Are we being more verbose here? -aneesh
"Aneesh Kumar K.V" <aneesh.kumar@linux.ibm.com> writes: > Vaibhav Jain <vaibhav@linux.ibm.com> writes: > >> 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. >> >> Fixes: 2d02bf835e57('powerpc/papr_scm: Fetch nvdimm performance stats from PHYP') >> Signed-off-by: Vaibhav Jain <vaibhav@linux.ibm.com> >> --- >> arch/powerpc/platforms/pseries/papr_scm.c | 37 ++++++++++++++++------- >> 1 file changed, 26 insertions(+), 11 deletions(-) >> >> diff --git a/arch/powerpc/platforms/pseries/papr_scm.c b/arch/powerpc/platforms/pseries/papr_scm.c >> index 12f1513f0fca..90f0af8fefe8 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); >> >> +umode_t papr_nd_attribute_visible(struct kobject *kobj, struct attribute *attr, >> + int n) >> +{ >> + struct device *dev = container_of(kobj, typeof(*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 == &dev_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[] = { >> &dev_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(&p->region_list, &papr_nd_regions); >> mutex_unlock(&papr_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(&p->pdev->dev, "Max perf-stat size %lu-bytes\n", >> - p->stat_buffer_len); >> - } else { >> - dev_info(&p->pdev->dev, "Dimm performance stats unavailable\n"); >> - } >> - >> return 0; >> >> err: nvdimm_bus_unregister(p->bus); >> @@ -1102,6 +1106,7 @@ static int papr_scm_probe(struct platform_device *pdev) >> u64 blocks, block_size; >> struct papr_scm_priv *p; >> const char *uuid_str; >> + ssize_t stat_size; >> u64 uuid[2]; >> int rc; >> >> @@ -1179,6 +1184,16 @@ static int papr_scm_probe(struct platform_device *pdev) >> p->res.name = pdev->name; >> p->res.flags = IORESOURCE_MEM; >> >> + /* 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(&p->pdev->dev, "Max perf-stat size %lu-bytes\n", >> + p->stat_buffer_len); >> + } else { >> + dev_info(&p->pdev->dev, "Dimm performance stats unavailable\n"); >> + } > > With this patch https://lore.kernel.org/linuxppc-dev/20210505191606.51666-1-vaibhav@linux.ibm.com > We are adding details of whyy performance stat query hcall failed. Do we > need to print again here? Are we being more verbose here? > Yes agree this looks more verbose with the other patch you mentioned. I have sent out a v2 of this patch with this dev_info removed. > -aneesh > _______________________________________________ > 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 12f1513f0fca..90f0af8fefe8 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); +umode_t papr_nd_attribute_visible(struct kobject *kobj, struct attribute *attr, + int n) +{ + struct device *dev = container_of(kobj, typeof(*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 == &dev_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[] = { &dev_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(&p->region_list, &papr_nd_regions); mutex_unlock(&papr_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(&p->pdev->dev, "Max perf-stat size %lu-bytes\n", - p->stat_buffer_len); - } else { - dev_info(&p->pdev->dev, "Dimm performance stats unavailable\n"); - } - return 0; err: nvdimm_bus_unregister(p->bus); @@ -1102,6 +1106,7 @@ static int papr_scm_probe(struct platform_device *pdev) u64 blocks, block_size; struct papr_scm_priv *p; const char *uuid_str; + ssize_t stat_size; u64 uuid[2]; int rc; @@ -1179,6 +1184,16 @@ static int papr_scm_probe(struct platform_device *pdev) p->res.name = pdev->name; p->res.flags = IORESOURCE_MEM; + /* 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(&p->pdev->dev, "Max perf-stat size %lu-bytes\n", + p->stat_buffer_len); + } else { + dev_info(&p->pdev->dev, "Dimm performance stats unavailable\n"); + } + rc = papr_scm_nvdimm_init(p); if (rc) goto err2;
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. Fixes: 2d02bf835e57('powerpc/papr_scm: Fetch nvdimm performance stats from PHYP') Signed-off-by: Vaibhav Jain <vaibhav@linux.ibm.com> --- arch/powerpc/platforms/pseries/papr_scm.c | 37 ++++++++++++++++------- 1 file changed, 26 insertions(+), 11 deletions(-)