[PATCH 0/6] Deduplicate string exposure in sysfs

[PATCH 0/6] Deduplicate string exposure in sysfs

[Lukas Wunner]

Introduce a generic ->show() callback to expose a string as a device
attribute in sysfs.  Deduplicate various identical callbacks across
the tree.

Result:  Minus 216 LoC, minus 1576 bytes vmlinux size (x86_64 allyesconfig).

This is a byproduct of my upcoming PCI device authentication v2 patches.


Lukas Wunner (6):
  driver core: Add device_show_string() helper for sysfs attributes
  hwmon: Use device_show_string() helper for sysfs attributes
  IB/qib: Use device_show_string() helper for sysfs attributes
  perf: Use device_show_string() helper for sysfs attributes
  platform/x86: Use device_show_string() helper for sysfs attributes
  scsi: Use device_show_string() helper for sysfs attributes

 arch/powerpc/perf/hv-24x7.c              | 10 ----
 arch/x86/events/intel/core.c             | 13 ++---
 drivers/base/core.c                      |  9 ++++
 drivers/hwmon/i5k_amb.c                  | 15 ++----
 drivers/hwmon/ibmpex.c                   | 14 ++----
 drivers/infiniband/hw/qib/qib.h          |  1 -
 drivers/infiniband/hw/qib/qib_driver.c   |  6 ---
 drivers/infiniband/hw/qib/qib_sysfs.c    | 10 +---
 drivers/perf/alibaba_uncore_drw_pmu.c    | 12 +----
 drivers/perf/arm-cci.c                   | 12 +----
 drivers/perf/arm-ccn.c                   | 11 +----
 drivers/perf/arm_cspmu/arm_cspmu.c       | 10 ----
 drivers/perf/arm_cspmu/arm_cspmu.h       |  7 +--
 drivers/perf/arm_dsu_pmu.c               | 11 +----
 drivers/perf/cxl_pmu.c                   | 13 +----
 drivers/perf/hisilicon/hisi_pcie_pmu.c   | 13 +----
 drivers/perf/hisilicon/hisi_uncore_pmu.c | 14 ------
 drivers/perf/hisilicon/hisi_uncore_pmu.h |  4 +-
 drivers/perf/hisilicon/hns3_pmu.c        | 12 +----
 drivers/perf/qcom_l3_pmu.c               | 11 +----
 drivers/perf/xgene_pmu.c                 | 11 +----
 drivers/platform/x86/asus-wmi.c          | 62 ++++++------------------
 drivers/platform/x86/thinkpad_acpi.c     | 10 +---
 drivers/platform/x86/toshiba_acpi.c      |  9 +---
 drivers/scsi/bfa/bfad_attr.c             | 28 +++--------
 drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c | 11 +----
 drivers/scsi/mvsas/mv_init.c             | 10 +---
 drivers/scsi/qla2xxx/qla_attr.c          | 11 +----
 drivers/scsi/smartpqi/smartpqi_init.c    | 11 ++---
 include/linux/device.h                   | 15 ++++++
 30 files changed, 85 insertions(+), 301 deletions(-)

-- 
2.43.0

[PATCH 1/6] driver core: Add device_show_string() helper for sysfs attributes

[Lukas Wunner]

For drivers wishing to expose an unsigned long, int or bool at a static
memory location in sysfs, the driver core provides ready-made helpers
such as device_show_ulong() to be used as ->show() callback.

Some drivers need to expose a string and so far they all provide their
own ->show() implementation.  arch/powerpc/perf/hv-24x7.c went so far
as to create a device_show_string() helper but kept it private.

Make it public for reuse by other drivers.  The pattern seems to be
sufficiently frequent to merit a public helper.

Add a DEVICE_STRING_ATTR_RO() macro in line with the existing
DEVICE_ULONG_ATTR() and similar macros to ease declaration of string
attributes.

Signed-off-by: Lukas Wunner <lukas@wunner.de>
---
 arch/powerpc/perf/hv-24x7.c | 10 ----------
 drivers/base/core.c         |  9 +++++++++
 include/linux/device.h      | 15 +++++++++++++++
 3 files changed, 24 insertions(+), 10 deletions(-)

diff --git a/arch/powerpc/perf/hv-24x7.c b/arch/powerpc/perf/hv-24x7.c
index 057ec2e3451d..d400fa391c27 100644
--- a/arch/powerpc/perf/hv-24x7.c
+++ b/arch/powerpc/perf/hv-24x7.c
@@ -425,16 +425,6 @@ static char *memdup_to_str(char *maybe_str, int max_len, gfp_t gfp)
 	return kasprintf(gfp, "%.*s", max_len, maybe_str);
 }
 
-static ssize_t device_show_string(struct device *dev,
-		struct device_attribute *attr, char *buf)
-{
-	struct dev_ext_attribute *d;
-
-	d = container_of(attr, struct dev_ext_attribute, attr);
-
-	return sprintf(buf, "%s\n", (char *)d->var);
-}
-
 static ssize_t cpumask_show(struct device *dev,
 			    struct device_attribute *attr, char *buf)
 {
diff --git a/drivers/base/core.c b/drivers/base/core.c
index 78dfa74ee18b..190d4a39c6a8 100644
--- a/drivers/base/core.c
+++ b/drivers/base/core.c
@@ -2523,6 +2523,15 @@ ssize_t device_show_bool(struct device *dev, struct device_attribute *attr,
 }
 EXPORT_SYMBOL_GPL(device_show_bool);
 
+ssize_t device_show_string(struct device *dev,
+			   struct device_attribute *attr, char *buf)
+{
+	struct dev_ext_attribute *ea = to_ext_attr(attr);
+
+	return sysfs_emit(buf, "%s\n", (char *)ea->var);
+}
+EXPORT_SYMBOL_GPL(device_show_string);
+
 /**
  * device_release - free device structure.
  * @kobj: device's kobject.
diff --git a/include/linux/device.h b/include/linux/device.h
index c515ba5756e4..63ac65db3ecb 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -132,6 +132,8 @@ ssize_t device_show_bool(struct device *dev, struct device_attribute *attr,
 			char *buf);
 ssize_t device_store_bool(struct device *dev, struct device_attribute *attr,
 			 const char *buf, size_t count);
+ssize_t device_show_string(struct device *dev, struct device_attribute *attr,
+			   char *buf);
 
 /**
  * DEVICE_ATTR - Define a device attribute.
@@ -251,6 +253,19 @@ ssize_t device_store_bool(struct device *dev, struct device_attribute *attr,
 	struct dev_ext_attribute dev_attr_##_name = \
 		{ __ATTR(_name, _mode, device_show_bool, device_store_bool), &(_var) }
 
+/**
+ * DEVICE_STRING_ATTR_RO - Define a device attribute backed by a r/o string.
+ * @_name: Attribute name.
+ * @_mode: File mode.
+ * @_var: Identifier of string.
+ *
+ * Like DEVICE_ULONG_ATTR(), but @_var is a string. Because the length of the
+ * string allocation is unknown, the attribute must be read-only.
+ */
+#define DEVICE_STRING_ATTR_RO(_name, _mode, _var) \
+	struct dev_ext_attribute dev_attr_##_name = \
+		{ __ATTR(_name, (_mode) & ~0222, device_show_string, NULL), (_var) }
+
 #define DEVICE_ATTR_IGNORE_LOCKDEP(_name, _mode, _show, _store) \
 	struct device_attribute dev_attr_##_name =		\
 		__ATTR_IGNORE_LOCKDEP(_name, _mode, _show, _store)
-- 
2.43.0

Re: [PATCH 1/6] driver core: Add device_show_string() helper for sysfs attributes

[Michael Ellerman]

Lukas Wunner <lukas@wunner.de> writes:
> For drivers wishing to expose an unsigned long, int or bool at a static
> memory location in sysfs, the driver core provides ready-made helpers
> such as device_show_ulong() to be used as ->show() callback.
>
> Some drivers need to expose a string and so far they all provide their
> own ->show() implementation.  arch/powerpc/perf/hv-24x7.c went so far
> as to create a device_show_string() helper but kept it private.
>
> Make it public for reuse by other drivers.  The pattern seems to be
> sufficiently frequent to merit a public helper.
>
> Add a DEVICE_STRING_ATTR_RO() macro in line with the existing
> DEVICE_ULONG_ATTR() and similar macros to ease declaration of string
> attributes.
>
> Signed-off-by: Lukas Wunner <lukas@wunner.de>
> ---
>  arch/powerpc/perf/hv-24x7.c | 10 ----------
>  drivers/base/core.c         |  9 +++++++++
>  include/linux/device.h      | 15 +++++++++++++++
>  3 files changed, 24 insertions(+), 10 deletions(-)

Acked-by: Michael Ellerman <mpe@ellerman.id.au> (powerpc)

cheers

Re: [PATCH 0/6] Deduplicate string exposure in sysfs

[Lukas Wunner]

Dear Greg,

On Sat, Apr 20, 2024 at 10:00:00PM +0200, Lukas Wunner wrote:
> Introduce a generic ->show() callback to expose a string as a device
> attribute in sysfs.  Deduplicate various identical callbacks across
> the tree.
> 
> Result:  Minus 216 LoC, minus 1576 bytes vmlinux size (x86_64 allyesconfig).
> 
> This is a byproduct of my upcoming PCI device authentication v2 patches.
> 
> 
> Lukas Wunner (6):
>   driver core: Add device_show_string() helper for sysfs attributes
>   hwmon: Use device_show_string() helper for sysfs attributes
>   IB/qib: Use device_show_string() helper for sysfs attributes
>   perf: Use device_show_string() helper for sysfs attributes
>   platform/x86: Use device_show_string() helper for sysfs attributes
>   scsi: Use device_show_string() helper for sysfs attributes

This series hasn't been applied to driver-core-next AFAICS and the
merge window is drawing closer.

So far only patches 1, 2 and 5 have been ack'ed by the respective
subsystem maintainers.  If the missing acks are the reason it hasn't
been applied, would it be possibe to apply only 1, 2 and 5?

I would then resubmit the other ones individually to the subsystem
maintainers in the next cycle.

Thanks!

Lukas

Re: [PATCH 0/6] Deduplicate string exposure in sysfs

[Greg Kroah-Hartman]

On Sat, May 04, 2024 at 04:31:42PM +0200, Lukas Wunner wrote:
> Dear Greg,
> 
> On Sat, Apr 20, 2024 at 10:00:00PM +0200, Lukas Wunner wrote:
> > Introduce a generic ->show() callback to expose a string as a device
> > attribute in sysfs.  Deduplicate various identical callbacks across
> > the tree.
> > 
> > Result:  Minus 216 LoC, minus 1576 bytes vmlinux size (x86_64 allyesconfig).
> > 
> > This is a byproduct of my upcoming PCI device authentication v2 patches.
> > 
> > 
> > Lukas Wunner (6):
> >   driver core: Add device_show_string() helper for sysfs attributes
> >   hwmon: Use device_show_string() helper for sysfs attributes
> >   IB/qib: Use device_show_string() helper for sysfs attributes
> >   perf: Use device_show_string() helper for sysfs attributes
> >   platform/x86: Use device_show_string() helper for sysfs attributes
> >   scsi: Use device_show_string() helper for sysfs attributes
> 
> This series hasn't been applied to driver-core-next AFAICS and the
> merge window is drawing closer.
> 
> So far only patches 1, 2 and 5 have been ack'ed by the respective
> subsystem maintainers.  If the missing acks are the reason it hasn't
> been applied, would it be possibe to apply only 1, 2 and 5?
> 
> I would then resubmit the other ones individually to the subsystem
> maintainers in the next cycle.

I'll just pick it up now, thanks!

greg k-h