[PATCH] ReiserFS v3 I/O error handling

[PATCH] ReiserFS v3 I/O error handling

[Jeff Mahoney]

-----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.

I've posted four patches at:
ftp://ftp.suse.com/pub/people/jeffm/reiserfs/kernel-v2.6/io-error/

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.

- -Jeff

[Resent: The patches initially were attached, and I suspect they were
too large to make it onto the list.]

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

iD8DBQFBRxC3LPWxlyuTD7IRAmyYAJ4t2zN0ZnGMWp4FV8CIVQYVcuOhqACfdMlJ
rSdILv0XFfcWh7lyCbQPyAY=
=iTX3
-----END PGP SIGNATURE-----

Re: [PATCH] ReiserFS v3 I/O error handling

[Hans Reiser]

Jeff Mahoney wrote:

> 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.
>
> I've posted four patches at:
> ftp://ftp.suse.com/pub/people/jeffm/reiserfs/kernel-v2.6/io-error/
>
> 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.
>
> -Jeff
>
> [Resent: The patches initially were attached, and I suspect they were
> too large to make it onto the list.]
>
> --
> Jeff Mahoney
> SuSE Labs

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

I am going to let zam review it on my behalf due to urgent personal 
issues.  If he does not respond in seven days, complain to me about it 
and my personal issues will be past me by then.

Hans

[PATCH] ReiserFS v3 I/O error handling

[Jeff Mahoney]

-----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-----

Re: [PATCH] ReiserFS v3 I/O error handling

[Alex Zarochentsev]

Hi Jeff,

On Mon, Sep 13, 2004 at 06:11:59PM -0400, Jeff Mahoney wrote:
> -----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.
ok

> * 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.
ok

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

> * 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. 

> 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-----

-- 
Alex.

Re: [PATCH] ReiserFS v3 I/O error handling

[Jeff Mahoney]

-----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-----

Re: [PATCH] ReiserFS v3 I/O error handling

[Alex Zarochentsev]

On Wed, Sep 15, 2004 at 09:47:45AM -0400, Jeff Mahoney wrote:
> -----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.
> 

I just checked that what we discussed for reiser4 is a right thing :)
But it is not fully implemented yet :-/

> 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.

I assume that was tested with some simulated i/o errors, wasn't it?.

> - -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.

Re: [PATCH] ReiserFS v3 I/O error handling

[Jeff Mahoney]

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

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
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.2.4 (GNU/Linux)
Comment: Using GnuPG with Thunderbird - http://enigmail.mozdev.org

iD8DBQFBSFIfLPWxlyuTD7IRAqljAKCoCuXGN4GSG+hQ2RuiSNtuMZSVuwCgnEMp
gQhDrTbctbjBYwD8U73EB5E=
=2OFI
-----END PGP SIGNATURE-----

Re: [PATCH] ReiserFS v3 I/O error handling

[Hans Reiser]

Jeff Mahoney wrote:

> 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.

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

>
> -Jeff
>
> --
> Jeff Mahoney
> SuSE Labs

Re: [PATCH] ReiserFS v3 I/O error handling

[Chris Mason]

On Wed, 2004-09-15 at 10:31, Hans Reiser wrote:
> Jeff Mahoney wrote:
> 
> > 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.
> 
> 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

Re: [PATCH] ReiserFS v3 I/O error handling

[Hans Reiser]

Chris Mason wrote:

>On Wed, 2004-09-15 at 10:31, Hans Reiser wrote:
>  
>
>>Jeff Mahoney wrote:
>>
>>    
>>
>>>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.
>>>      
>>>
>>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
>
>
>
>
>  
>
I'll let zam answer for me.

Re: [PATCH] ReiserFS v3 I/O error handling

[Alex Zarochentsev]

On Wed, Sep 22, 2004 at 08:39:06AM -0700, Hans Reiser wrote:
> Chris Mason wrote:
> 
> >On Wed, 2004-09-15 at 10:31, Hans Reiser wrote:
> > 
> >
> >>Jeff Mahoney wrote:
> >>
> >>   
> >>
> >>>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.
> >>>     
> >>>
> >>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?

no objections. 

> >
> >-chris
> >
> I'll let zam answer for me.


-- 
Alex.