Date: Sun, 21 May 2023 18:57:47 +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: <21c9532e-4ca7-a7fe-1ff6-07a94cbad6ab@freebsd.org> In-Reply-To: <54EF67D8-2A79-4EAB-8EFB-232F14FFE792@freebsd.org> References: <202305211621.34LGLsup059861@gitrepo.freebsd.org> <54EF67D8-2A79-4EAB-8EFB-232F14FFE792@freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On 5/21/23 18:33, Jessica Clarke wrote: > On 21 May 2023, at 17:21, Hans Petter Selasky <hselasky@FreeBSD.org> wrote: >> >> The branch main has been updated by hselasky: >> >> URL: https://cgit.FreeBSD.org/src/commit/?id=805d759338a2be939fffc8bf3f25cfaab981a9be >> >> commit 805d759338a2be939fffc8bf3f25cfaab981a9be >> Author: Hans Petter Selasky <hselasky@FreeBSD.org> >> AuthorDate: 2023-05-21 11:25:28 +0000 >> Commit: Hans Petter Selasky <hselasky@FreeBSD.org> >> CommitDate: 2023-05-21 16:20:16 +0000 >> >> mlx4: Move DEFINE_MUTEX() outside function body. >> >> Move static mutex declaration outside function body, to avoid global >> variables being declared on the stack, when using SYSINITs. > > What? This is nonsense. It’s not on the stack either way round. > > Please revert this. > > Jess Hi Jess, I think this is a false positive of yours. You need to look through all the macros used there. Basically DEFINE_MUTEX() expands to a bunch of structures, which are not in any block. The "static" you see in patch just covers the first mutex structure. SYSINITs use "static" in front of all structure definitions. If you want to change from static structures to global symbols, then my change is correct. Before: static DEFINE_MUTEX(xxx); Expands to something like: static struct yyy xxx; static struct sysinit zzz; .... If you want to change from "static struct sysinit zzz;" to "extern struct sysinit zzz" and also initialize the structure there, then that won't work, based on what I currently know about C-programming. I tried, but clang gave me a warning about it. You can't declare global variables inside a function or it is not good style. From what I can see, this location is the only place I've come accross in the FreeBSD kernel, where a SYSINIT() is used inside a function, and I thought I would just move that outside the function instead. This change also allows for: https://reviews.freebsd.org/D40193 --HPS
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?21c9532e-4ca7-a7fe-1ff6-07a94cbad6ab>