linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [patch] ext2: avoid WARN() messages when failing to write to buffers
@ 2012-01-04 14:28 Edward Shishkin
  2012-01-05 14:28 ` Jan Kara
  0 siblings, 1 reply; 2+ messages in thread
From: Edward Shishkin @ 2012-01-04 14:28 UTC (permalink / raw)
  To: Jan Kara, Andrew Morton, Eric Sandeen; +Cc: linux-fsdevel

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



^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: [patch] ext2: avoid WARN() messages when failing to write to buffers
  2012-01-04 14:28 [patch] ext2: avoid WARN() messages when failing to write to buffers Edward Shishkin
@ 2012-01-05 14:28 ` Jan Kara
  0 siblings, 0 replies; 2+ messages in thread
From: Jan Kara @ 2012-01-05 14:28 UTC (permalink / raw)
  To: Edward Shishkin; +Cc: Jan Kara, Andrew Morton, Eric Sandeen, linux-fsdevel

  Hello,

On Wed 04-01-12 15:28:14, Edward Shishkin wrote:
> 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 for the patch Edward but I have to say I don't quite like it.
Wouldn't it be cleaner to not clear the uptodate flag in the first place?
My idea would be that filesystems that properly check for
buffer_write_io_error (instead of buffer_uptodate) set a flag in
file_system_type structure and for these filesystems we don't clear the
uptodate flag. I believe that would be a move in a better direction.

I'll post in a minute three patches that do this for generic code and ext2.

								Honza

> 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
> 
> 
-- 
Jan Kara <jack@suse.cz>
SUSE Labs, CR

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2012-01-05 14:28 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-01-04 14:28 [patch] ext2: avoid WARN() messages when failing to write to buffers Edward Shishkin
2012-01-05 14:28 ` Jan Kara

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).