[PATCH 0/4] sysfs: constify bin_attribute argument of sysfs_bin_attr_simple_read()

[PATCH 0/4] sysfs: constify bin_attribute argument of sysfs_bin_attr_simple_read()

[Thomas Weißschuh]

Most users use this function through the BIN_ATTR_SIMPLE* macros,
they can handle the switch transparently.

This series is meant to be merged through the driver core tree.

Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
---
Thomas Weißschuh (4):
      sysfs: constify bin_attribute argument of sysfs_bin_attr_simple_read()
      platform/x86: wmi-bmof: Switch to sysfs_bin_attr_simple_read()
      btf: Switch vmlinux BTF attribute to sysfs_bin_attr_simple_read()
      btf: Switch module BTF attribute to sysfs_bin_attr_simple_read()

 arch/powerpc/platforms/powernv/opal.c |  2 +-
 drivers/platform/x86/wmi-bmof.c       | 12 ++----------
 fs/sysfs/file.c                       |  2 +-
 include/linux/sysfs.h                 |  4 ++--
 kernel/bpf/btf.c                      | 15 ++-------------
 kernel/bpf/sysfs_btf.c                | 12 ++----------
 kernel/module/sysfs.c                 |  2 +-
 7 files changed, 11 insertions(+), 38 deletions(-)
---
base-commit: feffde684ac29a3b7aec82d2df850fbdbdee55e4
change-id: 20241122-sysfs-const-bin_attr-simple-7c0ddb2fcf12

Best regards,
-- 
Thomas Weißschuh <linux@weissschuh.net>

[PATCH 1/4] sysfs: constify bin_attribute argument of sysfs_bin_attr_simple_read()

[Thomas Weißschuh]

Most users use this function through the BIN_ATTR_SIMPLE* macros,
they can handle the switch transparently.
Also adapt the two non-macro users in the same change.

Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
---
 arch/powerpc/platforms/powernv/opal.c | 2 +-
 fs/sysfs/file.c                       | 2 +-
 include/linux/sysfs.h                 | 4 ++--
 kernel/module/sysfs.c                 | 2 +-
 4 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/arch/powerpc/platforms/powernv/opal.c b/arch/powerpc/platforms/powernv/opal.c
index 5d0f35bb917ebced8c741cd3af2c511949a1d2ef..013637e2b2a8e6a4ec6b93a520f8d5d9d3245467 100644
--- a/arch/powerpc/platforms/powernv/opal.c
+++ b/arch/powerpc/platforms/powernv/opal.c
@@ -818,7 +818,7 @@ static int opal_add_one_export(struct kobject *parent, const char *export_name,
 	sysfs_bin_attr_init(attr);
 	attr->attr.name = name;
 	attr->attr.mode = 0400;
-	attr->read = sysfs_bin_attr_simple_read;
+	attr->read_new = sysfs_bin_attr_simple_read;
 	attr->private = __va(vals[0]);
 	attr->size = vals[1];
 
diff --git a/fs/sysfs/file.c b/fs/sysfs/file.c
index 785408861c01c89fc84c787848243a13c1338367..6931308876c4ac3b4c19878d5e1158ad8fe4f16f 100644
--- a/fs/sysfs/file.c
+++ b/fs/sysfs/file.c
@@ -817,7 +817,7 @@ EXPORT_SYMBOL_GPL(sysfs_emit_at);
  * Returns number of bytes written to @buf.
  */
 ssize_t sysfs_bin_attr_simple_read(struct file *file, struct kobject *kobj,
-				   struct bin_attribute *attr, char *buf,
+				   const struct bin_attribute *attr, char *buf,
 				   loff_t off, size_t count)
 {
 	memcpy(buf, attr->private + off, count);
diff --git a/include/linux/sysfs.h b/include/linux/sysfs.h
index 0f2fcd244523f050c5286f19d4fe1846506f9214..2205561159afdb57d0a250bb0439b28c01d9010e 100644
--- a/include/linux/sysfs.h
+++ b/include/linux/sysfs.h
@@ -511,7 +511,7 @@ __printf(3, 4)
 int sysfs_emit_at(char *buf, int at, const char *fmt, ...);
 
 ssize_t sysfs_bin_attr_simple_read(struct file *file, struct kobject *kobj,
-				   struct bin_attribute *attr, char *buf,
+				   const struct bin_attribute *attr, char *buf,
 				   loff_t off, size_t count);
 
 #else /* CONFIG_SYSFS */
@@ -774,7 +774,7 @@ static inline int sysfs_emit_at(char *buf, int at, const char *fmt, ...)
 
 static inline ssize_t sysfs_bin_attr_simple_read(struct file *file,
 						 struct kobject *kobj,
-						 struct bin_attribute *attr,
+						 const struct bin_attribute *attr,
 						 char *buf, loff_t off,
 						 size_t count)
 {
diff --git a/kernel/module/sysfs.c b/kernel/module/sysfs.c
index 456358e1fdc43e6b5b24f383bbefa37812971174..254017b58b645d4afcf6876d29bcc2e2113a8dc4 100644
--- a/kernel/module/sysfs.c
+++ b/kernel/module/sysfs.c
@@ -196,7 +196,7 @@ static int add_notes_attrs(struct module *mod, const struct load_info *info)
 			nattr->attr.mode = 0444;
 			nattr->size = info->sechdrs[i].sh_size;
 			nattr->private = (void *)info->sechdrs[i].sh_addr;
-			nattr->read = sysfs_bin_attr_simple_read;
+			nattr->read_new = sysfs_bin_attr_simple_read;
 			++nattr;
 		}
 		++loaded;

-- 
2.47.1

[PATCH 2/4] platform/x86: wmi-bmof: Switch to sysfs_bin_attr_simple_read()

[Thomas Weißschuh]

The generic function from the sysfs core can replace the custom one.

Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
---
 drivers/platform/x86/wmi-bmof.c | 12 ++----------
 1 file changed, 2 insertions(+), 10 deletions(-)

diff --git a/drivers/platform/x86/wmi-bmof.c b/drivers/platform/x86/wmi-bmof.c
index df6f0ae6e6c7904f97c125297a21166f56d0b1f0..e6c217d70086a2896dc70cf8ac1c27dafb501a95 100644
--- a/drivers/platform/x86/wmi-bmof.c
+++ b/drivers/platform/x86/wmi-bmof.c
@@ -25,15 +25,6 @@ struct bmof_priv {
 	struct bin_attribute bmof_bin_attr;
 };
 
-static ssize_t read_bmof(struct file *filp, struct kobject *kobj, struct bin_attribute *attr,
-			 char *buf, loff_t off, size_t count)
-{
-	struct bmof_priv *priv = container_of(attr, struct bmof_priv, bmof_bin_attr);
-
-	return memory_read_from_buffer(buf, count, &off, priv->bmofdata->buffer.pointer,
-				       priv->bmofdata->buffer.length);
-}
-
 static int wmi_bmof_probe(struct wmi_device *wdev, const void *context)
 {
 	struct bmof_priv *priv;
@@ -60,7 +51,8 @@ static int wmi_bmof_probe(struct wmi_device *wdev, const void *context)
 	sysfs_bin_attr_init(&priv->bmof_bin_attr);
 	priv->bmof_bin_attr.attr.name = "bmof";
 	priv->bmof_bin_attr.attr.mode = 0400;
-	priv->bmof_bin_attr.read = read_bmof;
+	priv->bmof_bin_attr.read_new = sysfs_bin_attr_simple_read;
+	priv->bmof_bin_attr.private = priv->bmofdata->buffer.pointer;
 	priv->bmof_bin_attr.size = priv->bmofdata->buffer.length;
 
 	ret = device_create_bin_file(&wdev->dev, &priv->bmof_bin_attr);

-- 
2.47.1

[PATCH 3/4] btf: Switch vmlinux BTF attribute to sysfs_bin_attr_simple_read()

[Thomas Weißschuh]

The generic function from the sysfs core can replace the custom one.

Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>

---
This is a replacement for [0], as Alexei was not happy about BIN_ATTR_SIMPLE_RO()

[0] https://lore.kernel.org/lkml/20241122-sysfs-const-bin_attr-bpf-v1-1-823aea399b53@weissschuh.net/
---
 kernel/bpf/sysfs_btf.c | 12 ++----------
 1 file changed, 2 insertions(+), 10 deletions(-)

diff --git a/kernel/bpf/sysfs_btf.c b/kernel/bpf/sysfs_btf.c
index fedb54c94cdb830a4890d33677dcc5a6e236c13f..81d6cf90584a7157929c50f62a5c6862e7a3d081 100644
--- a/kernel/bpf/sysfs_btf.c
+++ b/kernel/bpf/sysfs_btf.c
@@ -12,24 +12,16 @@
 extern char __start_BTF[];
 extern char __stop_BTF[];
 
-static ssize_t
-btf_vmlinux_read(struct file *file, struct kobject *kobj,
-		 struct bin_attribute *bin_attr,
-		 char *buf, loff_t off, size_t len)
-{
-	memcpy(buf, __start_BTF + off, len);
-	return len;
-}
-
 static struct bin_attribute bin_attr_btf_vmlinux __ro_after_init = {
 	.attr = { .name = "vmlinux", .mode = 0444, },
-	.read = btf_vmlinux_read,
+	.read_new = sysfs_bin_attr_simple_read,
 };
 
 struct kobject *btf_kobj;
 
 static int __init btf_vmlinux_init(void)
 {
+	bin_attr_btf_vmlinux.private = __start_BTF;
 	bin_attr_btf_vmlinux.size = __stop_BTF - __start_BTF;
 
 	if (bin_attr_btf_vmlinux.size == 0)

-- 
2.47.1

[PATCH 4/4] btf: Switch module BTF attribute to sysfs_bin_attr_simple_read()

[Thomas Weißschuh]

The generic function from the sysfs core can replace the custom one.

Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
---
 kernel/bpf/btf.c | 15 ++-------------
 1 file changed, 2 insertions(+), 13 deletions(-)

diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
index e7a59e6462a9331d0acb17a88a4ebf641509c050..69caa86ae6085dce17e95107c4497d2d8cf81544 100644
--- a/kernel/bpf/btf.c
+++ b/kernel/bpf/btf.c
@@ -7870,17 +7870,6 @@ struct btf_module {
 static LIST_HEAD(btf_modules);
 static DEFINE_MUTEX(btf_module_mutex);
 
-static ssize_t
-btf_module_read(struct file *file, struct kobject *kobj,
-		struct bin_attribute *bin_attr,
-		char *buf, loff_t off, size_t len)
-{
-	const struct btf *btf = bin_attr->private;
-
-	memcpy(buf, btf->data + off, len);
-	return len;
-}
-
 static void purge_cand_cache(struct btf *btf);
 
 static int btf_module_notify(struct notifier_block *nb, unsigned long op,
@@ -7941,8 +7930,8 @@ static int btf_module_notify(struct notifier_block *nb, unsigned long op,
 			attr->attr.name = btf->name;
 			attr->attr.mode = 0444;
 			attr->size = btf->data_size;
-			attr->private = btf;
-			attr->read = btf_module_read;
+			attr->private = btf->data;
+			attr->read_new = sysfs_bin_attr_simple_read;
 
 			err = sysfs_create_bin_file(btf_kobj, attr);
 			if (err) {

-- 
2.47.1

Re: [PATCH 3/4] btf: Switch vmlinux BTF attribute to sysfs_bin_attr_simple_read()

[Andrii Nakryiko]

On Thu, Dec 5, 2024 at 9:35 AM Thomas Weißschuh <linux@weissschuh.net> wrote:
>
> The generic function from the sysfs core can replace the custom one.
>
> Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
>
> ---
> This is a replacement for [0], as Alexei was not happy about BIN_ATTR_SIMPLE_RO()
>
> [0] https://lore.kernel.org/lkml/20241122-sysfs-const-bin_attr-bpf-v1-1-823aea399b53@weissschuh.net/
> ---
>  kernel/bpf/sysfs_btf.c | 12 ++----------
>  1 file changed, 2 insertions(+), 10 deletions(-)
>

Acked-by: Andrii Nakryiko <andrii@kernel.org>

> diff --git a/kernel/bpf/sysfs_btf.c b/kernel/bpf/sysfs_btf.c
> index fedb54c94cdb830a4890d33677dcc5a6e236c13f..81d6cf90584a7157929c50f62a5c6862e7a3d081 100644
> --- a/kernel/bpf/sysfs_btf.c
> +++ b/kernel/bpf/sysfs_btf.c
> @@ -12,24 +12,16 @@
>  extern char __start_BTF[];
>  extern char __stop_BTF[];
>
> -static ssize_t
> -btf_vmlinux_read(struct file *file, struct kobject *kobj,
> -                struct bin_attribute *bin_attr,
> -                char *buf, loff_t off, size_t len)
> -{
> -       memcpy(buf, __start_BTF + off, len);
> -       return len;
> -}
> -
>  static struct bin_attribute bin_attr_btf_vmlinux __ro_after_init = {
>         .attr = { .name = "vmlinux", .mode = 0444, },
> -       .read = btf_vmlinux_read,
> +       .read_new = sysfs_bin_attr_simple_read,
>  };
>
>  struct kobject *btf_kobj;
>
>  static int __init btf_vmlinux_init(void)
>  {
> +       bin_attr_btf_vmlinux.private = __start_BTF;
>         bin_attr_btf_vmlinux.size = __stop_BTF - __start_BTF;
>
>         if (bin_attr_btf_vmlinux.size == 0)
>
> --
> 2.47.1
>

Re: [PATCH 4/4] btf: Switch module BTF attribute to sysfs_bin_attr_simple_read()

[Andrii Nakryiko]

On Thu, Dec 5, 2024 at 9:35 AM Thomas Weißschuh <linux@weissschuh.net> wrote:
>
> The generic function from the sysfs core can replace the custom one.
>
> Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
> ---
>  kernel/bpf/btf.c | 15 ++-------------
>  1 file changed, 2 insertions(+), 13 deletions(-)
>

LGTM

Acked-by: Andrii Nakryiko <andrii@kernel.org>

> diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
> index e7a59e6462a9331d0acb17a88a4ebf641509c050..69caa86ae6085dce17e95107c4497d2d8cf81544 100644
> --- a/kernel/bpf/btf.c
> +++ b/kernel/bpf/btf.c
> @@ -7870,17 +7870,6 @@ struct btf_module {
>  static LIST_HEAD(btf_modules);
>  static DEFINE_MUTEX(btf_module_mutex);
>
> -static ssize_t
> -btf_module_read(struct file *file, struct kobject *kobj,
> -               struct bin_attribute *bin_attr,
> -               char *buf, loff_t off, size_t len)
> -{
> -       const struct btf *btf = bin_attr->private;
> -
> -       memcpy(buf, btf->data + off, len);
> -       return len;
> -}
> -
>  static void purge_cand_cache(struct btf *btf);
>
>  static int btf_module_notify(struct notifier_block *nb, unsigned long op,
> @@ -7941,8 +7930,8 @@ static int btf_module_notify(struct notifier_block *nb, unsigned long op,
>                         attr->attr.name = btf->name;
>                         attr->attr.mode = 0444;
>                         attr->size = btf->data_size;
> -                       attr->private = btf;
> -                       attr->read = btf_module_read;
> +                       attr->private = btf->data;
> +                       attr->read_new = sysfs_bin_attr_simple_read;
>
>                         err = sysfs_create_bin_file(btf_kobj, attr);
>                         if (err) {
>
> --
> 2.47.1
>

Re: [PATCH 2/4] platform/x86: wmi-bmof: Switch to sysfs_bin_attr_simple_read()

[Armin Wolf]

Am 05.12.24 um 18:35 schrieb Thomas Weißschuh:

> The generic function from the sysfs core can replace the custom one.

Sorry for taking quite a bit to respond, i totally overlooked this patch.

This patch is superseded by a patch of mine: https://lore.kernel.org/platform-driver-x86/20241206215650.2977-1-W_Armin@gmx.de/

This reworks the binary attribute handling inside the driver to use the new .bin_size() callback. This allows the
driver to have a static binary attribute which does not need a memory allocation.

Because i think we cannot use sysfs_bin_attr_simple_read() anymore. So maybe you can just drop this patch?

Thanks,
Armin Wolf

> Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
> ---
>   drivers/platform/x86/wmi-bmof.c | 12 ++----------
>   1 file changed, 2 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/platform/x86/wmi-bmof.c b/drivers/platform/x86/wmi-bmof.c
> index df6f0ae6e6c7904f97c125297a21166f56d0b1f0..e6c217d70086a2896dc70cf8ac1c27dafb501a95 100644
> --- a/drivers/platform/x86/wmi-bmof.c
> +++ b/drivers/platform/x86/wmi-bmof.c
> @@ -25,15 +25,6 @@ struct bmof_priv {
>   	struct bin_attribute bmof_bin_attr;
>   };
>
> -static ssize_t read_bmof(struct file *filp, struct kobject *kobj, struct bin_attribute *attr,
> -			 char *buf, loff_t off, size_t count)
> -{
> -	struct bmof_priv *priv = container_of(attr, struct bmof_priv, bmof_bin_attr);
> -
> -	return memory_read_from_buffer(buf, count, &off, priv->bmofdata->buffer.pointer,
> -				       priv->bmofdata->buffer.length);
> -}
> -
>   static int wmi_bmof_probe(struct wmi_device *wdev, const void *context)
>   {
>   	struct bmof_priv *priv;
> @@ -60,7 +51,8 @@ static int wmi_bmof_probe(struct wmi_device *wdev, const void *context)
>   	sysfs_bin_attr_init(&priv->bmof_bin_attr);
>   	priv->bmof_bin_attr.attr.name = "bmof";
>   	priv->bmof_bin_attr.attr.mode = 0400;
> -	priv->bmof_bin_attr.read = read_bmof;
> +	priv->bmof_bin_attr.read_new = sysfs_bin_attr_simple_read;
> +	priv->bmof_bin_attr.private = priv->bmofdata->buffer.pointer;
>   	priv->bmof_bin_attr.size = priv->bmofdata->buffer.length;
>
>   	ret = device_create_bin_file(&wdev->dev, &priv->bmof_bin_attr);
>

Re: [PATCH 2/4] platform/x86: wmi-bmof: Switch to sysfs_bin_attr_simple_read()

[Thomas Weißschuh]

Hi Armin,

On 2024-12-13 01:21:37+0100, Armin Wolf wrote:
> Am 05.12.24 um 18:35 schrieb Thomas Weißschuh:
> 
> > The generic function from the sysfs core can replace the custom one.
> 
> Sorry for taking quite a bit to respond, i totally overlooked this patch.
> 
> This patch is superseded by a patch of mine: https://lore.kernel.org/platform-driver-x86/20241206215650.2977-1-W_Armin@gmx.de/
> 
> This reworks the binary attribute handling inside the driver to use the new .bin_size() callback. This allows the
> driver to have a static binary attribute which does not need a memory allocation.
> 
> Because i think we cannot use sysfs_bin_attr_simple_read() anymore. So maybe you can just drop this patch?

Works for me, thanks!

Thomas

> > Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
> > ---
> >   drivers/platform/x86/wmi-bmof.c | 12 ++----------
> >   1 file changed, 2 insertions(+), 10 deletions(-)
> > 
> > diff --git a/drivers/platform/x86/wmi-bmof.c b/drivers/platform/x86/wmi-bmof.c
> > index df6f0ae6e6c7904f97c125297a21166f56d0b1f0..e6c217d70086a2896dc70cf8ac1c27dafb501a95 100644
> > --- a/drivers/platform/x86/wmi-bmof.c
> > +++ b/drivers/platform/x86/wmi-bmof.c
> > @@ -25,15 +25,6 @@ struct bmof_priv {
> >   	struct bin_attribute bmof_bin_attr;
> >   };
> > 
> > -static ssize_t read_bmof(struct file *filp, struct kobject *kobj, struct bin_attribute *attr,
> > -			 char *buf, loff_t off, size_t count)
> > -{
> > -	struct bmof_priv *priv = container_of(attr, struct bmof_priv, bmof_bin_attr);
> > -
> > -	return memory_read_from_buffer(buf, count, &off, priv->bmofdata->buffer.pointer,
> > -				       priv->bmofdata->buffer.length);
> > -}
> > -
> >   static int wmi_bmof_probe(struct wmi_device *wdev, const void *context)
> >   {
> >   	struct bmof_priv *priv;
> > @@ -60,7 +51,8 @@ static int wmi_bmof_probe(struct wmi_device *wdev, const void *context)
> >   	sysfs_bin_attr_init(&priv->bmof_bin_attr);
> >   	priv->bmof_bin_attr.attr.name = "bmof";
> >   	priv->bmof_bin_attr.attr.mode = 0400;
> > -	priv->bmof_bin_attr.read = read_bmof;
> > +	priv->bmof_bin_attr.read_new = sysfs_bin_attr_simple_read;
> > +	priv->bmof_bin_attr.private = priv->bmofdata->buffer.pointer;
> >   	priv->bmof_bin_attr.size = priv->bmofdata->buffer.length;
> > 
> >   	ret = device_create_bin_file(&wdev->dev, &priv->bmof_bin_attr);
> >