mmu_notifiers: turn off lockdep around mm_take_all_locks

Commit Message

Peter Zijlstra July 7, 2009, 8 p.m. UTC

On Tue, 2009-07-07 at 12:25 -0700, Linus Torvalds wrote:
> 
> On Tue, 7 Jul 2009, Peter Zijlstra wrote:
> > 
> > Another issue, at about >=256 vmas we'll overflow the preempt count. So
> > disabling lockdep will only 'fix' this for a short while, until you've
> > bloated beyond that ;-)
> 
> We would? 
> 
> I don't think so. Sure, we'd "overflow" into the softirq bits, but it's 
> all designed to faile very gracefully. Somebody who tests our "status" 
> might think we're in softirq context, but that really doesn't matter: we 
> still have preemption disabled.

Right, it might confuse the softirq (and when we extend the vma limit
and go wild maybe the hardirq) state.

> > Linus, Ingo, any opinions?
> 
> I do think that if lockdep can't handle it, we probably should turn it off 
> around it.
> 
> I don't think it's broken wrt regular preempt, though.

It does feel slightly weird to explicitly overflow that preempt count
though.

Hmm, the CONFIG_DEBUG_PREEMPT bits in kernel/sched.c
{sub,add}_preempt_count() will generate some splats though.

But sure, something like the below would disable lockdep for the
critical bits.. really don't like it though, but the alternative is
modifying the rmap locking and I don't like that either :/

---
 mm/mmap.c |    5 +++++
 1 files changed, 5 insertions(+), 0 deletions(-)



--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Andrea Arcangeli July 7, 2009, 11:30 p.m. UTC | #1

On Tue, Jul 07, 2009 at 10:00:25PM +0200, Peter Zijlstra wrote:
> It does feel slightly weird to explicitly overflow that preempt count
> though.

That is actually fixable there without adding more bits to preempt
count, just call a global preempt_disable after lockdep_off and call a
spinlock version that doesn't preempt_disable (or preempt_enable after
every spin_lock..).

> But sure, something like the below would disable lockdep for the
> critical bits.. really don't like it though, but the alternative is
> modifying the rmap locking and I don't like that either :/

Once we add that, I guess we can also remove the spin_lock_nest_lock.
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/mm/mmap.c b/mm/mmap.c
index 34579b2..cb7110e 100644
--- a/mm/mmap.c
+++ b/mm/mmap.c
@@ -2400,6 +2400,7 @@  int mm_take_all_locks(struct mm_struct *mm)
 
 	mutex_lock(&mm_all_locks_mutex);
 
+	lockdep_off()
 	for (vma = mm->mmap; vma; vma = vma->vm_next) {
 		if (signal_pending(current))
 			goto out_unlock;
@@ -2417,6 +2418,8 @@  int mm_take_all_locks(struct mm_struct *mm)
 	ret = 0;
 
 out_unlock:
+	lockdep_on();
+
 	if (ret)
 		mm_drop_all_locks(mm);
 
@@ -2470,12 +2473,14 @@  void mm_drop_all_locks(struct mm_struct *mm)
 	BUG_ON(down_read_trylock(&mm->mmap_sem));
 	BUG_ON(!mutex_is_locked(&mm_all_locks_mutex));
 
+	lockdep_off();
 	for (vma = mm->mmap; vma; vma = vma->vm_next) {
 		if (vma->anon_vma)
 			vm_unlock_anon_vma(vma->anon_vma);
 		if (vma->vm_file && vma->vm_file->f_mapping)
 			vm_unlock_mapping(vma->vm_file->f_mapping);
 	}
+	lockdep_on();
 
 	mutex_unlock(&mm_all_locks_mutex);
 }