linux-kernel - Re: [PATCH] locking/mutex: initialize osq lock in __MUTEX

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:   Wed, 14 Apr 2021 03:26:21 -0700
From:   Nikitas Angelinas <nikitas.angelinas@...il.com>
To:     Will Deacon <will@...nel.org>,
        Peter Zijlstra <peterz@...radead.org>,
        Ingo Molnar <mingo@...hat.com>
Cc:     Waiman Long <longman@...hat.com>,
        Boqun Feng <boqun.feng@...il.com>, linux-kernel@...r.kernel.org
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 <nikitas.angelinas@...il.com>
> > ---
> >  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