The patch titled
jbd: strictly check for write errors on data buffers
has been removed from the -mm tree. Its filename was
jbd-strictly-check-for-write-errors-on-data-buffers.patch
This patch was dropped because I don't think we want to go read-only on file data write errors
The current -mm tree may be found at http://userweb.kernel.org/~akpm/mmotm/
------------------------------------------------------
Subject: jbd: strictly check for write errors on data buffers
From: Hidehiro Kawai <[email protected]>
In ordered mode, we should abort journaling when an I/O error has occurred
on a file data buffer in the committing transaction. But there can be
data buffers which are not checked for error:
(a) the buffer which has already been written out by pdflush
(b) the buffer which has been unlocked before scanned in the
t_locked_list loop
This patch adds missing error checks and aborts journaling
appropriately.
Signed-off-by: Hidehiro Kawai <[email protected]>
Acked-by: Jan Kara <[email protected]>
Cc: <[email protected]>
Signed-off-by: Andrew Morton <[email protected]>
---
fs/jbd/commit.c | 14 +++++++++-----
1 file changed, 9 insertions(+), 5 deletions(-)
diff -puN fs/jbd/commit.c~jbd-strictly-check-for-write-errors-on-data-buffers fs/jbd/commit.c
--- a/fs/jbd/commit.c~jbd-strictly-check-for-write-errors-on-data-buffers
+++ a/fs/jbd/commit.c
@@ -172,7 +172,7 @@ static void journal_do_submit_data(struc
/*
* Submit all the data buffers to disk
*/
-static void journal_submit_data_buffers(journal_t *journal,
+static int journal_submit_data_buffers(journal_t *journal,
transaction_t *commit_transaction)
{
struct journal_head *jh;
@@ -180,6 +180,7 @@ static void journal_submit_data_buffers(
int locked;
int bufs = 0;
struct buffer_head **wbuf = journal->j_wbuf;
+ int err = 0;
/*
* Whenever we unlock the journal and sleep, things can get added
@@ -253,6 +254,8 @@ write_out_data:
put_bh(bh);
} else {
BUFFER_TRACE(bh, "writeout complete: unfile");
+ if (unlikely(!buffer_uptodate(bh)))
+ err = -EIO;
__journal_unfile_buffer(jh);
jbd_unlock_bh_state(bh);
if (locked)
@@ -271,6 +274,8 @@ write_out_data:
}
spin_unlock(&journal->j_list_lock);
journal_do_submit_data(wbuf, bufs);
+
+ return err;
}
/*
@@ -410,8 +415,7 @@ void journal_commit_transaction(journal_
* Now start flushing things to disk, in the order they appear
* on the transaction lists. Data blocks go first.
*/
- err = 0;
- journal_submit_data_buffers(journal, commit_transaction);
+ err = journal_submit_data_buffers(journal, commit_transaction);
/*
* Wait for all previously submitted IO to complete.
@@ -426,10 +430,10 @@ void journal_commit_transaction(journal_
if (buffer_locked(bh)) {
spin_unlock(&journal->j_list_lock);
wait_on_buffer(bh);
- if (unlikely(!buffer_uptodate(bh)))
- err = -EIO;
spin_lock(&journal->j_list_lock);
}
+ if (unlikely(!buffer_uptodate(bh)))
+ err = -EIO;
if (!inverted_lock(journal, bh)) {
put_bh(bh);
spin_lock(&journal->j_list_lock);
_
Patches currently in -mm which might be from [email protected] are
jbd-strictly-check-for-write-errors-on-data-buffers.patch
jbd-ordered-data-integrity-fix.patch
jbd-abort-when-failed-to-log-metadata-buffers.patch
jbd-fix-error-handling-for-checkpoint-io.patch
ext3-abort-ext3-if-the-journal-has-aborted.patch
ext3-abort-ext3-if-the-journal-has-aborted-warning-fix.patch
Hello Andrew,
[email protected] wrote:
> The patch titled
> jbd: strictly check for write errors on data buffers
> has been removed from the -mm tree. Its filename was
> jbd-strictly-check-for-write-errors-on-data-buffers.patch
>
> This patch was dropped because I don't think we want to go read-only on file data write errors
>
> The current -mm tree may be found at http://userweb.kernel.org/~akpm/mmotm/
>
> ------------------------------------------------------
> Subject: jbd: strictly check for write errors on data buffers
> From: Hidehiro Kawai <[email protected]>
This patch series doesn't change the behavior on file data write
errors as I stated before, but we found that the current behavior has
been made accidentally. So yesterday I sent an additional patch(*)
which removes the invocation of journal_abort() and thus stop making
the fs read-only on file data write errors, but it seems to be late
for the -mm release preparation.
Patch(*) can be found at:
http://marc.info/?l=linux-kernel&m=121300618614453&w=2
Anyway, as this patch series was dropped from -mm, I'm going to
send a revised version.
I plan to separate these pathces into three patche set.
The first patch (set) corrects the current behavior in ordered
writes, it means it removes the invocation of journal_abort() on file
data write errors. It is the almost same as the patch(*).
The second patch set fixes error handlings for metadata writes and
checkpointing. It should be applied independently of the first
patch set, and it is the same as PATCH 3/5 to 5/5.
The third patch set makes "abort the journal on file data write errors"
tunable for mission critical users. Of course, this feature depends
on the first patch set.
Any comments?
Regards,
--
Hidehiro Kawai
Hitachi, Systems Development Laboratory
Linux Technology Center
On Tue, 10 Jun 2008 17:51:35 +0900 Hidehiro Kawai <[email protected]> wrote:
> Hello Andrew,
>
> [email protected] wrote:
>
> > The patch titled
> > jbd: strictly check for write errors on data buffers
> > has been removed from the -mm tree. Its filename was
> > jbd-strictly-check-for-write-errors-on-data-buffers.patch
> >
> > This patch was dropped because I don't think we want to go read-only on file data write errors
> >
> > The current -mm tree may be found at http://userweb.kernel.org/~akpm/mmotm/
> >
> > ------------------------------------------------------
> > Subject: jbd: strictly check for write errors on data buffers
> > From: Hidehiro Kawai <[email protected]>
>
> This patch series doesn't change the behavior on file data write
> errors as I stated before, but we found that the current behavior has
> been made accidentally. So yesterday I sent an additional patch(*)
> which removes the invocation of journal_abort() and thus stop making
> the fs read-only on file data write errors, but it seems to be late
> for the -mm release preparation.
>
> Patch(*) can be found at:
> http://marc.info/?l=linux-kernel&m=121300618614453&w=2
>
> Anyway, as this patch series was dropped from -mm, I'm going to
> send a revised version.
>
> I plan to separate these pathces into three patche set.
> The first patch (set) corrects the current behavior in ordered
> writes, it means it removes the invocation of journal_abort() on file
> data write errors. It is the almost same as the patch(*).
> The second patch set fixes error handlings for metadata writes and
> checkpointing. It should be applied independently of the first
> patch set, and it is the same as PATCH 3/5 to 5/5.
> The third patch set makes "abort the journal on file data write errors"
> tunable for mission critical users. Of course, this feature depends
> on the first patch set.
>
That sounds like a good plan, thanks.
gah, I had html enabled... resend.
On Mon, Jul 14, 2008 at 10:08 AM, Mike Snitzer <[email protected]> wrote:
>
>
> On Tue, Jun 10, 2008 at 5:17 AM, Andrew Morton <[email protected]> wrote:
>>
>> On Tue, 10 Jun 2008 17:51:35 +0900 Hidehiro Kawai <[email protected]> wrote:
>>
>> > Hello Andrew,
>> >
>> > [email protected] wrote:
>> >
>> > > The patch titled
>> > > jbd: strictly check for write errors on data buffers
>> > > has been removed from the -mm tree. Its filename was
>> > > jbd-strictly-check-for-write-errors-on-data-buffers.patch
>> > >
>> > > This patch was dropped because I don't think we want to go read-only on file data write errors
>> > >
>> > > The current -mm tree may be found at http://userweb.kernel.org/~akpm/mmotm/
>> > >
>> > > ------------------------------------------------------
>> > > Subject: jbd: strictly check for write errors on data buffers
>> > > From: Hidehiro Kawai <[email protected]>
>> >
>> > This patch series doesn't change the behavior on file data write
>> > errors as I stated before, but we found that the current behavior has
>> > been made accidentally. So yesterday I sent an additional patch(*)
>> > which removes the invocation of journal_abort() and thus stop making
>> > the fs read-only on file data write errors, but it seems to be late
>> > for the -mm release preparation.
>> >
>> > Patch(*) can be found at:
>> > http://marc.info/?l=linux-kernel&m=121300618614453&w=2
>> >
>> > Anyway, as this patch series was dropped from -mm, I'm going to
>> > send a revised version.
>> >
>> > I plan to separate these pathces into three patche set.
>> > The first patch (set) corrects the current behavior in ordered
>> > writes, it means it removes the invocation of journal_abort() on file
>> > data write errors. It is the almost same as the patch(*).
>> > The second patch set fixes error handlings for metadata writes and
>> > checkpointing. It should be applied independently of the first
>> > patch set, and it is the same as PATCH 3/5 to 5/5.
>> > The third patch set makes "abort the journal on file data write errors"
>> > tunable for mission critical users. Of course, this feature depends
>> > on the first patch set.
>> >
>>
>> That sounds like a good plan, thanks.
>
> Hidehiro and Andrew,
>
> The first patch(set) has been in -mm with the following patches:
> jbd-dont-abort-if-flushing-file-data-failed.patch
> jbd-dont-abort-if-flushing-file-data-failed-fix.patch
>
> "PATCH 3/5 to 5/5" haven't made their way into -mm; nor has the tunable "abort the journal on file data write errors". Where do things stand on this work?
>
> Given the potential for corruption and the fact that -mm's series file justifiably has a place-holder comment of "jbd write-error stuff: scary" I'm wondering: how soon will all associated fixes be included in -mm?
>
> regards,
> Mike
On Mon, 14 Jul 2008 10:08:24 -0400 "Mike Snitzer" <[email protected]> wrote:
> On Tue, Jun 10, 2008 at 5:17 AM, Andrew Morton <[email protected]>
> wrote:
>
> > On Tue, 10 Jun 2008 17:51:35 +0900 Hidehiro Kawai <
> > [email protected]> wrote:
> >
> > > Hello Andrew,
> > >
> > > [email protected] wrote:
> > >
> > > > The patch titled
> > > > jbd: strictly check for write errors on data buffers
> > > > has been removed from the -mm tree. Its filename was
> > > > jbd-strictly-check-for-write-errors-on-data-buffers.patch
> > > >
> > > > This patch was dropped because I don't think we want to go read-only on
> > file data write errors
> > > >
> > > > The current -mm tree may be found at
> > http://userweb.kernel.org/~akpm/mmotm/<http://userweb.kernel.org/%7Eakpm/mmotm/>
> > > >
> > > > ------------------------------------------------------
> > > > Subject: jbd: strictly check for write errors on data buffers
> > > > From: Hidehiro Kawai <[email protected]>
> > >
> > > This patch series doesn't change the behavior on file data write
> > > errors as I stated before, but we found that the current behavior has
> > > been made accidentally. So yesterday I sent an additional patch(*)
> > > which removes the invocation of journal_abort() and thus stop making
> > > the fs read-only on file data write errors, but it seems to be late
> > > for the -mm release preparation.
> > >
> > > Patch(*) can be found at:
> > > http://marc.info/?l=linux-kernel&m=121300618614453&w=2
> > >
> > > Anyway, as this patch series was dropped from -mm, I'm going to
> > > send a revised version.
> > >
> > > I plan to separate these pathces into three patche set.
> > > The first patch (set) corrects the current behavior in ordered
> > > writes, it means it removes the invocation of journal_abort() on file
> > > data write errors. It is the almost same as the patch(*).
> > > The second patch set fixes error handlings for metadata writes and
> > > checkpointing. It should be applied independently of the first
> > > patch set, and it is the same as PATCH 3/5 to 5/5.
> > > The third patch set makes "abort the journal on file data write errors"
> > > tunable for mission critical users. Of course, this feature depends
> > > on the first patch set.
> > >
> >
> > That sounds like a good plan, thanks.
>
>
> Hidehiro and Andrew,
>
> The first patch(set) has been in -mm with the following patches:
> jbd-dont-abort-if-flushing-file-data-failed.patch
> jbd-dont-abort-if-flushing-file-data-failed-fix.patch
>
> "PATCH 3/5 to 5/5" haven't made their way into -mm; nor has the tunable
> "abort the journal on file data write errors". Where do things stand on
> this work?
>
> Given the potential for corruption and the fact that -mm's series file
> justifiably has a place-holder comment of "jbd write-error stuff: scary" I'm
> wondering: how soon will all associated fixes be included in -mm?
I assume they'll be resent if/when they're ready?
Mike Snitzer wrote:
> On Tue, Jun 10, 2008 at 5:17 AM, Andrew Morton <[email protected]>
> wrote:
>
>
>>On Tue, 10 Jun 2008 17:51:35 +0900 Hidehiro Kawai <
>>[email protected]> wrote:
>>
>>
>>>Hello Andrew,
>>>
>>>[email protected] wrote:
>>>
>>>
>>>>The patch titled
>>>> jbd: strictly check for write errors on data buffers
>>>>has been removed from the -mm tree. Its filename was
>>>> jbd-strictly-check-for-write-errors-on-data-buffers.patch
>>>>
>>>>This patch was dropped because I don't think we want to go read-only on
>>
>>file data write errors
>>
>>>>The current -mm tree may be found at
>>
>>http://userweb.kernel.org/~akpm/mmotm/<http://userweb.kernel.org/%7Eakpm/mmotm/>
>>
>>>>------------------------------------------------------
>>>>Subject: jbd: strictly check for write errors on data buffers
>>>>From: Hidehiro Kawai <[email protected]>
>>>
>>>This patch series doesn't change the behavior on file data write
>>>errors as I stated before, but we found that the current behavior has
>>>been made accidentally. So yesterday I sent an additional patch(*)
>>>which removes the invocation of journal_abort() and thus stop making
>>>the fs read-only on file data write errors, but it seems to be late
>>>for the -mm release preparation.
>>>
>>> Patch(*) can be found at:
>>> http://marc.info/?l=linux-kernel&m=121300618614453&w=2
>>>
>>>Anyway, as this patch series was dropped from -mm, I'm going to
>>>send a revised version.
>>>
>>>I plan to separate these pathces into three patche set.
>>>The first patch (set) corrects the current behavior in ordered
>>>writes, it means it removes the invocation of journal_abort() on file
>>>data write errors. It is the almost same as the patch(*).
>>>The second patch set fixes error handlings for metadata writes and
>>>checkpointing. It should be applied independently of the first
>>>patch set, and it is the same as PATCH 3/5 to 5/5.
>>>The third patch set makes "abort the journal on file data write errors"
>>>tunable for mission critical users. Of course, this feature depends
>>>on the first patch set.
>>>
>>
>>That sounds like a good plan, thanks.
>
> Hidehiro and Andrew,
>
> The first patch(set) has been in -mm with the following patches:
> jbd-dont-abort-if-flushing-file-data-failed.patch
> jbd-dont-abort-if-flushing-file-data-failed-fix.patch
>
> "PATCH 3/5 to 5/5" haven't made their way into -mm; nor has the tunable
> "abort the journal on file data write errors". Where do things stand on
> this work?
>
> Given the potential for corruption and the fact that -mm's series file
> justifiably has a place-holder comment of "jbd write-error stuff: scary" I'm
> wondering: how soon will all associated fixes be included in -mm?
Hello Mike,
Sorry for my late work. I'm going to send these two patch set soon,
but I have a trouble, 2.6.26-rc8-mm1 doesn't boot on my box.
So it may a bit more delay.
Regards,
--
Hidehiro Kawai
Hitachi, Systems Development Laboratory
Linux Technology Center