Patchworkβ [05/10] kbuild: sort the list of symbols exported by the kernel (__ksymtab)

Sam Ravnborg wrote:
> On Wed, Nov 04, 2009 at 10:00:50AM +0000, Alan Jenkins wrote:
>   
>> Rusty Russell wrote:
>>     
>>> On Tue, 3 Nov 2009 08:36:17 pm Alan Jenkins wrote:
>>>   
>>>       
>>>> +/*
>>>> + * We use CPP macros since they are more familiar than assembly macros.
>>>> + * Note that CPP macros eat newlines, so each statement must be terminated
>>>> + * by a semicolon.
>>>> + */
>>>> +
>>>> +#ifdef CONFIG_HAVE_SYMBOL_PREFIX
>>>> +#define __SYM(sym) _##sym
>>>> +#else
>>>> +#define __SYM(sym) sym
>>>> +#endif
>>>>     
>>>>         
>>> Ideally, you would used MODULE_SYMBOL_PREFIX here, but of course it's a
>>> string.  I don't think Kconfig can do arbitrary identifiers, so we can't
>>> make CONFIG_SYMBOL_PREFIX empty or _.
>>>
>>> Perhaps clarify it to a bool CONFIG_HAVE_MODULE_UNDERSCORE_PREFIX then,
>>> since that's what you're assuming here?
>>>
>>> Thanks,
>>> Rusty.
>>>       
>> I made the same assumption in patch 4.  The arch defines  
>> CONFIG_HAVE_SYMBOL_PREFIX, which then causes init/Kconfig to define  
>> CONFIG_SYMBOL_PREFIX="_".
>>
>> Mike suggested that I hack kbuild instead, to do something like
>>
>> unquote = ...
>> AFLAGS_.tmp_export-asm.o += -DSYMBOL_PREFIX=$(unquote CONFIG_SYMBOL_PREFIX)
>>
>> I'm experimenting with the idea, but I haven't managed to get it working  
>>     
>
> Something like this:
> unquote = $(patsubst "%",%,$($1))
>
> AFLAGS_.tmp_export-asm.o += -DSYMBOL_PREFIX=$(call unquote,CONFIG_SYMBOL_PREFIX)
>
> But the readability is low so we could be better off doing it direct:
>
> AFLAGS_.tmp_export-asm.o += -DSYMBOL_PREFIX=$(patsubst "%",",$(CONFIG_SYMBOL_PREFIX))
>
> 	Sam
>   

Thanks to both of you, I have something that works now. 

Unfortunately I had to export the variable AFLAGS_.tmp_export-asm.o, 
otherwise it had no effect.  I assume it's a limitation of the top-level 
Makefile. 

To make SYMBOL_PREFIX available for linker scripts, I also added it to 
"cpp_flags" in scripts/Makefile.lib.  (As far as I can see, cpp_flags is 
_only_ used for preprocessing linker scripts).

If this is all getting too brittle, I guess I could be less timid and 
add it to  the global KBUILD_CPPFLAGS instead :).

Here's the incremental diff:




--
To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

On Thu, Nov 5, 2009 at 09:24, Alan Jenkins wrote:
> Thanks to both of you, I have something that works now.
> Unfortunately I had to export the variable AFLAGS_.tmp_export-asm.o,
> otherwise it had no effect.  I assume it's a limitation of the top-level
> Makefile.
> To make SYMBOL_PREFIX available for linker scripts, I also added it to
> "cpp_flags" in scripts/Makefile.lib.  (As far as I can see, cpp_flags is
> _only_ used for preprocessing linker scripts).
>
> If this is all getting too brittle, I guess I could be less timid and add it
> to  the global KBUILD_CPPFLAGS instead :).

this stuff looks fine to me, thanks.  pushing the Blackfin stuff via
other trees as part of the set is OK in my book.
Acked-By: Mike Frysinger <vapier@gentoo.org>
-mike
--
To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

On Fri, 6 Nov 2009 12:54:10 am Alan Jenkins wrote:
> Sam Ravnborg wrote:
> > On Wed, Nov 04, 2009 at 10:00:50AM +0000, Alan Jenkins wrote:
> >   
> >> Rusty Russell wrote:
> >>     
> >>> On Tue, 3 Nov 2009 08:36:17 pm Alan Jenkins wrote:
> >>>   
> >>>       
> >>>> +/*
> >>>> + * We use CPP macros since they are more familiar than assembly macros.
> >>>> + * Note that CPP macros eat newlines, so each statement must be terminated
> >>>> + * by a semicolon.
> >>>> + */
> >>>> +
> >>>> +#ifdef CONFIG_HAVE_SYMBOL_PREFIX
> >>>> +#define __SYM(sym) _##sym
> >>>> +#else
> >>>> +#define __SYM(sym) sym
> >>>> +#endif
> >>>>     
> >>>>         
> >>> Ideally, you would used MODULE_SYMBOL_PREFIX here, but of course it's a
> >>> string.  I don't think Kconfig can do arbitrary identifiers, so we can't
> >>> make CONFIG_SYMBOL_PREFIX empty or _.
> >>>
> >>> Perhaps clarify it to a bool CONFIG_HAVE_MODULE_UNDERSCORE_PREFIX then,
> >>> since that's what you're assuming here?
> >>>
> >>> Thanks,
> >>> Rusty.
> >>>       
> >> I made the same assumption in patch 4.  The arch defines  
> >> CONFIG_HAVE_SYMBOL_PREFIX, which then causes init/Kconfig to define  
> >> CONFIG_SYMBOL_PREFIX="_".
> >>
> >> Mike suggested that I hack kbuild instead, to do something like
> >>
> >> unquote = ...
> >> AFLAGS_.tmp_export-asm.o += -DSYMBOL_PREFIX=$(unquote CONFIG_SYMBOL_PREFIX)
> >>
> >> I'm experimenting with the idea, but I haven't managed to get it working  
> >>     
> >
> > Something like this:
> > unquote = $(patsubst "%",%,$($1))
> >
> > AFLAGS_.tmp_export-asm.o += -DSYMBOL_PREFIX=$(call unquote,CONFIG_SYMBOL_PREFIX)
> >
> > But the readability is low so we could be better off doing it direct:
> >
> > AFLAGS_.tmp_export-asm.o += -DSYMBOL_PREFIX=$(patsubst "%",",$(CONFIG_SYMBOL_PREFIX))
> >
> > 	Sam
> >   
> 
> Thanks to both of you, I have something that works now. 

I've taken this, but really, it's overkill.  Changing the config to a fixed
"underscore or nothing" was simpler and demonstrably sufficient for all cases
in the last 5 years.

Thanks,
Rusty.
--
To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

The sorted ksymtab breaks ia64 (and possibly ppc64 and
parisc too).

Alex Chiang did the bisect to find this change as
the cause of the breakage.  The problem is that ia64
expects that the first item in each ksymtab entry to be
a function pointer.  The code in modpost that creates
.tmp_exports-asm.S doesn't know about types of exported
objects, so it uses __EXPORT_SYMBOL from linux/mod_export.h
for everything. This results in

        PTR SYM(sym);
        PTR SYM(__kstrtab_##sym);

which the preprocessor expands to entries like:

        .long ____pagevec_lru_add
        .long __kstrtab____pagevec_lru_add

which puts the address of the first instruction of the
function into the table, rather than the address of a
function pointer (which on ia64 is a two element data
object containing the code address and the global data
pointer).

The syntax you need for this* is:

        .long @fptr(____pagevec_lru_add)
        .long __kstrtab____pagevec_lru_add

Note that you must only use the @fptr(name) syntax for
function exports. Exported data items just need an address.

-Tony

* On ia64 ... powerpc and parisc might need something else.
--
To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Tony Luck wrote:
> The sorted ksymtab breaks ia64 (and possibly ppc64 and
> parisc too).
>
> Alex Chiang did the bisect to find this change as
> the cause of the breakage.  The problem is that ia64
> expects that the first item in each ksymtab entry to be
> a function pointer.  The code in modpost that creates
> .tmp_exports-asm.S doesn't know about types of exported
> objects, so it uses __EXPORT_SYMBOL from linux/mod_export.h
> for everything. This results in
>
>         PTR SYM(sym);
>         PTR SYM(__kstrtab_##sym);
>
> which the preprocessor expands to entries like:
>
>         .long ____pagevec_lru_add
>         .long __kstrtab____pagevec_lru_add
>
> which puts the address of the first instruction of the
> function into the table, rather than the address of a
> function pointer (which on ia64 is a two element data
> object containing the code address and the global data
> pointer).
>
> The syntax you need for this* is:
>
>         .long @fptr(____pagevec_lru_add)
>         .long __kstrtab____pagevec_lru_add
>
> Note that you must only use the @fptr(name) syntax for
> function exports. Exported data items just need an address.
>
> -Tony
>
> * On ia64 ... powerpc and parisc might need something else.
>   

Thanks!  It doesn't sound too hard to retro-fit your suggestion.

Still, I can't help wondering if I've done this all wrong :-/.  Perhaps 
I should avoid the assembler.  Instead, I could write a tool to sort the 
ksymtab elf sections in-place (and mangle their relocations 
accordingly).  That should preserve any special handling for function 
symbols without arch-specific special cases.  It would also concentrate 
all the magic in one tool - rather than it being scattered between the 
modpost tool, mod_export.h, tmp_exports.S, and vmlinux.lds.h.

Alan
--
To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html