Messages in this thread Patch in this message | | | Date | Fri, 13 May 2022 22:27:47 +0800 | From | Muchun Song <> | Subject | Re: [PATCH] mm: change "char *bdi_unknown_name" to "char bdi_unknown_name[]" |
| |
On Fri, May 13, 2022 at 11:06:23AM +0000, David Laight wrote: > From: Andrew Morton > > Sent: 12 May 2022 21:01 > > > > On Thu, 12 May 2022 16:26:37 +0800 liqiong <liqiong@nfschina.com> wrote: > > > > > "char bdi_unknown_nam[]" string form declares a single variable. > > > It is better then "char *bdi_unknown_name" which creates two > > > variables. > > > > > > ... > > > > > > --- a/mm/backing-dev.c > > > +++ b/mm/backing-dev.c > > > @@ -20,7 +20,7 @@ struct backing_dev_info noop_backing_dev_info; > > > EXPORT_SYMBOL_GPL(noop_backing_dev_info); > > > > > > static struct class *bdi_class; > > > -static const char *bdi_unknown_name = "(unknown)"; > > > +static const char bdi_unknown_name[] = "(unknown)"; > > > > > > > heh, fun patch. We actually do this quite a lot. > > > > grep -r "^[a-z].*char \*[a-z].*= \"" . > > > > is a pathetic pattern which catches a lot of them. > > > > > > However. I expected your patch to shrink the kernel a bit, but it has > > the opposite effect: > > > > hp2:/usr/src/25> size mm/backing-dev.o > > text data bss dec hex filename > > 21288 9396 3808 34492 86bc mm/backing-dev.o-before > > 21300 9428 3808 34536 86e8 mm/backing-dev.o-after > > > > Even .data became larger. I didn't investigate why. > > The linker can merge replicated strings > (ie data in .rodata.str1.n sections) > but I don't think the compiler puts variables into that section. > > So if you have: > static const char *const foo_xxx = "foo"; > in multiple source/object files you get lots of pointers > but only one string. > OTOH with: > static const char foo_xxx[] = "foo"; > you get lots of copies of the string. > Which is smaller depends on the number of variables and the length > of the string. >
Good point. I have searched the whole code. There are 19 places where use the string "(unknown)". Seems it is better to drop this change.
arch/mips/alchemy/devboards/db1xxx.c:48: return "(unknown)"; drivers/acpi/device_pm.c:44: return "(unknown)"; drivers/base/component.c:101: component ? dev_name(component->dev) : "(unknown)", drivers/block/rbd.c:5137: spec->image_id, spec->image_name ?: "(unknown)", drivers/gpu/drm/i915/display/intel_dsi_vbt.c:570: return "(unknown)"; drivers/i3c/master/mipi-i3c-hci/ext_caps.c:50: "(unknown)", "master only", "target only", drivers/net/wireless/broadcom/brcm80211/brcmfmac/fwil.c:85: return "(unknown)"; drivers/platform/x86/thinkpad_acpi.c:7506: thinkpad_id.ec_version_str : "(unknown)"); drivers/usb/gadget/udc/omap_udc.c:2800: type = "(unknown)"; fs/cifs/cifs_swn.c:653: seq_puts(m, "(unknown)"); fs/cifs/cifsfs.c:434: seq_puts(s, "(unknown)"); fs/ext4/super.c:835: path = "(unknown)"; include/drm/drm_mode_object.h:118: return "(unknown)"; \ lib/error-inject.c:187: return "(unknown)"; mm/backing-dev.c:23:static const char *bdi_unknown_name = "(unknown)"; mm/filemap.c:3664: path = "(unknown)"; net/ipv4/cipso_ipv4.c:444: type_str = "(unknown)"; net/ipv6/calipso.c:384: type_str = "(unknown)"; sound/firewire/dice/dice-proc.c:35: return "(unknown)";
Thanks.
| |