2008-09-20 23:10:30

by Theodore Ts'o

[permalink] [raw]
Subject: [PATCH] ext3/jbd: Avoid WARN() messages when failing to write to the superblock

This fixes some very common warnings reported by kerneloops.org

Signed-off-by: "Theodore Ts'o" <[email protected]>
---
fs/ext3/super.c | 23 ++++++++++++++++++++++-
fs/jbd/journal.c | 29 +++++++++++++++++++++++++++--
2 files changed, 49 insertions(+), 3 deletions(-)

diff --git a/fs/ext3/super.c b/fs/ext3/super.c
index f38a5af..b3f8eb8 100644
--- a/fs/ext3/super.c
+++ b/fs/ext3/super.c
@@ -2259,13 +2259,34 @@ static void ext3_commit_super (struct super_block * sb,

if (!sbh)
return;
+ if (buffer_write_io_error(sbh)) {
+ /*
+ * Oh, dear. A previous attempt to write the
+ * superblock failed. This could happen because the
+ * USB device was yanked out. Or it could happen to
+ * be a transient write error and maybe the block will
+ * be remapped. Nothing we can do but to retry the
+ * write and hope for the best.
+ */
+ printk(KERN_ERR "ext3: previous I/O error to "
+ "superblock detected for %s.\n", sb->s_id);
+ clear_buffer_write_io_error(sbh);
+ set_buffer_uptodate(sbh);
+ }
es->s_wtime = cpu_to_le32(get_seconds());
es->s_free_blocks_count = cpu_to_le32(ext3_count_free_blocks(sb));
es->s_free_inodes_count = cpu_to_le32(ext3_count_free_inodes(sb));
BUFFER_TRACE(sbh, "marking dirty");
mark_buffer_dirty(sbh);
- if (sync)
+ if (sync) {
sync_dirty_buffer(sbh);
+ if (buffer_write_io_error(sbh)) {
+ printk(KERN_ERR "ext3: I/O error while writing "
+ "superblock for %s.\n", sb->s_id);
+ clear_buffer_write_io_error(sbh);
+ set_buffer_uptodate(sbh);
+ }
+ }
}


diff --git a/fs/jbd/journal.c b/fs/jbd/journal.c
index aa7143a..8214175 100644
--- a/fs/jbd/journal.c
+++ b/fs/jbd/journal.c
@@ -83,6 +83,7 @@ EXPORT_SYMBOL(journal_force_commit);

static int journal_convert_superblock_v1(journal_t *, journal_superblock_t *);
static void __journal_abort_soft (journal_t *journal, int errno);
+static const char *journal_dev_name(journal_t *journal, char *buffer);

/*
* Helper function used to manage commit timeouts
@@ -931,6 +932,7 @@ void journal_update_superblock(journal_t *journal, int wait)
{
journal_superblock_t *sb = journal->j_superblock;
struct buffer_head *bh = journal->j_sb_buffer;
+ char b[BDEVNAME_SIZE];

/*
* As a special case, if the on-disk copy is already marked as needing
@@ -948,6 +950,22 @@ void journal_update_superblock(journal_t *journal, int wait)
goto out;
}

+ if (buffer_write_io_error(bh)) {
+ /*
+ * Oh, dear. A previous attempt to write the journal
+ * superblock failed. This could happen because the
+ * USB device was yanked out. Or it could happen to
+ * be a transient write error and maybe the block will
+ * be remapped. Nothing we can do but to retry the
+ * write and hope for the best.
+ */
+ printk(KERN_ERR "JBD: previous I/O error detected "
+ "for journal superblock update for %s.\n",
+ journal_dev_name(journal, b));
+ clear_buffer_write_io_error(bh);
+ set_buffer_uptodate(bh);
+ }
+
spin_lock(&journal->j_state_lock);
jbd_debug(1,"JBD: updating superblock (start %ld, seq %d, errno %d)\n",
journal->j_tail, journal->j_tail_sequence, journal->j_errno);
@@ -959,9 +977,16 @@ void journal_update_superblock(journal_t *journal, int wait)

BUFFER_TRACE(bh, "marking dirty");
mark_buffer_dirty(bh);
- if (wait)
+ if (wait) {
sync_dirty_buffer(bh);
- else
+ if (buffer_write_io_error(bh)) {
+ printk(KERN_ERR "JBD: I/O error detected "
+ "when updating journal superblock for %s.\n",
+ journal_dev_name(journal, b));
+ clear_buffer_write_io_error(bh);
+ set_buffer_uptodate(bh);
+ }
+ } else
ll_rw_block(SWRITE, 1, &bh);

out:
--
1.5.6.1.205.ge2c7.dirty



2008-09-21 02:14:42

by Arjan van de Ven

[permalink] [raw]
Subject: Re: [PATCH] ext3/jbd: Avoid WARN() messages when failing to write to the superblock

On Sat, 20 Sep 2008 17:35:47 -0400
Theodore Ts'o <[email protected]> wrote:

> This fixes some very common warnings reported by kerneloops.org

just to quantify "very common"; I did a quick count on the database and
there have been 4920 reports of the WARNing fixed by this patch...
that's quite a bit.


--
Arjan van de Ven Intel Open Source Technology Centre
For development, discussion and tips for power savings,
visit http://www.lesswatts.org

2008-09-21 17:33:29

by Jan Kara

[permalink] [raw]
Subject: Re: [PATCH] ext3/jbd: Avoid WARN() messages when failing to write to the superblock

> This fixes some very common warnings reported by kerneloops.org
>
> Signed-off-by: "Theodore Ts'o" <[email protected]>
The patch looks nice but does it really fix the warnings? I see
at least ext3_put_super() calling mark_buffer_dirty() before calling
ext3_commit_super(). We should just remove that mark_buffer_dirty()
call.
BTW: Do you plan doing a similar fix for ext2 and ext4?

Honza
--
Jan Kara <[email protected]>
SuSE CR Labs

2008-09-22 02:51:34

by Theodore Ts'o

[permalink] [raw]
Subject: Re: [PATCH] ext3/jbd: Avoid WARN() messages when failing to write to the superblock

On Sun, Sep 21, 2008 at 07:33:28PM +0200, Jan Kara wrote:
> > This fixes some very common warnings reported by kerneloops.org
> >
> > Signed-off-by: "Theodore Ts'o" <[email protected]>
> The patch looks nice but does it really fix the warnings? I see
> at least ext3_put_super() calling mark_buffer_dirty() before calling
> ext3_commit_super(). We should just remove that mark_buffer_dirty()
> call.

I tested it, and in practice it works, since mark_buffer_dirty() only
causes a problem if an attempt to write to the superblock has already
failed. I agree that we should just remove that mark_buffer_dirty()
call, though, since ext3_put_super() calls ext3_commit_super() which
calls mark_buffer_dirty() anyway. I'll respin the patch.

> BTW: Do you plan doing a similar fix for ext2 and ext4?

I have a similar fix for ext4 that is queued up to be pushed once the
merge window opens. I haven't back-ported it to ext2 yet, but I will.

- Ted