Re: git: cff79fd02636 - main - linuxkpi: Fix spin_lock_init

From: John Baldwin <jhb_at_FreeBSD.org>
Date: Fri, 17 May 2024 16:07:53 UTC
On 5/16/24 10:59 PM, Emmanuel Vadot wrote:
> The branch main has been updated by manu:
> 
> URL: https://cgit.FreeBSD.org/src/commit/?id=cff79fd02636f34010d8b835cc9e55401fa76e74
> 
> commit cff79fd02636f34010d8b835cc9e55401fa76e74
> Author:     Emmanuel Vadot <manu@FreeBSD.org>
> AuthorDate: 2024-05-17 04:52:53 +0000
> Commit:     Emmanuel Vadot <manu@FreeBSD.org>
> CommitDate: 2024-05-17 05:58:59 +0000
> 
>      linuxkpi: Fix spin_lock_init
>      
>      Some linux code re-init some spinlock so add MTX_NEW to mtx_init.
>      
>      Reported by:    David Wolfskill <david@catwhisker.org>
>      Fixes:          ae38a1a1bfdf ("linuxkpi: spinlock: Simplify code")
> ---
>   sys/compat/linuxkpi/common/include/linux/spinlock.h | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/sys/compat/linuxkpi/common/include/linux/spinlock.h b/sys/compat/linuxkpi/common/include/linux/spinlock.h
> index 3f6eb4bb70f6..2992e41c9c02 100644
> --- a/sys/compat/linuxkpi/common/include/linux/spinlock.h
> +++ b/sys/compat/linuxkpi/common/include/linux/spinlock.h
> @@ -140,7 +140,7 @@ typedef struct mtx spinlock_t;
>   #define	spin_lock_name(name)		_spin_lock_name(name, __FILE__, __LINE__)
>   
>   #define	spin_lock_init(lock)	mtx_init(lock, spin_lock_name("lnxspin"), \
> -				  NULL, MTX_DEF | MTX_NOWITNESS)
> +				  NULL, MTX_DEF | MTX_NOWITNESS | MTX_NEW)
>   
>   #define	spin_lock_destroy(_l)	mtx_destroy(_l)

This is only ok because of MTX_NOWITNESS.  Reiniting locks without destroying
them corrupts the internal linked lists in WITNESS for locks using witness.
That may warrant a comment here explaining why we disable witness.

It might be nice to add an extension to the various lock inits for code that
wants to opt-int to using WITNESS where a name can be passed.  Using those would
be relatively small diffs in the client code and let select locks opt into
using WITNESS.  You could make it work by adding an optional second argument
to spin_lock_init, etc. that takes the name.

-- 
John Baldwin