Re: [rfc] lockdep: check fs reclaim recursion

Ingo Molnar
2008-11-28T12:27:15Z

* 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