getting rid of __cpuinit

Hi Linus,

this is one of those pull requests for which the best timing is likely at the end of the merge window due to touching a large number of files in a trivial way. In fact, 6 of the 8 patches were done via regexps:

find -name "*.c" | xargs sed -i "s/^__cpuinit //g" find arch/ -name "*.h" | xargs sed -i "s/ __cpuinit//g"

find -name "*.c" | xargs sed -i "s/ __cpuinit$//g"
find -name "*.c" | xargs sed -i "s/ __cpuinitdata//g"
find -name "*.c" | xargs sed -i "s/ __cpuexit / /g"
find -name "*.c" | xargs sed -i "s/ __cpuinit / /g"

(I suppose asking for a "git sed" is too much to do...)

The point of this series is to get rid of __cpuinit. (I'll be working on other __XXXXinit after this) __cpuinit is a NOOP for CONFIG_HOTPLUG_CPU, which on for just about anybody in practice (in fact I had to disable 6 common options to even be ABLE to turn it off for size testing) and all distros also ship with it enabled.

Yet it's a rather error prone construct since the rules of this sort of __XXXXinit thing are rather complex and convoluted.. better to just get rid of it entirely.

As for the code size effect, after I spent a bunch of time cursing and trying to find out how to disable HOTPLUG_CPU, I managed to get an x86-64 build that actually shows how much space we ACTUALLY save with all the __cpuinit's in the tree... it's less than 2Kb. That is, if you manage to turn it actually off.

text data bss dec hex filename
4618096 1358684 848588 6825368 682598 vmlinux.before
4619965 1356908 848716 6825589 682675 vmlinux.after

Please consider pulling; or if you think another time is more appropriate just let me know; re-running the regexps isn't all that much work.

Note: the patch leaves the __cpuinit macro still existent so that in-flight code that might use it won't immediately break, but over time I want to kill that as well. (just not right now, because it could break various people's workflow and a few empty defines don't hurt all that much)

The following changes since commit e013e13bf605b9e6b702adffbe2853cfc60e7806:
Jens Axboe (1):
libata: fix bug with non-ncq devices

are available in the git repository at:

git://git.kernel.org/pub/scm/linux/kernel/git/arjan/linux-2.6-init.git ..BRANCH.NOT.VERIFIED..

Arjan van de Ven (8):
cpuinit: get rid of __cpuinit, first regexp
cpuinit: get rid of __cpuinit, second regexp
cpuinit: get rid of __cpuinit, third regexp
cpuinit: get rid of __cpuinit, fourth regexp
cpuinit: get rid of __cpuinit, fifth regexp
cpuinit: get rid of __cpuinit, sixth regexp
cpuinit: get rid of __cpuinit, manual fixups
cpuinit: turn __cpuinit into nops

 Documentation/cpu-hotplug.txt              |    4 +-
 arch/alpha/kernel/smp.c                    |    4 +-
 arch/arm/common/gic.c                      |    2 +-
 arch/arm/kernel/smp.c                      |   12 ++--
 arch/arm/mach-realview/localtimer.c        |    8 ++--
 arch/arm/mach-realview/platsmp.c           |    6 +-
 arch/cris/arch-v32/kernel/smp.c            |    2 +-
 arch/frv/kernel/setup.c                    |    2 +-
 arch/ia64/kernel/err_inject.c              |    8 ++--
 arch/ia64/kernel/mca.c                     |   12 ++--
 arch/ia64/kernel/numa.c                    |    4 +-
 arch/ia64/kernel/palinfo.c                 |    4 +-
 arch/ia64/kernel/salinfo.c                 |    4 +-
 arch/ia64/kernel/setup.c                   |   10 ++--
 arch/ia64/kernel/smpboot.c                 |   12 ++--
 arch/ia64/kernel/topology.c                |   18 ++++----
 arch/ia64/mm/contig.c                      |    2 +-
 arch/ia64/mm/discontig.c                   |    2 +-
 arch/ia64/sn/kernel/setup.c                |    4 +-
 arch/ia64/xen/hypervisor.c                 |    2 +-
 arch/m32r/kernel/smpboot.c                 |    2 +-
 arch/mips/kernel/cevt-bcm1480.c            |    2 +-
 arch/mips/kernel/cevt-r4k.c                |    2 +-
 arch/mips/kernel/cevt-sb1250.c             |    2 +-
 arch/mips/kernel/cevt-smtc.c               |    2 +-
 arch/mips/kernel/cpu-bugs64.c              |    4 +-
 arch/mips/kernel/cpu-probe.c               |   10 ++--
 arch/mips/kernel/smp-mt.c                  |    6 +-
 arch/mips/kernel/smp-up.c                  |    6 +-
 arch/mips/kernel/smp.c                     |    6 +-
 arch/mips/kernel/smtc.c                    |    2 +-
 arch/mips/kernel/spram.c                   |   14 +++---
 arch/mips/kernel/time.c                    |    2 +-
 arch/mips/kernel/traps.c                   |    8 ++--
 arch/mips/kernel/watch.c                   |    2 +-
 arch/mips/lib/uncached.c                   |    2 +-
 arch/mips/mipssim/sim_smtc.c               |    6 +-
 arch/mips/mipssim/sim_time.c               |    2 +-
 arch/mips/mm/c-r3k.c                       |    8 ++--
 arch/mips/mm/c-r4k.c                       |   34 +++++++-------
 arch/mips/mm/c-tx39.c                      |    2 +-
 arch/mips/mm/page.c                        |   40 ++++++++--------
 arch/mips/mm/sc-ip22.c                     |    2 +-
 arch/mips/mm/sc-mips.c                     |    2 +-
 arch/mips/mm/sc-r5k.c                      |    2 +-
 arch/mips/mm/sc-rm7k.c                     |    6 +-
 arch/mips/mm/tlb-r3k.c                     |    2 +-
 arch/mips/mm/tlb-r4k.c                     |    8 ++--
 arch/mips/mm/tlb-r8k.c                     |    4 +-
 arch/mips/mm/tlbex.c                       |   70 ++++++++++++++--------------
 arch/mips/mm/uasm.c                        |   70 ++++++++++++++--------------
 arch/mips/mm/uasm.h                        |   24 +++++-----
 arch/mips/mti-malta/malta-smtc.c           |    6 +-
 arch/mips/mti-malta/malta-time.c           |    2 +-
 arch/mips/pci/pci-ip27.c                   |    2 +-
 arch/mips/pmc-sierra/yosemite/smp.c        |   12 ++--
 arch/mips/sgi-ip27/ip27-init.c             |    4 +-
 arch/mips/sgi-ip27/ip27-smp.c              |    6 +-
 arch/mips/sgi-ip27/ip27-timer.c            |    6 +-
 arch/mips/sgi-ip27/ip27-xtalk.c            |    6 +-
 arch/mips/sibyte/bcm1480/smp.c             |    8 ++--
 arch/mips/sibyte/sb1250/smp.c              |    8 ++--
 arch/parisc/kernel/hardware.c              |    2 +-
 arch/parisc/kernel/processor.c             |    2 +-
 arch/parisc/kernel/smp.c                   |    4 +-
 arch/powerpc/kernel/smp.c                  |    2 +-
 arch/powerpc/kernel/sysfs.c                |   12 ++--
 arch/powerpc/mm/numa.c                     |   12 ++--
 arch/s390/appldata/appldata_base.c         |    8 ++--
 arch/s390/kernel/entry.h                   |    2 +-
 arch/s390/kernel/setup.c                   |    4 +-
 arch/s390/kernel/smp.c                     |   10 ++--
 arch/sh/kernel/setup.c                     |    2 +-
 arch/sh/kernel/smp.c                       |    4 +-
 arch/sh/kernel/timers/timer-broadcast.c    |    2 +-
 arch/sh/kernel/traps_32.c                  |    2 +-
 arch/sparc/kernel/smp.c                    |    8 ++--
 arch/sparc/kernel/sun4d_smp.c              |    2 +-
 arch/sparc/kernel/sun4m_smp.c              |    4 +-
 arch/sparc64/kernel/ds.c                   |    4 +-
 arch/sparc64/kernel/entry.h                |    2 +-
 arch/sparc64/kernel/irq.c                  |    4 +-
 arch/sparc64/kernel/mdesc.c                |    2 +-
 arch/sparc64/kernel/smp.c                  |    4 +-
 arch/sparc64/kernel/sysfs.c                |    4 +-
 arch/sparc64/mm/init.c                     |    2 +-
 arch/x86/include/asm/ds.h                  |    2 +-
 arch/x86/include/asm/mmconfig.h            |    4 +-
 arch/x86/include/asm/mpspec.h              |    2 +-
 arch/x86/include/asm/numa_64.h             |    8 ++--
 arch/x86/include/asm/ptrace.h              |    2 +-
 arch/x86/include/asm/smp.h                 |    4 +-
 arch/x86/kernel/acpi/boot.c                |    4 +-
 arch/x86/kernel/apic.c                     |   20 ++++----
 arch/x86/kernel/cpu/addon_cpuid_features.c |    6 +-
 arch/x86/kernel/cpu/amd.c                  |   22 ++++----
 arch/x86/kernel/cpu/centaur.c              |   26 +++++-----
 arch/x86/kernel/cpu/centaur_64.c           |    6 +-
 arch/x86/kernel/cpu/common.c               |   58 +++++++++++-----------
 arch/x86/kernel/cpu/cpufreq/powernow-k8.c  |    4 +-
 arch/x86/kernel/cpu/cyrix.c                |   40 ++++++++--------
 arch/x86/kernel/cpu/intel.c                |   22 ++++----
 arch/x86/kernel/cpu/intel_cacheinfo.c      |   44 +++++++++---------
 arch/x86/kernel/cpu/mcheck/mce_64.c        |   16 +++---
 arch/x86/kernel/cpu/mcheck/mce_amd_64.c    |   10 ++--
 arch/x86/kernel/cpu/mcheck/mce_intel_64.c  |    4 +-
 arch/x86/kernel/cpu/mcheck/therm_throt.c   |    8 ++--
 arch/x86/kernel/cpu/transmeta.c            |    6 +-
 arch/x86/kernel/cpu/umc.c                  |    2 +-
 arch/x86/kernel/cpuid.c                    |    4 +-
 arch/x86/kernel/ds.c                       |    2 +-
 arch/x86/kernel/genx2apic_uv_x.c           |    4 +-
 arch/x86/kernel/i387.c                     |    6 +-
 arch/x86/kernel/irq_32.c                   |    2 +-
 arch/x86/kernel/microcode_core.c           |    2 +-
 arch/x86/kernel/mmconf-fam10h_64.c         |   14 +++---
 arch/x86/kernel/msr.c                      |    4 +-
 arch/x86/kernel/numaq_32.c                 |    4 +-
 arch/x86/kernel/process.c                  |    8 ++--
 arch/x86/kernel/ptrace.c                   |    2 +-
 arch/x86/kernel/setup.c                    |    2 +-
 arch/x86/kernel/setup_percpu.c             |   16 +++---
 arch/x86/kernel/smpboot.c                  |   28 ++++++------
 arch/x86/kernel/smpcommon.c                |    2 +-
 arch/x86/kernel/tlb_64.c                   |    2 +-
 arch/x86/kernel/tsc.c                      |    2 +-
 arch/x86/kernel/tsc_sync.c                 |   18 ++++----
 arch/x86/kernel/vsyscall_64.c              |    6 +-
 arch/x86/kernel/xsave.c                    |    2 +-
 arch/x86/mach-voyager/voyager_smp.c        |    4 +-
 arch/x86/mm/init_64.c                      |    4 +-
 arch/x86/mm/numa_64.c                      |    2 +-
 arch/x86/mm/pat.c                          |    2 +-
 arch/x86/pci/amd_bus.c                     |    4 +-
 arch/x86/xen/setup.c                       |    6 +-
 arch/x86/xen/smp.c                         |    8 ++--
 arch/x86/xen/spinlock.c                    |    2 +-
 arch/x86/xen/xen-ops.h                     |    2 +-
 arch/xtensa/kernel/time.c                  |    2 +-
 block/blk-softirq.c                        |    4 +-
 drivers/acpi/bus.c                         |    2 +-
 drivers/acpi/ec.c                          |    2 +-
 drivers/acpi/numa.c                        |    2 +-
 drivers/acpi/processor_core.c              |    4 +-
 drivers/acpi/processor_idle.c              |    6 +-
 drivers/base/cpu.c                         |    4 +-
 drivers/base/topology.c                    |    8 ++--
 drivers/cpufreq/cpufreq.c                  |    2 +-
 drivers/cpufreq/cpufreq_stats.c            |    2 +-
 drivers/hwmon/coretemp.c                   |    4 +-
 drivers/infiniband/hw/ehca/ehca_irq.c      |    6 +-
 drivers/s390/sysinfo.c                     |    2 +-
 include/linux/cpu.h                        |    2 +-
 include/linux/init.h                       |   12 ++--
 init/calibrate.c                           |    6 +-
 kernel/cpu.c                               |    6 +-
 kernel/fork.c                              |    4 +-
 kernel/hrtimer.c                           |    6 +-
 kernel/rcuclassic.c                        |    6 +-
 kernel/rcupreempt.c                        |    6 +-
 kernel/relay.c                             |    2 +-
 kernel/sched.c                             |    8 ++--
 kernel/smp.c                               |    2 +-
 kernel/softirq.c                           |    8 ++--
 kernel/softlockup.c                        |    4 +-
 kernel/timer.c                             |   10 ++--
 lib/percpu_counter.c                       |    2 +-
 mm/page-writeback.c                        |    4 +-
 mm/page_alloc.c                            |    6 +-
 mm/slab.c                                  |   10 ++--
 mm/slub.c                                  |    4 +-
 mm/vmstat.c                                |    6 +-
 net/iucv/iucv.c                            |    2 +-
 173 files changed, 645 insertions(+), 645 deletions(-)

Arjan van de Ven 	Intel Open Source Technology Centre
For development, discussion and tips for power savings, 
visit http://www.lesswatts.org

With the removal of cpuinit/cpuexit we no longer need to do section mismatch checks on these sections.

Drop it from modpost.

---

Can you add this to the queue too.

Thanks,
Sam

index 8892161..58041e3 100644
@@ -753,14 +753,14 @@ static int check_section(const char *modname, const char *sec)
 
 
 #define ALL_INIT_DATA_SECTIONS \
-	".init.data$", ".devinit.data$", ".cpuinit.data$", ".meminit.data$"
+	".init.data$", ".devinit.data$", ".meminit.data$"
 #define ALL_EXIT_DATA_SECTIONS \
-	".exit.data$", ".devexit.data$", ".cpuexit.data$", ".memexit.data$"
+	".exit.data$", ".devexit.data$", ".memexit.data$"
 
 #define ALL_INIT_TEXT_SECTIONS \
-	".init.text$", ".devinit.text$", ".cpuinit.text$", ".meminit.text$"
+	".init.text$", ".devinit.text$", ".meminit.text$"
 #define ALL_EXIT_TEXT_SECTIONS \
-	".exit.text$", ".devexit.text$", ".cpuexit.text$", ".memexit.text$"
+	".exit.text$", ".devexit.text$", ".memexit.text$"
 
 #define ALL_INIT_SECTIONS ALL_INIT_DATA_SECTIONS, ALL_INIT_TEXT_SECTIONS
 #define ALL_EXIT_SECTIONS ALL_EXIT_DATA_SECTIONS, ALL_EXIT_TEXT_SECTIONS

@@ -770,12 +770,10 @@ static int check_section(const char *modname, const char *sec)
 
 #define INIT_SECTIONS      ".init.data$", ".init.text$"
 #define DEV_INIT_SECTIONS  ".devinit.data$", ".devinit.text$"
-#define CPU_INIT_SECTIONS  ".cpuinit.data$", ".cpuinit.text$"
 #define MEM_INIT_SECTIONS  ".meminit.data$", ".meminit.text$"
 
 #define EXIT_SECTIONS      ".exit.data$", ".exit.text$"
 #define DEV_EXIT_SECTIONS  ".devexit.data$", ".devexit.text$"
-#define CPU_EXIT_SECTIONS  ".cpuexit.data$", ".cpuexit.text$"
 #define MEM_EXIT_SECTIONS  ".memexit.data$", ".memexit.text$"
 
 /* init data sections */

@@ -862,15 +860,15 @@ const struct sectioncheck sectioncheck[] = {
 	.tosec   = { ALL_EXIT_SECTIONS, NULL },
 	.mismatch = DATA_TO_EXIT,
 },
-/* Do not reference init code/data from devinit/cpuinit/meminit code/data */
+/* Do not reference init code/data from devinit/meminit code/data */
 {
-	.fromsec = { DEV_INIT_SECTIONS, CPU_INIT_SECTIONS, MEM_INIT_SECTIONS, NULL },
+	.fromsec = { DEV_INIT_SECTIONS, MEM_INIT_SECTIONS, NULL },
 	.tosec   = { INIT_SECTIONS, NULL },
 	.mismatch = XXXINIT_TO_INIT,
 },
-/* Do not reference exit code/data from devexit/cpuexit/memexit code/data */
+/* Do not reference exit code/data from devexit/memexit code/data */
 {
-	.fromsec = { DEV_EXIT_SECTIONS, CPU_EXIT_SECTIONS, MEM_EXIT_SECTIONS, NULL },
+	.fromsec = { DEV_EXIT_SECTIONS, MEM_EXIT_SECTIONS, NULL },
 	.tosec   = { EXIT_SECTIONS, NULL },
 	.mismatch = XXXEXIT_TO_EXIT,
 },

@@ -1085,7 +1083,6 @@ static Elf_Sym *find_elf_symbol2(struct elf_info *elf, Elf_Addr addr,
 /*
  * Convert a section name to the function/data attribute
  * .init.text => __init
- * .cpuinit.data => __cpudata
  * .memexitconst => __memconst
  * etc.
 */

applied and git-pushed

Arjan van de Ven 	Intel Open Source Technology Centre
For development, discussion and tips for power savings, 
visit http://www.lesswatts.org

The interesting number here is the size of the cpuinit/cpuexit sections. cpuinit sections are discarded after init and cpuexit sections are discarded if you do not use modules.

Can you share these numbers with us?

Sam

those sections don't actually exist because of the linker script magic...

Arjan van de Ven 	Intel Open Source Technology Centre
For development, discussion and tips for power savings, 
visit http://www.lesswatts.org

The code exist in both the before and after case, only in the after case you cannot distingush them.
Your patch simply moves code/data from the cpuinit/exit sections to .text/.data.

So if you look at the size of the cpuinit/exit sections in the before case you get the real space saving that is lost in the embedded case after your patch is applied.
The space savings that happen after early init.

As for the complexity of especially the cpuinit/exit stuff the reason is that whoever started to use this mechnishm extended the __cpuinit case to be:

a) Code that is only used during early init and can be discarded
b) Code that is only used for CPU hotplugging

The a) case is the traditional way __fooinit is used. The b) case is usually handled by our configuration but for CPU hotplugging it was misused.

Let us please get the numbers correct before we rip it out.

Sam

[linux.trees.git]# objdump -h vmlinux.before | grep init.text
25 .init.text 00037c25 ffffffff8152a000 000000000152a000 0092a000
[inux.trees.git]# objdump -h vmlinux.after | grep init.text
25 .init.text 000316c2 ffffffff81542000 0000000001542000 00942000

So with your config we will loose a saving of 25955 bytes of text.

I tried to look a the numbers of a defconfig build here. cpuinit.text equals 0x5d97 = 23959
cpuinit.data equals 0x3574 = 13684

[objdump of vmlinux.o gives you the size of the cpuinit sections]

So if we decide to drop cpuinit then it should be based on the above figure and not the 2k figure you gave in the original mail.

Or even better we should see the difference with a typical embedded configuration and not some big defconfig build. I will assume the saving is less on a typical embedded configuration.

Sam

it will be; on x86 you end up inheriting all cpu vendors init code. on embedded you only have the dedicated CPU code.

... just that on x86... nobody seems to be in this position, it's almost impossble to have HOTPLUG_CPU even be asked.

Arjan van de Ven 	Intel Open Source Technology Centre
For development, discussion and tips for power savings, 
visit http://www.lesswatts.org

FWIW, I like the idea of removing __cpuinit. It often is quite difficult to determine if given piece of code should be marked as __cpuinit and that leads to bugs that are not so easy to resolve.

Thanks,
Rafael

Getting rid of __cpuinit would be good - I do not questions this. I only want the decision to be taken based on the correct numbers, which Arjan failed to provide initially.

I decided to try to build a typical arm target - netwinder. It had HOTPLUG disabled which is why I took this one.

objdump -h a/vmlinux.o | grep cpu

  4 .cpuinit.text 000006ac  00000000  00000000  0023e3e4  2**2
 41 .cpuinit.data 00000040  00000000  00000000  002c31a8  2**2

objdump -h a/vmlinux.o | grep text

  0 .text         00227b44  00000000  00000000  00000040  2**5
  1 .text.head    00000240  00000000  00000000  00227ba0  2**5
  2 .init.text    000150b0  00000000  00000000  00227de0  2**5
  4 .cpuinit.text 000006ac  00000000  00000000  0023e3e4  2**2
  7 .devinit.text 00005c24  00000000  00000000  0023f78c  2**2
  9 .exit.text    00001124  00000000  00000000  002453e4  2**2
 11 .meminit.text 000002d4  00000000  00000000  002482b0  2**2
 12 .devexit.text 00000348  00000000  00000000  00248584  2**2

So we see that for this config on arm we only save ~1700 bytes by the cpuinit stuff. And for data we save even less.

__cpuinit is almost not used in arm specific code so what we have here is the savings from the shared code.

Considering these numbers the gain/pain ratio is too low and I agree that the cpuinit stuff should go.

The reports that removing cpuinit/exit stuff adds a new set of warnings should just be looked at.
Except from the tricky __cpuinit cases it is almost always trivial to fix when you have understood the problem.

Sam