From: Hugh Dickins <hugh@veritas.com>
To: Ingo Molnar <mingo@elte.hu>
Cc: Andrew Morton <akpm@osdl.org>,
Nick Piggin <nickpiggin@yahoo.com.au>,
linux-kernel@vger.kernel.org
Subject: [PATCH] break_lock forever broken
Date: Fri, 11 Mar 2005 18:51:48 +0000 (GMT) [thread overview]
Message-ID: <Pine.LNX.4.61.0503111847450.9320@goblin.wat.veritas.com> (raw)
lock->break_lock is set when a lock is contended, but cleared only in
cond_resched_lock. Users of need_lockbreak (journal_commit_transaction,
copy_pte_range, unmap_vmas) don't necessarily use cond_resched_lock on it.
So, if the lock has been contended at some time in the past, break_lock
remains set thereafter, and the fastpath keeps dropping lock unnecessarily.
Hanging the system if you make a change like I did, forever restarting a
loop before making any progress.
Should it be cleared when contending to lock, just the other side of the
cpu_relax loop? No, that loop is preemptible, we don't want break_lock
set all the while the contender has been preempted. It should be cleared
when we unlock - any remaining contenders will quickly set it again.
So cond_resched_lock's spin_unlock will clear it, no need for it to do
that; and use need_lockbreak there too, preferring optimizer to #ifdefs.
Or would you prefer the few need_lockbreak users to clear it in advance?
Less overhead, more errorprone.
Signed-off-by: Hugh Dickins <hugh@veritas.com>
--- 2.6.11-bk7/kernel/sched.c 2005-03-11 13:33:09.000000000 +0000
+++ linux/kernel/sched.c 2005-03-11 17:46:50.000000000 +0000
@@ -3753,14 +3753,11 @@ EXPORT_SYMBOL(cond_resched);
*/
int cond_resched_lock(spinlock_t * lock)
{
-#if defined(CONFIG_SMP) && defined(CONFIG_PREEMPT)
- if (lock->break_lock) {
- lock->break_lock = 0;
+ if (need_lockbreak(lock)) {
spin_unlock(lock);
cpu_relax();
spin_lock(lock);
}
-#endif
if (need_resched()) {
_raw_spin_unlock(lock);
preempt_enable_no_resched();
--- 2.6.11-bk7/kernel/spinlock.c 2005-03-02 07:38:52.000000000 +0000
+++ linux/kernel/spinlock.c 2005-03-11 17:46:50.000000000 +0000
@@ -163,6 +163,8 @@ void __lockfunc _write_lock(rwlock_t *lo
EXPORT_SYMBOL(_write_lock);
+#define _stop_breaking(lock)
+
#else /* CONFIG_PREEMPT: */
/*
@@ -250,12 +252,19 @@ BUILD_LOCK_OPS(spin, spinlock);
BUILD_LOCK_OPS(read, rwlock);
BUILD_LOCK_OPS(write, rwlock);
+#define _stop_breaking(lock) \
+ do { \
+ if ((lock)->break_lock) \
+ (lock)->break_lock = 0; \
+ } while (0)
+
#endif /* CONFIG_PREEMPT */
void __lockfunc _spin_unlock(spinlock_t *lock)
{
_raw_spin_unlock(lock);
preempt_enable();
+ _stop_breaking(lock);
}
EXPORT_SYMBOL(_spin_unlock);
@@ -263,6 +272,7 @@ void __lockfunc _write_unlock(rwlock_t *
{
_raw_write_unlock(lock);
preempt_enable();
+ _stop_breaking(lock);
}
EXPORT_SYMBOL(_write_unlock);
@@ -270,6 +280,7 @@ void __lockfunc _read_unlock(rwlock_t *l
{
_raw_read_unlock(lock);
preempt_enable();
+ _stop_breaking(lock);
}
EXPORT_SYMBOL(_read_unlock);
@@ -278,6 +289,7 @@ void __lockfunc _spin_unlock_irqrestore(
_raw_spin_unlock(lock);
local_irq_restore(flags);
preempt_enable();
+ _stop_breaking(lock);
}
EXPORT_SYMBOL(_spin_unlock_irqrestore);
@@ -286,6 +298,7 @@ void __lockfunc _spin_unlock_irq(spinloc
_raw_spin_unlock(lock);
local_irq_enable();
preempt_enable();
+ _stop_breaking(lock);
}
EXPORT_SYMBOL(_spin_unlock_irq);
@@ -294,6 +307,7 @@ void __lockfunc _spin_unlock_bh(spinlock
_raw_spin_unlock(lock);
preempt_enable();
local_bh_enable();
+ _stop_breaking(lock);
}
EXPORT_SYMBOL(_spin_unlock_bh);
@@ -302,6 +316,7 @@ void __lockfunc _read_unlock_irqrestore(
_raw_read_unlock(lock);
local_irq_restore(flags);
preempt_enable();
+ _stop_breaking(lock);
}
EXPORT_SYMBOL(_read_unlock_irqrestore);
@@ -310,6 +325,7 @@ void __lockfunc _read_unlock_irq(rwlock_
_raw_read_unlock(lock);
local_irq_enable();
preempt_enable();
+ _stop_breaking(lock);
}
EXPORT_SYMBOL(_read_unlock_irq);
@@ -318,6 +334,7 @@ void __lockfunc _read_unlock_bh(rwlock_t
_raw_read_unlock(lock);
preempt_enable();
local_bh_enable();
+ _stop_breaking(lock);
}
EXPORT_SYMBOL(_read_unlock_bh);
@@ -326,6 +343,7 @@ void __lockfunc _write_unlock_irqrestore
_raw_write_unlock(lock);
local_irq_restore(flags);
preempt_enable();
+ _stop_breaking(lock);
}
EXPORT_SYMBOL(_write_unlock_irqrestore);
@@ -334,6 +352,7 @@ void __lockfunc _write_unlock_irq(rwlock
_raw_write_unlock(lock);
local_irq_enable();
preempt_enable();
+ _stop_breaking(lock);
}
EXPORT_SYMBOL(_write_unlock_irq);
@@ -342,6 +361,7 @@ void __lockfunc _write_unlock_bh(rwlock_
_raw_write_unlock(lock);
preempt_enable();
local_bh_enable();
+ _stop_breaking(lock);
}
EXPORT_SYMBOL(_write_unlock_bh);
next reply other threads:[~2005-03-11 18:58 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2005-03-11 18:51 Hugh Dickins [this message]
2005-03-12 4:34 ` Andrew Morton
2005-03-12 23:20 ` Hugh Dickins
2005-03-13 8:23 ` Arjan van de Ven
2005-03-13 9:35 ` Hugh Dickins
2005-03-13 13:52 ` Arjan van de Ven
2005-03-14 5:01 ` Nick Piggin
2005-03-14 7:02 ` Ingo Molnar
2005-03-14 8:03 ` Nick Piggin
2005-03-14 8:14 ` Ingo Molnar
2005-03-14 8:24 ` Nick Piggin
2005-03-14 8:34 ` Arjan van de Ven
2005-03-14 8:43 ` Nick Piggin
2005-03-14 10:46 ` Ingo Molnar
2005-03-14 11:01 ` Nick Piggin
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=Pine.LNX.4.61.0503111847450.9320@goblin.wat.veritas.com \
--to=hugh@veritas.com \
--cc=akpm@osdl.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mingo@elte.hu \
--cc=nickpiggin@yahoo.com.au \
/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
all inboxes | Powered by JetHome®