[PATCH] ext3 journal commit I/O error fix

From: Hua Zhong <hzhong@cisco.com>

The current ext3 totally ignores I/O errors that happened during a
journal_force_commit time, causing user space to falsely believe it has
succeeded, which actually did not.

This patch  checks IO error during  journal_commit_transaction. and aborts
the journal when there is I/O error.

Originally I thought about reporting the error without doing aborting the
journal, but it probably needs a new flag. Aborting the journal seems to be
the easy way to  signal "hey sth is wrong..".

fs/ext3/fsync.c

@@ -72,6 +72,5 @@ int ext3_sync_file(struct file * file, struct dentry *dentry, int datasync)
 	 *  (they were dirtied by commit).  But that's OK - the blocks are
 	 *  safe in-journal, which is all fsync() needs to ensure.
 	 */
-	ext3_force_commit(inode->i_sb);
-	return 0;
+	return ext3_force_commit(inode->i_sb);
 }

fs/jbd/commit.c

@@ -73,7 +73,7 @@ void journal_commit_transaction(journal_t *journal)
 #endif
 
 	lock_kernel();
-	
+
 	J_ASSERT (journal->j_running_transaction != NULL);
 	J_ASSERT (journal->j_committing_transaction == NULL);
 
@@ -173,6 +173,7 @@ void journal_commit_transaction(journal_t *journal)
 	 * on the transaction lists.  Data blocks go first.
 	 */
 
+	err = 0;
 	/*
 	 * Whenever we unlock the journal and sleep, things can get added
 	 * onto ->t_datalist, so we have to keep looping back to write_out_data
@@ -251,9 +252,13 @@ void journal_commit_transaction(journal_t *journal)
 		jh = jh->b_tprev;	/* Wait on the last written */
 		bh = jh2bh(jh);
 		if (buffer_locked(bh)) {
+			get_bh(bh);
 			spin_unlock(&journal_datalist_lock);
 			unlock_journal(journal);
 			wait_on_buffer(bh);
+			if (unlikely(!buffer_uptodate(bh)))
+				err = -EIO;
+			put_bh(bh);
 			/* the journal_head may have been removed now */
 			lock_journal(journal);
 			goto write_out_data;
@@ -446,7 +451,10 @@ void journal_commit_transaction(journal_t *journal)
 
 	jbd_debug(3, "JBD: commit phase 4\n");
 
-	/* akpm: these are BJ_IO, and journal_datalist_lock is not needed */
+	/*
+	 * akpm: these are BJ_IO, and journal_datalist_lock is not needed.
+	 * See __journal_try_to_free_buffer.
+	 */
  wait_for_iobuf:
 	while (commit_transaction->t_iobuf_list != NULL) {
 		struct buffer_head *bh;
@@ -455,6 +463,8 @@ void journal_commit_transaction(journal_t *journal)
 		if (buffer_locked(bh)) {
 			unlock_journal(journal);
 			wait_on_buffer(bh);
+			if (unlikely(!buffer_uptodate(bh)))
+				err = -EIO;
 			lock_journal(journal);
 			goto wait_for_iobuf;
 		}
@@ -516,6 +526,8 @@ void journal_commit_transaction(journal_t *journal)
 		if (buffer_locked(bh)) {
 			unlock_journal(journal);
 			wait_on_buffer(bh);
+			if (unlikely(!buffer_uptodate(bh)))
+				err = -EIO;
 			lock_journal(journal);
 			goto wait_for_ctlbuf;
 		}
@@ -563,7 +575,9 @@ void journal_commit_transaction(journal_t *journal)
 		struct buffer_head *bh = jh2bh(descriptor);
 		set_buffer_uptodate(bh);
 		sync_dirty_buffer(bh);
-		__brelse(bh);		/* One for getblk() */
+		if (unlikely(!buffer_uptodate(bh)))
+			err = -EIO;
+		put_bh(bh);		/* One for getblk() */
 		journal_unlock_journal_head(descriptor);
 	}
 
@@ -574,6 +588,12 @@ void journal_commit_transaction(journal_t *journal)
 
 skip_commit: /* The journal should be unlocked by now. */
 
+	if (err) {
+		lock_journal(journal);
+		__journal_abort_hard(journal);
+		unlock_journal(journal);
+	}
+	
 	/* Call any callbacks that had been registered for handles in this
 	 * transaction.  It is up to the callback to free any allocated
 	 * memory.

fs/jbd/journal.c

@@ -580,8 +580,10 @@ tid_t log_start_commit (journal_t *journal, transaction_t *transaction)
  * Wait for a specified commit to complete.
  * The caller may not hold the journal lock.
  */
-void log_wait_commit (journal_t *journal, tid_t tid)
+int log_wait_commit (journal_t *journal, tid_t tid)
 {
+	int err = 0;
+
 	lock_kernel();
 #ifdef CONFIG_JBD_DEBUG
 	lock_journal(journal);
@@ -598,7 +600,14 @@ void log_wait_commit (journal_t *journal, tid_t tid)
 		wake_up(&journal->j_wait_commit);
 		sleep_on(&journal->j_wait_done_commit);
 	}
+
+	if (unlikely(is_journal_aborted(journal))) {
+		printk(KERN_EMERG "journal commit I/O error\n");
+		err = -EIO;
+	}
+
 	unlock_kernel();
+	return err;
 }
 
 /*

fs/jbd/transaction.c

@@ -1402,7 +1402,7 @@ int journal_stop(handle_t *handle)
 		 * to wait for the commit to complete.  
 		 */
 		if (handle->h_sync && !(current->flags & PF_MEMALLOC))
-			log_wait_commit(journal, tid);
+			err = log_wait_commit(journal, tid);
 	}
 	jbd_free_handle(handle);
 	return err;
@@ -1418,7 +1418,7 @@ int journal_stop(handle_t *handle)
 int journal_force_commit(journal_t *journal)
 {
 	handle_t *handle;
-	int ret = 0;
+	int ret;
 
 	lock_kernel();
 	handle = journal_start(journal, 1);
@@ -1427,7 +1427,7 @@ int journal_force_commit(journal_t *journal)
 		goto out;
 	}
 	handle->h_sync = 1;
-	journal_stop(handle);
+	ret = journal_stop(handle);
 out:
 	unlock_kernel();
 	return ret;

include/linux/jbd.h

@@ -855,7 +855,7 @@ extern void	   journal_brelse_array(struct buffer_head *b[], int n);
 
 extern int	log_space_left (journal_t *); /* Called with journal locked */
 extern tid_t	log_start_commit (journal_t *, transaction_t *);
-extern void	log_wait_commit (journal_t *, tid_t);
+extern int	log_wait_commit (journal_t *, tid_t);
 extern int	log_do_checkpoint (journal_t *, int);
 
 extern void	log_wait_for_space(journal_t *, int nblocks);