All of lore.kernel.org
 help / color / mirror / Atom feed
From: Darren Hart <dvhart@linux.intel.com>
To: Maarten Lankhorst <m.b.lankhorst@gmail.com>
Cc: Thomas Gleixner <tglx@linutronix.de>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	Steven Rostedt <rostedt@goodmis.org>
Subject: Re: rt_mutex: restore wait_lock init in __rt_mutex_init
Date: Wed, 27 Jul 2011 08:43:40 -0700	[thread overview]
Message-ID: <4E30322C.5000309@linux.intel.com> (raw)
In-Reply-To: <4E3021E6.30409@gmail.com>



On 07/27/2011 07:34 AM, Maarten Lankhorst wrote:
> On 07/27/2011 11:37 AM, Thomas Gleixner wrote:
>> On Wed, 27 Jul 2011, Darren Hart wrote:
>>
>>> Without the raw_spin_lock_init(), the wait_lock does not get properly
>>> initialized with CONFIG_DEBUG_SPINLOCK. This can manifest in a BUG() in the
>>> futex requeue_pi path when the pi_state->pi_mutex->wait_lock fails the magic
>>> test in rt_mutex_start_proxy_lock()->raw_spin_lock(&lock->wait_lock).
>> That's actively wrong. You reinitialize the lock for all other cases
>> which call this via rt_mutex_init(). There is a reason why I moved the
>> spin lock initializer out of __rt_mutex_init() into
>> rt_mutex_init().

Ah... I didn't notice the move. I saw the change to the line after which
add _raw and passed the wait_lock and thought it was expected that that
call did the init (which it doesn't).

>> The lock name stuff for lockdep ends up to be
>> "lock->wait_lock" for all rt_mutexes, which is pretty useless when you
>> have to analyze a lockdep splat. Thanks for finding it nevertheless.

Right, makes sense.

>> So the correct fix is:
>>
>> Index: linux-2.6/kernel/rtmutex.c
>> ===================================================================
>> --- linux-2.6.orig/kernel/rtmutex.c
>> +++ linux-2.6/kernel/rtmutex.c
>> @@ -1296,7 +1296,7 @@ EXPORT_SYMBOL_GPL(__rt_mutex_init);
>>  void rt_mutex_init_proxy_locked(struct rt_mutex *lock,
>>  				struct task_struct *proxy_owner)
>>  {
>> -	__rt_mutex_init(lock, NULL);
>> +	rt_mutex_init(lock);

So obvious now in hindsight :)

>>  	debug_rt_mutex_proxy_lock(lock, proxy_owner);
>>  	rt_mutex_set_owner(lock, proxy_owner);
>>  	rt_mutex_deadlock_account_lock(lock, proxy_owner);
>>
>>  
> Seems to work. I no longer get a warning from pulseaudio either.

I still don't understand how pulseaudio ever caused this.

> Also darren, at least on fedora 15 glibc supports requeue_pi.
> support for that is in nptl/sysdeps/unix/sysv/linux/x86_64/pthread_cond_*.S

Duh, sorry, wasn't thinking straight. The requeue_pi support went in a
while ago - what is missing is PI aware condvars which complete
requeue_pi support.

Thanks!

-- 
Darren Hart
Intel Open Source Technology Center
Yocto Project - Linux Kernel

      reply	other threads:[~2011-07-27 15:43 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-07-27  8:18 rt_mutex: restore wait_lock init in __rt_mutex_init Darren Hart
2011-07-27  9:37 ` Thomas Gleixner
2011-07-27 14:34   ` Maarten Lankhorst
2011-07-27 15:43     ` Darren Hart [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4E30322C.5000309@linux.intel.com \
    --to=dvhart@linux.intel.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=m.b.lankhorst@gmail.com \
    --cc=rostedt@goodmis.org \
    --cc=tglx@linutronix.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.