From: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
To: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>,
Mike Galbraith <bitbucket@online.de>,
Frederic Weisbecker <fweisbec@gmail.com>,
LKML <linux-kernel@vger.kernel.org>,
RT <linux-rt-users@vger.kernel.org>,
"Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
Subject: Re: [PATCH v2] rtmutex: take the waiter lock with irqs off
Date: Fri, 22 Nov 2013 14:59:31 +0100 [thread overview]
Message-ID: <20131122135931.GA8698@linutronix.de> (raw)
In-Reply-To: <20131118141021.GA10022@twins.programming.kicks-ass.net>
* Peter Zijlstra | 2013-11-18 15:10:21 [+0100]:
>Its get_next_timer_interrupt() that does a trylock() and only for
>PREEMPT_RT_FULL.
>
>Also; on IRC you said:
>
> "<bigeasy> I'm currently not sure if we should do
>the _irq() lock or a trylock for the wait_lock in
>rt_mutex_slowtrylock()"
>
>Which I misread and dismissed -- but yes that might actually work too
>and would be a much smaller patch. You'd only need trylock and unlock.
Yes and the patch is at the end of the mail. I also added your "lockdep:
Correctly annotate hardirq context in irq_exit()" to the -RT queue and
saw the splat. After the trylock replacement I needed something similar
for the unlock path _or_ lockdep would complain. So unless there is a
better way…
>That said, allowing such usage from actual IRQ context is iffy; suppose
>the trylock succeeds, who then is the lock owner?
>
>I suppose it would be whatever task we interrupted and boosting will
>'work' because we're non-preemptable, but still *YUCK*.
You are right, this ain't pretty. I hope this timer mess can be replaced
with something else so the whole trylock thingy can be removed.
Sebastian
--- a/include/linux/spinlock_rt.h
+++ b/include/linux/spinlock_rt.h
@@ -22,6 +22,7 @@ extern void __lockfunc rt_spin_lock(spin
extern unsigned long __lockfunc rt_spin_lock_trace_flags(spinlock_t *lock);
extern void __lockfunc rt_spin_lock_nested(spinlock_t *lock, int subclass);
extern void __lockfunc rt_spin_unlock(spinlock_t *lock);
+extern void __lockfunc rt_spin_try_unlock(spinlock_t *lock);
extern void __lockfunc rt_spin_unlock_wait(spinlock_t *lock);
extern int __lockfunc rt_spin_trylock_irqsave(spinlock_t *lock, unsigned long *flags);
extern int __lockfunc rt_spin_trylock_bh(spinlock_t *lock);
--- a/kernel/rtmutex.c
+++ b/kernel/rtmutex.c
@@ -816,10 +816,8 @@ static void noinline __sched rt_spin_lo
/*
* Slow path to release a rt_mutex spin_lock style
*/
-static void noinline __sched rt_spin_lock_slowunlock(struct rt_mutex *lock)
+static void __sched __rt_spin_lock_slowunlock(struct rt_mutex *lock)
{
- raw_spin_lock(&lock->wait_lock);
-
debug_rt_mutex_unlock(lock);
rt_mutex_deadlock_account_unlock(current);
@@ -838,6 +836,21 @@ static void noinline __sched rt_spin_lo
rt_mutex_adjust_prio(current);
}
+static void noinline __sched rt_spin_lock_slowunlock(struct rt_mutex *lock)
+{
+ raw_spin_lock(&lock->wait_lock);
+ __rt_spin_lock_slowunlock(lock);
+}
+
+static void noinline __sched rt_spin_lock_try_slowunlock(struct rt_mutex *lock)
+{
+ int ret;
+
+ ret = raw_spin_trylock(&lock->wait_lock);
+ BUG_ON(!ret);
+ __rt_spin_lock_slowunlock(lock);
+}
+
void __lockfunc rt_spin_lock(spinlock_t *lock)
{
rt_spin_lock_fastlock(&lock->lock, rt_spin_lock_slowlock);
@@ -868,6 +881,13 @@ void __lockfunc rt_spin_unlock(spinlock_
}
EXPORT_SYMBOL(rt_spin_unlock);
+void __lockfunc rt_spin_try_unlock(spinlock_t *lock)
+{
+ /* NOTE: we always pass in '1' for nested, for simplicity */
+ spin_release(&lock->dep_map, 1, _RET_IP_);
+ rt_spin_lock_fastunlock(&lock->lock, rt_spin_lock_try_slowunlock);
+}
+
void __lockfunc __rt_spin_unlock(struct rt_mutex *lock)
{
rt_spin_lock_fastunlock(lock, rt_spin_lock_slowunlock);
@@ -1191,7 +1211,8 @@ rt_mutex_slowtrylock(struct rt_mutex *lo
{
int ret = 0;
- raw_spin_lock(&lock->wait_lock);
+ if (!raw_spin_trylock(&lock->wait_lock))
+ return ret;
init_lists(lock);
if (likely(rt_mutex_owner(lock) != current)) {
--- a/kernel/timer.c
+++ b/kernel/timer.c
@@ -1400,7 +1400,7 @@ unsigned long get_next_timer_interrupt(u
expires = base->next_timer;
}
#ifdef CONFIG_PREEMPT_RT_FULL
- rt_spin_unlock(&base->lock);
+ rt_spin_try_unlock(&base->lock);
#else
spin_unlock(&base->lock);
#endif
next prev parent reply other threads:[~2013-11-22 13:59 UTC|newest]
Thread overview: 39+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-10-31 14:07 CONFIG_NO_HZ_FULL + CONFIG_PREEMPT_RT_FULL = nogo Mike Galbraith
2013-11-06 17:49 ` Thomas Gleixner
2013-11-07 3:26 ` Mike Galbraith
2013-11-07 4:31 ` Mike Galbraith
2013-11-07 11:21 ` Thomas Gleixner
2013-11-07 12:59 ` Frederic Weisbecker
2013-11-07 13:13 ` Thomas Gleixner
2013-11-12 8:06 ` Mike Galbraith
2013-11-12 9:28 ` Thomas Gleixner
2013-11-15 16:30 ` [PATCH] rtmutex: take the waiter lock with irqs off Sebastian Andrzej Siewior
2013-11-15 20:14 ` [PATCH v2] " Sebastian Andrzej Siewior
2013-11-18 14:10 ` Peter Zijlstra
2013-11-18 17:56 ` Peter Zijlstra
2013-11-18 23:59 ` Frederic Weisbecker
2013-11-19 8:30 ` Peter Zijlstra
2013-11-22 13:59 ` Sebastian Andrzej Siewior [this message]
2013-11-22 16:08 ` Peter Zijlstra
2013-11-22 16:21 ` Sebastian Andrzej Siewior
2013-11-22 17:44 ` Sebastian Andrzej Siewior
2013-11-25 18:33 ` Paul Gortmaker
2013-11-07 13:07 ` CONFIG_NO_HZ_FULL + CONFIG_PREEMPT_RT_FULL = nogo Mike Galbraith
2013-12-20 15:41 ` Sebastian Andrzej Siewior
2013-12-21 9:11 ` Mike Galbraith
2013-12-21 17:21 ` Muli Baron
2013-12-22 4:17 ` Mike Galbraith
2013-12-22 5:10 ` Mike Galbraith
2013-12-22 5:37 ` Mike Galbraith
2013-12-22 5:16 ` Mike Galbraith
2013-11-08 3:23 ` Paul E. McKenney
2013-11-08 7:31 ` Mike Galbraith
2013-11-08 12:37 ` Paul E. McKenney
2013-11-08 13:26 ` Mike Galbraith
2013-11-08 14:03 ` Paul E. McKenney
2013-11-08 14:21 ` Mike Galbraith
2013-11-08 14:29 ` Frederic Weisbecker
2013-11-08 14:45 ` Paul E. McKenney
2013-11-08 16:03 ` Frederic Weisbecker
2013-11-08 14:53 ` Mike Galbraith
2013-11-08 16:04 ` Frederic Weisbecker
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=20131122135931.GA8698@linutronix.de \
--to=bigeasy@linutronix.de \
--cc=bitbucket@online.de \
--cc=fweisbec@gmail.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-rt-users@vger.kernel.org \
--cc=paulmck@linux.vnet.ibm.com \
--cc=peterz@infradead.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).