Re: git: 805d759338a2 - main - mlx4: Move DEFINE_MUTEX() outside function body.
- In reply to: Mark Millard : "RE: git: 805d759338a2 - main - mlx4: Move DEFINE_MUTEX() outside function body."
- Go to: [ bottom of page ] [ top of archives ] [ this month ]
Date: Sun, 21 May 2023 17:44:24 UTC
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 : > >> 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. > > DEFINE_MUTEX is from/for linux related code: > > # 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) \ > > and looks like: > > #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. >> >> 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; .... > > 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 >> 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 >> > > === Mark Millard marklmi at yahoo.com