lkml.org 
[lkml]   [2008]   [May]   [16]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH] Fix DIO EIO error caused by race between jbd_commit_transaction() and journal_try_to_drop_buffers()
    On Fri, May 16, 2008 at 07:14:10AM -0700, Mingming Cao wrote:
    > 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 specific, 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, and 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 06:42:30.000000000 -0700
    > @@ -1648,12 +1648,47 @@ out:
    > return;
    > }
    >
    > +/*
    > + * Journal_try_to_free_buffers() will call this function to w
    > + * waiting for the current transaction finishing syncing data buffers, before
    > + * try to free that buffer.
    > + *
    > + * 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.
    > + */
    > +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;
    > +}
    >

    Got a couple of whitespace problems above it looks like. Thanks,

    Josef



    \
     
     \ /
      Last update: 2008-05-16 17:23    [W:0.035 / U:30.656 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site