2017-08-08 13:26 GMT+09:00 Nicholas Piggin <npiggin@xxxxxxxxx>: > On Tue, 8 Aug 2017 12:42:18 +0900 > Masahiro Yamada <yamada.masahiro@xxxxxxxxxxxxx> wrote: > >> Hi Nicholas, >> >> 2017-08-08 12:19 GMT+09:00 Nicholas Piggin <npiggin@xxxxxxxxx>: >> > On Tue, 8 Aug 2017 10:53:56 +0900 >> > Masahiro Yamada <yamada.masahiro@xxxxxxxxxxxxx> wrote: >> > >> >> Hi Nicholas, >> >> >> >> 2017-07-26 21:46 GMT+09:00 Nicholas Piggin <npiggin@xxxxxxxxx>: >> >> > The .data and .bss sections were modified in the generic linker script to >> >> > pull in sections named .data.<C identifier>, which are generated by gcc with >> >> > -ffunction-sections and -fdata-sections options. >> >> > >> >> > The problem with this pattern is it can also match section names that Linux >> >> > defines explicitly, e.g., .data.unlikely. This can cause Linux sections to >> >> > get moved into the wrong place. >> >> > >> >> > The way to avoid this is to use ".." separators for explicit section names >> >> > (the dot character is valid in a section name but not a C identifier). >> >> > However currently there are sections which don't follow this rule, so for >> >> > now just disable the wild card by default. >> >> > >> >> > Example: http://marc.info/?l=linux-arm-kernel&m=150106824024221&w=2 >> >> > >> >> > Cc: <stable@xxxxxxxxxxxxxxx> # 4.9 >> >> > Fixes: b67067f1176df ("kbuild: allow archs to select link dead code/data elimination") >> >> > Signed-off-by: Nicholas Piggin <npiggin@xxxxxxxxx> >> >> > --- >> >> > include/asm-generic/vmlinux.lds.h | 38 ++++++++++++++++++++++++++------------ >> >> > 1 file changed, 26 insertions(+), 12 deletions(-) >> >> > >> >> > diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h >> >> > index da0be9a8d1de..9623d78f8494 100644 >> >> > --- a/include/asm-generic/vmlinux.lds.h >> >> > +++ b/include/asm-generic/vmlinux.lds.h >> >> > @@ -60,6 +60,22 @@ >> >> > #define ALIGN_FUNCTION() . = ALIGN(8) >> >> > >> >> > /* >> >> > + * LD_DEAD_CODE_DATA_ELIMINATION option enables -fdata-sections, which >> >> > + * generates .data.identifier sections, which need to be pulled in with >> >> > + * .data. We don't want to pull in .data..other sections, which Linux >> >> > + * has defined. Same for text and bss. >> >> > + */ >> >> > +#ifdef CONFIG_LD_DEAD_CODE_DATA_ELIMINATION >> >> > +#define TEXT_MAIN .text .text.[0-9a-zA-Z_]* >> >> > +#define DATA_MAIN .data .data.[0-9a-zA-Z_]* >> >> > +#define BSS_MAIN .bss .bss.[0-9a-zA-Z_]* >> >> > +#else >> >> > +#define TEXT_MAIN .text >> >> > +#define DATA_MAIN .data >> >> > +#define BSS_MAIN .bss >> >> > +#endif >> >> >> >> >> >> From the git-log, I think you are planning to rename >> >> .data.unlikely -> .data..unlikey >> >> then the problem will be fixed. >> >> >> >> ("git grep data.unlikely" gave me only 7 instances.) >> >> >> >> >> >> Why is this patch necessary? >> >> >> >> >> >> You are trying to migrate some architectures to DCDE, >> >> so this #ifdef does not solve the problem, I think. >> > >> > Well basically I want to preserve previous behaviour and avoid >> > regressions as far as possible. Patches to rename existing >> > sections I think would not be so suitable for @stable. And >> > there could be other things we've missed. >> > >> > Yes I would like to ensure all our sections use .. separators >> > which (hopefully). Until then I think this is a minimal fix. >> > >> >> >> After you rename the sections, will you revert this patch? >> >> I think the #ifdef CONFIG_LD_DEAD_CODE_DATA_ELIMINATION >> should not be permanent. > > Once we have the code reasonably well used on a number of archs, > I think we should remove the ifdefs, yes we definitely should be > moving toward sharing common code wherever possible. > OK, I will pick up this for fixes. Thanks. -- Best Regards Masahiro Yamada