On Thu, 14 Nov 2024, Masahiro Yamada wrote: > > > The _stext symbol is intended to reference the start of the text section. > > > However, it currently relies on a fragile link order because the existing > > > EXPORT(_stext) resides within the .text section, which is not guaranteed > > > to be placed first. > > > > Umm, arch/mips/kernel/head.S does mean to be linked first. We rely on it > > for environments where there's no entry point is available and execution > > starts from the beginning of the image. See the comment right below your > > change. > > > > > Move the _stext definition to the linker script to enforce an explicit > > > ordering. > > > > So if you say that the link order is fragile (which it may well be), then > > that problem has to be fixed instead, likely with the linker script too, > > and then perhaps an ASSERT placed there to verify that it has worked and > > `_stext' refers to the beginning, taking into account what follows too. > > > arch/mips/kernel/head.S is always passed as the first object > in the link command because it is listed in scripts/head-object-list.txt > > What you missed to understand is, the .text section of the first object > is NOT guaranteed to be placed at the start of the image. I know how linker scripts work, thank you very much. However there was nothing to understand from the commit description as hardly any has been given. > Assume, we pass 3 objects, head.o, foo.o, bar.o to the linker > in this order. > > - head.o contains a .text section > - foo.o contains .text and .text.hot sections > - bar.o contains .text and .text.hot sections > > > The output will contain the sections in this order: > foo.o#.text.hot > bar.o#.text.hot > head.o#.text > foo.o#.text > bar.o#.text > > > This result comes from the fact that TEXT_MAIN > is not necessarily placed first. > > See the macro in include/asm-generic/vmlinux.lds.h > > #define TEXT_TEXT \ > ALIGN_FUNCTION(); \ > *(.text.hot .text.hot.*) \ > *(TEXT_MAIN .text.fixup) \ > *(.text.unlikely .text.unlikely.*) \ > *(.text.unknown .text.unknown.*) \ It corresponds to what I suggested in the second paragraph of my previous reply, which I retained quoted above for your convenience. The reviewer is not required to give the submitter a complete solution, but rather some guidance for the submitter to find the correct solution themselves. > BTW, "head.o must be passed to the linker as the first object" > is a bad convention in old days. > If you expect the entry point at the beginning of the kernel image, > it must be marked as __HEAD, which is placed in the .head.text section. > > See commit ce697ccee1a8 I do not disagree, however such details need to be given in the change description. The purpose of the change description is not to repeat in the texual form what the patch does, because everyone can see it, but to give rationale and any reasoning beyond the change being made, especially when fiddling with something that has worked for 30 years now. It's not that the old is always good, but at least it has proved in the field, so you need to convince the reviewer why the new is more suitable. > Well-maintained architectures got rid of > stupid "head.o must be passed first" requirement: > > - 2348e6bf4421 > - 994b7ac1697b > - 5353fff29e42 > > If MIPS migrates to the cleaner __HEAD solution, > it will be appreciated, but this is another story. Patches are welcome. Maciej
