Re: git: 805d759338a2 - main - mlx4: Move DEFINE_MUTEX() outside function body.

On 21 May 2023, at 23:13, Hans Petter Selasky <hselasky@freebsd.org> wrote:
> 
> On 5/21/23 23:37, Hans Petter Selasky wrote:
>> DEFINE_MUTEX is Linux’s API, implemented in LinuxKPI. So Linux’s behaviour is absolutely relevant. Any deviation from Linux’s semantics is an incompatibility that requires patching any sources that are built for FreeBSD using LinuxKPI. It is generally best to minimise that.
> 
> Jess, listen:
> 
> 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.
> 
> 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’re giving yet another reason here why it isn’t needed for the changes you’re 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.
> 
> 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’s time.

Jess

> # grep -r DEFINE_MUTEX * | grep -vE ":static " | grep -vE ":DEFINE_MUTEX" | wc -l
>      57
> 
> # grep -r DEFINE_MUTEX * | wc -l
>    1293
> 
> --HPS