Date: Sun, 21 May 2023 23:37:13 +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: <ac11ff8e-eeb9-a7e6-28d3-da5c9fad6567@freebsd.org> In-Reply-To: <2BF2F7CF-82E6-4D98-800D-C35D7C29E948@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> <ffb3f6ea-ef74-ebe3-884d-da567defd2e6@freebsd.org> <2BF2F7CF-82E6-4D98-800D-C35D7C29E948@freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On 5/21/23 21:41, Jessica Clarke wrote:
>> 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.
Hi Jess,
> I am aware of all of this. Please do not talk to me like I am an idiot. Your tone here is extremely patronising.
I can not know what you know before you tell me. I'm not a mind reader
and I for sure don't think you are an idiot. Those are in such case your
own words about yourself and not mine, to make it clear.
>> 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.
> You could. But that’s a pretty gross hack IMO, and depending on how you do it may still not be unique. Not to mention it’s going to further bloat the symbol tables with these long names.
Why do you think I don't know what a hack is?
In the case about the "lock" name being global:
- My solution is to pass a define to the compiler to help make the
identifier unique across the monotolithic kernel build.
- When building the LINT target, the linker will fail if there are
duplicate global symbols. Then the debug information output by the
linker will tell the file and line. Go there and change the identifier.
It should be rare. Problem solved.
- All symbols being made global for the sake of processing them, are
listed by name. And after the final sorted sysinit table is created and
linked, all those symbol names can be stripped away from the resulting
binary, and then only the relocations remain, if I'm not mistaken. I
didn't do that in my differential review, because it is a prototype. The
argument about symbol table bloat, because of prefixing or suffixing
global symbols is not valid the way I see it.
>
> Given the sysinit subsystem still needs to be able to merge in new sysinits registered dynamically, one might as well just do the sort dynamically, it’s very little added complexity on top, especially when sorting functions are just a libkern call away. Much less complexity than all this scripting to generate tables.
Do you agree about the following statement or not?
If you have two already sorted lists and a pointer to each of them, then:
Method a) place the two lists after each other in memory and then call
qsort() on the resulting list, is relatively slow.
Method b) look at the next element from each list, and store the
smallest one into the destination list, and repeat until the end of both
source lists are reached, is relatively much faster.
>
>> 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 ...
> DEFINE_MUTEX is Linux’s API, implemented in LinuxKPI. So Linux’s behaviour is absolutely relevant. Any deviation from Linux’s semantics is an incompatibility that requires patching any sources that are built for FreeBSD using LinuxKPI. It is generally best to minimise that.
One side of this is compile time issues. If a developer needs to change
something to make the code compile for FreeBSD, that's acceptable. And
then the developer can also say the code was made for FreeBSD and not
Linux, which is good with regards to the GPLv2 license, and not just
copying solutions from Linux or being force to implement things exactly
like on Linux, so-called bug-by-bug compliance.
The other side is runtime issues. Here the goal should be to minimise
issues to zero and try to outperform Linux.
> So, once again, I am asking you to revert your change.
I still cannot see why this change should be reverted.
Does the following code example and compiler warning explain why this
change should remain:
# cat << EOF > test.c
int
main()
{
extern char string[];
char string[] = { "test" };
return (0);
}
EOF
# cc -O2 -Wall test.c
test.c:11:7: error: non-extern declaration of 'string' follows extern
declaration
char string[] = { "test" };
^
test.c:10:14: note: previous declaration is here
extern char string[];
Like my commit message says, you cannot declare global variables on the
stack. When variables are on the stack, they are inside a function body.
There is no stack outside any C-functions. It's just another way to say
it and what is the problem about that?
--HPS
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?ac11ff8e-eeb9-a7e6-28d3-da5c9fad6567>