From: Ingo Molnar Subject: Re: [PATCH] fix pgd_lock deadlock Date: Wed, 16 Feb 2011 11:28:01 +0100 Message-ID: <20110216102801.GA23082@elte.hu> References: <20110215190710.GL5935@random.random> <20110215195450.GO5935@random.random> <20110215225234.GP5935@random.random> <20110215231733.GR5935@random.random> <1297850294.2413.30.camel@twins> <20110216101521.GU5935@random.random> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Return-path: Content-Disposition: inline In-Reply-To: <20110216101521.GU5935@random.random> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xensource.com Errors-To: xen-devel-bounces@lists.xensource.com To: Andrea Arcangeli Cc: Jeremy Fitzhardinge , "Xen-devel@lists.xensource.com" , Ian Campbell , Peter Zijlstra , the arch/x86 maintainers , Hugh Dickins , Linux Kernel Mailing List , Jan Beulich , Andi Kleen , Andrew Morton , Johannes Weiner , "H. Peter Anvin" , Thomas Gleixner , Larry Woodman List-Id: xen-devel@lists.xenproject.org * Andrea Arcangeli wrote: > On Wed, Feb 16, 2011 at 10:58:14AM +0100, Peter Zijlstra wrote: > > On Wed, 2011-02-16 at 00:17 +0100, Andrea Arcangeli wrote: > > > On Wed, Feb 16, 2011 at 12:03:30AM +0100, Thomas Gleixner wrote: > > > > I assume you run it with a lockdep enabled kernel as well, right ? > > > > > > Yes, I always run with lockdep and prove locking enabled on my test > > > box, not sure how it's meant to trigger more bugs in this case, the > > > debug check that should be relevant for this is DEBUG_VM and that is > > > enabled too of course. I didn't try DEBUG_PAGEALLOC yet. > > > > I think what Thomas tried to tell you is that your > > VM_BUG_ON(in_interrupt()) is fully redundant if you have lockdep > > enabled. > > > > Lockdep will warn you if a !irqsave lock is taken from IRQ context, > > since that is a clear inversion problem. > > Ah I get it now, but I prefer to have it on an all my builds, and > I don't keep lockdep on for all builds (but I keep DEBUG_VM on). It's > still only debug code that no production system will ever deal with, > so it should be good to exercise it in more than on debug .config > considering it's very low overhead (pgd_lock is never taken in fast > paths) so it's suitable for a VM_BUG_ON. The point both Thomas and Peter tried to point out to you, that adding 7 instances of redundant debug checks: + VM_BUG_ON(in_interrupt()); + VM_BUG_ON(in_interrupt()); + VM_BUG_ON(in_interrupt()); + VM_BUG_ON(in_interrupt()); + VM_BUG_ON(in_interrupt()); + VM_BUG_ON(in_interrupt()); + VM_BUG_ON(in_interrupt()); to arch/x86/ is not acceptable, for the reasons stated. Please remove it from the patch. Thanks, Ingo