Date: Sun, 21 May 2023 10:44:24 -0700 From: Mark Millard <marklmi@yahoo.com> To: dev-commits-src-main@freebsd.org Subject: Re: git: 805d759338a2 - main - mlx4: Move DEFINE_MUTEX() outside function body. Message-ID: <333A3097-9FD1-4FE2-B7D1-5CE85F208C82@yahoo.com> In-Reply-To: <C375C14D-C185-415A-A2B4-82123F1EA5FF@yahoo.com> References: <C375C14D-C185-415A-A2B4-82123F1EA5FF@yahoo.com>
next in thread | previous in thread | raw e-mail | index | archive | help
On May 21, 2023, at 10:14, Mark Millard <marklmi@yahoo.com> wrote: > Hans Petter Selasky <hselasky_at_freebsd.org> wrote on > Date: Sun, 21 May 2023 16:57:47 UTC : >=20 >> On 5/21/23 18:33, Jessica Clarke wrote: >>> On 21 May 2023, at 17:21, Hans Petter Selasky <hselasky@FreeBSD.org> = wrote: >>>>=20 >>>> The branch main has been updated by hselasky: >>>>=20 >>>> URL: = https://cgit.FreeBSD.org/src/commit/?id=3D805d759338a2be939fffc8bf3f25cfaa= b981a9be >>>>=20 >>>> 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 >>>>=20 >>>> mlx4: Move DEFINE_MUTEX() outside function body. >>>>=20 >>>> Move static mutex declaration outside function body, to avoid = global >>>> variables being declared on the stack, when using SYSINITs. >>>=20 >>> What? This is nonsense. It=E2=80=99s not on the stack either way = round. >>>=20 >>> Please revert this. >>>=20 >>> Jess >>=20 >> Hi Jess, >>=20 >> I think this is a false positive of yours. You need to look through = all=20 >> the macros used there. >>=20 >> Basically DEFINE_MUTEX() expands to a bunch of structures, which are = not=20 >> in any block. >=20 > DEFINE_MUTEX is from/for linux related code: >=20 > # grep -r "define.*DEFINE_MUTEX" /usr/main-src/sys/ | more > /usr/main-src/sys/compat/linuxkpi/common/include/linux/mutex.h:#define = DEFINE_MUTEX(lock) \ >=20 > and looks like: >=20 > #define DEFINE_MUTEX(lock) = \ > mutex_t lock; = \ > SX_SYSINIT_FLAGS(lock, &(lock).sx, mutex_name(#lock), SX_DUPOK) Never mind: starting with mutex_t was not essential. >> The "static" you see in patch just covers the first mutex structure. >>=20 >> SYSINITs use "static" in front of all structure definitions. >> If you want to change from static structures to global symbols, then = my=20 >> change is correct. >>=20 >> Before: >>=20 >> static DEFINE_MUTEX(xxx); >>=20 >> Expands to something like: >>=20 >> static struct yyy xxx; static struct sysinit zzz; .... >=20 > Not true. Never mind: starting with mutex_t was not essential, despite it not having a "struct" explicitly. Sorry for the noise. >> If you want to change from "static struct sysinit zzz;" to "extern=20 >> struct sysinit zzz" and also initialize the structure there, then = that=20 >> won't work, based on what I currently know about C-programming. I = tried,=20 >> but clang gave me a warning about it. >>=20 >>=20 >>=20 >> You can't declare global variables inside a function or it is not = good=20 >> style. >>=20 >>=20 >>=20 >> =46rom what I can see, this location is the only place I've come = accross=20 >> in the FreeBSD kernel, where a SYSINIT() is used inside a function, = and=20 >> I thought I would just move that outside the function instead. >>=20 >> This change also allows for: >>=20 >> https://reviews.freebsd.org/D40193 >>=20 >=20 >=20 =3D=3D=3D Mark Millard marklmi at yahoo.com
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?333A3097-9FD1-4FE2-B7D1-5CE85F208C82>