Date: Sun, 21 May 2023 21:13:09 +0200 From: Hans Petter Selasky <hselasky@freebsd.org> To: Jessica Clarke <jrtc27@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: <ffb3f6ea-ef74-ebe3-884d-da567defd2e6@freebsd.org> In-Reply-To: <26465813-3B51-4E52-9E9D-F93A0F2AF6BD@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> <26465813-3B51-4E52-9E9D-F93A0F2AF6BD@freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On 5/21/23 20:02, Jessica Clarke wrote:
> On 21 May 2023, at 18:46, Hans Petter Selasky <hselasky@freebsd.org> wrote:
>>
>> 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’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.
Hi Jess,
Thanks for sharing your view. I know some words in English have multiple
meanings, and should not be used, because the receiver may pick a
different interpretation than you intended at the time of writing. The
way you react makes me think "avoid" is one more of those words. I have
a list of do-not-use words already. And it grows from time to time.
Maybe an automated tool exists, to analyze texts for frequent multiple
meanings, not just spell-checking.
>> 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.
OK, no problem.
>
>> 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.
Yes, that is correct for Linux (and possibly other OS'es), but not for
FreeBSD! Maybe I can take this opportunity to give you some insight.
In the Linux kernel there may be multiple C- and object files resulting
from compilation, sharing the same basename. Linux and FreeBSD both
compile C-files into object files by substituting the ".c" suffix by
".o" (or something very similar). It depends a bit.
In FreeBSD there are two modes of compilation:
1) Monotolith: /boot/kernel/kernel
2) Modules: /boot/kernel/*.ko
If there is a kernel module, there is also a corresponding configuration
keyword to include that kernel module into the monotolithic kernel. See
for example "sys/amd64/conf/GENERIC". There are very few exceptions. The
LINT target builds almost all sources into a monotolithic kernel.
One difference between Linux and FreeBSD when doing the monotolithic
kernel build: FreeBSD puts all object files resulting from compilation
into the same object directory. This implies there cannot be two object
files sharing the same name as a result of compilation. This in turn
implies all source file names are unique across the whole of "sys/*",
because converting a source file name into the corresponding object file
name, is simply done by changing the suffix of the file in question.
If you are worried multiple DEFINE_MUTEX(lock) will result in multiple
global symbols having the same "lock" name, all you need to do is pass
through the ${.TARGET} variable from "make" as a define, stripping a few
invalid characters, and macro-concat that to the locally generated
global variable name, and you are all good.
Your comment is correct based on your prior knowledge about Linux and
compilers. But at this point FreeBSD is different. That's why porting
code from Linux to FreeBSD is sometimes difficult, because you need to
keep track of how source files are renamed. You cannot just use "meld
Linux/blah FreeBSD/blah" to compare directories ...
--HPS
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?ffb3f6ea-ef74-ebe3-884d-da567defd2e6>