* [PATCH 0/2] Deduplicate bin_attribute simple read() callbacks @ 2024-04-06 13:52 Lukas Wunner 2024-04-06 13:52 ` [PATCH 1/2] sysfs: Add sysfs_bin_attr_simple_read() helper Lukas Wunner ` (4 more replies) 0 siblings, 5 replies; 10+ messages in thread From: Lukas Wunner @ 2024-04-06 13:52 UTC (permalink / raw) To: Greg Kroah-Hartman, Rafael J. Wysocki, linux-kernel Cc: Michael Ellerman, linuxppc-dev, linux-acpi, Jean Delvare, Ard Biesheuvel, linux-efi, Zhenyu Wang, Zhi Wang, intel-gvt-dev, Daniel Lezcano, linux-pm, Luis Chamberlain, linux-modules For my upcoming PCI device authentication v2 patches, I have the need to expose a simple buffer in virtual memory as a bin_attribute. It turns out we've duplicated the ->read() callback for such simple buffers a fair number of times across the tree. So instead of reinventing the wheel, I decided to introduce a common helper and eliminate all duplications I could find. I'm open to a bikeshedding discussion on the sysfs_bin_attr_simple_read() name. ;) Lukas Wunner (2): sysfs: Add sysfs_bin_attr_simple_read() helper treewide: Use sysfs_bin_attr_simple_read() helper arch/powerpc/platforms/powernv/opal.c | 10 +------- drivers/acpi/bgrt.c | 9 +------- drivers/firmware/dmi_scan.c | 12 ++-------- drivers/firmware/efi/rci2-table.c | 10 +------- drivers/gpu/drm/i915/gvt/firmware.c | 26 +++++---------------- .../intel/int340x_thermal/int3400_thermal.c | 9 +------- fs/sysfs/file.c | 27 ++++++++++++++++++++++ include/linux/sysfs.h | 15 ++++++++++++ init/initramfs.c | 10 +------- kernel/module/sysfs.c | 13 +---------- 10 files changed, 56 insertions(+), 85 deletions(-) -- 2.43.0 ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 1/2] sysfs: Add sysfs_bin_attr_simple_read() helper 2024-04-06 13:52 [PATCH 0/2] Deduplicate bin_attribute simple read() callbacks Lukas Wunner @ 2024-04-06 13:52 ` Lukas Wunner 2024-04-11 13:07 ` Greg Kroah-Hartman 2024-04-06 13:52 ` [PATCH 2/2] treewide: Use " Lukas Wunner ` (3 subsequent siblings) 4 siblings, 1 reply; 10+ messages in thread From: Lukas Wunner @ 2024-04-06 13:52 UTC (permalink / raw) To: Greg Kroah-Hartman, Rafael J. Wysocki, linux-kernel Cc: Michael Ellerman, linuxppc-dev, linux-acpi, Jean Delvare, Ard Biesheuvel, linux-efi, Zhenyu Wang, Zhi Wang, intel-gvt-dev, Daniel Lezcano, linux-pm, Luis Chamberlain, linux-modules When drivers expose a bin_attribute in sysfs which is backed by a buffer in memory, a common pattern is to set the @private and @size members in struct bin_attribute to the buffer's location and size. The ->read() callback then merely consists of a single memcpy() call. It's not even necessary to perform bounds checks as these are already handled by sysfs_kf_bin_read(). However each driver is so far providing its own ->read() implementation. The pattern is sufficiently frequent to merit a public helper, so add sysfs_bin_attr_simple_read() as well as BIN_ATTR_SIMPLE_RO() and BIN_ATTR_SIMPLE_ADMIN_RO() macros to ease declaration of such bin_attributes and reduce LoC and .text section size. Signed-off-by: Lukas Wunner <lukas@wunner.de> --- fs/sysfs/file.c | 27 +++++++++++++++++++++++++++ include/linux/sysfs.h | 15 +++++++++++++++ 2 files changed, 42 insertions(+) diff --git a/fs/sysfs/file.c b/fs/sysfs/file.c index 6b7652f..289b57d 100644 --- a/fs/sysfs/file.c +++ b/fs/sysfs/file.c @@ -783,3 +783,30 @@ int sysfs_emit_at(char *buf, int at, const char *fmt, ...) return len; } EXPORT_SYMBOL_GPL(sysfs_emit_at); + +/** + * sysfs_bin_attr_simple_read - read callback to simply copy from memory. + * @file: attribute file which is being read. + * @kobj: object to which the attribute belongs. + * @attr: attribute descriptor. + * @buf: destination buffer. + * @off: offset in bytes from which to read. + * @count: maximum number of bytes to read. + * + * Simple ->read() callback for bin_attributes backed by a buffer in memory. + * The @private and @size members in struct bin_attribute must be set to the + * buffer's location and size before the bin_attribute is created in sysfs. + * + * Bounds check for @off and @count is done in sysfs_kf_bin_read(). + * Negative value check for @off is done in vfs_setpos() and default_llseek(). + * + * 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, + loff_t off, size_t count) +{ + memcpy(buf, attr->private + off, count); + return count; +} +EXPORT_SYMBOL_GPL(sysfs_bin_attr_simple_read); diff --git a/include/linux/sysfs.h b/include/linux/sysfs.h index 326341c..a7d725f 100644 --- a/include/linux/sysfs.h +++ b/include/linux/sysfs.h @@ -371,6 +371,17 @@ struct bin_attribute bin_attr_##_name = __BIN_ATTR_ADMIN_RO(_name, _size) #define BIN_ATTR_ADMIN_RW(_name, _size) \ struct bin_attribute bin_attr_##_name = __BIN_ATTR_ADMIN_RW(_name, _size) +#define __BIN_ATTR_SIMPLE_RO(_name, _mode) { \ + .attr = { .name = __stringify(_name), .mode = _mode }, \ + .read = sysfs_bin_attr_simple_read, \ +} + +#define BIN_ATTR_SIMPLE_RO(_name) \ +struct bin_attribute bin_attr_##_name = __BIN_ATTR_SIMPLE_RO(_name, 0444) + +#define BIN_ATTR_SIMPLE_ADMIN_RO(_name) \ +struct bin_attribute bin_attr_##_name = __BIN_ATTR_SIMPLE_RO(_name, 0400) + struct sysfs_ops { ssize_t (*show)(struct kobject *, struct attribute *, char *); ssize_t (*store)(struct kobject *, struct attribute *, const char *, size_t); @@ -478,6 +489,10 @@ int sysfs_group_change_owner(struct kobject *kobj, __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, + loff_t off, size_t count); + #else /* CONFIG_SYSFS */ static inline int sysfs_create_dir_ns(struct kobject *kobj, const void *ns) -- 2.43.0 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 1/2] sysfs: Add sysfs_bin_attr_simple_read() helper 2024-04-06 13:52 ` [PATCH 1/2] sysfs: Add sysfs_bin_attr_simple_read() helper Lukas Wunner @ 2024-04-11 13:07 ` Greg Kroah-Hartman 0 siblings, 0 replies; 10+ messages in thread From: Greg Kroah-Hartman @ 2024-04-11 13:07 UTC (permalink / raw) To: Lukas Wunner Cc: Rafael J. Wysocki, linux-kernel, Michael Ellerman, linuxppc-dev, linux-acpi, Jean Delvare, Ard Biesheuvel, linux-efi, Zhenyu Wang, Zhi Wang, intel-gvt-dev, Daniel Lezcano, linux-pm, Luis Chamberlain, linux-modules On Sat, Apr 06, 2024 at 03:52:01PM +0200, Lukas Wunner wrote: > When drivers expose a bin_attribute in sysfs which is backed by a buffer > in memory, a common pattern is to set the @private and @size members in > struct bin_attribute to the buffer's location and size. > > The ->read() callback then merely consists of a single memcpy() call. > It's not even necessary to perform bounds checks as these are already > handled by sysfs_kf_bin_read(). > > However each driver is so far providing its own ->read() implementation. > The pattern is sufficiently frequent to merit a public helper, so add > sysfs_bin_attr_simple_read() as well as BIN_ATTR_SIMPLE_RO() and > BIN_ATTR_SIMPLE_ADMIN_RO() macros to ease declaration of such > bin_attributes and reduce LoC and .text section size. > > Signed-off-by: Lukas Wunner <lukas@wunner.de> > --- > fs/sysfs/file.c | 27 +++++++++++++++++++++++++++ > include/linux/sysfs.h | 15 +++++++++++++++ > 2 files changed, 42 insertions(+) Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 2/2] treewide: Use sysfs_bin_attr_simple_read() helper 2024-04-06 13:52 [PATCH 0/2] Deduplicate bin_attribute simple read() callbacks Lukas Wunner 2024-04-06 13:52 ` [PATCH 1/2] sysfs: Add sysfs_bin_attr_simple_read() helper Lukas Wunner @ 2024-04-06 13:52 ` Lukas Wunner 2024-04-08 10:42 ` Zhi Wang 2024-04-08 11:11 ` Michael Ellerman 2024-04-08 8:42 ` [PATCH 0/2] Deduplicate bin_attribute simple read() callbacks Ard Biesheuvel ` (2 subsequent siblings) 4 siblings, 2 replies; 10+ messages in thread From: Lukas Wunner @ 2024-04-06 13:52 UTC (permalink / raw) To: Greg Kroah-Hartman, Rafael J. Wysocki, linux-kernel Cc: Michael Ellerman, linuxppc-dev, linux-acpi, Jean Delvare, Ard Biesheuvel, linux-efi, Zhenyu Wang, Zhi Wang, intel-gvt-dev, Daniel Lezcano, linux-pm, Luis Chamberlain, linux-modules Deduplicate ->read() callbacks of bin_attributes which are backed by a simple buffer in memory: Use the newly introduced sysfs_bin_attr_simple_read() helper instead, either by referencing it directly or by declaring such bin_attributes with BIN_ATTR_SIMPLE_RO() or BIN_ATTR_SIMPLE_ADMIN_RO(). Aside from a reduction of LoC, this shaves off a few bytes from vmlinux (304 bytes on an x86_64 allyesconfig). No functional change intended. Signed-off-by: Lukas Wunner <lukas@wunner.de> --- arch/powerpc/platforms/powernv/opal.c | 10 +-------- drivers/acpi/bgrt.c | 9 +------- drivers/firmware/dmi_scan.c | 12 ++-------- drivers/firmware/efi/rci2-table.c | 10 +-------- drivers/gpu/drm/i915/gvt/firmware.c | 26 +++++----------------- .../intel/int340x_thermal/int3400_thermal.c | 9 +------- init/initramfs.c | 10 +-------- kernel/module/sysfs.c | 13 +---------- 8 files changed, 14 insertions(+), 85 deletions(-) diff --git a/arch/powerpc/platforms/powernv/opal.c b/arch/powerpc/platforms/powernv/opal.c index 45dd77e..5d0f35b 100644 --- a/arch/powerpc/platforms/powernv/opal.c +++ b/arch/powerpc/platforms/powernv/opal.c @@ -792,14 +792,6 @@ static int __init opal_sysfs_init(void) return 0; } -static ssize_t export_attr_read(struct file *fp, struct kobject *kobj, - struct bin_attribute *bin_attr, char *buf, - loff_t off, size_t count) -{ - return memory_read_from_buffer(buf, count, &off, bin_attr->private, - bin_attr->size); -} - static int opal_add_one_export(struct kobject *parent, const char *export_name, struct device_node *np, const char *prop_name) { @@ -826,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 = export_attr_read; + attr->read = sysfs_bin_attr_simple_read; attr->private = __va(vals[0]); attr->size = vals[1]; diff --git a/drivers/acpi/bgrt.c b/drivers/acpi/bgrt.c index e4fb9e2..d1d9c92 100644 --- a/drivers/acpi/bgrt.c +++ b/drivers/acpi/bgrt.c @@ -29,14 +29,7 @@ BGRT_SHOW(xoffset, image_offset_x); BGRT_SHOW(yoffset, image_offset_y); -static ssize_t image_read(struct file *file, struct kobject *kobj, - struct bin_attribute *attr, char *buf, loff_t off, size_t count) -{ - memcpy(buf, attr->private + off, count); - return count; -} - -static BIN_ATTR_RO(image, 0); /* size gets filled in later */ +static BIN_ATTR_SIMPLE_RO(image); static struct attribute *bgrt_attributes[] = { &bgrt_attr_version.attr, diff --git a/drivers/firmware/dmi_scan.c b/drivers/firmware/dmi_scan.c index 015c95a..3d0f773 100644 --- a/drivers/firmware/dmi_scan.c +++ b/drivers/firmware/dmi_scan.c @@ -746,16 +746,8 @@ static void __init dmi_scan_machine(void) pr_info("DMI not present or invalid.\n"); } -static ssize_t raw_table_read(struct file *file, struct kobject *kobj, - struct bin_attribute *attr, char *buf, - loff_t pos, size_t count) -{ - memcpy(buf, attr->private + pos, count); - return count; -} - -static BIN_ATTR(smbios_entry_point, S_IRUSR, raw_table_read, NULL, 0); -static BIN_ATTR(DMI, S_IRUSR, raw_table_read, NULL, 0); +static BIN_ATTR_SIMPLE_ADMIN_RO(smbios_entry_point); +static BIN_ATTR_SIMPLE_ADMIN_RO(DMI); static int __init dmi_init(void) { diff --git a/drivers/firmware/efi/rci2-table.c b/drivers/firmware/efi/rci2-table.c index de1a9a1..4fd45d6 100644 --- a/drivers/firmware/efi/rci2-table.c +++ b/drivers/firmware/efi/rci2-table.c @@ -40,15 +40,7 @@ struct rci2_table_global_hdr { static u32 rci2_table_len; unsigned long rci2_table_phys __ro_after_init = EFI_INVALID_TABLE_ADDR; -static ssize_t raw_table_read(struct file *file, struct kobject *kobj, - struct bin_attribute *attr, char *buf, - loff_t pos, size_t count) -{ - memcpy(buf, attr->private + pos, count); - return count; -} - -static BIN_ATTR(rci2, S_IRUSR, raw_table_read, NULL, 0); +static BIN_ATTR_SIMPLE_ADMIN_RO(rci2); static u16 checksum(void) { diff --git a/drivers/gpu/drm/i915/gvt/firmware.c b/drivers/gpu/drm/i915/gvt/firmware.c index 4dd52ac..5e66a26 100644 --- a/drivers/gpu/drm/i915/gvt/firmware.c +++ b/drivers/gpu/drm/i915/gvt/firmware.c @@ -50,21 +50,7 @@ struct gvt_firmware_header { #define dev_to_drm_minor(d) dev_get_drvdata((d)) -static ssize_t -gvt_firmware_read(struct file *filp, struct kobject *kobj, - struct bin_attribute *attr, char *buf, - loff_t offset, size_t count) -{ - memcpy(buf, attr->private + offset, count); - return count; -} - -static struct bin_attribute firmware_attr = { - .attr = {.name = "gvt_firmware", .mode = (S_IRUSR)}, - .read = gvt_firmware_read, - .write = NULL, - .mmap = NULL, -}; +static BIN_ATTR_SIMPLE_ADMIN_RO(gvt_firmware); static int expose_firmware_sysfs(struct intel_gvt *gvt) { @@ -107,10 +93,10 @@ static int expose_firmware_sysfs(struct intel_gvt *gvt) crc32_start = offsetof(struct gvt_firmware_header, version); h->crc32 = crc32_le(0, firmware + crc32_start, size - crc32_start); - firmware_attr.size = size; - firmware_attr.private = firmware; + bin_attr_gvt_firmware.size = size; + bin_attr_gvt_firmware.private = firmware; - ret = device_create_bin_file(&pdev->dev, &firmware_attr); + ret = device_create_bin_file(&pdev->dev, &bin_attr_gvt_firmware); if (ret) { vfree(firmware); return ret; @@ -122,8 +108,8 @@ static void clean_firmware_sysfs(struct intel_gvt *gvt) { struct pci_dev *pdev = to_pci_dev(gvt->gt->i915->drm.dev); - device_remove_bin_file(&pdev->dev, &firmware_attr); - vfree(firmware_attr.private); + device_remove_bin_file(&pdev->dev, &bin_attr_gvt_firmware); + vfree(bin_attr_gvt_firmware.private); } /** diff --git a/drivers/thermal/intel/int340x_thermal/int3400_thermal.c b/drivers/thermal/intel/int340x_thermal/int3400_thermal.c index 427d370..6d4b51a 100644 --- a/drivers/thermal/intel/int340x_thermal/int3400_thermal.c +++ b/drivers/thermal/intel/int340x_thermal/int3400_thermal.c @@ -73,14 +73,7 @@ struct odvp_attr { struct device_attribute attr; }; -static ssize_t data_vault_read(struct file *file, struct kobject *kobj, - struct bin_attribute *attr, char *buf, loff_t off, size_t count) -{ - memcpy(buf, attr->private + off, count); - return count; -} - -static BIN_ATTR_RO(data_vault, 0); +static BIN_ATTR_SIMPLE_RO(data_vault); static struct bin_attribute *data_attributes[] = { &bin_attr_data_vault, diff --git a/init/initramfs.c b/init/initramfs.c index da79760..5193fae 100644 --- a/init/initramfs.c +++ b/init/initramfs.c @@ -575,15 +575,7 @@ static int __init initramfs_async_setup(char *str) #include <linux/initrd.h> #include <linux/kexec.h> -static ssize_t raw_read(struct file *file, struct kobject *kobj, - struct bin_attribute *attr, char *buf, - loff_t pos, size_t count) -{ - memcpy(buf, attr->private + pos, count); - return count; -} - -static BIN_ATTR(initrd, 0440, raw_read, NULL, 0); +static BIN_ATTR(initrd, 0440, sysfs_bin_attr_simple_read, NULL, 0); void __init reserve_initrd_mem(void) { diff --git a/kernel/module/sysfs.c b/kernel/module/sysfs.c index d964167..26efe13 100644 --- a/kernel/module/sysfs.c +++ b/kernel/module/sysfs.c @@ -146,17 +146,6 @@ struct module_notes_attrs { struct bin_attribute attrs[] __counted_by(notes); }; -static ssize_t module_notes_read(struct file *filp, struct kobject *kobj, - struct bin_attribute *bin_attr, - char *buf, loff_t pos, size_t count) -{ - /* - * The caller checked the pos and count against our size. - */ - memcpy(buf, bin_attr->private + pos, count); - return count; -} - static void free_notes_attrs(struct module_notes_attrs *notes_attrs, unsigned int i) { @@ -205,7 +194,7 @@ static void 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 = module_notes_read; + nattr->read = sysfs_bin_attr_simple_read; ++nattr; } ++loaded; -- 2.43.0 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] treewide: Use sysfs_bin_attr_simple_read() helper 2024-04-06 13:52 ` [PATCH 2/2] treewide: Use " Lukas Wunner @ 2024-04-08 10:42 ` Zhi Wang 2024-04-08 11:11 ` Michael Ellerman 1 sibling, 0 replies; 10+ messages in thread From: Zhi Wang @ 2024-04-08 10:42 UTC (permalink / raw) To: Lukas Wunner Cc: Greg Kroah-Hartman, Rafael J. Wysocki, linux-kernel, Michael Ellerman, linuxppc-dev, linux-acpi, Jean Delvare, Ard Biesheuvel, linux-efi, Zhenyu Wang, Zhi Wang, intel-gvt-dev, Daniel Lezcano, linux-pm, Luis Chamberlain, linux-modules On Sat, 6 Apr 2024 15:52:02 +0200 Lukas Wunner <lukas@wunner.de> wrote: > Deduplicate ->read() callbacks of bin_attributes which are backed by a > simple buffer in memory: > > Use the newly introduced sysfs_bin_attr_simple_read() helper instead, > either by referencing it directly or by declaring such bin_attributes > with BIN_ATTR_SIMPLE_RO() or BIN_ATTR_SIMPLE_ADMIN_RO(). > > Aside from a reduction of LoC, this shaves off a few bytes from > vmlinux (304 bytes on an x86_64 allyesconfig). > > No functional change intended. > As for GVT, looks good to me. Acked-by: Zhi Wang <zhiwang@kernel.org> > Signed-off-by: Lukas Wunner <lukas@wunner.de> > --- > arch/powerpc/platforms/powernv/opal.c | 10 +-------- > drivers/acpi/bgrt.c | 9 +------- > drivers/firmware/dmi_scan.c | 12 ++-------- > drivers/firmware/efi/rci2-table.c | 10 +-------- > drivers/gpu/drm/i915/gvt/firmware.c | 26 > +++++----------------- .../intel/int340x_thermal/int3400_thermal.c > | 9 +------- init/initramfs.c > | 10 +-------- kernel/module/sysfs.c | > 13 +---------- 8 files changed, 14 insertions(+), 85 deletions(-) > > diff --git a/arch/powerpc/platforms/powernv/opal.c > b/arch/powerpc/platforms/powernv/opal.c index 45dd77e..5d0f35b 100644 > --- a/arch/powerpc/platforms/powernv/opal.c > +++ b/arch/powerpc/platforms/powernv/opal.c > @@ -792,14 +792,6 @@ static int __init opal_sysfs_init(void) > return 0; > } > > -static ssize_t export_attr_read(struct file *fp, struct kobject > *kobj, > - struct bin_attribute *bin_attr, char > *buf, > - loff_t off, size_t count) > -{ > - return memory_read_from_buffer(buf, count, &off, > bin_attr->private, > - bin_attr->size); > -} > - > static int opal_add_one_export(struct kobject *parent, const char > *export_name, struct device_node *np, const char *prop_name) > { > @@ -826,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 = export_attr_read; > + attr->read = sysfs_bin_attr_simple_read; > attr->private = __va(vals[0]); > attr->size = vals[1]; > > diff --git a/drivers/acpi/bgrt.c b/drivers/acpi/bgrt.c > index e4fb9e2..d1d9c92 100644 > --- a/drivers/acpi/bgrt.c > +++ b/drivers/acpi/bgrt.c > @@ -29,14 +29,7 @@ > BGRT_SHOW(xoffset, image_offset_x); > BGRT_SHOW(yoffset, image_offset_y); > > -static ssize_t image_read(struct file *file, struct kobject *kobj, > - struct bin_attribute *attr, char *buf, loff_t off, > size_t count) -{ > - memcpy(buf, attr->private + off, count); > - return count; > -} > - > -static BIN_ATTR_RO(image, 0); /* size gets filled in later */ > +static BIN_ATTR_SIMPLE_RO(image); > > static struct attribute *bgrt_attributes[] = { > &bgrt_attr_version.attr, > diff --git a/drivers/firmware/dmi_scan.c b/drivers/firmware/dmi_scan.c > index 015c95a..3d0f773 100644 > --- a/drivers/firmware/dmi_scan.c > +++ b/drivers/firmware/dmi_scan.c > @@ -746,16 +746,8 @@ static void __init dmi_scan_machine(void) > pr_info("DMI not present or invalid.\n"); > } > > -static ssize_t raw_table_read(struct file *file, struct kobject > *kobj, > - struct bin_attribute *attr, char *buf, > - loff_t pos, size_t count) > -{ > - memcpy(buf, attr->private + pos, count); > - return count; > -} > - > -static BIN_ATTR(smbios_entry_point, S_IRUSR, raw_table_read, NULL, > 0); -static BIN_ATTR(DMI, S_IRUSR, raw_table_read, NULL, 0); > +static BIN_ATTR_SIMPLE_ADMIN_RO(smbios_entry_point); > +static BIN_ATTR_SIMPLE_ADMIN_RO(DMI); > > static int __init dmi_init(void) > { > diff --git a/drivers/firmware/efi/rci2-table.c > b/drivers/firmware/efi/rci2-table.c index de1a9a1..4fd45d6 100644 > --- a/drivers/firmware/efi/rci2-table.c > +++ b/drivers/firmware/efi/rci2-table.c > @@ -40,15 +40,7 @@ struct rci2_table_global_hdr { > static u32 rci2_table_len; > unsigned long rci2_table_phys __ro_after_init = > EFI_INVALID_TABLE_ADDR; > -static ssize_t raw_table_read(struct file *file, struct kobject > *kobj, > - struct bin_attribute *attr, char *buf, > - loff_t pos, size_t count) > -{ > - memcpy(buf, attr->private + pos, count); > - return count; > -} > - > -static BIN_ATTR(rci2, S_IRUSR, raw_table_read, NULL, 0); > +static BIN_ATTR_SIMPLE_ADMIN_RO(rci2); > > static u16 checksum(void) > { > diff --git a/drivers/gpu/drm/i915/gvt/firmware.c > b/drivers/gpu/drm/i915/gvt/firmware.c index 4dd52ac..5e66a26 100644 > --- a/drivers/gpu/drm/i915/gvt/firmware.c > +++ b/drivers/gpu/drm/i915/gvt/firmware.c > @@ -50,21 +50,7 @@ struct gvt_firmware_header { > > #define dev_to_drm_minor(d) dev_get_drvdata((d)) > > -static ssize_t > -gvt_firmware_read(struct file *filp, struct kobject *kobj, > - struct bin_attribute *attr, char *buf, > - loff_t offset, size_t count) > -{ > - memcpy(buf, attr->private + offset, count); > - return count; > -} > - > -static struct bin_attribute firmware_attr = { > - .attr = {.name = "gvt_firmware", .mode = (S_IRUSR)}, > - .read = gvt_firmware_read, > - .write = NULL, > - .mmap = NULL, > -}; > +static BIN_ATTR_SIMPLE_ADMIN_RO(gvt_firmware); > > static int expose_firmware_sysfs(struct intel_gvt *gvt) > { > @@ -107,10 +93,10 @@ static int expose_firmware_sysfs(struct > intel_gvt *gvt) crc32_start = offsetof(struct gvt_firmware_header, > version); h->crc32 = crc32_le(0, firmware + crc32_start, size - > crc32_start); > - firmware_attr.size = size; > - firmware_attr.private = firmware; > + bin_attr_gvt_firmware.size = size; > + bin_attr_gvt_firmware.private = firmware; > > - ret = device_create_bin_file(&pdev->dev, &firmware_attr); > + ret = device_create_bin_file(&pdev->dev, > &bin_attr_gvt_firmware); if (ret) { > vfree(firmware); > return ret; > @@ -122,8 +108,8 @@ static void clean_firmware_sysfs(struct intel_gvt > *gvt) { > struct pci_dev *pdev = to_pci_dev(gvt->gt->i915->drm.dev); > > - device_remove_bin_file(&pdev->dev, &firmware_attr); > - vfree(firmware_attr.private); > + device_remove_bin_file(&pdev->dev, &bin_attr_gvt_firmware); > + vfree(bin_attr_gvt_firmware.private); > } > > /** > diff --git a/drivers/thermal/intel/int340x_thermal/int3400_thermal.c > b/drivers/thermal/intel/int340x_thermal/int3400_thermal.c index > 427d370..6d4b51a 100644 --- > a/drivers/thermal/intel/int340x_thermal/int3400_thermal.c +++ > b/drivers/thermal/intel/int340x_thermal/int3400_thermal.c @@ -73,14 > +73,7 @@ struct odvp_attr { struct device_attribute attr; > }; > > -static ssize_t data_vault_read(struct file *file, struct kobject > *kobj, > - struct bin_attribute *attr, char *buf, loff_t off, > size_t count) -{ > - memcpy(buf, attr->private + off, count); > - return count; > -} > - > -static BIN_ATTR_RO(data_vault, 0); > +static BIN_ATTR_SIMPLE_RO(data_vault); > > static struct bin_attribute *data_attributes[] = { > &bin_attr_data_vault, > diff --git a/init/initramfs.c b/init/initramfs.c > index da79760..5193fae 100644 > --- a/init/initramfs.c > +++ b/init/initramfs.c > @@ -575,15 +575,7 @@ static int __init initramfs_async_setup(char > *str) #include <linux/initrd.h> > #include <linux/kexec.h> > > -static ssize_t raw_read(struct file *file, struct kobject *kobj, > - struct bin_attribute *attr, char *buf, > - loff_t pos, size_t count) > -{ > - memcpy(buf, attr->private + pos, count); > - return count; > -} > - > -static BIN_ATTR(initrd, 0440, raw_read, NULL, 0); > +static BIN_ATTR(initrd, 0440, sysfs_bin_attr_simple_read, NULL, 0); > > void __init reserve_initrd_mem(void) > { > diff --git a/kernel/module/sysfs.c b/kernel/module/sysfs.c > index d964167..26efe13 100644 > --- a/kernel/module/sysfs.c > +++ b/kernel/module/sysfs.c > @@ -146,17 +146,6 @@ struct module_notes_attrs { > struct bin_attribute attrs[] __counted_by(notes); > }; > > -static ssize_t module_notes_read(struct file *filp, struct kobject > *kobj, > - struct bin_attribute *bin_attr, > - char *buf, loff_t pos, size_t count) > -{ > - /* > - * The caller checked the pos and count against our size. > - */ > - memcpy(buf, bin_attr->private + pos, count); > - return count; > -} > - > static void free_notes_attrs(struct module_notes_attrs *notes_attrs, > unsigned int i) > { > @@ -205,7 +194,7 @@ static void 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 = module_notes_read; > + nattr->read = sysfs_bin_attr_simple_read; > ++nattr; > } > ++loaded; ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] treewide: Use sysfs_bin_attr_simple_read() helper 2024-04-06 13:52 ` [PATCH 2/2] treewide: Use " Lukas Wunner 2024-04-08 10:42 ` Zhi Wang @ 2024-04-08 11:11 ` Michael Ellerman 1 sibling, 0 replies; 10+ messages in thread From: Michael Ellerman @ 2024-04-08 11:11 UTC (permalink / raw) To: Lukas Wunner, Greg Kroah-Hartman, Rafael J. Wysocki, linux-kernel Cc: linuxppc-dev, linux-acpi, Jean Delvare, Ard Biesheuvel, linux-efi, Zhenyu Wang, Zhi Wang, intel-gvt-dev, Daniel Lezcano, linux-pm, Luis Chamberlain, linux-modules Lukas Wunner <lukas@wunner.de> writes: > Deduplicate ->read() callbacks of bin_attributes which are backed by a > simple buffer in memory: > > Use the newly introduced sysfs_bin_attr_simple_read() helper instead, > either by referencing it directly or by declaring such bin_attributes > with BIN_ATTR_SIMPLE_RO() or BIN_ATTR_SIMPLE_ADMIN_RO(). > > Aside from a reduction of LoC, this shaves off a few bytes from vmlinux > (304 bytes on an x86_64 allyesconfig). > > No functional change intended. > > Signed-off-by: Lukas Wunner <lukas@wunner.de> > --- > arch/powerpc/platforms/powernv/opal.c | 10 +-------- > drivers/acpi/bgrt.c | 9 +------- > drivers/firmware/dmi_scan.c | 12 ++-------- > drivers/firmware/efi/rci2-table.c | 10 +-------- > drivers/gpu/drm/i915/gvt/firmware.c | 26 +++++----------------- > .../intel/int340x_thermal/int3400_thermal.c | 9 +------- > init/initramfs.c | 10 +-------- > kernel/module/sysfs.c | 13 +---------- > 8 files changed, 14 insertions(+), 85 deletions(-) > > diff --git a/arch/powerpc/platforms/powernv/opal.c b/arch/powerpc/platforms/powernv/opal.c > index 45dd77e..5d0f35b 100644 > --- a/arch/powerpc/platforms/powernv/opal.c > +++ b/arch/powerpc/platforms/powernv/opal.c > @@ -792,14 +792,6 @@ static int __init opal_sysfs_init(void) > return 0; > } > > -static ssize_t export_attr_read(struct file *fp, struct kobject *kobj, > - struct bin_attribute *bin_attr, char *buf, > - loff_t off, size_t count) > -{ > - return memory_read_from_buffer(buf, count, &off, bin_attr->private, > - bin_attr->size); > -} > - > static int opal_add_one_export(struct kobject *parent, const char *export_name, > struct device_node *np, const char *prop_name) > { > @@ -826,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 = export_attr_read; > + attr->read = sysfs_bin_attr_simple_read; > attr->private = __va(vals[0]); > attr->size = vals[1]; I gave it a quick boot and checked I could still read the attributes, everything seems fine. Acked-by: Michael Ellerman <mpe@ellerman.id.au> (powerpc) cheers ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 0/2] Deduplicate bin_attribute simple read() callbacks 2024-04-06 13:52 [PATCH 0/2] Deduplicate bin_attribute simple read() callbacks Lukas Wunner 2024-04-06 13:52 ` [PATCH 1/2] sysfs: Add sysfs_bin_attr_simple_read() helper Lukas Wunner 2024-04-06 13:52 ` [PATCH 2/2] treewide: Use " Lukas Wunner @ 2024-04-08 8:42 ` Ard Biesheuvel 2024-04-08 15:01 ` Rafael J. Wysocki 2024-04-11 13:07 ` Greg Kroah-Hartman 4 siblings, 0 replies; 10+ messages in thread From: Ard Biesheuvel @ 2024-04-08 8:42 UTC (permalink / raw) To: Lukas Wunner Cc: Greg Kroah-Hartman, Rafael J. Wysocki, linux-kernel, Michael Ellerman, linuxppc-dev, linux-acpi, Jean Delvare, linux-efi, Zhenyu Wang, Zhi Wang, intel-gvt-dev, Daniel Lezcano, linux-pm, Luis Chamberlain, linux-modules On Sat, 6 Apr 2024 at 15:52, Lukas Wunner <lukas@wunner.de> wrote: > > For my upcoming PCI device authentication v2 patches, I have the need > to expose a simple buffer in virtual memory as a bin_attribute. > > It turns out we've duplicated the ->read() callback for such simple > buffers a fair number of times across the tree. > > So instead of reinventing the wheel, I decided to introduce a common > helper and eliminate all duplications I could find. > > I'm open to a bikeshedding discussion on the sysfs_bin_attr_simple_read() > name. ;) > > Lukas Wunner (2): > sysfs: Add sysfs_bin_attr_simple_read() helper > treewide: Use sysfs_bin_attr_simple_read() helper > Acked-by: Ard Biesheuvel <ardb@kernel.org> > arch/powerpc/platforms/powernv/opal.c | 10 +------- > drivers/acpi/bgrt.c | 9 +------- > drivers/firmware/dmi_scan.c | 12 ++-------- > drivers/firmware/efi/rci2-table.c | 10 +------- > drivers/gpu/drm/i915/gvt/firmware.c | 26 +++++---------------- > .../intel/int340x_thermal/int3400_thermal.c | 9 +------- > fs/sysfs/file.c | 27 ++++++++++++++++++++++ > include/linux/sysfs.h | 15 ++++++++++++ > init/initramfs.c | 10 +------- > kernel/module/sysfs.c | 13 +---------- > 10 files changed, 56 insertions(+), 85 deletions(-) > > -- > 2.43.0 > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 0/2] Deduplicate bin_attribute simple read() callbacks 2024-04-06 13:52 [PATCH 0/2] Deduplicate bin_attribute simple read() callbacks Lukas Wunner ` (2 preceding siblings ...) 2024-04-08 8:42 ` [PATCH 0/2] Deduplicate bin_attribute simple read() callbacks Ard Biesheuvel @ 2024-04-08 15:01 ` Rafael J. Wysocki 2024-04-11 13:07 ` Greg Kroah-Hartman 4 siblings, 0 replies; 10+ messages in thread From: Rafael J. Wysocki @ 2024-04-08 15:01 UTC (permalink / raw) To: Lukas Wunner Cc: Greg Kroah-Hartman, Rafael J. Wysocki, linux-kernel, Michael Ellerman, linuxppc-dev, linux-acpi, Jean Delvare, Ard Biesheuvel, linux-efi, Zhenyu Wang, Zhi Wang, intel-gvt-dev, Daniel Lezcano, linux-pm, Luis Chamberlain, linux-modules On Sat, Apr 6, 2024 at 3:52 PM Lukas Wunner <lukas@wunner.de> wrote: > > For my upcoming PCI device authentication v2 patches, I have the need > to expose a simple buffer in virtual memory as a bin_attribute. > > It turns out we've duplicated the ->read() callback for such simple > buffers a fair number of times across the tree. > > So instead of reinventing the wheel, I decided to introduce a common > helper and eliminate all duplications I could find. > > I'm open to a bikeshedding discussion on the sysfs_bin_attr_simple_read() > name. ;) > > Lukas Wunner (2): > sysfs: Add sysfs_bin_attr_simple_read() helper > treewide: Use sysfs_bin_attr_simple_read() helper > > arch/powerpc/platforms/powernv/opal.c | 10 +------- > drivers/acpi/bgrt.c | 9 +------- > drivers/firmware/dmi_scan.c | 12 ++-------- > drivers/firmware/efi/rci2-table.c | 10 +------- > drivers/gpu/drm/i915/gvt/firmware.c | 26 +++++---------------- > .../intel/int340x_thermal/int3400_thermal.c | 9 +------- > fs/sysfs/file.c | 27 ++++++++++++++++++++++ > include/linux/sysfs.h | 15 ++++++++++++ > init/initramfs.c | 10 +------- > kernel/module/sysfs.c | 13 +---------- > 10 files changed, 56 insertions(+), 85 deletions(-) > > -- For the series Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 0/2] Deduplicate bin_attribute simple read() callbacks 2024-04-06 13:52 [PATCH 0/2] Deduplicate bin_attribute simple read() callbacks Lukas Wunner ` (3 preceding siblings ...) 2024-04-08 15:01 ` Rafael J. Wysocki @ 2024-04-11 13:07 ` Greg Kroah-Hartman 2024-04-11 13:39 ` Lukas Wunner 4 siblings, 1 reply; 10+ messages in thread From: Greg Kroah-Hartman @ 2024-04-11 13:07 UTC (permalink / raw) To: Lukas Wunner Cc: Rafael J. Wysocki, linux-kernel, Michael Ellerman, linuxppc-dev, linux-acpi, Jean Delvare, Ard Biesheuvel, linux-efi, Zhenyu Wang, Zhi Wang, intel-gvt-dev, Daniel Lezcano, linux-pm, Luis Chamberlain, linux-modules On Sat, Apr 06, 2024 at 03:52:00PM +0200, Lukas Wunner wrote: > For my upcoming PCI device authentication v2 patches, I have the need > to expose a simple buffer in virtual memory as a bin_attribute. > > It turns out we've duplicated the ->read() callback for such simple > buffers a fair number of times across the tree. > > So instead of reinventing the wheel, I decided to introduce a common > helper and eliminate all duplications I could find. > > I'm open to a bikeshedding discussion on the sysfs_bin_attr_simple_read() > name. ;) Seems like no one objects, should I just take this through my driver-core tree for 6.10? thanks, greg k-h ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 0/2] Deduplicate bin_attribute simple read() callbacks 2024-04-11 13:07 ` Greg Kroah-Hartman @ 2024-04-11 13:39 ` Lukas Wunner 0 siblings, 0 replies; 10+ messages in thread From: Lukas Wunner @ 2024-04-11 13:39 UTC (permalink / raw) To: Greg Kroah-Hartman Cc: Rafael J. Wysocki, linux-kernel, Michael Ellerman, linuxppc-dev, linux-acpi, Jean Delvare, Ard Biesheuvel, linux-efi, Zhenyu Wang, Zhi Wang, intel-gvt-dev, Daniel Lezcano, linux-pm, Luis Chamberlain, linux-modules On Thu, Apr 11, 2024 at 03:07:46PM +0200, Greg Kroah-Hartman wrote: > On Sat, Apr 06, 2024 at 03:52:00PM +0200, Lukas Wunner wrote: > > For my upcoming PCI device authentication v2 patches, I have the need > > to expose a simple buffer in virtual memory as a bin_attribute. > > > > It turns out we've duplicated the ->read() callback for such simple > > buffers a fair number of times across the tree. > > > > So instead of reinventing the wheel, I decided to introduce a common > > helper and eliminate all duplications I could find. > > > > I'm open to a bikeshedding discussion on the sysfs_bin_attr_simple_read() > > name. ;) > > Seems like no one objects, should I just take this through my > driver-core tree for 6.10? That would be awesome, thank you! Lukas ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2024-04-11 13:39 UTC | newest] Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2024-04-06 13:52 [PATCH 0/2] Deduplicate bin_attribute simple read() callbacks Lukas Wunner 2024-04-06 13:52 ` [PATCH 1/2] sysfs: Add sysfs_bin_attr_simple_read() helper Lukas Wunner 2024-04-11 13:07 ` Greg Kroah-Hartman 2024-04-06 13:52 ` [PATCH 2/2] treewide: Use " Lukas Wunner 2024-04-08 10:42 ` Zhi Wang 2024-04-08 11:11 ` Michael Ellerman 2024-04-08 8:42 ` [PATCH 0/2] Deduplicate bin_attribute simple read() callbacks Ard Biesheuvel 2024-04-08 15:01 ` Rafael J. Wysocki 2024-04-11 13:07 ` Greg Kroah-Hartman 2024-04-11 13:39 ` Lukas Wunner
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).