All of lore.kernel.org
 help / color / mirror / Atom feed
From: Badari Pulavarty <pbadari@us.ibm.com>
To: Jan Kara <jack@suse.cz>
Cc: Mingming Cao <cmm@us.ibm.com>,
	akpm@linux-foundation.org, linux-ext4@vger.kernel.org,
	linux-kernel@vger.kernel.org
Subject: Re: Possible race between direct IO and JBD?
Date: Mon, 28 Apr 2008 10:11:34 -0700	[thread overview]
Message-ID: <1209402694.23575.5.camel@badari-desktop> (raw)
In-Reply-To: <20080428122626.GC17054@duck.suse.cz>


On Mon, 2008-04-28 at 14:26 +0200, Jan Kara wrote:
> Hi,
> 
> On Fri 25-04-08 16:38:23, Mingming Cao wrote:
> > While looking at a bug related to direct IO returns to EIO, after
> > looking at the code, I found there is a window that
> > try_to_free_buffers() from direct IO could race with JBD, which holds
> > the reference to the data buffers before journal_commit_transaction()
> > ensures the data buffers has reached to the disk.
> > 
> > A little more detail: to prepare for direct IO, generic_file_direct_IO()
> > calls invalidate_inode_pages2_range() to invalidate the pages in the
> > cache before performaning direct IO.  invalidate_inode_pages2_range()
> > tries to free the buffers via try_to free_buffers(), but sometimes it
> > can't, due to the buffers is possible still on some transaction's
> > t_sync_datalist or t_locked_list waiting for
> > journal_commit_transaction() to process it. 
> > 
> > Currently Direct IO simply returns EIO if try_to_free_buffers() finds
> > the buffer is busy, as it has no clue that JBD is referencing it.
> > 
> > Is this a known issue and expected behavior? Any thoughts?
>   Are you seeing this in data=ordered mode? As Andrew pointed out we do
> filemap_write_and_wait() so all the relevant data buffers of the inode
> should be already on disk. In __journal_try_to_free_buffer() we check
> whether the buffer is already-written-out data buffer and unfile and free
> it in that case. It shouldn't happen that a data buffer has
> b_next_transaction set so really the only idea why try_to_free_buffers()
> could fail is that somebody manages to write to a page via mmap before
> invalidate_inode_pages2_range() gets to it. Under which kind of load do you
> observe the problem? Do you know exactly because of which condition does
> journal_try_to_free_buffers() fail?
> 

Thank you for your reply.

What we are noticing is invalidate_inode_pages2_range() fails with -EIO
(from try_to_free_buffers() since b_count > 0).

I don't think the file is being updated through mmap(). Previous
writepage() added these buffers to t_sync_data list (data=ordered).
filemap_write_and_wait() waits for pagewrite back to be cleared.
So, buffers are no longer dirty, but still on the t_sync_data and
kjournald didn't get chance to process them yet :(

Since we have elevated b_count on these buffers, try_to_free_buffers()
fails. How can we make filemap_write_and_wait() to wait for kjournald
to unfile these buffers ?

Does this makes sense ? Am I missing something here ?

Thanks,
Badari


  reply	other threads:[~2008-04-28 17:11 UTC|newest]

Thread overview: 71+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-03-06 17:42 [RFC] JBD ordered mode rewrite Jan Kara
2008-03-06 19:05 ` Josef Bacik
2008-03-10 16:30   ` Jan Kara
2008-03-06 23:53 ` Andrew Morton
2008-03-10 17:38   ` Jan Kara
2008-03-07  1:34 ` Mark Fasheh
2008-03-10 18:00   ` Jan Kara
2008-03-07 10:55 ` Mingming Cao
2008-03-10 18:29   ` Jan Kara
2008-03-07 23:52 ` Andreas Dilger
2008-03-08  0:08   ` Mingming Cao
2008-03-08 12:14   ` Christoph Hellwig
2008-03-10 19:54   ` Jan Kara
2008-03-10 21:37     ` Andreas Dilger
2008-04-25 23:38 ` Possible race between direct IO and JBD? Mingming Cao
2008-04-26 10:41   ` Andrew Morton
2008-04-28 12:26   ` Jan Kara
2008-04-28 17:11     ` Badari Pulavarty [this message]
2008-04-28 18:09       ` Jan Kara
2008-04-28 19:09         ` Mingming Cao
2008-04-29 12:43           ` Jan Kara
2008-04-29 17:49             ` Mingming Cao
2008-05-01 15:16             ` [PATCH] jbd_commit_transaction() races with journal_try_to_drop_buffers() causing DIO failures Badari Pulavarty
2008-05-01 22:08               ` Mingming Cao
2008-05-05 17:06               ` Jan Kara
2008-05-05 17:53                 ` Mingming Cao
2008-05-06  0:10                 ` Badari Pulavarty
2008-05-09 22:27                 ` Mingming Cao
2008-05-09 22:39                   ` [PATCH] JBD:need hold j_state_lock to updates to transaction t_state to T_COMMIT Mingming Cao
2008-05-12  9:34                     ` Jan Kara
2008-05-12 15:54                   ` [PATCH] jbd_commit_transaction() races with journal_try_to_drop_buffers() causing DIO failures Jan Kara
2008-05-12 19:23                     ` Mingming Cao
2008-05-13 14:20                       ` Jan Kara
2008-05-13  0:39                     ` Mingming Cao
2008-05-13 14:54                       ` Jan Kara
2008-05-13 16:37                         ` Mingming Cao
2008-05-13 22:23                         ` Mingming Cao
2008-05-14 17:08                           ` Jan Kara
2008-05-14 17:41                             ` Mingming Cao
2008-05-14 18:14                               ` Jan Kara
2008-05-16 14:13                                 ` Mingming Cao
2008-05-16 14:14                                 ` [PATCH] Fix DIO EIO error caused by race between jbd_commit_transaction() and journal_try_to_drop_buffers() Mingming Cao
2008-05-16 15:01                                   ` Josef Bacik
2008-05-16 17:11                                     ` Mingming Cao
2008-05-16 17:17                                       ` Badari Pulavarty
2008-05-16 17:30                                         ` Mingming Cao
2008-05-16 17:12                                   ` Badari Pulavarty
2008-05-16 21:01                                     ` [PATCH] JBD: Fix DIO EIO error caused by race between free buffer and commit trasanction Mingming Cao
2008-05-18 22:37                                       ` Jan Kara
2008-05-19 19:59                                         ` Mingming Cao
2008-05-19 20:25                                           ` Andrew Morton
2008-05-19 22:07                                             ` Mingming Cao
2008-05-20  9:30                                               ` Jens Axboe
2008-05-20 17:47                                                 ` Mingming Cao
2008-05-20 18:02                                               ` [PATCH-v2] JBD: Fix " Mingming Cao
2008-05-20 23:53                                                 ` Jan Kara
2008-05-21 17:14                                                   ` Mingming
2008-05-24 22:44                                                     ` Jan Kara
2008-05-28 18:18                                                       ` Mingming Cao
2008-05-28 18:55                                                         ` Jan Kara
2008-05-29  0:15                                                           ` Mingming Cao
2008-05-29  0:16                                                           ` [PATCH][take 5] " Mingming Cao
2008-05-29  0:18                                                             ` [PATCH][take 5] JBD2: " Mingming Cao
2008-05-30  6:24                                                               ` Aneesh Kumar K.V
2008-05-30 15:17                                                                 ` Mingming Cao
2008-05-21 23:38                                                 ` [PATCH 1/2][TAKE3] JBD: " Mingming
2008-05-22  5:57                                                   ` Andrew Morton
2008-05-21 23:39                                                 ` [PATCH 2/2][TAKE3] JBD2: " Mingming
2008-05-20 18:03                                               ` [PATCH -v2] JBD2: Fix race between journal " Mingming Cao
2008-05-16 21:01                                     ` [PATCH] JBD2: Fix DIO EIO error caused by race between " Mingming Cao
2008-05-09 22:39                 ` [PATCH] JBD2:need hold j_state_lock to updates to transaction t_state to T_COMMIT Mingming Cao

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1209402694.23575.5.camel@badari-desktop \
    --to=pbadari@us.ibm.com \
    --cc=akpm@linux-foundation.org \
    --cc=cmm@us.ibm.com \
    --cc=jack@suse.cz \
    --cc=linux-ext4@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.