2021-03-29 07:19:14

by Nikitas Angelinas

Subject: [PATCH] locking/mutex: initialize osq lock in __MUTEX_INITIALIZER()

Since __MUTEX_INITIALIZER() is used on memory that is initialized to 0
anyway this change should not have an effect, but it seems better to
initialize osq explicitly for completeness, as done in other macros and
functions that initialize mutex and rwsem.

Signed-off-by: Nikitas Angelinas <[email protected]>
---
include/linux/mutex.h | 8 ++++++++
1 file changed, 8 insertions(+)

diff --git a/include/linux/mutex.h b/include/linux/mutex.h
index 515cff7..bff47f8 100644
--- a/include/linux/mutex.h
+++ b/include/linux/mutex.h
@@ -129,10 +129,18 @@ do { \
# define __DEP_MAP_MUTEX_INITIALIZER(lockname)
#endif

+#ifdef CONFIG_MUTEX_SPIN_ON_OWNER
+# define __OSQ_MUTEX_INITIALIZER(lockname) \
+ , .osq = OSQ_LOCK_UNLOCKED
+#else
+# define __OSQ_MUTEX_INITIALIZER(lockname)
+#endif
+
#define __MUTEX_INITIALIZER(lockname) \
{ .owner = ATOMIC_LONG_INIT(0) \
, .wait_lock = __SPIN_LOCK_UNLOCKED(lockname.wait_lock) \
, .wait_list = LIST_HEAD_INIT(lockname.wait_list) \
+ __OSQ_MUTEX_INITIALIZER(lockname) \
__DEBUG_MUTEX_INITIALIZER(lockname) \
__DEP_MAP_MUTEX_INITIALIZER(lockname) }

--
2.10.0

2021-03-29 14:55:42

by Will Deacon

Subject: Re: [PATCH] locking/mutex: initialize osq lock in __MUTEX_INITIALIZER()

On Mon, Mar 29, 2021 at 12:15:16AM -0700, Nikitas Angelinas wrote:
> Since __MUTEX_INITIALIZER() is used on memory that is initialized to 0
> anyway this change should not have an effect, but it seems better to
> initialize osq explicitly for completeness, as done in other macros and
> functions that initialize mutex and rwsem.
>
> Signed-off-by: Nikitas Angelinas <[email protected]>
> ---
> include/linux/mutex.h | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
> diff --git a/include/linux/mutex.h b/include/linux/mutex.h
> index 515cff7..bff47f8 100644
> --- a/include/linux/mutex.h
> +++ b/include/linux/mutex.h
> @@ -129,10 +129,18 @@ do { \
> # define __DEP_MAP_MUTEX_INITIALIZER(lockname)
> #endif
>
> +#ifdef CONFIG_MUTEX_SPIN_ON_OWNER
> +# define __OSQ_MUTEX_INITIALIZER(lockname) \
> + , .osq = OSQ_LOCK_UNLOCKED
> +#else
> +# define __OSQ_MUTEX_INITIALIZER(lockname)
> +#endif
> +
> #define __MUTEX_INITIALIZER(lockname) \
> { .owner = ATOMIC_LONG_INIT(0) \
> , .wait_lock = __SPIN_LOCK_UNLOCKED(lockname.wait_lock) \
> , .wait_list = LIST_HEAD_INIT(lockname.wait_list) \
> + __OSQ_MUTEX_INITIALIZER(lockname) \

You don't need the lockname parameter for this macro.

Will

2021-04-14 23:11:26

by Nikitas Angelinas

Subject: Re: [PATCH] locking/mutex: initialize osq lock in __MUTEX_INITIALIZER()

On Mon, Mar 29, 2021 at 03:50:56PM +0100, Will Deacon wrote:
> On Mon, Mar 29, 2021 at 12:15:16AM -0700, Nikitas Angelinas wrote:
> > Since __MUTEX_INITIALIZER() is used on memory that is initialized to 0
> > anyway this change should not have an effect, but it seems better to
> > initialize osq explicitly for completeness, as done in other macros and
> > functions that initialize mutex and rwsem.
> >
> > Signed-off-by: Nikitas Angelinas <[email protected]>
> > ---
> > include/linux/mutex.h | 8 ++++++++
> > 1 file changed, 8 insertions(+)
> >
> > diff --git a/include/linux/mutex.h b/include/linux/mutex.h
> > index 515cff7..bff47f8 100644
> > --- a/include/linux/mutex.h
> > +++ b/include/linux/mutex.h
> > @@ -129,10 +129,18 @@ do { \
> > # define __DEP_MAP_MUTEX_INITIALIZER(lockname)
> > #endif
> >
> > +#ifdef CONFIG_MUTEX_SPIN_ON_OWNER
> > +# define __OSQ_MUTEX_INITIALIZER(lockname) \
> > + , .osq = OSQ_LOCK_UNLOCKED
> > +#else
> > +# define __OSQ_MUTEX_INITIALIZER(lockname)
> > +#endif
> > +
> > #define __MUTEX_INITIALIZER(lockname) \
> > { .owner = ATOMIC_LONG_INIT(0) \
> > , .wait_lock = __SPIN_LOCK_UNLOCKED(lockname.wait_lock) \
> > , .wait_list = LIST_HEAD_INIT(lockname.wait_list) \
> > + __OSQ_MUTEX_INITIALIZER(lockname) \
>
> You don't need the lockname parameter for this macro.
>
> Will

Hi,

Please excuse this late reply.

I included the unnecessary lockname parameter as the counterpart macro in
__RWSEM_INITIALIZER(), __RWSEM_OPT_INIT() and also __RWSEM_COUNT_INIT() do the
same thing, thinking that was done on purpose, e.g. so that all macros used take
a parameter in order to satisfy some dubious notion of symmetry; I realize this
is not a good reason, of course.

I'll send a v2, possibly in a series with changes to the aforementioned bits in
rwsem, fwiw.


Cheers,
Nikitas