login
Login
/
Register
Search
Header Space
Forums
News
Jobs
Blogs
Features
Man Pages
Site
Home
»
Mailing list archives
»
linux-kernel
»
2008
»
May
»
16
Re: [PATCH] Fix DIO EIO error caused by race between jbd_commit_transaction() and journal_try_to_drop_buffers()
view
thread
Score:
Previous message: [
thread
] [
date
] [
author
]
Next message: [
thread
] [
date
] [
author
]
[view in full thread]
From:
Badari Pulavarty <pbadari@...>
To: <cmm@...>
Cc: Josef Bacik <jbacik@...>, Jan Kara <jack@...>, <akpm@...>, <linux-ext4@...>, <linux-kernel@...>
Subject:
Re: [PATCH] Fix DIO EIO error caused by race between jbd_commit_transaction() and journal_try_to_drop_buffers()
Date: Friday, May 16, 2008 - 1:17 pm
On Fri, 2008-05-16 at 10:11 -0700, Mingming Cao wrote:
quoted text
> On Fri, 2008-05-16 at 11:01 -0400, Josef Bacik wrote: > > > > > Got a couple of whitespace problems above it looks like. Thanks, > > > > Thanks for catching this, below is updated patch, fixed the whitespace > and comments. > > > --------------------------------------------------- > JBD: fix journal_try_to_free_buffers race with > journal_commit_transaction > > From: Mingming Cao <cmm@us.ibm.com> > > This patch fixed a few races between direct IO and kjournld commit > transaction. > An unexpected EIO error gets returned to direct IO caller when it failed > to > free those data buffers. This could be reproduced easily with parallel > direct write and buffered write to the same file > > More specificly, those races could cause journal_try_to_free_buffers() > fail to free the data buffers, when jbd is committing the transaction > that has > those data buffers on its t_syncdata_list or t_locked_list. > journal_commit_transaction() still holds the reference to those buffers > before data reach to disk and buffers are removed from the > t_syncdata_list of t_locked_list. This prevent the concurrent > journal_try_to_free_buffers() to free those buffers at the same time, > but cause > EIO error returns back to direct IO. > > With this patch, in case of direct IO and when try_to_free_buffers() > failed, > let's waiting for journal_commit_transaction() to finish > flushing the current committing transaction's data buffers to disk, > then try to free those buffers again. > > Signed-off-by: Mingming Cao <cmm@us.ibm.com> > Reviewed-by: Badari Pulavarty <pbadari@us.ibm.com> > --- > fs/jbd/commit.c | 1 + > fs/jbd/journal.c | 1 + > fs/jbd/transaction.c | 46 > ++++++++++++++++++++++++++++++++++++++++++++++ > include/linux/jbd.h | 3 +++ > 4 files changed, 51 insertions(+) > > Index: linux-2.6.26-rc1/include/linux/jbd.h > =================================================================== > --- linux-2.6.26-rc1.orig/include/linux/jbd.h 2008-05-14 > 16:36:41.000000000 -0700 > +++ linux-2.6.26-rc1/include/linux/jbd.h 2008-05-15 14:12:10.000000000 > -0700 > @@ -667,6 +667,9 @@ struct journal_s > */ > wait_queue_head_t j_wait_transaction_locked; > > + /* Wait queu for waiting for data buffers to flushed to disk*/ > + wait_queue_head_t j_wait_data_flushed; > + > /* Wait queue for waiting for checkpointing to complete */ > wait_queue_head_t j_wait_logspace; > > Index: linux-2.6.26-rc1/fs/jbd/commit.c > =================================================================== > --- linux-2.6.26-rc1.orig/fs/jbd/commit.c 2008-05-03 11:59:44.000000000 > -0700 > +++ linux-2.6.26-rc1/fs/jbd/commit.c 2008-05-15 14:12:46.000000000 -0700 > @@ -462,6 +462,7 @@ void journal_commit_transaction(journal_ > * clean by now, so check that it is in fact empty. > */ > J_ASSERT (commit_transaction->t_sync_datalist == NULL); > + wake_up(&journal->j_wait_data_flushed) > > jbd_debug (3, "JBD: commit phase 3\n"); > > Index: linux-2.6.26-rc1/fs/jbd/journal.c > =================================================================== > --- linux-2.6.26-rc1.orig/fs/jbd/journal.c 2008-05-14 16:36:41.000000000 > -0700 > +++ linux-2.6.26-rc1/fs/jbd/journal.c 2008-05-15 14:13:02.000000000 > -0700 > @@ -660,6 +660,7 @@ static journal_t * journal_init_common ( > goto fail; > > init_waitqueue_head(&journal->j_wait_transaction_locked); > + init_waitqueue_head(&journal->j_wait_data_flushed); > init_waitqueue_head(&journal->j_wait_logspace); > init_waitqueue_head(&journal->j_wait_done_commit); > init_waitqueue_head(&journal->j_wait_checkpoint); > Index: linux-2.6.26-rc1/fs/jbd/transaction.c > =================================================================== > --- linux-2.6.26-rc1.orig/fs/jbd/transaction.c 2008-05-03 > 11:59:44.000000000 -0700 > +++ linux-2.6.26-rc1/fs/jbd/transaction.c 2008-05-16 09:27:21.000000000 > -0700 > @@ -1648,12 +1648,49 @@ out: > return; > } > > +/* > + * journal_try_to_free_buffers() could race with > journal_commit_transaction() > + * The later might still hold the reference count to the buffers when > inspecting > + * them on t_syncdata_list or t_locked_list. > + * > + * Journal_try_to_free_buffers() will call this function to > + * wait for the current transaction finishing syncing data buffers, > before > + * try to free that buffer. > + * > + * Called with journal->j_state_lock hold.
Fix wrapping lines ?
quoted text
> + */ > +static void journal_wait_for_transaction_sync_data(journal_t *journal) > +{ > + transaction_t *transaction = NULL; > + > + transaction = journal->j_committing_transaction; > + > + if (!transaction) > + return; > + > + /* > + * If the current transaction is flushing and waiting for data buffers > + * (t_state is T_FLUSH), wait for the j_wait_data_flushed event > + */ > + if (transaction->t_state == T_FLUSH) { > + DEFINE_WAIT(wait); > + > + prepare_to_wait(&journal->j_wait_data_flushed, > + &wait, TASK_UNINTERRUPTIBLE); > + spin_unlock(&journal->j_state_lock); > + schedule(); > + finish_wait(&journal->j_wait_data_flushed, &wait); > + spin_lock(&journal->j_state_lock); > + } > + return; > +} > > /** > * int journal_try_to_free_buffers() - try to free page buffers. > * @journal: journal for operation > * @page: to try and free > - * @unused_gfp_mask: unused > + * @gfp_mask: unused for allocation purpose. Here is used > + * as a flag to tell if direct IO is attemping to free buffers. > * > * > * For all the buffers on this page, > @@ -1682,13 +1719,16 @@ out: > * journal_try_to_free_buffer() is changing its state. But that > * cannot happen because we never reallocate freed data as metadata > * while the data is part of a transaction. Yes? > + * > + * Return 0 on failure, 1 on success > */ > int journal_try_to_free_buffers(journal_t *journal, > - struct page *page, gfp_t unused_gfp_mask) > + struct page *page, gfp_t gfp_mask) > { > struct buffer_head *head; > struct buffer_head *bh; > int ret = 0; > + int dio = gfp_mask & __GFP_REPEAT; > > J_ASSERT(PageLocked(page)); > > @@ -1713,7 +1753,31 @@ int journal_try_to_free_buffers(journal_ > if (buffer_jbd(bh)) > goto busy; > } while ((bh = bh->b_this_page) != head); > + > ret = try_to_free_buffers(page); > + > + /* > + * In the case of concurrent direct IO and buffered IO, > + * There are a number of places where we > + * could race with journal_commit_transaction(), the later still > + * helds the reference to the buffers to free while processing them. > + * try_to_free_buffers() failed to free those buffers, > + * resulting in an unexpected EIO error > + * returns back to the generic_file_direct_IO() > + * > + * So let's wait for the current transaction finished flush > + * dirty data buffers before we try to free those buffers > + * again. This wait is needed by direct IO code path only, > + * gfp_mask __GFP_REPEAT is passed from the direct IO code > + * path to flag if we need to wait and retry free buffers. > + */ > + if (ret == 0 && dio) {
drop "dio" variable and compare here, like if (ret == 0 && (gfp_mask & __GFP_REPEAT)
quoted text
> + spin_lock(&journal->j_state_lock); > + journal_wait_for_transaction_sync_data(journal); > + ret = try_to_free_buffers(page); > + spin_unlock(&journal->j_state_lock); > + }
Thanks, Badari --
unsubscribe notice
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to
majordomo@vger.kernel.org
More majordomo info at
http://vger.kernel.org/majordomo-info.html
Please read the FAQ at
http://www.tux.org/lkml/
Previous message: [
thread
] [
date
] [
author
]
Next message: [
thread
] [
date
] [
author
]
Messages in current thread:
[RFC] JBD ordered mode rewrite
, Jan Kara
, (Thu Mar 6, 1:42 pm)
Possible race between direct IO and JBD?
, Mingming Cao
, (Fri Apr 25, 7:38 pm)
Re: Possible race between direct IO and JBD?
, Jan Kara
, (Mon Apr 28, 8:26 am)
Re: Possible race between direct IO and JBD?
, Badari Pulavarty
, (Mon Apr 28, 1:11 pm)
Re: Possible race between direct IO and JBD?
, Jan Kara
, (Mon Apr 28, 2:09 pm)
Re: Possible race between direct IO and JBD?
, Mingming Cao
, (Mon Apr 28, 3:09 pm)
Re: Possible race between direct IO and JBD?
, Jan Kara
, (Tue Apr 29, 8:43 am)
[PATCH] jbd_commit_transaction() races with journal_try_to_d...
, Badari Pulavarty
, (Thu May 1, 11:16 am)
Re: [PATCH] jbd_commit_transaction() races with journal_try_...
, Jan Kara
, (Mon May 5, 1:06 pm)
Re: [PATCH] jbd_commit_transaction() races with journal_try_...
, Mingming Cao
, (Fri May 9, 6:27 pm)
Re: [PATCH] jbd_commit_transaction() races with journal_try_...
, Jan Kara
, (Mon May 12, 11:54 am)
Re: [PATCH] jbd_commit_transaction() races with journal_try_...
, Mingming Cao
, (Mon May 12, 8:39 pm)
Re: [PATCH] jbd_commit_transaction() races with journal_try_...
, Jan Kara
, (Tue May 13, 10:54 am)
Re: [PATCH] jbd_commit_transaction() races with journal_try_...
, Mingming Cao
, (Tue May 13, 6:23 pm)
Re: [PATCH] jbd_commit_transaction() races with journal_try_...
, Jan Kara
, (Wed May 14, 1:08 pm)
Re: [PATCH] jbd_commit_transaction() races with journal_try_...
, Mingming Cao
, (Wed May 14, 1:41 pm)
Re: [PATCH] jbd_commit_transaction() races with journal_try_...
, Jan Kara
, (Wed May 14, 2:14 pm)
[PATCH] Fix DIO EIO error caused by race between jbd_commit_...
, Mingming Cao
, (Fri May 16, 10:14 am)
Re: [PATCH] Fix DIO EIO error caused by race between jbd_com...
, Badari Pulavarty
, (Fri May 16, 1:12 pm)
[PATCH] JBD2: Fix DIO EIO error caused by race between free ...
, Mingming Cao
, (Fri May 16, 5:01 pm)
[PATCH] JBD: Fix DIO EIO error caused by race between free b...
, Mingming Cao
, (Fri May 16, 5:01 pm)
Re: [PATCH] JBD: Fix DIO EIO error caused by race between fr...
, Jan Kara
, (Sun May 18, 6:37 pm)
Re: [PATCH] JBD: Fix DIO EIO error caused by race between fr...
, Mingming Cao
, (Mon May 19, 3:59 pm)
Re: [PATCH] JBD: Fix DIO EIO error caused by race between fr...
, Andrew Morton
, (Mon May 19, 4:25 pm)
Re: [PATCH] JBD: Fix DIO EIO error caused by race between fr...
, Mingming Cao
, (Mon May 19, 6:07 pm)
[PATCH -v2] JBD2: Fix race between journal free buffer and c...
, Mingming Cao
, (Tue May 20, 2:03 pm)
[PATCH-v2] JBD: Fix race between free buffer and commit tras...
, Mingming Cao
, (Tue May 20, 2:02 pm)
[PATCH 2/2][TAKE3] JBD2: Fix race between free buffer and co...
, Mingming
, (Wed May 21, 7:39 pm)
[PATCH 1/2][TAKE3] JBD: Fix race between free buffer and com...
, Mingming
, (Wed May 21, 7:38 pm)
Re: [PATCH 1/2][TAKE3] JBD: Fix race between free buffer and...
, Andrew Morton
, (Thu May 22, 1:57 am)
Re: [PATCH-v2] JBD: Fix race between free buffer and commit ...
, Jan Kara
, (Tue May 20, 7:53 pm)
Re: [PATCH-v2] JBD: Fix race between free buffer and commit ...
, Mingming
, (Wed May 21, 1:14 pm)
Re: [PATCH-v2] JBD: Fix race between free buffer and commit ...
, Jan Kara
, (Sat May 24, 6:44 pm)
Re: [PATCH-v2] JBD: Fix race between free buffer and commit ...
, Mingming Cao
, (Wed May 28, 2:18 pm)
Re: [PATCH-v2] JBD: Fix race between free buffer and commit ...
, Jan Kara
, (Wed May 28, 2:55 pm)
[PATCH][take 5] JBD: Fix race between free buffer and commit...
, Mingming Cao
, (Wed May 28, 8:16 pm)
[PATCH][take 5] JBD2: Fix race between free buffer and commi...
, Mingming Cao
, (Wed May 28, 8:18 pm)
Re: [PATCH][take 5] JBD2: Fix race between free buffer and c...
, Aneesh Kumar K.V
, (Fri May 30, 2:24 am)
Re: [PATCH][take 5] JBD2: Fix race between free buffer and c...
, Mingming Cao
, (Fri May 30, 11:17 am)
Re: [PATCH-v2] JBD: Fix race between free buffer and commit ...
, Mingming Cao
, (Wed May 28, 8:15 pm)
Re: [PATCH] JBD: Fix DIO EIO error caused by race between fr...
, Jens Axboe
, (Tue May 20, 5:30 am)
Re: [PATCH] JBD: Fix DIO EIO error caused by race between fr...
, Mingming Cao
, (Tue May 20, 1:47 pm)
Re: [PATCH] Fix DIO EIO error caused by race between jbd_com...
, Josef Bacik
, (Fri May 16, 11:01 am)
Re: [PATCH] Fix DIO EIO error caused by race between jbd_com...
, Mingming Cao
, (Fri May 16, 1:11 pm)
Re: [PATCH] Fix DIO EIO error caused by race between jbd_com...
, Badari Pulavarty
, (Fri May 16, 1:17 pm)
Re: [PATCH] Fix DIO EIO error caused by race between jbd_com...
, Mingming Cao
, (Fri May 16, 1:30 pm)
Re: [PATCH] jbd_commit_transaction() races with journal_try_...
, Mingming Cao
, (Fri May 16, 10:13 am)
Re: [PATCH] jbd_commit_transaction() races with journal_try_...
, Mingming Cao
, (Tue May 13, 12:37 pm)
Re: [PATCH] jbd_commit_transaction() races with journal_try_...
, Mingming Cao
, (Mon May 12, 3:23 pm)
Re: [PATCH] jbd_commit_transaction() races with journal_try_...
, Jan Kara
, (Tue May 13, 10:20 am)
Re: [PATCH] jbd_commit_transaction() races with journal_try_...
, Badari Pulavarty
, (Mon May 5, 8:10 pm)
Re: [PATCH] jbd_commit_transaction() races with journal_try_...
, Mingming Cao
, (Mon May 5, 1:53 pm)
Re: [PATCH] jbd_commit_transaction() races with journal_try_...
, Mingming Cao
, (Thu May 1, 6:08 pm)
Re: Possible race between direct IO and JBD?
, Mingming Cao
, (Tue Apr 29, 1:49 pm)
Re: Possible race between direct IO and JBD?
, Andrew Morton
, (Sat Apr 26, 6:41 am)
Re: [RFC] JBD ordered mode rewrite
, Andreas Dilger
, (Fri Mar 7, 7:52 pm)
Re: [RFC] JBD ordered mode rewrite
, Jan Kara
, (Mon Mar 10, 3:54 pm)
Re: [RFC] JBD ordered mode rewrite
, Andreas Dilger
, (Mon Mar 10, 5:37 pm)
Re: [RFC] JBD ordered mode rewrite
, Christoph Hellwig
, (Sat Mar 8, 8:14 am)
Re: [RFC] JBD ordered mode rewrite
, Mingming Cao
, (Fri Mar 7, 8:08 pm)
Re: [RFC] JBD ordered mode rewrite
, Mingming Cao
, (Fri Mar 7, 6:55 am)
Re: [RFC] JBD ordered mode rewrite
, Jan Kara
, (Mon Mar 10, 2:29 pm)
Re: [RFC] JBD ordered mode rewrite
, Mark Fasheh
, (Thu Mar 6, 9:34 pm)
Re: [RFC] JBD ordered mode rewrite
, Jan Kara
, (Mon Mar 10, 2:00 pm)
Re: [RFC] JBD ordered mode rewrite
, Andrew Morton
, (Thu Mar 6, 7:53 pm)
Re: [RFC] JBD ordered mode rewrite
, Jan Kara
, (Mon Mar 10, 1:38 pm)
Re: [RFC] JBD ordered mode rewrite
, Josef Bacik
, (Thu Mar 6, 3:05 pm)
Re: [RFC] JBD ordered mode rewrite
, Jan Kara
, (Mon Mar 10, 12:30 pm)
Navigation
Create content
Mailing list archives
Recent posts
Mail archive search
Enter your search terms.
all mailing lists
alsa-devel
dragonflybsd-bugs
dragonflybsd-commit
dragonflybsd-docs
dragonflybsd-kernel
dragonflybsd-submit
dragonflybsd-user
freebsd-announce
freebsd-bugs
freebsd-chat
freebsd-cluster
freebsd-current
freebsd-drivers
freebsd-embeded
freebsd-fs
freebsd-hackers
freebsd-hardware
freebsd-mobile
freebsd-net
freebsd-performance
freebsd-pf
freebsd-security
freebsd-security-notifications
freebsd-threads
git
git-commits-head
linux-activists
linux-arm
linux-ath5k-devel
linux-btrfs
linux-c-programming
linux-driver-devel
linux-ext4
linux-fsdevel
linux-ia64
linux-input
linux-kernel
linux-kernel-janitors
linux-kernel-mentors
linux-kernel-newbies
linux-kvm
linux-net
linux-netdev
linux-newbie
linux-nfs
linux-raid
linux-scsi
linux-security-module
linux-sparse
linux-usb
linux-usb-devel
madwifi-devel
netbsd-announce
netbsd-tech-kern
openbsd-announce
openbsd-bugs
openbsd-ipv6
openbsd-misc
openbsd-security-announce
openbsd-smp
openbsd-source-changes
openbsd-tech
openfabrics-general
openmoko-community
openmoko-devel
openmoko-kernel
reiserfs-devel
tux3
ucarp
Optionally limit your search to a specific mailing list.
advanced
Popular discussions
linux-kernel
:
Arjan van de Ven
[Patch v2] Make PCI extended config space (MMCONFIG) a driver opt-in
Linus Torvalds
Linux 2.6.27-rc8
Tilman Schmidt
git guidance
Greg KH
[GIT PATCH] driver core patches against 2.6.24
git
:
Martin Langhoff
Re: pack operation is thrashing my server
Alan Larkin
fatal: Out of memory, malloc failed
Mark Junker
git on MacOSX and files with decomposed utf-8 file names
Alex Riesen
Re: How do get a specific version of a particular file?
openbsd-misc
:
Leon Dippenaar
New tcp stack attack
Richard Stallman
Real men don't attack straw men
Pieter Verberne
Remove escape characters from file
Juan Miscaro
removing sendmail
linux-netdev
:
Gerrit Renker
[PATCH 27/37] dccp: Integration of dynamic feature activation - part 2 (server side)
David Miller
[GIT]: Networking
Chuck Lever
Re: [bug?] tg3: Failed to load firmware "tigon/tg3_tso.bin"
David Miller
Re: [PATCH] pkt_sched: Destroy gen estimators under rtnl_lock().
Latest forum posts
usb mic not detected
18 minutes ago
Applications and Utilities
Extended configuration Read/Write
33 minutes ago
Windows
Add ext2 inode field
10 hours ago
Linux kernel
the kernel how to power off the machine
20 hours ago
Linux kernel
struct gendisk via request_queue
22 hours ago
Linux kernel
page initialization during kernel initialization
1 day ago
Linux kernel
Read Transport Layer Data form network packets (tcp/ip)
2 days ago
Linux kernel
Getting blinking screen in Fedora 9
2 days ago
Linux general
Problem with kernel + libata
3 days ago
Linux kernel
How to detect usb device insertioin and removal event ?
3 days ago
Linux general
Show all forums...
Recent Tags
-rc
Intel
2.6.27-rc8
Linus Torvalds
2.6.27
quote
Linux
bugs
-rc8
more tags
Colocation donated by:
Who's online
There are currently
3 users
and
1185 guests
online.
Online users
Anusha
strcmp
kingneutron
Syndicate
speck-geostationary