Hi, > I delete the following patch > "commit 3f31fddfa26b7594b44ff2b34f9a04ba409e0f91 > Author: Mingming Cao <cmm@xxxxxxxxxx> > Date: Fri Jul 25 01:46:22 2008 -0700 > > jbd: fix race between free buffer and commit transaction > " > This patch is no longer needed because if race between freeing buffer > and committing transaction functionality occurs and dio gets error, > currently dio falls back to buffered IO by the following patch. > > commit 6ccfa806a9cfbbf1cd43d5b6aa47ef2c0eb518fd > Author: Hisashi Hifumi <hifumi.hisashi@xxxxxxxxxxxxx> > Date: Tue Sep 2 14:35:40 2008 -0700 > > VFS: fix dio write returning EIO when try_to_release_page fails > > Thanks. > > Signed-off-by: Hisashi Hifumi <hifumi.hisashi@xxxxxxxxxxxxx> OK, makes sence. At least it has an advantage that we don't have to wait for a transaction commit in DIO path. Acked-by: Jan Kara <jack@xxxxxxx> Honza > > diff -Nrup linux-2.6.30-rc8.org/fs/jbd/transaction.c linux-2.6.30-rc8.ext3_4/fs/jbd/transaction.c > --- linux-2.6.30-rc8.org/fs/jbd/transaction.c 2009-06-04 16:26:26.000000000 +0900 > +++ linux-2.6.30-rc8.ext3_4/fs/jbd/transaction.c 2009-06-04 19:14:53.000000000 +0900 > @@ -1686,35 +1686,6 @@ out: > return; > } > > -/* > - * journal_try_to_free_buffers() could race with journal_commit_transaction() > - * The latter might still hold the a count on 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 to finish syncing data buffers, before > - * tryinf to free that buffer. > - * > - * Called with journal->j_state_lock held. > - */ > -static void journal_wait_for_transaction_sync_data(journal_t *journal) > -{ > - transaction_t *transaction = NULL; > - tid_t tid; > - > - spin_lock(&journal->j_state_lock); > - transaction = journal->j_committing_transaction; > - > - if (!transaction) { > - spin_unlock(&journal->j_state_lock); > - return; > - } > - > - tid = transaction->t_tid; > - spin_unlock(&journal->j_state_lock); > - log_wait_commit(journal, tid); > -} > - > /** > * int journal_try_to_free_buffers() - try to free page buffers. > * @journal: journal for operation > @@ -1786,25 +1757,6 @@ int journal_try_to_free_buffers(journal_ > > ret = try_to_free_buffers(page); > > - /* > - * There are a number of places where journal_try_to_free_buffers() > - * could race with journal_commit_transaction(), the later still > - * holds the reference to the buffers to free while processing them. > - * try_to_free_buffers() failed to free those buffers. Some of the > - * caller of releasepage() request page buffers to be dropped, otherwise > - * treat the fail-to-free as errors (such as generic_file_direct_IO()) > - * > - * So, if the caller of try_to_release_page() wants the synchronous > - * behaviour(i.e make sure buffers are dropped upon return), > - * let's wait for the current transaction to finish flush of > - * dirty data buffers, then try to free those buffers again, > - * with the journal locked. > - */ > - if (ret == 0 && (gfp_mask & __GFP_WAIT) && (gfp_mask & __GFP_FS)) { > - journal_wait_for_transaction_sync_data(journal); > - ret = try_to_free_buffers(page); > - } > - > busy: > return ret; > } > diff -Nrup linux-2.6.30-rc8.org/fs/jbd2/transaction.c linux-2.6.30-rc8.ext3_4/fs/jbd2/transaction.c > --- linux-2.6.30-rc8.org/fs/jbd2/transaction.c 2009-06-04 16:26:26.000000000 +0900 > +++ linux-2.6.30-rc8.ext3_4/fs/jbd2/transaction.c 2009-06-04 19:17:12.000000000 +0900 > @@ -1547,36 +1547,6 @@ out: > return; > } > > -/* > - * jbd2_journal_try_to_free_buffers() could race with > - * jbd2_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. > - * > - * jbd2_journal_try_to_free_buffers() will call this function to > - * wait for the current transaction to finish syncing data buffers, before > - * try to free that buffer. > - * > - * Called with journal->j_state_lock hold. > - */ > -static void jbd2_journal_wait_for_transaction_sync_data(journal_t *journal) > -{ > - transaction_t *transaction; > - tid_t tid; > - > - spin_lock(&journal->j_state_lock); > - transaction = journal->j_committing_transaction; > - > - if (!transaction) { > - spin_unlock(&journal->j_state_lock); > - return; > - } > - > - tid = transaction->t_tid; > - spin_unlock(&journal->j_state_lock); > - jbd2_log_wait_commit(journal, tid); > -} > - > /** > * int jbd2_journal_try_to_free_buffers() - try to free page buffers. > * @journal: journal for operation > @@ -1649,25 +1619,6 @@ int jbd2_journal_try_to_free_buffers(jou > > ret = try_to_free_buffers(page); > > - /* > - * There are a number of places where jbd2_journal_try_to_free_buffers() > - * could race with jbd2_journal_commit_transaction(), the later still > - * holds the reference to the buffers to free while processing them. > - * try_to_free_buffers() failed to free those buffers. Some of the > - * caller of releasepage() request page buffers to be dropped, otherwise > - * treat the fail-to-free as errors (such as generic_file_direct_IO()) > - * > - * So, if the caller of try_to_release_page() wants the synchronous > - * behaviour(i.e make sure buffers are dropped upon return), > - * let's wait for the current transaction to finish flush of > - * dirty data buffers, then try to free those buffers again, > - * with the journal locked. > - */ > - if (ret == 0 && (gfp_mask & __GFP_WAIT) && (gfp_mask & __GFP_FS)) { > - jbd2_journal_wait_for_transaction_sync_data(journal); > - ret = try_to_free_buffers(page); > - } > - > busy: > return ret; > } > > -- > To unsubscribe from this list: send the line "unsubscribe linux-ext4" in > the body of a message to majordomo@xxxxxxxxxxxxxxx > More majordomo info at http://vger.kernel.org/majordomo-info.html -- Jan Kara <jack@xxxxxxx> SuSE CR Labs -- To unsubscribe from this list: send the line "unsubscribe linux-ext4" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html