Re: [PATCH] jbd_commit_transaction() races with journal_try_to_drop_buffers() causing DIO failures

[Jan Kara <jack@suse.cz>]

On Wed 14-05-08 10:41:12, Mingming Cao wrote:
> > > Bummer, we can't free buffers in ext3_launder_page() before calling
> > > try_to_free_page, as later
> > > invalidate_complete_page2()->try_to_free_page() expecting the page
> > > buffers are still here, and will return EIO if it launder_page() has
> > > already freed those buffers.:(
> >   Are you sure? Because if bufferes are released in ext3_launder_page(),
> > PagePrivate() has been set to 0 and we should directly fall through to
> > releasing the page without ever calling try_to_release_page()... So I'd
> > want to find out why PagePrivate is still set in
> > invalidate_complete_page2().
> > 
> 
> You are right. PagePrivate() is being set to 0 in drop_buffers(). 
> 
> The problem is do_launder_page() returns successfully if the page is not
> dirty (our case), so ext3_launder_page() is not even get called. This
> also explains why the log_wait_commit() approach doesn't work for me:(
  I didn't realize PageDirty() is going to be already cleared by previous
writes... :(

> Have to think other ways...could we pass some flag to
> journal_try_to_free_buffers(), and ask journal_try_to_free_buffers()
> wait for jbd commit to finish flushing the data, if the request is from
> directo IO?
  Well, we could do that but we'd have to change try_to_release_page() call
to accept an extra argument which would consequently mean changing all the
filesystems... But yes, it probably makes sence because it is really
different whether we should just release the page because of memory
pressure or because direct IO needs to write to that area of the file.
So adding the parameter to releasepage() callback is probably a reasonable
thing to do.

								Honza
-- 
Jan Kara <jack@suse.cz>
SUSE Labs, CR