| Message ID | 92ee0a0e83a5a3f3474845db6c8575297698933a.1712410202.git.lukas@wunner.de |
|---|---|
| State | Accepted |
| Commit | 66bc1a173328dec3e37c203a999f2a2914c96b56 |
| Headers | show |
| Series | Deduplicate bin_attribute simple read() callbacks | expand |
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
On Wed, May 22, 2024 at 07:51:35PM -0700, Guenter Roeck wrote: > Hi, > > On Sat, Apr 06, 2024 at 03:52:02PM +0200, Lukas Wunner 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. > > > > Not really; see below. > > > Signed-off-by: Lukas Wunner <lukas@wunner.de> > > Acked-by: Michael Ellerman <mpe@ellerman.id.au> (powerpc) > > --- > ... > > 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); > > > > sysfs_bin_attr_simple_read is only declared and available if CONFIG_SYSFS=y. > With m68k:m5208evb_defconfig + CONFIG_BLK_DEV_INITRD=y, this results in > > /opt/buildbot/slave/qemu-m68k/build/init/initramfs.c:578:31: > error: 'sysfs_bin_attr_simple_read' undeclared here (not in a function) > > This happens because CONFIG_SYSFS=n and there is no dummy function for > sysfs_bin_attr_simple_read(). Presumably the problem will be seen for all > configurations with CONFIG_BLK_DEV_INITRD=y and CONFIG_SYSFS=n. Lukas, can you send a patch adding a dummy function? thanks, greg k-h
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;
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(-)