Message ID | 7bb361261001292241t7dda80d8sc5f467677ad34954@mail.gmail.com |
---|---|
State | New, archived |
Headers | show |
> From eb6567de576ac4a9337c3971a2d0549af64e15f0 Mon Sep 17 00:00:00 2001 > From: dingdinghua <dingdinghua@nrchpc.ac.cn> > Date: Sat, 30 Jan 2010 14:23:28 +0800 > Subject: [PATCH] Jbd2: delay discarding buffers in journal_unmap_buffer > > We should delay discarding buffers until we know that "add to orphan" > operation has definitely been committed, otherwise the log space of > committing transation may be freed and reused before truncate get > committed, updates may get lost if crash happens. Thanks for the patch. For us to be able to merge the patch, we need a Signed-off-by signature from you. Please add it in the next round of submission. > diff --git a/fs/jbd2/commit.c b/fs/jbd2/commit.c > index 1bc74b6..c5d1c7c 100644 > --- a/fs/jbd2/commit.c > +++ b/fs/jbd2/commit.c > @@ -936,8 +936,10 @@ restart_loop: > * behind for writeback and gets reallocated for another > * use in a different page. */ > if (buffer_freed(bh)) { > - clear_buffer_freed(bh); > - clear_buffer_jbddirty(bh); > + if (!jh->b_next_transaction) { > + clear_buffer_freed(bh); > + clear_buffer_jbddirty(bh); > + } > } When can merge the 'if' here. Also the comment above this code needs updating after your change. > if (buffer_jbddirty(bh)) { > diff --git a/fs/jbd2/transaction.c b/fs/jbd2/transaction.c > index a051270..0d019f1 100644 > --- a/fs/jbd2/transaction.c > +++ b/fs/jbd2/transaction.c > @@ -1788,11 +1788,8 @@ static int journal_unmap_buffer(journal_t *journal, struct buffer_head *bh) > * running transaction if that is set, but nothing > * else. */ > set_buffer_freed(bh); > - if (jh->b_next_transaction) { > - J_ASSERT(jh->b_next_transaction == > - journal->j_running_transaction); > - jh->b_next_transaction = NULL; > - } > + if (journal->j_running_transaction && buffer_jbddirty(bh)) > + jh->b_next_transaction = journal->j_running_transaction; Also here you have to update the comment above set_buffer_freed to match the new reality. Otherwise I'm fine with the patch. Honza
From eb6567de576ac4a9337c3971a2d0549af64e15f0 Mon Sep 17 00:00:00 2001 From: dingdinghua <dingdinghua@nrchpc.ac.cn> Date: Sat, 30 Jan 2010 14:23:28 +0800 Subject: [PATCH] Jbd2: delay discarding buffers in journal_unmap_buffer We should delay discarding buffers until we know that "add to orphan" operation has definitely been committed, otherwise the log space of committing transation may be freed and reused before truncate get committed, updates may get lost if crash happens. --- fs/jbd2/commit.c | 6 ++++-- fs/jbd2/transaction.c | 18 ++++++++++-------- 2 files changed, 14 insertions(+), 10 deletions(-) diff --git a/fs/jbd2/commit.c b/fs/jbd2/commit.c index 1bc74b6..c5d1c7c 100644 --- a/fs/jbd2/commit.c +++ b/fs/jbd2/commit.c @@ -936,8 +936,10 @@ restart_loop: * behind for writeback and gets reallocated for another * use in a different page. */ if (buffer_freed(bh)) { - clear_buffer_freed(bh); - clear_buffer_jbddirty(bh); + if (!jh->b_next_transaction) { + clear_buffer_freed(bh); + clear_buffer_jbddirty(bh); + } } if (buffer_jbddirty(bh)) { diff --git a/fs/jbd2/transaction.c b/fs/jbd2/transaction.c index a051270..0d019f1 100644 --- a/fs/jbd2/transaction.c +++ b/fs/jbd2/transaction.c @@ -1788,11 +1788,8 @@ static int journal_unmap_buffer(journal_t *journal, struct buffer_head *bh) * running transaction if that is set, but nothing * else. */ set_buffer_freed(bh); - if (jh->b_next_transaction) { - J_ASSERT(jh->b_next_transaction == - journal->j_running_transaction); - jh->b_next_transaction = NULL; - } + if (journal->j_running_transaction && buffer_jbddirty(bh)) + jh->b_next_transaction = journal->j_running_transaction; jbd2_journal_put_journal_head(jh); spin_unlock(&journal->j_list_lock); jbd_unlock_bh_state(bh); @@ -1969,7 +1966,7 @@ void jbd2_journal_file_buffer(struct journal_head *jh, */ void __jbd2_journal_refile_buffer(struct journal_head *jh) { - int was_dirty; + int was_dirty, jlist; struct buffer_head *bh = jh2bh(jh); J_ASSERT_JH(jh, jbd_is_locked_bh_state(bh)); @@ -1991,8 +1988,13 @@ void __jbd2_journal_refile_buffer(struct journal_head *jh) __jbd2_journal_temp_unlink_buffer(jh); jh->b_transaction = jh->b_next_transaction; jh->b_next_transaction = NULL; - __jbd2_journal_file_buffer(jh, jh->b_transaction, - jh->b_modified ? BJ_Metadata : BJ_Reserved); + if (buffer_freed(bh)) + jlist = BJ_Forget; + else if (jh->b_modified) + jlist = BJ_Metadata; + else + jlist = BJ_Reserved; + __jbd2_journal_file_buffer(jh, jh->b_transaction, jlist); J_ASSERT_JH(jh, jh->b_transaction->t_state == T_RUNNING); if (was_dirty) -- 1.5.5.6