[patch] ext2: avoid WARN() messages when failing to write to buffers

[Edward Shishkin <edward@redhat.com>]

Hello.

There is a number of complaints: people yunk out usb cables
of their external hard drives and see kernel warnings, since
BH_Uptodate flag is cleared up because of io error:
http://bugzilla.redhat.com/show_bug.cgi?id=679930

There was a related commit 7bf0dc9b0ca9e9b6524b1f70e0898c7f11eb10be
however it fixes nothing: after ext2_clear_super_error() the buffer
can successfully become not-uptodate again, so that the warnings are
easily observed in the latest upstream kernels.

Also the problem takes place not only for superblock's buffers:
any buffer can become not-uptodate because of io errors.

Please consider more substantial fixup below. I performed regression
testing for the patched stuff: mongo benchmark doesn't show any
performance drop:
http://bugzilla.redhat.com/attachment.cgi?id=550660

Thanks,
Edward.

ext2: avoid WARN() messages when failing to write to buffers
( more substantial fixup than 7bf0dc9b0ca9e9b6524b1f70e0898c7f11eb10be )

Signed-off-by: Edward Shishkin <edward@redhat.com>
---
 fs/ext2/balloc.c |   12 +++++++++++-
 fs/ext2/ext2.h   |    2 ++
 fs/ext2/ialloc.c |   17 +++++++++++++++++
 fs/ext2/inode.c  |   25 +++++++++++++++++++++----
 fs/ext2/super.c  |   23 +++++++++++++----------
 fs/ext2/xattr.c  |   10 +++++++++-
 6 files changed, 73 insertions(+), 16 deletions(-)

Index: linux-3.2-rc6/fs/ext2/balloc.c
===================================================================
--- linux-3.2-rc6.orig/fs/ext2/balloc.c
+++ linux-3.2-rc6/fs/ext2/balloc.c
@@ -181,7 +181,10 @@ static void group_adjust_blocks(struct s
 		desc->bg_free_blocks_count = cpu_to_le16(free_blocks + count);
 		spin_unlock(sb_bgl_lock(sbi, group_no));
 		sb->s_dirt = 1;
+		lock_buffer(bh);
+		ext2_clear_buffer_error(sb, bh, "updating group descriptor");
 		mark_buffer_dirty(bh);
+		unlock_buffer(bh);
 	}
 }
 
@@ -555,8 +558,11 @@ do_more:
 			group_freed++;
 		}
 	}
-
+	lock_buffer(bitmap_bh);
+	ext2_clear_buffer_error(sb, bitmap_bh, "updating bitmap");
 	mark_buffer_dirty(bitmap_bh);
+	unlock_buffer(bitmap_bh);
+
 	if (sb->s_flags & MS_SYNCHRONOUS)
 		sync_dirty_buffer(bitmap_bh);
 
@@ -1411,7 +1417,11 @@ allocated:
 	group_adjust_blocks(sb, group_no, gdp, gdp_bh, -num);
 	percpu_counter_sub(&sbi->s_freeblocks_counter, num);
 
+	lock_buffer(bitmap_bh);
+	ext2_clear_buffer_error(sb, bitmap_bh, "updating bitmap");
 	mark_buffer_dirty(bitmap_bh);
+	unlock_buffer(bitmap_bh);
+
 	if (sb->s_flags & MS_SYNCHRONOUS)
 		sync_dirty_buffer(bitmap_bh);
 
Index: linux-3.2-rc6/fs/ext2/ialloc.c
===================================================================
--- linux-3.2-rc6.orig/fs/ext2/ialloc.c
+++ linux-3.2-rc6/fs/ext2/ialloc.c
@@ -82,7 +82,10 @@ static void ext2_release_inode(struct su
 	if (dir)
 		percpu_counter_dec(&EXT2_SB(sb)->s_dirs_counter);
 	sb->s_dirt = 1;
+	lock_buffer(bh);
+	ext2_clear_buffer_error(sb, bh, "updating group descriptor");
 	mark_buffer_dirty(bh);
+	unlock_buffer(bh);
 }
 
 /*
@@ -145,7 +148,12 @@ void ext2_free_inode (struct inode * ino
 			      "bit already cleared for inode %lu", ino);
 	else
 		ext2_release_inode(sb, block_group, is_directory);
+
+	lock_buffer(bitmap_bh);
+	ext2_clear_buffer_error(sb, bitmap_bh, "updating bitmap");
 	mark_buffer_dirty(bitmap_bh);
+	unlock_buffer(bitmap_bh);
+
 	if (sb->s_flags & MS_SYNCHRONOUS)
 		sync_dirty_buffer(bitmap_bh);
 
@@ -512,7 +520,11 @@ repeat_in_this_group:
 	err = -ENOSPC;
 	goto fail;
 got:
+	lock_buffer(bitmap_bh);
+	ext2_clear_buffer_error(sb, bitmap_bh, "updating bitmap");
 	mark_buffer_dirty(bitmap_bh);
+	unlock_buffer(bitmap_bh);
+
 	if (sb->s_flags & MS_SYNCHRONOUS)
 		sync_dirty_buffer(bitmap_bh);
 	brelse(bitmap_bh);
@@ -544,7 +556,12 @@ got:
 	spin_unlock(sb_bgl_lock(sbi, group));
 
 	sb->s_dirt = 1;
+
+	lock_buffer(bh2);
+	ext2_clear_buffer_error(sb, bh2, "updating group descriptor");
 	mark_buffer_dirty(bh2);
+	unlock_buffer(bh2);
+
 	if (test_opt(sb, GRPID)) {
 		inode->i_mode = mode;
 		inode->i_uid = current_fsuid();
Index: linux-3.2-rc6/fs/ext2/inode.c
===================================================================
--- linux-3.2-rc6.orig/fs/ext2/inode.c
+++ linux-3.2-rc6/fs/ext2/inode.c
@@ -514,8 +514,8 @@ static int ext2_alloc_branch(struct inod
 				*(branch[n].p + i) = cpu_to_le32(++current_block);
 		}
 		set_buffer_uptodate(bh);
-		unlock_buffer(bh);
 		mark_buffer_dirty_inode(bh, inode);
+		unlock_buffer(bh);
 		/* We used to sync bh here if IS_SYNC(inode).
 		 * But we now rely upon generic_write_sync()
 		 * and b_inode_buffers.  But not for directories.
@@ -577,9 +577,13 @@ static void ext2_splice_branch(struct in
 	/* We are done with atomic stuff, now do the rest of housekeeping */
 
 	/* had we spliced it onto indirect block? */
-	if (where->bh)
+	if (where->bh) {
+		lock_buffer(where->bh);
+		ext2_clear_buffer_error(inode->i_sb, where->bh,
+					"updating indirect block");
 		mark_buffer_dirty_inode(where->bh, inode);
-
+		unlock_buffer(where->bh);
+	}
 	inode->i_ctime = CURRENT_TIME_SEC;
 	mark_inode_dirty(inode);
 }
@@ -1105,8 +1109,13 @@ static void __ext2_truncate_blocks(struc
 	if (nr) {
 		if (partial == chain)
 			mark_inode_dirty(inode);
-		else
+		else {
+			lock_buffer(partial->bh);
+			ext2_clear_buffer_error(inode->i_sb, partial->bh,
+						"updating indirect block");
 			mark_buffer_dirty_inode(partial->bh, inode);
+			unlock_buffer(partial->bh);
+		}
 		ext2_free_branches(inode, &nr, &nr+1, (chain+n-1) - partial);
 	}
 	/* Clear the ends of indirect blocks on the shared branch */
@@ -1115,7 +1124,12 @@ static void __ext2_truncate_blocks(struc
 				   partial->p + 1,
 				   (__le32*)partial->bh->b_data+addr_per_block,
 				   (chain+n-1) - partial);
+		lock_buffer(partial->bh);
+		ext2_clear_buffer_error(inode->i_sb, partial->bh,
+					"updating indirect block");
 		mark_buffer_dirty_inode(partial->bh, inode);
+		unlock_buffer(partial->bh);
+
 		brelse (partial->bh);
 		partial--;
 	}
@@ -1504,7 +1518,10 @@ static int __ext2_write_inode(struct ino
 		}
 	} else for (n = 0; n < EXT2_N_BLOCKS; n++)
 		raw_inode->i_block[n] = ei->i_data[n];
+	lock_buffer(bh);
+	ext2_clear_buffer_error(sb, bh, "writing inode");
 	mark_buffer_dirty(bh);
+	unlock_buffer(bh);
 	if (do_sync) {
 		sync_dirty_buffer(bh);
 		if (buffer_req(bh) && !buffer_uptodate(bh)) {
Index: linux-3.2-rc6/fs/ext2/super.c
===================================================================
--- linux-3.2-rc6.orig/fs/ext2/super.c
+++ linux-3.2-rc6/fs/ext2/super.c
@@ -1129,37 +1129,40 @@ failed_unlock:
 	return ret;
 }
 
-static void ext2_clear_super_error(struct super_block *sb)
+void ext2_clear_buffer_error(struct super_block *sb, struct buffer_head *bh,
+			     const char *when)
 {
-	struct buffer_head *sbh = EXT2_SB(sb)->s_sbh;
-
-	if (buffer_write_io_error(sbh)) {
+	if (buffer_write_io_error(bh)) {
 		/*
-		 * Oh, dear.  A previous attempt to write the
-		 * superblock failed.  This could happen because the
+		 * Oh, dear.  A previous attempt to write the buffer
+		 * 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.
 		 */
 		ext2_msg(sb, KERN_ERR,
-		       "previous I/O error to superblock detected\n");
-		clear_buffer_write_io_error(sbh);
-		set_buffer_uptodate(sbh);
+			 "previous I/O error to buffer detected when %s\n",
+			 when);
+		clear_buffer_write_io_error(bh);
+		set_buffer_uptodate(bh);
 	}
 }
 
 static void ext2_sync_super(struct super_block *sb, struct ext2_super_block *es,
 			    int wait)
 {
-	ext2_clear_super_error(sb);
 	spin_lock(&EXT2_SB(sb)->s_lock);
 	es->s_free_blocks_count = cpu_to_le32(ext2_count_free_blocks(sb));
 	es->s_free_inodes_count = cpu_to_le32(ext2_count_free_inodes(sb));
 	es->s_wtime = cpu_to_le32(get_seconds());
 	/* unlock before we do IO */
 	spin_unlock(&EXT2_SB(sb)->s_lock);
+
+	lock_buffer(EXT2_SB(sb)->s_sbh);
+	ext2_clear_buffer_error(sb, EXT2_SB(sb)->s_sbh, "writing superblock");
 	mark_buffer_dirty(EXT2_SB(sb)->s_sbh);
+	unlock_buffer(EXT2_SB(sb)->s_sbh);
 	if (wait)
 		sync_dirty_buffer(EXT2_SB(sb)->s_sbh);
 	sb->s_dirt = 0;
Index: linux-3.2-rc6/fs/ext2/xattr.c
===================================================================
--- linux-3.2-rc6.orig/fs/ext2/xattr.c
+++ linux-3.2-rc6/fs/ext2/xattr.c
@@ -341,7 +341,10 @@ static void ext2_xattr_update_super_bloc
 	EXT2_SET_COMPAT_FEATURE(sb, EXT2_FEATURE_COMPAT_EXT_ATTR);
 	spin_unlock(&EXT2_SB(sb)->s_lock);
 	sb->s_dirt = 1;
+	lock_buffer(EXT2_SB(sb)->s_sbh);
+	ext2_clear_buffer_error(sb, EXT2_SB(sb)->s_sbh, "updating super");
 	mark_buffer_dirty(EXT2_SB(sb)->s_sbh);
+	unlock_buffer(EXT2_SB(sb)->s_sbh);
 }
 
 /*
@@ -678,7 +681,10 @@ ext2_xattr_set2(struct inode *inode, str
 			
 			ext2_xattr_update_super_block(sb);
 		}
+		lock_buffer(new_bh);
+		ext2_clear_buffer_error(sb, new_bh, "setting xattrs");
 		mark_buffer_dirty(new_bh);
+		unlock_buffer(new_bh);
 		if (IS_SYNC(inode)) {
 			sync_dirty_buffer(new_bh);
 			error = -EIO;
@@ -734,6 +740,7 @@ ext2_xattr_set2(struct inode *inode, str
 				mb_cache_entry_release(ce);
 			dquot_free_block_nodirty(inode, 1);
 			mark_inode_dirty(inode);
+			ext2_clear_buffer_error(sb, old_bh, "setting xattrs");
 			mark_buffer_dirty(old_bh);
 			ea_bdebug(old_bh, "refcount now=%d",
 				le32_to_cpu(HDR(old_bh)->h_refcount));
@@ -792,8 +799,9 @@ ext2_xattr_delete_inode(struct inode *in
 			mb_cache_entry_release(ce);
 		ea_bdebug(bh, "refcount now=%d",
 			le32_to_cpu(HDR(bh)->h_refcount));
-		unlock_buffer(bh);
+		ext2_clear_buffer_error(inode->i_sb, bh, "deleting xattr");
 		mark_buffer_dirty(bh);
+		unlock_buffer(bh);
 		if (IS_SYNC(inode))
 			sync_dirty_buffer(bh);
 		dquot_free_block_nodirty(inode, 1);
Index: linux-3.2-rc6/fs/ext2/ext2.h
===================================================================
--- linux-3.2-rc6.orig/fs/ext2/ext2.h
+++ linux-3.2-rc6/fs/ext2/ext2.h
@@ -141,6 +141,8 @@ extern __printf(3, 4)
 void ext2_msg(struct super_block *, const char *, const char *, ...);
 extern void ext2_update_dynamic_rev (struct super_block *sb);
 extern void ext2_write_super (struct super_block *);
+void ext2_clear_buffer_error(struct super_block *sb, struct buffer_head *bh,
+			     const char *when);
 
 /*
  * Inodes and files operations