[PATCH] [RFC] Fix data missing when reusing bh which is ready to be checkpointed

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



From: zhanchengbin <zhanchengbin1@xxxxxxxxxx>

Following process will make data lost and could lead to a filesystem
corrupted problem:

1. jh(bh) is inserted into T1->t_checkpoint_list, bh is dirty, and
   jh->b_transaction = NULL
2. T1 is added into journal->j_checkpoint_transactions.
3. Get bh prepare to write while doing checkpoing:
           PA				    PB
   do_get_write_access             jbd2_log_do_checkpoint
    spin_lock(&jh->b_state_lock)
     if (buffer_dirty(bh))
      clear_buffer_dirty(bh)   // clear buffer dirty
       set_buffer_jbddirty(bh)
				    transaction =
				    journal->j_checkpoint_transactions
				    jh = transaction->t_checkpoint_list
				    if (!buffer_dirty(bh))
		                      __jbd2_journal_remove_checkpoint(jh)
				      // bh won't be flushed
		                    jbd2_cleanup_journal_tail
    __jbd2_journal_file_buffer(jh, transaction, BJ_Reserved)
4. Aborting journal/Power-cut before writing latest bh on journal area.

In this way we get a corrupted filesystem with bh'data lost.

Fix it by wrapping clear_buffer_dirty(bh) and jh->b_transaction setting
into journal->j_list_lock, so that jbd2_log_do_checkpoint() will wait
until jh's new transaction fininshed even bh is currently not dirty.

Cc: <stable@xxxxxxxxxx>
Signed-off-by: zhanchengbin <zhanchengbin1@xxxxxxxxxx>
Signed-off-by: Baokun Li <libaokun1@xxxxxxxxxx>
Signed-off-by: Zhihao Cheng <chengzhihao1@xxxxxxxxxx>
---
 This is a quick fix, I need some suggestions about this patch, whether
 it will import new problems if this patch is applied.
 Yi suggests that the formal solution could be splitting
 journal->j_list_lock into two locks: one protects checkpoint list and
 the other one for other lists. Besides, jh->b_state_lock should be
 held while traversing transaction->t_checkpoint_list in
 jbd2_log_do_checkpoint()/journal_shrink_one_cp_list().

 fs/jbd2/transaction.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/fs/jbd2/transaction.c b/fs/jbd2/transaction.c
index 6a404ac1c178..d22460001d6b 100644
--- a/fs/jbd2/transaction.c
+++ b/fs/jbd2/transaction.c
@@ -990,6 +990,7 @@ do_get_write_access(handle_t *handle, struct journal_head *jh,
  	start_lock = jiffies;
 	lock_buffer(bh);
 	spin_lock(&jh->b_state_lock);
+	spin_lock(&journal->j_list_lock);
 
 	/* If it takes too long to lock the buffer, trace it */
 	time_lock = jbd2_time_diff(start_lock, jiffies);
@@ -1039,6 +1040,7 @@ do_get_write_access(handle_t *handle, struct journal_head *jh,
 
 	error = -EROFS;
 	if (is_handle_aborted(handle)) {
+		spin_unlock(&journal->j_list_lock);
 		spin_unlock(&jh->b_state_lock);
 		goto out;
 	}
@@ -1049,8 +1051,10 @@ do_get_write_access(handle_t *handle, struct journal_head *jh,
 	 * b_next_transaction points to it
 	 */
 	if (jh->b_transaction == transaction ||
-	    jh->b_next_transaction == transaction)
+	    jh->b_next_transaction == transaction) {
+		spin_unlock(&journal->j_list_lock);
 		goto done;
+	}
 
 	/*
 	 * this is the first time this transaction is touching this buffer,
@@ -1073,11 +1077,11 @@ do_get_write_access(handle_t *handle, struct journal_head *jh,
 		 * Paired with barrier in jbd2_write_access_granted()
 		 */
 		smp_wmb();
-		spin_lock(&journal->j_list_lock);
 		__jbd2_journal_file_buffer(jh, transaction, BJ_Reserved);
 		spin_unlock(&journal->j_list_lock);
 		goto done;
 	}
+	spin_unlock(&journal->j_list_lock);
 	/*
 	 * If there is already a copy-out version of this buffer, then we don't
 	 * need to make another one
-- 
2.31.1




[Index of Archives]     [Reiser Filesystem Development]     [Ceph FS]     [Kernel Newbies]     [Security]     [Netfilter]     [Bugtraq]     [Linux FS]     [Yosemite National Park]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Device Mapper]     [Linux Media]

  Powered by Linux