ReiserFS v3 I/O error handling

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Hey all -

One of the most common complaints I've heard about ReiserFS is how graceless it is in handling critical I/O errors.

ext[23] can handle I/O errors anywhere, with the results being up to the system admin to determine: continue, go read only, or panic.

ReiserFS doesn't offer the admin any such choice, instead panicking on any I/O error in the journal.

Attached are four patches, against 2.6.9-rc2:
* reiserfs-cleanup-buffer-heads.diff
- Cleans up handling of buffer head bitfields - uses the kernel supplied FNS_BUFFER macros instead.

* reiserfs-cleanup-sb-journal.diff
- Cleans up accessing of the journal structure, prefering

~          to create a temporary variable in functions that access
~          the journal structure non-trivially. Should make 0 difference
~          at compile time.

* reiserfs-write-lock.diff
- Fixes two missing reiserfs_write_unlock() calls on error paths
~ that are unrelated to the last patch.

* reiserfs-io-error-handling.diff
- Allows ReiserFS to gracefully handle I/O errors in critical code paths. The admin has the option to go read-only or panic. Since ReiserFS has no option to ignore the use of the journal,
~ the "continue" method is not enabled.

These patches have seen a lot of testing in the SuSE Linux Enterprise Server 9 kernel, and are considered ready for mainline.

Hans - please take a look.

Resent: Since the namesys.com mail server refuses messages over 40k, I've posted the patches at ftp://ftp.suse.com/pub/people/jeffm/reiserfs/kernel-v2.6/io-error/

- -Jeff

- --
Jeff Mahoney
SuSE Labs
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.2.4 (GNU/Linux)
Comment: Using GnuPG with Thunderbird - http://enigmail.mozdev.org

iD8DBQFBRhsvLPWxlyuTD7IRAqa7AJ9ZJa+HnOGYU7OIouWMu34H2mU0MQCgn0H5 QBdLo95dLiXsQdwojQSHAos=
=kPTJ
-----END PGP SIGNATURE-----

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Alex Zarochentsev wrote:

| * reiserfs-io-error-handling.diff
| 	- Allows ReiserFS to gracefully handle I/O errors in critical
| 	  code paths. The admin has the option to go read-only or panic.
| 	  Since ReiserFS has no option to ignore the use of the journal,
|         the "continue" method is not enabled.
|
|
|> What reiserfs will do with already dirty blocks on the R/O fs.  Those
blocks
|> remain dirty, yes?
|
|> The whole patch is enough complex, one look is not enough :) I think

Elena may
|> do additional testing to be sure that nothing is broken while improving i/o
|> handling.

Yes, blocks that are already dirty will remain dirty. This is safe.

Here are the cases:
* Transaction hasn't started yet, and the journal has aborted ~ - Deny the transaction start in do_journal_begin_r, returning -EROFS
* Transaction has started, journal was aborted in the meantime ~ - Finish cleaning up, don't write the commit block, return -EIO
~ from do_journal_end
* Transaction has completed, and aborts during flush_commit_list() ~ - Finish writing, but don't write commit block
* Transaction has completed, and aborts during flush_journal_list() ~ - Finish writing, but don't update journal header

Since this is an error path for I/O errors, NOT internal inconsistencies, it's safe to continue writing to the disk so long as we do so with the expectation that whatever we write there will be handled elsewhere.

One of the tenets of journaling filesystems is that if the transaction isn't commited, it never existed. The I/O error handling code takes advantage of this by completing whatever I/O is outstanding, which makes the code *far* less intrusive. My initial implementation tried cleaning stuff up, and it's extremely involved for very little gain. Once the
journal has aborted, updates to the journal's state just don't happen. Yes, blocks get written into it -- but the commit block doesn't. The transaction isn't commited: It never existed. Outstanding journal flushes can still flush to the disk, but it won't update the journal header. The transaction can be replayed the next time the filesystem is mounted.

I view it like this: An aborted filesystem is no different than one on a system which has lost power. The journal protects us here. Nothing after reiserfs_abort() exists, since the journal operations aren't commited. There will be incomplete transactions and incomplete flushes, but that's what the journal is there to handle in the first place.

The admin will be able to perform the normal operations before umounting a filesystem (closing open files, chdir'ing out of it), and the filesystem will be umountable. The admin can check it, and remount if they choose.

- -Jeff
- --
Jeff Mahoney
SuSE Labs
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.2.4 (GNU/Linux)
Comment: Using GnuPG with Thunderbird - http://enigmail.mozdev.org

iD8DBQFBSEgBLPWxlyuTD7IRAvyBAJ0XJW7eP6eThwSK6YT9K3hdxlTT0ACfTLxY LIJEucE1EisIn9VQPXon/dw=
=cQx8
-----END PGP SIGNATURE-----

Alex Zarochentsev wrote:
|> I assume that was tested with some simulated i/o errors, wasn't it?.

Of course. The debugging code is since removed, but every place there was a !buffer_uptodate(bh) check, I added a trigger such that I could trigger each error path individually. I triggered various error paths while running fsx, stress.sh, and/or ltp's fsstress.

-Jeff

--
Jeff Mahoney
SuSE Labs

Quoted 3 lines · Jeff Mahoney

Alex Zarochentsev wrote:
|> I assume that was tested with some simulated i/o errors, wasn't it?.

Quoted 4 lines

Of course. The debugging code is since removed, but every place there was a !buffer_uptodate(bh) check, I added a trigger such that I could trigger each error path individually. I triggered various error paths while running fsx, stress.sh, and/or ltp's fsstress.

Your patch is much needed stuff. Would be nice to see it for reiser4 someday.....;-)

Quoted 13 lines · Hans Reiser

Quoted 9 lines · Jeff Mahoney

Alex Zarochentsev wrote:
|> I assume that was tested with some simulated i/o errors, wasn't it?.

Of course. The debugging code is since removed, but every place there was a !buffer_uptodate(bh) check, I added a trigger such that I could trigger each error path individually. I triggered various error paths while running fsx, stress.sh, and/or ltp's fsstress.

Quoted 3 lines

Your patch is much needed stuff. Would be nice to see it for reiser4 someday.....;-)

;-) Any objections if we start by sending this to Andrew for v3?

-chris

Quoted 16 lines · Chris Mason

Quoted 13 lines · Hans Reiser

Quoted 9 lines · Jeff Mahoney

Alex Zarochentsev wrote:
|> I assume that was tested with some simulated i/o errors, wasn't it?.

Of course. The debugging code is since removed, but every place there was a !buffer_uptodate(bh) check, I added a trigger such that I could trigger each error path individually. I triggered various error paths while running fsx, stress.sh, and/or ltp's fsstress.

Quoted 6 lines

Quoted 4 lines

Your patch is much needed stuff. Would be nice to see it for reiser4 someday.....;-)

;-) Any objections if we start by sending this to Andrew for v3?

Quoted 3 lines

-chris

I'll let zam answer for me.