From patchwork Thu Jul 9 08:41:27 2009 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: dingdinghua X-Patchwork-Id: 29624 Return-Path: X-Original-To: patchwork-incoming@bilbo.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from ozlabs.org (ozlabs.org [203.10.76.45]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (Client CN "mx.ozlabs.org", Issuer "CA Cert Signing Authority" (verified OK)) by bilbo.ozlabs.org (Postfix) with ESMTPS id E182CB7082 for ; Thu, 9 Jul 2009 18:41:38 +1000 (EST) Received: by ozlabs.org (Postfix) id D047ADDDE7; Thu, 9 Jul 2009 18:41:38 +1000 (EST) Delivered-To: patchwork-incoming@ozlabs.org Received: from vger.kernel.org (vger.kernel.org [209.132.176.167]) by ozlabs.org (Postfix) with ESMTP id 32B5BDDDA0 for ; Thu, 9 Jul 2009 18:41:38 +1000 (EST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1750995AbZGIIlg (ORCPT ); Thu, 9 Jul 2009 04:41:36 -0400 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S1752107AbZGIIlg (ORCPT ); Thu, 9 Jul 2009 04:41:36 -0400 Received: from rv-out-0506.google.com ([209.85.198.232]:62602 "EHLO rv-out-0506.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750995AbZGIIle (ORCPT ); Thu, 9 Jul 2009 04:41:34 -0400 Received: by rv-out-0506.google.com with SMTP id f6so1029rvb.1 for ; Thu, 09 Jul 2009 01:41:33 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=domainkey-signature:received:received:from:to:cc:subject:date :message-id:x-mailer; bh=KkDnO2DNayPKBDFooMeDEmxfOXrc3UYGePOVhkfrbDo=; b=WqbyDpLkWwzuivBBy5/fGa7uNY1OZ/YFGtsHeBgqdxGz2mRSyfiHQtuNTublzvYKXv pqvkgqK/+VX94jw5wBB10u4SZPq2jNuvglAxNyjTsIfklQVDfnrzZX8gct8ZrfVRR56U u855aQDhAfNQXkTMUygUjAgh9jY0B0Rci5Xbg= DomainKey-Signature: a=rsa-sha1; c=nofws; d=gmail.com; s=gamma; h=from:to:cc:subject:date:message-id:x-mailer; b=VfDa5oveYZ5aUMGSfqTVltViei5BISPg+6zxIGZy5/FHb8nbj2evB+2xrPdGJ9fW8L 33phc2RMNSPxcHLSytGioABHhf7/wr2Vi2QKtJyB+pnVJuX5jJXoYt/uVxvXeXCro+g2 BxzfEwnM1y0BncJ7QBOaQHUvG35djNQY0OGRU= Received: by 10.141.12.15 with SMTP id p15mr317923rvi.41.1247128893734; Thu, 09 Jul 2009 01:41:33 -0700 (PDT) Received: from localhost ([159.226.39.251]) by mx.google.com with ESMTPS id g31sm8167420rvb.20.2009.07.09.01.41.31 (version=TLSv1/SSLv3 cipher=RC4-MD5); Thu, 09 Jul 2009 01:41:32 -0700 (PDT) From: dingdinghua To: linux-ext4@vger.kernel.org Cc: dingdinghua Subject: [PATCH] set h_transaction to NULL when restart handle Date: Thu, 9 Jul 2009 16:41:27 +0800 Message-Id: <1247128887-25588-1-git-send-email-dingdinghua85@gmail.com> X-Mailer: git-send-email 1.6.0.4 Sender: linux-ext4-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-ext4@vger.kernel.org We may restart journal handle when do truncate or resize work, but current journal restart doesn't set handle->h_transaction to NULL when remove the handle from old transaction. This may results bugs if restart handle failed, since this handle doesn't belongs to the old transaction any more, and the transaction may have been committed, checkpointed and dropped while this handle still refer to it. So an active handle has a state that doesn't belongs to any transaction now, but current journal codes suppose that handle must belongs to an transaction, I add h_journal to handle_s, which points to journal. Signed-off-by: dingdinghua --- fs/ext3/ext3_jbd.c | 8 +++++ fs/ext3/super.c | 6 +++- fs/jbd/revoke.c | 12 ++++++-- fs/jbd/transaction.c | 68 +++++++++++++++++++++++++++++++++------------- include/linux/ext3_jbd.h | 9 +++--- include/linux/jbd.h | 1 + 6 files changed, 75 insertions(+), 29 deletions(-) diff --git a/fs/ext3/ext3_jbd.c b/fs/ext3/ext3_jbd.c index d401f14..17d43a5 100644 --- a/fs/ext3/ext3_jbd.c +++ b/fs/ext3/ext3_jbd.c @@ -57,3 +57,11 @@ int __ext3_journal_dirty_metadata(const char *where, ext3_journal_abort_handle(where, __func__, bh, handle,err); return err; } + +int __ext3_journal_restart(const char *where, handle_t *handle, int nblocks) +{ + int err = journal_restart(handle, nblocks); + if (err) + ext3_journal_abort_handle(where, __func__, NULL, handle, err); + return err; +} diff --git a/fs/ext3/super.c b/fs/ext3/super.c index 524b349..1f4c694 100644 --- a/fs/ext3/super.c +++ b/fs/ext3/super.c @@ -105,11 +105,13 @@ handle_t *ext3_journal_start_sb(struct super_block *sb, int nblocks) */ int __ext3_journal_stop(const char *where, handle_t *handle) { - struct super_block *sb; + struct super_block *sb = handle->h_journal->j_private; int err; int rc; - sb = handle->h_transaction->t_journal->j_private; + if (unlikely(handle->h_transaction == NULL)) + BUG_ON(!is_handle_aborted(handle)); + err = handle->h_err; rc = journal_stop(handle); diff --git a/fs/jbd/revoke.c b/fs/jbd/revoke.c index da6cd9b..712f6c4 100644 --- a/fs/jbd/revoke.c +++ b/fs/jbd/revoke.c @@ -336,7 +336,7 @@ int journal_revoke(handle_t *handle, unsigned long blocknr, struct buffer_head *bh_in) { struct buffer_head *bh = NULL; - journal_t *journal; + journal_t *journal = handle->h_journal; struct block_device *bdev; int err; @@ -344,7 +344,11 @@ int journal_revoke(handle_t *handle, unsigned long blocknr, if (bh_in) BUFFER_TRACE(bh_in, "enter"); - journal = handle->h_transaction->t_journal; + if (unlikely(handle->h_transaction == NULL)) { + J_ASSERT(is_handle_aborted(handle)); + return -EROFS; + } + if (!journal_set_features(journal, 0, 0, JFS_FEATURE_INCOMPAT_REVOKE)){ J_ASSERT (!"Cannot set revoke feature!"); return -EINVAL; @@ -426,13 +430,15 @@ int journal_revoke(handle_t *handle, unsigned long blocknr, int journal_cancel_revoke(handle_t *handle, struct journal_head *jh) { struct jbd_revoke_record_s *record; - journal_t *journal = handle->h_transaction->t_journal; + journal_t *journal = handle->h_journal; int need_cancel; int did_revoke = 0; /* akpm: debug */ struct buffer_head *bh = jh2bh(jh); jbd_debug(4, "journal_head %p, cancelling revoke\n", jh); + J_ASSERT(handle->h_transaction != NULL); + /* Is the existing Revoke bit valid? If so, we trust it, and * only perform the full cancel if the revoke bit is set. If * not, we can't trust the revoke bit, and we need to do the diff --git a/fs/jbd/transaction.c b/fs/jbd/transaction.c index 73242ba..8e61801 100644 --- a/fs/jbd/transaction.c +++ b/fs/jbd/transaction.c @@ -220,6 +220,7 @@ repeat_locked: * use and add the handle to the running transaction. */ handle->h_transaction = transaction; + handle->h_journal = transaction->t_journal; transaction->t_outstanding_credits += nblocks; transaction->t_updates++; transaction->t_handle_count++; @@ -274,7 +275,7 @@ handle_t *journal_start(journal_t *journal, int nblocks) return ERR_PTR(-EROFS); if (handle) { - J_ASSERT(handle->h_transaction->t_journal == journal); + J_ASSERT(handle->h_journal == journal); handle->h_ref++; return handle; } @@ -322,11 +323,15 @@ out: int journal_extend(handle_t *handle, int nblocks) { transaction_t *transaction = handle->h_transaction; - journal_t *journal = transaction->t_journal; + journal_t *journal = handle->h_journal; int result; int wanted; result = -EIO; + + if (unlikely(transaction == NULL)) + J_ASSERT(is_handle_aborted(handle)); + if (is_handle_aborted(handle)) goto out; @@ -388,13 +393,16 @@ out: int journal_restart(handle_t *handle, int nblocks) { transaction_t *transaction = handle->h_transaction; - journal_t *journal = transaction->t_journal; + journal_t *journal = handle->h_journal; int ret; + if (unlikely(transaction == NULL)) + J_ASSERT(is_handle_aborted(handle)); + /* If we've had an abort of any type, don't even think about * actually doing the restart! */ if (is_handle_aborted(handle)) - return 0; + return -EROFS; /* * First unlink the handle from its current transaction, and start the @@ -407,6 +415,7 @@ int journal_restart(handle_t *handle, int nblocks) spin_lock(&transaction->t_handle_lock); transaction->t_outstanding_credits -= handle->h_buffer_credits; transaction->t_updates--; + handle->h_transaction = NULL; if (!transaction->t_updates) wake_up(&journal->j_wait_updates); @@ -534,18 +543,18 @@ do_get_write_access(handle_t *handle, struct journal_head *jh, int force_copy) { struct buffer_head *bh; - transaction_t *transaction; - journal_t *journal; + transaction_t *transaction = handle->h_transaction; + journal_t *journal = handle->h_journal; int error; char *frozen_buffer = NULL; int need_copy = 0; + if (unlikely(transaction == NULL)) + J_ASSERT(is_handle_aborted(handle)); + if (is_handle_aborted(handle)) return -EROFS; - transaction = handle->h_transaction; - journal = transaction->t_journal; - jbd_debug(5, "buffer_head %p, force_copy %d\n", jh, force_copy); JBUFFER_TRACE(jh, "entry"); @@ -797,12 +806,15 @@ int journal_get_write_access(handle_t *handle, struct buffer_head *bh) int journal_get_create_access(handle_t *handle, struct buffer_head *bh) { transaction_t *transaction = handle->h_transaction; - journal_t *journal = transaction->t_journal; + journal_t *journal = handle->h_journal; struct journal_head *jh = journal_add_journal_head(bh); int err; jbd_debug(5, "journal_head %p\n", jh); err = -EROFS; + if (unlikely(transaction == NULL)) + J_ASSERT(is_handle_aborted(handle)); + if (is_handle_aborted(handle)) goto out; err = 0; @@ -951,11 +963,14 @@ out: */ int journal_dirty_data(handle_t *handle, struct buffer_head *bh) { - journal_t *journal = handle->h_transaction->t_journal; + journal_t *journal = handle->h_journal; int need_brelse = 0; struct journal_head *jh; int ret = 0; + if (unlikely(handle->h_transaction == NULL)) + J_ASSERT(is_handle_aborted(handle)); + if (is_handle_aborted(handle)) return ret; @@ -1143,11 +1158,15 @@ no_journal: int journal_dirty_metadata(handle_t *handle, struct buffer_head *bh) { transaction_t *transaction = handle->h_transaction; - journal_t *journal = transaction->t_journal; + journal_t *journal = handle->h_journal; struct journal_head *jh = bh2jh(bh); jbd_debug(5, "journal_head %p\n", jh); JBUFFER_TRACE(jh, "entry"); + + if (unlikely(transaction == NULL)) + J_ASSERT(is_handle_aborted(handle)); + if (is_handle_aborted(handle)) goto out; @@ -1241,7 +1260,7 @@ journal_release_buffer(handle_t *handle, struct buffer_head *bh) int journal_forget (handle_t *handle, struct buffer_head *bh) { transaction_t *transaction = handle->h_transaction; - journal_t *journal = transaction->t_journal; + journal_t *journal = handle->h_journal; struct journal_head *jh; int drop_reserve = 0; int err = 0; @@ -1249,6 +1268,11 @@ int journal_forget (handle_t *handle, struct buffer_head *bh) BUFFER_TRACE(bh, "entry"); + if (unlikely(transaction == NULL)) { + J_ASSERT(is_handle_aborted(handle)); + return err; + } + jbd_lock_bh_state(bh); spin_lock(&journal->j_list_lock); @@ -1370,18 +1394,14 @@ drop: int journal_stop(handle_t *handle) { transaction_t *transaction = handle->h_transaction; - journal_t *journal = transaction->t_journal; - int err; + journal_t *journal = handle->h_journal; + int err = 0; pid_t pid; J_ASSERT(journal_current_handle() == handle); if (is_handle_aborted(handle)) err = -EIO; - else { - J_ASSERT(transaction->t_updates > 0); - err = 0; - } if (--handle->h_ref > 0) { jbd_debug(4, "h_ref %d -> %d\n", handle->h_ref + 1, @@ -1389,6 +1409,16 @@ int journal_stop(handle_t *handle) return err; } + if (unlikely(transaction == NULL)) { + /*isolated handle, can only exist when restart handle failed*/ + J_ASSERT(is_handle_aborted(handle)); + current->journal_info = NULL; + jbd_free_handle(handle); + return err; + } + + J_ASSERT(transaction->t_updates > 0); + jbd_debug(4, "Handle %p going down\n", handle); /* diff --git a/include/linux/ext3_jbd.h b/include/linux/ext3_jbd.h index cf82d51..2a3e9fd 100644 --- a/include/linux/ext3_jbd.h +++ b/include/linux/ext3_jbd.h @@ -136,6 +136,8 @@ int __ext3_journal_get_create_access(const char *where, int __ext3_journal_dirty_metadata(const char *where, handle_t *handle, struct buffer_head *bh); +int __ext3_journal_restart(const char *where, handle_t *handle, int nblocks); + #define ext3_journal_get_undo_access(handle, bh) \ __ext3_journal_get_undo_access(__func__, (handle), (bh)) #define ext3_journal_get_write_access(handle, bh) \ @@ -148,6 +150,8 @@ int __ext3_journal_dirty_metadata(const char *where, __ext3_journal_dirty_metadata(__func__, (handle), (bh)) #define ext3_journal_forget(handle, bh) \ __ext3_journal_forget(__func__, (handle), (bh)) +#define ext3_journal_restart(handle, nblocks) \ + __ext3_journal_restart(__func__, (handle), (nblocks)) int ext3_journal_dirty_data(handle_t *handle, struct buffer_head *bh); @@ -172,11 +176,6 @@ static inline int ext3_journal_extend(handle_t *handle, int nblocks) return journal_extend(handle, nblocks); } -static inline int ext3_journal_restart(handle_t *handle, int nblocks) -{ - return journal_restart(handle, nblocks); -} - static inline int ext3_journal_blocks_per_page(struct inode *inode) { return journal_blocks_per_page(inode); diff --git a/include/linux/jbd.h b/include/linux/jbd.h index c2049a0..9e5d134 100644 --- a/include/linux/jbd.h +++ b/include/linux/jbd.h @@ -360,6 +360,7 @@ struct handle_s { /* Which compound transaction is this update a part of? */ transaction_t *h_transaction; + journal_t *h_journal; /* Number of remaining buffers we are allowed to dirty: */ int h_buffer_credits;