Re: [rfc] lockdep: check fs reclaim recursion

[Ingo Molnar <mingo@elte.hu>]

* Nick Piggin <npiggin@suse.de> wrote:

> On Fri, Nov 28, 2008 at 01:11:27PM +0100, Ingo Molnar wrote:
> > 
> > * Nick Piggin <npiggin@suse.de> wrote:
> > 
> > > Hi,
> > > 
> > > After yesterday noticing some code in mm/filemap.c accidentally perform 
> > > a __GFP_FS allocation when it should not have been, I thought it might 
> > > be a good idea to try to catch this kind of thing with lockdep.
> > > 
> > > I coded up a little idea that seems to work. Unfortunately the system 
> > > has to actually be in __GFP_FS page reclaim, then take the lock, before 
> > > it will mark it. But at least that might still be some orders of 
> > > magnitude more common (and more debuggable) than an actual deadlock 
> > > condition, so we have some improvement I hope.
> > > 
> > > I guess we could do the same thing with __GFP_IO and even GFP_NOIO 
> > > locks too, but I don't know how expensive it is to add these 
> > > annotations to lockdep. [...]
> > 
> > Same cost as normal locking, i.e. as cheap and local as it gets. Lockdep 
> > is only expensive computationally when new rules are discovered and have 
> > to be validated - but that is rare.
> 
> OK, good... I'll think about whether it makes sense to add those locks.
> Actually, it probably makes sense to merge the __GFP_FS thing first,
> with a design that will allow further types to be added easily.
> 
>  
> > Nice feature - and we want more of this type of preventive dependency 
> > tracking - so feel free to add it whenever you run into an example like 
> > this.
> 
> Well, lockdep has most of the support with iits "recusion possibility"
> checking for interrupts. All the names in the lockdep code are geared
> completely toward interrupts, but the concept is almost exactly the same
> here (I can't think if there are any other important points in the kernel
> where similar situation can arise, but it wouldn't surprise me if there
> is). 
> 
> 
> > What merge route would you prefer? tip/core/locking would be the natural 
> > home of it (we already have a fair bit of lockdep stuff queued up there 
> > for v2.6.29) - it also touches a few FS bits.
> 
> I'm happy for you or Peter to merge yet though there, sure. Just let me 
> get some more input and then I'll try fix it up and make it merge 
> worthy :)
> 
> BTW. Do you have the might_lock annotations in there? I thought I'd see 
> them in 2.6.28, but they don't seem to be there. No problems with them?

yes, they are still there, lined up for v2.6.29. They are working fine.

	Ingo