Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Sender: Nikitas Angelinas <nikitasangelinas@gmail.com>
Date:   Wed, 14 Apr 2021 03:26:21 -0700
From:   Nikitas Angelinas <nikitas.angelinas@gmail.com>
To:     Will Deacon <will@kernel.org>,
        Peter Zijlstra <peterz@infradead.org>,
        Ingo Molnar <mingo@redhat.com>
Cc:     Waiman Long <longman@redhat.com>,
        Boqun Feng <boqun.feng@gmail.com>, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] locking/mutex: initialize osq lock in
 __MUTEX_INITIALIZER()
Message-ID: <YHbDTX9XNQYZ0UZl@vostro>
References: <YGF+hDytO8r9Iv2o@vostro>
 <20210329145055.GC4203@willie-the-truck>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20210329145055.GC4203@willie-the-truck>
User-Agent: Mutt/2.0.6 (98f8cb83) (2021-03-06)
Precedence: bulk

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@gmail.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