x86: pgtable_32.h - prototype and section mismatch fixes
PATCH 1/1 x86: pgtable_32.h - prototype and section mismatch fixes
This patch adds extern to native_pagetable_setup_[start,done]() protypes and fixes following section mismatch warning:
WARNING: arch/x86/mm/built-in.o(.text+0xf2): Section mismatch in reference from the function paravirt_pagetable_setup_start()
paravirt_pagetable_setup_[start,done]() is used by __init pagetable_init(). Annotate both functions with __init.
--- pgtable_32.h | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)
include/asm-x86/pgtable_32.h +4 -4
@@ -204,16 +204,16 @@ do { \*/ #define update_mmu_cache (vma, address, pte) do { } while (0) -void native_pagetable_setup_start (pgd_t *base); -void native_pagetable_setup_done (pgd_t *base); +extern void native_pagetable_setup_start (pgd_t *base); +extern void native_pagetable_setup_done (pgd_t *base);#ifndef CONFIG_PARAVIRT -static inline void paravirt_pagetable_setup_start (pgd_t *base) +static inline void __init paravirt_pagetable_setup_start (pgd_t *base) {native_pagetable_setup_start (base); } -static inline void paravirt_pagetable_setup_done (pgd_t *base) +static inline void __init paravirt_pagetable_setup_done (pgd_t *base) {native_pagetable_setup_done (base); }
include/asm-x86/pgtable_32.h do { \ · line 214
Quoted 9 lines · Jacek Luczak
paravirt_pagetable_setup_[start,done]() is used by
__initpagetable_init(). Annotate both functions with__init.#ifndef
CONFIG_PARAVIRTinclude/asm-x86/pgtable_32.h +1
-1-static inline void paravirt_pagetable_setup_start (pgd_t *base) +static inline void __init paravirt_pagetable_setup_start (pgd_t *base) {native_pagetable_setup_start (base); }
hm, that's an interesting case: we need those annotations probably because gcc decided to not inline those functions. (this is possible via the new CONFIG_OPTIMIZE_INLINING=y option) Sam, what's your take on that?
Ingo
Quoted 6 lines · Ingo Molnar
Quoted 2 lines · Jacek Luczak
paravirt_pagetable_setup_[start,done]() is used by
__initpagetable_init(). Annotate both functions with__init.
Quoted 11 lines
include/asm-x86/pgtable_32.h do { \ · line 214
Quoted 6 lines
#ifndef
CONFIG_PARAVIRTinclude/asm-x86/pgtable_32.h +1
-1-static inline void paravirt_pagetable_setup_start (pgd_t *base) +static inline void __init paravirt_pagetable_setup_start (pgd_t *base) {native_pagetable_setup_start (base); }hm, that's an interesting case: we need those annotations probably because gcc decided to not inline those functions. (this is possible via the new
CONFIG_OPTIMIZE_INLINING=yoption) Sam, what's your take on that?
gcc uses different heuristics for inlining between the different versions. Therefore to achieve somehow predictable results I added -fno-inline-functions-called-once when CONFIG_DEBUG_SECTION_MISMATCH is enabled.
So in the above case for any normal kernel build we would see that gcc inlined the above and everything is fine.
But for the CONFIG_DEBUG_SECTION_MISMTCH cases we do not inline and thus we see that we have a section mismatch.
The rationale was that it is better to annotate a few more functions and reliable results across gcc versions than it is to see section mismatch warnings only with older but not newer gcc versions.
Sam
Quoted 10 lines · Sam Ravnborg
Quoted 4 lines
hm, that's an interesting case: we need those annotations probably because gcc decided to not inline those functions. (this is possible via the new
CONFIG_OPTIMIZE_INLINING=yoption) Sam, what's your take on that?gcc uses different heuristics for inlining between the different versions. Therefore to achieve somehow predictable results I added -fno-inline-functions-called-once when
CONFIG_DEBUG_SECTION_MISMATCHis enabled.
Quoted 4 lines
So in the above case for any normal kernel build we would see that gcc inlined the above and everything is fine. But for the
CONFIG_DEBUG_SECTION_MISMTCHcases we do not inline and thus we see that we have a section mismatch.
ah, ok. So i guess this will result in a few isolated cases of __init annotations added to inline functions - Jacek fixed one such case - but it should not result in the general spreading of __init annotations to inline functions, correct? (which i was worried about)
Ingo
Quoted 18 lines · Ingo Molnar
Quoted 10 lines · Sam Ravnborg
Quoted 4 lines
hm, that's an interesting case: we need those annotations probably because gcc decided to not inline those functions. (this is possible via the new
CONFIG_OPTIMIZE_INLINING=yoption) Sam, what's your take on that?gcc uses different heuristics for inlining between the different versions. Therefore to achieve somehow predictable results I added -fno-inline-functions-called-once when
CONFIG_DEBUG_SECTION_MISMATCHis enabled.Quoted 4 lines
So in the above case for any normal kernel build we would see that gcc inlined the above and everything is fine. But for the
CONFIG_DEBUG_SECTION_MISMTCHcases we do not inline and thus we see that we have a section mismatch.
Quoted 4 lines
ah, ok. So i guess this will result in a few isolated cases of
__initannotations added to inline functions - Jacek fixed one such case - but it should not result in the general spreading of__initannotations to inline functions, correct? (which i was worried about)
I do not think so. The need for small isolated inline functions in the init paths are minimal and last I did a section mismatch free sweep on the kernel it was only few if any inline functions(*) I had to annotate.
(*) Considering only the minimal amount of function that ought to be annotated inlined.
Sam
Sam Ravnborg pisze:
Quoted 20 lines · Ingo Molnar
Quoted 11 lines · Sam Ravnborg
Quoted 10 lines
Quoted 5 lines
hm, that's an interesting case: we need those annotations probably because gcc decided to not inline those functions. (this is possible via the new
CONFIG_OPTIMIZE_INLINING=yoption) Sam, what's your take on that?gcc uses different heuristics for inlining between the different versions. Therefore to achieve somehow predictable results I added -fno-inline-functions-called-once when
CONFIG_DEBUG_SECTION_MISMATCHis enabled.Quoted 8 lines
Quoted 4 lines
So in the above case for any normal kernel build we would see that gcc inlined the above and everything is fine. But for the
CONFIG_DEBUG_SECTION_MISMTCHcases we do not inline and thus we see that we have a section mismatch.ah, ok. So i guess this will result in a few isolated cases of
__initannotations added to inline functions - Jacek fixed one such case - but it should not result in the general spreading of__initannotations to inline functions, correct? (which i was worried about)
Quoted 7 lines
I do not think so. The need for small isolated inline functions in the init paths are minimal and last I did a section mismatch free sweep on the kernel it was only few if any inline
functions(*)I had to annotate.(*) Considering only the minimal amount of function that ought to be annotated inlined.
There's a lot of inline __init functions already - which, on the other hand should not result in more of such. Attached you can find grep on tree which shows all inline __init functions (you won't find my paravirt_pagetable_setup_* functions in output as I removed __init for test).
Sam, do -fno-inline-functions-called-once could affect paravirt_pagetable_setup_done? In general there was no section mismatch warning for this function, but I've annotated it also.
-Jacek
1225:static inline int __init ace_of_register(void)
1237:static inline int __init ace_of_register(void) { return 0; } ./drivers/block/xsysace.c
809:static inline int __init hwicap_of_register(void)
821:static inline int __init hwicap_of_register(void) { return 0; } ./drivers/char/xilinx_hwicap/xilinx_hwicap.c
195:static inline int __init./drivers/net/wan/sbni.c
776:static inline void __init fs_subset_descriptors(void)
876:static inline void __init hs_subset_descriptors(void) ./drivers/usb/gadget/ether.c
47:static inline void __init show_version (void) {./drivers/atm/ambassador.c
57:static inline void __init show_version (void) {./drivers/atm/horizon.c
66:static inline void __init set_phy_reg(struct ti_ohci *ohci, u8 addr, u8 data) 81:static inline void __init init_ohci1394_soft_reset(struct ti_ohci *ohci) { 95:static inline void __init init_ohci1394_initialize(struct ti_ohci *ohci) 167:static inline void __init init_ohci1394_wait_for_busresets(struct ti_ohci *ohci) 185:static inline void __init init_ohci1394_enable_physical_dma(struct ti_ohci *hci) 196:static inline void __init init_ohci1394_reset_and_init_dma(struct ti_ohci *ohci) 225:static inline void __init init_ohci1394_controller(int num, int slot, int func) ./drivers/ieee1394/init_ohci1394_dma.c
91:static inline int __init doccheck(void __iomem *potential, unsigned long physadr) ./drivers/mtd/devices/docprobe.c
1111:static inline int __init nftl_partscan(struct mtd_info *mtd, struct mtd_partition *parts) 1214:static inline int __init inftl_partscan(struct mtd_info *mtd, struct mtd_partition *parts) 1433:static inline int __init doc2000_init(struct mtd_info *mtd)
1450:static inline int __init doc2001_init(struct mtd_info *mtd)
1481:static inline int __init doc2001plus_init(struct mtd_info *mtd) ./drivers/mtd/nand/diskonchip.c
975:static inline int __init./drivers/parisc/pdc_stable.c
19:static inline void __init pcmcia_setup_ioctl(void) { return; } ./drivers/pcmcia/ds_internal.h
616:static inline int __init ulite_of_register(void)
628:static inline int __init ulite_of_register(void) { return 0; } ./drivers/serial/uartlite.c
480:static inline int __init xilinxfb_of_register(void)
492:static inline int __init xilinxfb_of_register(void) { return 0; } ./drivers/video/xilinxfb.c
136:static inline void __init eat(unsigned n)./init/initramfs.c
3290:static inline void __init set_pageblock_order(unsigned int order) ./mm/page_alloc.c
1086:static inline int __init./net/irda/irnet/irnet_ppp.c
352:static inline int __init probe_pas(struct address_info *hw_config) ./sound/oss/pas2_card.c
257:static inline int __init probe_v_midi(struct address_info *hw_config) ./sound/oss/v_midi.c
2071:static inline void __init unlock_ExtINT_logic(void)
2129:static inline void __init check_timer(void)./arch/x86/kernel/io_apic_32.c
493:static inline void __init construct_default_ISA_mptable(int mpc_default_type) ./arch/x86/kernel/mpparse.c
126:static inline int __init
146:static inline int __init./arch/x86/kernel/e820_64.c
1602:static inline void __init unlock_ExtINT_logic(void)
1657:static inline void __init check_timer(void)./arch/x86/kernel/io_apic_64.c
1259:static inline int __init determine_tce_table_size(u64 ram) ./arch/x86/kernel/pci-calgary_64.c
540:static inline void __init reserve_crashkernel(void) ./arch/x86/kernel/setup_32.c
259:static inline void __init reserve_crashkernel(void) ./arch/x86/kernel/setup_64.c
596:static inline int __init check_vmi_rom(struct vrom_header *rom)
663:static inline int __init probe_vmi_rom(void)
744:static inline int __init activate_vmi(void)./arch/x86/kernel/vmi_32.c
478:static inline void __init early_set_fixmap(enum fixed_addresses idx, 487:static inline void __init early_clear_fixmap(enum fixed_addresses idx) ./arch/x86/mm/ioremap.c
84:static inline void __init
96:static inline void __init./arch/alpha/kernel/smp.c
193:static inline int __init
223:static inline int __init
238:static inline void __init
245:static inline void __init
294:static inline int __init
311:static inline void __init./arch/alpha/kernel/sys_cabriolet.c
57:static inline void __init
147:static inline int __init
186:static inline int __init
205:static inline void __init
214:static inline void __init./arch/alpha/kernel/sys_sio.c
123:static inline void __init sdp2430_init_smc91x(void) ./arch/arm/mach-omap2/board-2430sdp.c
186:static inline void __init apollon_init_smc91x(void) ./arch/arm/mach-omap2/board-apollon.c
256:static inline void __init h4_init_debug(void) ./arch/arm/mach-omap2/board-h4.c
82:static inline void __init omap_serial_reset(struct plat_serial8250_port *p) ./arch/arm/mach-omap2/serial.c
254:static inline void __init setup_crashkernel(unsigned long total, int *n) 386:static inline int __init./arch/ia64/kernel/setup.c
203:static inline int __init is_shub_1_1(int nasid)
696:static inline int __init board_needs_cnode(int type) ./arch/ia64/sn/kernel/setup.c
148:static inline int __init indy_sc_probe(void)./arch/mips/mm/sc-ip22.c
55:static inline int __init mips_sc_probe(void)./arch/mips/mm/sc-mips.c
80:static inline int __init r5k_sc_probe(void)./arch/mips/mm/sc-r5k.c
270:static inline void __init./arch/parisc/kernel/processor.c
171:static inline void __init sio_write(u8 val, u8 index) ./arch/powerpc/platforms/chrp/setup.c
60:static inline int __init./arch/ppc/platforms/lopec.c
27:static inline int __init./arch/ppc/platforms/pal4_pci.c
300:static inline int __init./arch/ppc/platforms/powerpmc250.c
166:static inline void __init reserve_crashkernel(void)./arch/sh/kernel/setup.c
46:static inline void __init smp_store_cpu_info(unsigned int cpu) ./arch/sh/kernel/smp.c
777:static inline void __init gdb_vbr_init(void)./arch/sh/kernel/traps_32.c
16:static inline void __init check_bugs(void)./include/asm-mn10300/bugs.h
36:static inline void __init smpboot_setup_io_apic(void) ./include/asm-x86/mach-default/smpboot_hooks.h
Quoted 26 lines · Jacek Luczak
Sam Ravnborg pisze:
Quoted 25 lines · Ingo Molnar
Quoted 11 lines · Sam Ravnborg
Quoted 10 lines
Quoted 5 lines
hm, that's an interesting case: we need those annotations probably because gcc decided to not inline those functions. (this is possible via the new
CONFIG_OPTIMIZE_INLINING=yoption) Sam, what's your take on that?gcc uses different heuristics for inlining between the different versions. Therefore to achieve somehow predictable results I added -fno-inline-functions-called-once when
CONFIG_DEBUG_SECTION_MISMATCHis enabled.Quoted 8 lines
Quoted 4 lines
So in the above case for any normal kernel build we would see that gcc inlined the above and everything is fine. But for the
CONFIG_DEBUG_SECTION_MISMTCHcases we do not inline and thus we see that we have a section mismatch.ah, ok. So i guess this will result in a few isolated cases of
__initannotations added to inline functions - Jacek fixed one such case - but it should not result in the general spreading of__initannotations to inline functions, correct? (which i was worried about)I do not think so. The need for small isolated inline functions in the init paths are minimal and last I did a section mismatch free sweep on the kernel it was only few if any inline
functions(*)I had to annotate.
Quoted 7 lines
Quoted 2 lines
(*) Considering only the minimal amount of function that ought to be annotated inlined.
There's a lot of inline
__initfunctions already - which, on the other hand should not result in more of such. Attached you can find grep on tree which shows all inline__initfunctions (you won't find my paravirt_pagetable_setup_* functions in output as I removed__initfor test).
Most of the functionas listed should never have been annotated inline. If you filter aways the functions that are in one one of their incarnations longer than 8 lines or are located in a .c file the list is much shorter. This is the real candidates (the figure '8' is just bad way to filter away 'complex' functions).
Your paravirt_pagetable_setup_done is a real one btw.
Quoted 3 lines
Sam, do -fno-inline-functions-called-once could affect
paravirt_pagetable_setup_done? In general there was no section mismatch warning for this function, but I've annotated it also.
Most liekly yes - but I did not check.
Sam
Sam Ravnborg pisze:
Quoted 34 lines · Jacek Luczak
Quoted 26 lines
Sam Ravnborg pisze:
Quoted 25 lines · Ingo Molnar
Quoted 11 lines · Sam Ravnborg
Quoted 10 lines
Quoted 5 lines
hm, that's an interesting case: we need those annotations probably because gcc decided to not inline those functions. (this is possible via the new
CONFIG_OPTIMIZE_INLINING=yoption) Sam, what's your take on that?gcc uses different heuristics for inlining between the different versions. Therefore to achieve somehow predictable results I added -fno-inline-functions-called-once when
CONFIG_DEBUG_SECTION_MISMATCHis enabled.Quoted 8 lines
Quoted 4 lines
So in the above case for any normal kernel build we would see that gcc inlined the above and everything is fine. But for the
CONFIG_DEBUG_SECTION_MISMTCHcases we do not inline and thus we see that we have a section mismatch.ah, ok. So i guess this will result in a few isolated cases of
__initannotations added to inline functions - Jacek fixed one such case - but it should not result in the general spreading of__initannotations to inline functions, correct? (which i was worried about)I do not think so. The need for small isolated inline functions in the init paths are minimal and last I did a section mismatch free sweep on the kernel it was only few if any inline
functions(*)I had to annotate.Quoted 6 lines
Quoted 2 lines
(*) Considering only the minimal amount of function that ought to be annotated inlined.
There's a lot of inline
__initfunctions already - which, on the other hand should not result in more of such. Attached you can find grep on tree which shows all inline__initfunctions (you won't find my paravirt_pagetable_setup_* functions in output as I removed__initfor test).
Quoted 6 lines
Most of the functionas listed should never have been annotated inline. If you filter aways the functions that are in one one of their incarnations longer than 8 lines or are located in a .c file the list is much shorter. This is the real candidates (the figure '8' is just bad way to filter away 'complex' functions).
Yourparavirt_pagetable_setup_doneis a real one btw.
It's worth of closer investigation I think.
Quoted 4 lines
Quoted 3 lines
Sam, do -fno-inline-functions-called-once could affect
paravirt_pagetable_setup_done? In general there was no section mismatch warning for this function, but I've annotated it also.Most liekly yes - but I did not check.
GCC is ok, I mean it inlines paravirt_pagetable_setup_[start,done] even with CONFIG_OPTIMIZE_INLINING=y:
$ objdump -t vmlinux.o | grep pagetable_setup_start 0000a418 g F .init.text 000000a1 native_pagetable_setup_start
While running with CONFIG_DEBUG_SECTION_MISMTCH=y it does not inline paravirt_pagetable_setup_start (_done is OK):
$ objdump -t vmlinux.o | grep pagetable_setup_start 00017100 l F .text 0000000b paravirt_pagetable_setup_start 00009fb0 g F .init.text 00000089 native_pagetable_setup_start
DEBUG_SECTION_MISMATCH probably generated mismatch with smpboot_setup_io_apic() (commit: 96c968742fa1e6d64f979464acf2fd90cdc117b3, already merged). I will check and if __init is really superfluous I will post delta patch to remove all those __init annotations.
Similar situation might be found with unlock_ExtINT_logic() (arch/x86/kernel/io_apic_32.c), which I annotated with __init in one of previous patches, but here, this function IMO shouldn't be marked inline.
-Jacek
Quoted 12 lines
GCC is ok, I mean it inlines paravirt_pagetable_setup_[start,done] even with
CONFIG_OPTIMIZE_INLINING=y:$ objdump -t vmlinux.o | grep pagetable_setup_start 0000a418 g F .init.text 000000a1 native_pagetable_setup_startWhile running with
CONFIG_DEBUG_SECTION_MISMTCH=yit does not inlineparavirt_pagetable_setup_start(_doneis OK):$ objdump -t vmlinux.o | grep pagetable_setup_start 00017100 l F .text 0000000b paravirt_pagetable_setup_start 00009fb0 g F .init.text 00000089 native_pagetable_setup_start
Quoted 8 lines
DEBUG_SECTION_MISMATCHprobably generated mismatch withsmpboot_setup_io_apic()(commit:96c968742fa1e6d64f979464acf2fd90cdc117b3, already merged). I will check and if__initis really superfluous I will post delta patch to remove all those__initannotations.Similar situation might be found with
unlock_ExtINT_logic()(arch/x86/kernel/io_apic_32.c), which I annotated with__initin one of previous patches, but here, this function IMO shouldn't be marked inline.
Hi Jacek.
Please consider following code snippet:
static void __init foo() {}
static void __init bar() { foo(); }
static void __init baz() { bar(); }
Browsing this code it makes perfect sense that when baz() is annotated __init bar and suddenly foo() has same annotation.
There is no point in removing the __init annotation of bar() just becasue we happen to know it will be inlined by gcc. Removing the __init anotation is actually wrong - because the __init annotation has a two-fold purpose.
It is used to locate this function in a section that is later discarded.
But it is also used to document that this function is only used in the early init of this driver/what-ever. So using this function in non-__init code is wrong.
Therefore I recommend to keep the __init annotation also for the inline functions - assuming these functions are supposed to be used only in the early init as the annotation says they are.
Sam
Ingo Molnar pisze:
Quoted 16 lines · Sam Ravnborg
Quoted 10 lines
Quoted 5 lines
hm, that's an interesting case: we need those annotations probably because gcc decided to not inline those functions. (this is possible via the new
CONFIG_OPTIMIZE_INLINING=yoption) Sam, what's your take on that?gcc uses different heuristics for inlining between the different versions. Therefore to achieve somehow predictable results I added -fno-inline-functions-called-once when
CONFIG_DEBUG_SECTION_MISMATCHis enabled.Quoted 4 lines
So in the above case for any normal kernel build we would see that gcc inlined the above and everything is fine. But for the
CONFIG_DEBUG_SECTION_MISMTCHcases we do not inline and thus we see that we have a section mismatch.
Quoted 5 lines
ah, ok. So i guess this will result in a few isolated cases of
__initannotations added to inline functions - Jacek fixed one such case - but it should not result in the general spreading of__initannotations to inline functions, correct? (which i was worried about)
Yep, I was confused about that patch, thus I didn't sent it previously.
Hmm...I forgot that all my kernels where compiled with CONFIG_OPTIMIZE_INLINING=y. There's no mismatch warning printed while OPTIMIZE_INLINING is not set. Also I've made a fast look into objects and looks like those two functions are really inlined. I will also take a look on those functions with CONFIG_OPTIMIZE_INLINING=y && ONFIG_DEBUG_SECTION_MISMATCH=n.
-Jacek