Re: [PATCH 1/5] jbd: strictly check for write errors on data buffers

[Hidehiro Kawai]

Andreas Dilger wrote:
> On Jun 05, 2008  09:20 -0700, Andrew Morton wrote:

>>I guess we need to undo this.  And yes, propagating errors into AS_EIO
>>is the way.  I guess that's safe without holding lock_page(), as long
>>as the bh is pinned.
> 
> Something like the following instead if -EIO and journal abort:
> 
> 		if (!buffer_uptodate(bh)) {
> 			set_bit(AS_EIO, &bh->b_page->mapping->flags);
> 			SetPageError(bh->b_page);
> 		}
> 
> It seems end_buffer_async_write() does this already, but
> journal_do_submit_data() uses end_buffer_write_sync() and it does not
> do either of those operations.

Thank you for your suggestion.  I wrote an additional patch to do
that below.  Please apply it as the 6th patch of this patch series.

BTW, I'm developing a patch which makes "abort the journal if a file
data buffer has an error" tunable.  I'll send it in another thread
because it's not a bug fix patch.

Regards,
-- 
Hidehiro Kawai
Hitachi, Systems Development Laboratory
Linux Technology Center


Subject: JBD: don't abort if flushing file data failed

In ordered mode, it is not appropriate behavior to abort the journal
when we failed to write file data.  This patch calls printk()
instead of aborting the journal.  Additionally, set AS_EIO into
the address_space object of the buffer which is written out by
journal_do_submit_data() and failed so that fsync() can get -EIO.

Signed-off-by: Hidehiro Kawai <hidehiro.kawai.ez@xxxxxxxxxxx>
---
 fs/jbd/commit.c |   12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

Index: linux-2.6.26-rc4/fs/jbd/commit.c
===================================================================
--- linux-2.6.26-rc4.orig/fs/jbd/commit.c
+++ linux-2.6.26-rc4/fs/jbd/commit.c
@@ -432,8 +432,11 @@ void journal_commit_transaction(journal_
 			wait_on_buffer(bh);
 			spin_lock(&journal->j_list_lock);
 		}
-		if (unlikely(!buffer_uptodate(bh)))
+		if (unlikely(!buffer_uptodate(bh))) {
+			set_bit(AS_EIO, &bh->b_page->mapping->flags);
+			SetPageError(bh->b_page);
 			err = -EIO;
+		}
 		if (!inverted_lock(journal, bh)) {
 			put_bh(bh);
 			spin_lock(&journal->j_list_lock);
@@ -452,8 +455,11 @@ void journal_commit_transaction(journal_
 	}
 	spin_unlock(&journal->j_list_lock);
 
-	if (err)
-		journal_abort(journal, err);
+	if (err) {
+		printk(KERN_WARNING
+		       "JBD: Detected IO errors during flushing file data\n");
+		err = 0;
+	}
 
 	journal_write_revoke_records(journal, commit_transaction);
 



--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/