Re: kjournald() with DIO

[Mingming Cao <cmm@us.ibm.com>]

On Mon, 2005-09-12 at 17:29 -0700, Andrew Morton wrote:
> Badari Pulavarty <pbadari@us.ibm.com> wrote:
> >
> > Didn't help. How can this help ?
> 
> Oh well.  Different bug.  See http://bugzilla.kernel.org/show_bug.cgi?id=4964
> 
> > DIO code is kicking back to buffered mode, since its trying to
> > fill-in the holes. The fix you suggested might have helped, if
> > IO is beyond file size or file size not ending on page boundary.
> > What am I missing ?
> 
> Have you spoken with Mingming about this?  She's chasing a similar race. 
> She's currently working on getting the ext3-debug patch working so we can
> find out how those buffers (which apparently aren't even attached to the
> journal) came to have an elevated refcount.
> 
Hi Andrew, Badari was looking at the race with me and he came up with
some hack patch to "remember" who are the last two caller to get_bh(),
and once we found a buffer whose reference count is >1 but the journal
head is NULL, it start trace this bufferhead.(dump who is the last two
caller to get_bh())

The analysis results shows that there are race between
journal_try_to_free_buffers() and kjournald.  It possible that when
kjournald is waiting for a buffer unlocked, it released the journal-
>j_list_lock(commit.c, line 398), but it is still holding a reference
count on that buffer while wait_on_buffer().  This allowed the
__journal_try_to_free_buffer() to proceed (which is waiting for the
j_list_lock and will unlink the journal head from the buffer
eventually). journal_try_to_free_buffer() will call try_to_free_buffer()
if the journal head is NULL, at that point, since kjournald is still
holding a reference count on that buffer, try_to_free_buffers()-
>drop_buffers() failed because buffer is busy.


This race happened in SLES9 SP1/SP2, also it happend in 2.6.11, but
could not reproduce in mainline 2.6.13. SPLES9 sp1/sp2 and 2.6.11 all
have the patch to allow invalidate_inode_page() to return -EIO to deal
with the DIO-with-dirty-mapped-write case.  But in 2.6.13, it
invalidates using a range, probably that's why mainline doesn't 
show the problem (the chance of calling invalidate_inode_pages2_range on
a busy page is reduced if the range is passed as parameter)

> Apparently the debug patch is currently oopsing.  We need to fix that.

Sorry about the lack of response on this, it certainly very powerful and
useful tool. Last time I tried it again the kernel would not boot. I am
working on it, slowly:)

Mingming