Date: Sun, 21 May 2023 19:02:55 +0100 From: Jessica Clarke <jrtc27@freebsd.org> To: Hans Petter Selasky <hselasky@freebsd.org> Cc: "src-committers@freebsd.org" <src-committers@FreeBSD.org>, "dev-commits-src-all@freebsd.org" <dev-commits-src-all@FreeBSD.org>, "dev-commits-src-main@freebsd.org" <dev-commits-src-main@FreeBSD.org> Subject: Re: git: 805d759338a2 - main - mlx4: Move DEFINE_MUTEX() outside function body. Message-ID: <26465813-3B51-4E52-9E9D-F93A0F2AF6BD@freebsd.org> In-Reply-To: <e802dfc0-0a16-add1-cf5d-a8811b3bd3e6@freebsd.org> References: <202305211621.34LGLsup059861@gitrepo.freebsd.org> <54EF67D8-2A79-4EAB-8EFB-232F14FFE792@freebsd.org> <21c9532e-4ca7-a7fe-1ff6-07a94cbad6ab@freebsd.org> <3066464F-E4C6-4B84-ADFF-E578AFAFE622@freebsd.org> <e802dfc0-0a16-add1-cf5d-a8811b3bd3e6@freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On 21 May 2023, at 18:46, Hans Petter Selasky <hselasky@freebsd.org> = wrote: >=20 > On 5/21/23 19:05, Jessica Clarke wrote: >> On 21 May 2023, at 17:57, Hans Petter Selasky <hselasky@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=E2=80=99t = state this as your goal, you stated it as a behavioural change *today*, = which it=E2=80=99s 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. >=20 > Hi Jess, >=20 > 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. >=20 > For example declaring a static mutex: >=20 > 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. >=20 > 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=E2=80=99s 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=E2=80=99re 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=E2=80=99s DEFINE_MUTEX lets you do = that. So shoving a bunch of symbols into the global namespace is a = non-starter. Jess >>>=20 >>> This change also allows for: >>>=20 >>> 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. >=20 > --HPS
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?26465813-3B51-4E52-9E9D-F93A0F2AF6BD>