All of lore.kernel.org
 help / color / mirror / Atom feed
From: arnd@arndb.de (Arnd Bergmann)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH 12/17] ARM: iop13xx: mark iop13xx_scan_bus as __devinit
Date: Thu, 4 Oct 2012 10:32:20 +0000	[thread overview]
Message-ID: <201210041032.20300.arnd@arndb.de> (raw)
In-Reply-To: <CAErSpo7rkN_F=AxweoEG3oXj392FvzVGPSKnkXCd=JX6cg0aTA@mail.gmail.com>

(+Greg)

On Tuesday 02 October 2012, Bjorn Helgaas wrote:
> 
> On Tue, Oct 2, 2012 at 10:36 AM, Arnd Bergmann <arnd@arndb.de> wrote:
> > pci_scan_root_bus is __devinit, so iop13xx_scan_bus has to be the
> > same in order to safely call it. This is ok because the function
> > itself is only called from the hwpci->scan callback.
> >
> > WARNING: vmlinux.o(.text+0x10138): Section mismatch in reference from the function iop13xx_scan_bus() to the function .devinit.text:pci_scan_root_bus()
> > The function iop13xx_scan_bus() references
> > the function __devinit pci_scan_root_bus().
> > This is often because iop13xx_scan_bus lacks a __devinit
> > annotation or the annotation of pci_scan_root_bus is wrong.
> 
> With CONFIG_HOTPLUG going away (I think the current state is that it
> is always set to "y"), __devinit will effectively become a no-op, so I
> expect we'll remove it from pci_scan_root_bus().
> 
> Therefore, I would skip this patch and live with the warning a little longer.

Hmm, I'm just trying to get rid of all build time warnings in the defconfigs
right now, and modpost still complains about the section mismatches. I have
a bunch more of these patches, but it would also be fine with me if we can
patch mostpost to ignore these cases.

I've also redone the analysis that Greg cited in the commit message for
45f035ab9b8 "CONFIG_HOTPLUG should be always on"

   It is quite hard to disable it these days, and even if you do, it
    only saves you about 200 bytes.  However, if it is disabled, lots of
    bugs show up because it is almost never tested if the option is disabled.

My test case (ARM omap2plus_defconfig, one of the most common configurations)
shows these size -A differences:



section		nohotplug	hotplug		difference
.head.text	392		392		0
.text		4829940		4881140		51200
.rodata		1630360		1633056		2696
__ksymtab	25720		25720		0
__ksymtab_gpl	17096		17136		40
__kcrctab	12860		12860		0
__kcrctab_gpl	8548		8568		20
__ksymtab_stri	96427		96509		82
__init_rodata	0		9800		9800
__param		2320		2320		0
__modver	716		364		-352
.ARM.unwind_idx	160360		160792		432
.ARM.unwind_tab	24312		24312		0
.init.text	234632		195688		-38944
.exit.text	8680		5116		-3564
.init.proc.info	312		312		0
.init.arch.info	2964		2964		0
.init.tagtable	72		72		0
.init.smpalt	776		776		0
.init.pv_table	880		880		0
.init.data	123356		111348		-12008
.exit.data	0		0		0
.data..percpu	12928		12928		0
.data		560160		562688		2528
.notes		36		36		0
.bss		5605324		5605580		256

total		13359171	13371357	12186
after boot	13001183	13054521	53338

That is over 50kb difference after discarding the init sections,
significantly more than the 200 bytes that Greg found.
The point about lack of testing is still valid of course, and I'm
not saying we need to keep the option around, but it's really
not as obvious as before. An argument in favor of removing the
__devinit logic is that these 50kb is still just 0.4% of the
kernel size.

For the five ARM defconfig files that actually turn off hotplug,
the absolute numbers are a bit lower, but the percentage is similar.

This is the amount of space wasted by enabling on CONFIG_HOTPLUG
on them, in bytes after discarding the init sections, and as a
percentage of the vmlinux size:

at91x40_defconfig	3448	0.27%
edb7211_defconfig	8912	0.41%
footbridge_defconfig	33347	0.97%
fortunet_defconfig	4592	0.25%
pleb_defconfig		7405	0.28%

Footbridge is the only config among these that enables PCI and USB, so
it has a bunch more drivers that actually have notable functions that 
can be discarded.

	Arnd

WARNING: multiple messages have this Message-ID (diff)
From: Arnd Bergmann <arnd@arndb.de>
To: Bjorn Helgaas <bhelgaas@google.com>
Cc: linux-arm-kernel@lists.infradead.org, arm@kernel.org,
	linux-kernel@vger.kernel.org,
	Lennert Buytenhek <kernel@wantstofly.org>,
	Dan Williams <djbw@fb.com>, Greg KH <greg@kroah.com>
Subject: Re: [PATCH 12/17] ARM: iop13xx: mark iop13xx_scan_bus as __devinit
Date: Thu, 4 Oct 2012 10:32:20 +0000	[thread overview]
Message-ID: <201210041032.20300.arnd@arndb.de> (raw)
In-Reply-To: <CAErSpo7rkN_F=AxweoEG3oXj392FvzVGPSKnkXCd=JX6cg0aTA@mail.gmail.com>

(+Greg)

On Tuesday 02 October 2012, Bjorn Helgaas wrote:
> 
> On Tue, Oct 2, 2012 at 10:36 AM, Arnd Bergmann <arnd@arndb.de> wrote:
> > pci_scan_root_bus is __devinit, so iop13xx_scan_bus has to be the
> > same in order to safely call it. This is ok because the function
> > itself is only called from the hwpci->scan callback.
> >
> > WARNING: vmlinux.o(.text+0x10138): Section mismatch in reference from the function iop13xx_scan_bus() to the function .devinit.text:pci_scan_root_bus()
> > The function iop13xx_scan_bus() references
> > the function __devinit pci_scan_root_bus().
> > This is often because iop13xx_scan_bus lacks a __devinit
> > annotation or the annotation of pci_scan_root_bus is wrong.
> 
> With CONFIG_HOTPLUG going away (I think the current state is that it
> is always set to "y"), __devinit will effectively become a no-op, so I
> expect we'll remove it from pci_scan_root_bus().
> 
> Therefore, I would skip this patch and live with the warning a little longer.

Hmm, I'm just trying to get rid of all build time warnings in the defconfigs
right now, and modpost still complains about the section mismatches. I have
a bunch more of these patches, but it would also be fine with me if we can
patch mostpost to ignore these cases.

I've also redone the analysis that Greg cited in the commit message for
45f035ab9b8 "CONFIG_HOTPLUG should be always on"

   It is quite hard to disable it these days, and even if you do, it
    only saves you about 200 bytes.  However, if it is disabled, lots of
    bugs show up because it is almost never tested if the option is disabled.

My test case (ARM omap2plus_defconfig, one of the most common configurations)
shows these size -A differences:



section		nohotplug	hotplug		difference
.head.text	392		392		0
.text		4829940		4881140		51200
.rodata		1630360		1633056		2696
__ksymtab	25720		25720		0
__ksymtab_gpl	17096		17136		40
__kcrctab	12860		12860		0
__kcrctab_gpl	8548		8568		20
__ksymtab_stri	96427		96509		82
__init_rodata	0		9800		9800
__param		2320		2320		0
__modver	716		364		-352
.ARM.unwind_idx	160360		160792		432
.ARM.unwind_tab	24312		24312		0
.init.text	234632		195688		-38944
.exit.text	8680		5116		-3564
.init.proc.info	312		312		0
.init.arch.info	2964		2964		0
.init.tagtable	72		72		0
.init.smpalt	776		776		0
.init.pv_table	880		880		0
.init.data	123356		111348		-12008
.exit.data	0		0		0
.data..percpu	12928		12928		0
.data		560160		562688		2528
.notes		36		36		0
.bss		5605324		5605580		256

total		13359171	13371357	12186
after boot	13001183	13054521	53338

That is over 50kb difference after discarding the init sections,
significantly more than the 200 bytes that Greg found.
The point about lack of testing is still valid of course, and I'm
not saying we need to keep the option around, but it's really
not as obvious as before. An argument in favor of removing the
__devinit logic is that these 50kb is still just 0.4% of the
kernel size.

For the five ARM defconfig files that actually turn off hotplug,
the absolute numbers are a bit lower, but the percentage is similar.

This is the amount of space wasted by enabling on CONFIG_HOTPLUG
on them, in bytes after discarding the init sections, and as a
percentage of the vmlinux size:

at91x40_defconfig	3448	0.27%
edb7211_defconfig	8912	0.41%
footbridge_defconfig	33347	0.97%
fortunet_defconfig	4592	0.25%
pleb_defconfig		7405	0.28%

Footbridge is the only config among these that enables PCI and USB, so
it has a bunch more drivers that actually have notable functions that 
can be discarded.

	Arnd

  reply	other threads:[~2012-10-04 10:32 UTC|newest]

Thread overview: 56+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <1349195816-2225-1-git-send-email-arnd@arndb.de>
2012-10-02 16:36 ` [PATCH 01/17] ARM: shmobile: fix memory size for kota2_defconfig Arnd Bergmann
2012-10-02 16:36   ` Arnd Bergmann
2012-10-03  0:37   ` Simon Horman
2012-10-03  0:37     ` Simon Horman
2012-10-04  8:34     ` Arnd Bergmann
2012-10-04  8:34       ` Arnd Bergmann
2012-10-04  8:58       ` Simon Horman
2012-10-04  8:58         ` Simon Horman
2012-11-30 22:10         ` Olof Johansson
2012-11-30 22:10           ` Olof Johansson
2012-12-01  0:26           ` Simon Horman
2012-12-01  0:26             ` Simon Horman
2013-01-07  1:59           ` Simon Horman
2013-01-07  1:59             ` Simon Horman
2013-01-07 13:33             ` Arnd Bergmann
2013-01-07 13:33               ` Arnd Bergmann
2013-01-08  0:19               ` Simon Horman
2013-01-08  0:19                 ` Simon Horman
2013-01-08 19:06                 ` Arnd Bergmann
2013-01-08 19:06                   ` Arnd Bergmann
2012-10-02 16:36 ` [PATCH 02/17] ARM: shark: fix shark_pci_init return code Arnd Bergmann
2012-10-02 16:36 ` [PATCH 03/17] ARM: pxa: Wunused-result warning in viper board file Arnd Bergmann
2012-10-02 16:50   ` Marc Zyngier
2012-10-02 16:36 ` [PATCH 04/17] ARM: pxa: define palmte2_pxa_keys conditionally Arnd Bergmann
2012-10-03 11:23   ` Marek Vasut
2012-10-02 16:36 ` [PATCH 05/17] ARM: pxa: remove sharpsl_fatal_check function Arnd Bergmann
2012-10-08  2:38   ` Eric Miao
2012-10-08  3:19     ` Haojian Zhuang
2012-10-02 16:36 ` [PATCH 06/17] ARM: pxa: work around duplicate definition of GPIO24_SSP1_SFRM Arnd Bergmann
2012-10-03  6:38   ` Igor Grinberg
2012-10-02 16:36 ` [PATCH 07/17] ARM: at91: skip at91_io_desc definition for NOMMU Arnd Bergmann
2012-10-02 18:56   ` Jean-Christophe PLAGNIOL-VILLARD
2012-10-04  8:23     ` Arnd Bergmann
2012-10-02 16:36 ` [PATCH 08/17] ARM: at91: unused variable in at91_pm_verify_clocks Arnd Bergmann
2012-10-02 18:55   ` Jean-Christophe PLAGNIOL-VILLARD
2012-10-04  8:28     ` Arnd Bergmann
2012-10-04 13:05       ` Jean-Christophe PLAGNIOL-VILLARD
2012-10-02 16:36 ` [PATCH 09/17] ARM: imx: select ARM_CPU_SUSPEND if necessary Arnd Bergmann
2012-10-07  4:12   ` Shawn Guo
2012-10-07  8:33     ` Arnd Bergmann
2012-10-02 16:36 ` [PATCH 10/17] ARM: s3c24xx: fix multiple section mismatch warnings Arnd Bergmann
2012-10-02 16:36 ` [PATCH 11/17] ARM: mv78xx0: mark mv78xx0_timer_init as __init_refok Arnd Bergmann
2012-10-02 16:36 ` [PATCH 12/17] ARM: iop13xx: mark iop13xx_scan_bus as __devinit Arnd Bergmann
2012-10-02 20:08   ` Bjorn Helgaas
2012-10-04 10:32     ` Arnd Bergmann [this message]
2012-10-04 10:32       ` Arnd Bergmann
2012-10-04 14:30       ` Greg KH
2012-10-04 14:30         ` Greg KH
2012-10-02 16:36 ` [PATCH 13/17] ARM: iop13xx: fix iq81340sc_atux_map_irq prototype Arnd Bergmann
2012-10-02 16:36 ` [PATCH 14/17] ARM: davinci: don't mark da850_register_cpufreq as __init Arnd Bergmann
2012-10-04 13:18   ` Sekhar Nori
2012-10-02 16:36 ` [PATCH 15/17] ARM: rpc: check device_register return code in ecard_probe Arnd Bergmann
2012-10-02 16:36 ` [PATCH 16/17] ARM: ks8695: __arch_virt_to_dma type handling Arnd Bergmann
2012-10-02 16:36 ` [PATCH 17/17] ARM: soc: dependency warnings for errata Arnd Bergmann
2012-10-02 17:16   ` Stephen Warren
2012-10-03 11:23   ` Linus Walleij

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=201210041032.20300.arnd@arndb.de \
    --to=arnd@arndb.de \
    --cc=linux-arm-kernel@lists.infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.