From mboxrd@z Thu Jan 1 00:00:00 1970 From: Peter Zijlstra Subject: Re: mmu_notifiers: turn off lockdep around mm_take_all_locks Date: Tue, 07 Jul 2009 22:00:25 +0200 Message-ID: <1246996825.5197.34.camel@laptop> References: <20090707180630.GA8008@amt.cnet> <1246990505.5197.2.camel@laptop> <4A53917C.6080208@redhat.com> <20090707183741.GA8393@amt.cnet> <1246993442.5197.15.camel@laptop> Mime-Version: 1.0 Content-Type: text/plain Content-Transfer-Encoding: 7bit Cc: Marcelo Tosatti , Avi Kivity , Andrea Arcangeli , kvm , Ingo Molnar To: Linus Torvalds Return-path: Received: from casper.infradead.org ([85.118.1.10]:42204 "EHLO casper.infradead.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754655AbZGGUAf (ORCPT ); Tue, 7 Jul 2009 16:00:35 -0400 In-Reply-To: Sender: kvm-owner@vger.kernel.org List-ID: 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(-) 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); }