Date: Sun, 21 May 2023 23:32:22 +0100 From: Jessica Clarke <jrtc27@freebsd.org> To: Hans Petter Selasky <hselasky@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: <1787C08C-AD82-48EC-92E2-49360B1AB8B0@freebsd.org> In-Reply-To: <8f46512a-f086-6da0-2fdd-3ffc89f4dfdd@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> <e802dfc0-0a16-add1-cf5d-a8811b3bd3e6@freebsd.org> <26465813-3B51-4E52-9E9D-F93A0F2AF6BD@freebsd.org> <ffb3f6ea-ef74-ebe3-884d-da567defd2e6@freebsd.org> <2BF2F7CF-82E6-4D98-800D-C35D7C29E948@freebsd.org> <ac11ff8e-eeb9-a7e6-28d3-da5c9fad6567@freebsd.org> <8f46512a-f086-6da0-2fdd-3ffc89f4dfdd@freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On 21 May 2023, at 23:13, Hans Petter Selasky <hselasky@freebsd.org> = wrote: >=20 > On 5/21/23 23:37, Hans Petter Selasky wrote: >> DEFINE_MUTEX is Linux=E2=80=99s API, implemented in LinuxKPI. So = Linux=E2=80=99s behaviour is absolutely relevant. Any deviation from = Linux=E2=80=99s semantics is an incompatibility that requires patching = any sources that are built for FreeBSD using LinuxKPI. It is generally = best to minimise that. >=20 > Jess, listen: >=20 > Probably at some point, there could be an initialization macro for the = SX locks used in the LinuxKPI, being of type SX_NOWITNESS, simply = plotting in the constants needed in the sx structure, exactly like they = do under Linux. >=20 > Then neither SYSINITs, nor my patch will be needed for the sake of = DEFINE_MUTEX(). Can we wait for that? No. If anything this is another reason why your patch is unnecessary; = you=E2=80=99re giving yet another reason here why it isn=E2=80=99t = needed for the changes you=E2=80=99re proposing. > The reason for this is Linux has a bad habit of not destroying locks, = so FreeBSD cannot add any WITNESS objects for locks that are not = destroyed anyway. Most of the work done by sx_init() is putting a few = constants into various SX structure fields. Only the WITNESS part needs = execution, if you get what I mean. >=20 > This change is made so I can let others easily test another patchset. = Also this change is harmless as such. Once again I will remind you that the FreeBSD tree is not somewhere for = you to play around with experiments. Changes should be properly = justified. This one is completely unjustified today, and should remain = unjustified in future lest LinuxKPI compatibility be broken. > Looking at the whole Linux tree, the most common way is to use = DEFINE_MUTEX() outside macros and functions: Because a lot of the time, mutexes are needed in multiple functions? All = this tells me is there are 57 cases of the pattern you want to break; = i.e. Linux supports this and makes use of it. Unless, and until, there is consensus that `static DEFINE_MUTEX(...);` = within a function body should be broken by LinuxKPI, this patch does not = belong in the tree. So, for the fourth time, please revert it. I do not = want to have to escalate this further; it would be a colossal waste of = everyone=E2=80=99s time. Jess > # grep -r DEFINE_MUTEX * | grep -vE ":static " | grep -vE = ":DEFINE_MUTEX" | wc -l > 57 >=20 > # grep -r DEFINE_MUTEX * | wc -l > 1293 >=20 > --HPS
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?1787C08C-AD82-48EC-92E2-49360B1AB8B0>