From nobody Sun May 21 19:13:09 2023 X-Original-To: dev-commits-src-all@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 4QPVbX41nXz4CLy5; Sun, 21 May 2023 19:13:12 +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 4QPVbX0NQSz3xXh; Sun, 21 May 2023 19:13:12 +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 0C8192600D2; Sun, 21 May 2023 21:13:09 +0200 (CEST) Message-ID: Date: Sun, 21 May 2023 21:13:09 +0200 List-Id: Commit messages for all branches of the src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-all List-Help: List-Post: List-Subscribe: List-Unsubscribe: Sender: owner-dev-commits-src-all@freebsd.org X-BeenThere: dev-commits-src-all@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> From: Hans Petter Selasky In-Reply-To: <26465813-3B51-4E52-9E9D-F93A0F2AF6BD@freebsd.org> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-Rspamd-Queue-Id: 4QPVbX0NQSz3xXh 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 20:02, Jessica Clarke wrote: > On 21 May 2023, at 18:46, Hans Petter Selasky wrote: >> >> On 5/21/23 19:05, Jessica Clarke wrote: >>> On 21 May 2023, at 17:57, Hans Petter Selasky 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