*_pagetable_setup_[start,done] crap ? - Kernel

This is a discussion on *_pagetable_setup_[start,done] crap ? - Kernel ; Hi, while investigating following section mismatch warning: WARNING: arch/x86/mm/built-in.o(.text+0xf2): Section mismatch in reference from the function paravirt_pagetable_setup_start() to the function .init.text:native_pagetable_setup_start() I have found something with looks quite strange for me. This section mismatch is really easy to fix, as ...

+ Reply to Thread
Results 1 to 3 of 3

Thread: *_pagetable_setup_[start,done] crap ?

  1. *_pagetable_setup_[start,done] crap ?


    Hi,

    while investigating following section mismatch warning:
    WARNING: arch/x86/mm/built-in.o(.text+0xf2): Section mismatch in reference from the function paravirt_pagetable_setup_start() to the function
    .init.text:native_pagetable_setup_start()

    I have found something with looks quite strange for me. This section mismatch is really easy to fix, as paravirt_pagetable_setup_start() is used by __init pagetable_init().
    paravirt_pagetable_setup_start() (body in include/asm-x86/pgtable_32.h) calls, #ifndef CONFIG_PARAVIRT, __init native_pagetable_setup_start(). Here everything is OK and case is clear: to fix that mismatch, we need to annotate paravirt_pagetable_setup_start() with __init.

    Same situation applies to paravirt_pagetable_setup_done(), but here it calls __init native_pagetable_setup_done(). On the other hand native_pagetable_setup_done() is quite strange:
    void __init native_pagetable_setup_done(pgd_t *base)
    {
    }

    This ,,change'' was introduced by commit: 551889a6e2a24a9c06fd453ea03b57b7746ffdc0 (x86: construct 32-bit boot time page tables in native format.), diff:
    void __init native_pagetable_setup_done(pgd_t *base)
    {
    -#ifdef CONFIG_X86_PAE
    - /*
    - * Add low memory identity-mappings - SMP needs it when
    - * starting up on an AP from real-mode. In the non-PAE
    - * case we already have these mappings through head.S.
    - * All user-space mappings are explicitly cleared after
    - * SMP startup.
    - */
    - set_pgd(&base[0], base[USER_PTRS_PER_PGD]);
    -#endif
    }

    native_pagetable_setup_done() is used in __init pagetable_init() through paravirt_pagetable_setup_done(), nothing really scary.
    One can take a look at arch/x86/kernel/paravirt.c:
    struct pv_mmu_ops pv_mmu_ops = {
    #ifndef CONFIG_X86_64
    .pagetable_setup_start = native_pagetable_setup_start,
    .pagetable_setup_done = native_pagetable_setup_done,
    #endif
    So when paravirt_pagetable_setup_done() will be called while CONFIG_PARAVIRT set it will be exactly same function as aravirt_pagetable_setup_done() while CONFIG_PARAVIRT not set.

    Ohh... and prototypes are wrong:
    diff --git a/include/asm-x86/pgtable_32.h b/include/asm-x86/pgtable_32.h
    index 6cbc520..2167879 100644
    --- a/include/asm-x86/pgtable_32.h
    +++ b/include/asm-x86/pgtable_32.h
    @@ -204,8 +204,8 @@ 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) __init;
    +extern void native_pagetable_setup_done(pgd_t *base) __init;

    For me it looks like a complete crap Maybe it's OK, can anybody convince me?

    -Jacek
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  2. Re: *_pagetable_setup_[start,done] crap ?


    * Jacek Luczak wrote:

    > Ohh... and prototypes are wrong:
    > diff --git a/include/asm-x86/pgtable_32.h b/include/asm-x86/pgtable_32.h
    > index 6cbc520..2167879 100644
    > --- a/include/asm-x86/pgtable_32.h
    > +++ b/include/asm-x86/pgtable_32.h
    > @@ -204,8 +204,8 @@ 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) __init;
    > +extern void native_pagetable_setup_done(pgd_t *base) __init;


    while generally it's nice to add 'extern' to prototypes, it's not an
    outright bug to not have it. Also, __init is a must-have in the
    definition only - it should not be added to the prototype.

    Ingo
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  3. Re: *_pagetable_setup_[start,done] crap ?

    Ingo Molnar pisze:
    > * Jacek Luczak wrote:
    >
    >> Ohh... and prototypes are wrong:
    >> diff --git a/include/asm-x86/pgtable_32.h b/include/asm-x86/pgtable_32.h
    >> index 6cbc520..2167879 100644
    >> --- a/include/asm-x86/pgtable_32.h
    >> +++ b/include/asm-x86/pgtable_32.h
    >> @@ -204,8 +204,8 @@ 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) __init;
    >> +extern void native_pagetable_setup_done(pgd_t *base) __init;

    >
    > while generally it's nice to add 'extern' to prototypes, it's not an
    > outright bug to not have it. Also, __init is a must-have in the
    > definition only - it should not be added to the prototype.
    >


    Sounds reasonably. Sorry for calling something crap. When I slept well it looks fine and clear for me.

    -Jacek
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

+ Reply to Thread