From nobody Sun May 21 21:37:13 2023 X-Original-To: dev-commits-src-main@mlmmj.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mlmmj.nyi.freebsd.org (Postfix) with ESMTP id 4QPYnn6P45z4CWFW; Sun, 21 May 2023 21:37:17 +0000 (UTC) (envelope-from hselasky@freebsd.org) Received: from mail.turbocat.net (turbocat.net [88.99.82.50]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id 4QPYnn1VVbz4JSY; Sun, 21 May 2023 21:37:17 +0000 (UTC) (envelope-from hselasky@freebsd.org) Authentication-Results: mx1.freebsd.org; none Received: from [10.36.2.145] (unknown [46.212.121.255]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by mail.turbocat.net (Postfix) with ESMTPSA id C2EFC260D48; Sun, 21 May 2023 23:37:15 +0200 (CEST) Message-ID: Date: Sun, 21 May 2023 23:37:13 +0200 List-Id: Commit messages for the main branch of the src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-main List-Help: List-Post: List-Subscribe: List-Unsubscribe: Sender: owner-dev-commits-src-main@freebsd.org X-BeenThere: dev-commits-src-main@freebsd.org MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; FreeBSD amd64; rv:102.0) Gecko/20100101 Thunderbird/102.10.1 Subject: Re: git: 805d759338a2 - main - mlx4: Move DEFINE_MUTEX() outside function body. Content-Language: en-US To: Jessica Clarke Cc: "src-committers@freebsd.org" , "dev-commits-src-all@freebsd.org" , "dev-commits-src-main@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> <26465813-3B51-4E52-9E9D-F93A0F2AF6BD@freebsd.org> <2BF2F7CF-82E6-4D98-800D-C35D7C29E948@freebsd.org> From: Hans Petter Selasky In-Reply-To: <2BF2F7CF-82E6-4D98-800D-C35D7C29E948@freebsd.org> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-Rspamd-Queue-Id: 4QPYnn1VVbz4JSY X-Spamd-Bar: ---- X-Spamd-Result: default: False [-4.00 / 15.00]; REPLY(-4.00)[]; ASN(0.00)[asn:24940, ipnet:88.99.0.0/16, country:DE] X-Rspamd-Pre-Result: action=no action; module=replies; Message is reply to one we originated X-ThisMailContainsUnwantedMimeParts: N 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