On 21 May 2023, at 18:46, Hans Petter Selasky <hsela...@freebsd.org> wrote: > > On 5/21/23 19:05, Jessica Clarke wrote: >> On 21 May 2023, at 17:57, Hans Petter Selasky <hsela...@freebsd.org> wrote: >>> If you want to change from static structures to global symbols, then my >>> change is correct. >> Which will bloat symbol tables excessively. But you didn’t state this as >> your goal, you stated it as a behavioural change *today*, which it’s not >> (other than changing the scope). Your commit message was entirely nonsense, >> and so I told you that. If your goal is instead to allow for future changes, >> put that in your commit message. I am not a mind reader, nor is anyone else. >> It is extremely unhelpful to have commits that say one thing but intend to >> achieve a different thing. > > Hi Jess, > > To me the word "avoid" is agnostic of time. And that's why I used it there. > It doesn't mean there is a bug, but there may easily be a bug there.
I strongly disagree. Your wording heavily implied that you were avoiding something that currently occurs. > If you have time, I can add you for review more often. Let me know what you > prefer. Code review is encouraged by the project, whether from me or anyone else. > When the kernel uses dynamic linking, you end up having tons of relocation > entries in the resulting ELF file. Getting rid of symbol names doesn't stop > relocation entries from piling up. > > For example declaring a static mutex: > > At first you have the static mutex. Then the sysinit structure needs one > relocation to refer to the static mutex. Then after that the dataset > mechanism needs another relocation to refer to the sysinit structure. > > sysinit's work, but there may be better ways to do it. I am well aware of how relocations work. I am a compiler and linker developer (and co-chair of RISC-V’s psABI task group); my expertise lies in the world of linking. Relocations serve a purpose at run time. Symbols like this do not. Moreover they now risk clashing as they’re now visible outside the translation unit. For example, ib_addr.c and ib_cma.c both have static DEFINE_MUTEX(lock); and that needs to work as-is because Linux’s DEFINE_MUTEX lets you do that. So shoving a bunch of symbols into the global namespace is a non-starter. Jess >>> >>> This change also allows for: >>> >>> https://reviews.freebsd.org/D40193 >> Which looks like a mess to me. > If you have a better way in your mind to do this, then implement it, and let > me know. > > --HPS
