Re: lockdep warning with LTP dio test (v2.6.24-rc6-125-g5356f66)

[Chris Mason <chris.mason@oracle.com>]

On Mon, 14 Jan 2008 18:06:09 +0100
Jan Kara <jack@suse.cz> wrote:

> On Wed 02-01-08 12:42:19, Zach Brown wrote:
> > Erez Zadok wrote:
> > > Setting: ltp-full-20071031, dio01 test on ext3 with Linus's
> > > latest tree. Kernel w/ SMP, preemption, and lockdep configured.
> > 
> > This is a real lock ordering problem.  Thanks for reporting it.
> > 
> > The updating of atime inside sys_mmap() orders the mmap_sem in the
> > vfs outside of the journal handle in ext3's inode dirtying:
> > 

[ lock inversion traces ]
 
> > Two fixes come to mind:
> > 
> > 1) use something like Peter's ->mmap_prepare() to update atime
> > before acquiring the mmap_sem.
> > ( http://lkml.org/lkml/2007/11/11/97 ).  I don't know if this would
> > leave more paths which do a journal_start() while holding the
> > mmap_sem.
> > 
> > 2) rework ext3's dio to only hold the jbd handle in
> > ext3_get_block(). Chris has a patch for this kicking around
> > somewhere but I'm told it has problems exposing old blocks in
> > ordered data mode.
> > 
> > Does anyone have preferences?  I could go either way.  I certainly
> > don't like the idea of journal handles being held across the
> > entirety of fs/direct-io.c.  It's yet another case of O_DIRECT
> > differing wildly from the buffered path :(.
>   I've looked more into it and I think that 2) is the only way to go
> since transaction start ranks below page lock (standard buffered
> write path) and page lock ranks below mmap_sem. So we have at least
> one more dependency mmap_sem must go before transaction start...

Just to clarify a little bit:

If ext3's DIO code only touches transactions in get_block, then it can
violate data=ordered rules.  Basically the transaction that allocates
the blocks might commit before the DIO code gets around to writing them.

A crash in the wrong place will expose stale data on disk.

-chris