Re: [PATCH] ext2_free_inode(): remove useless call to brelse()

[Jan Kara <jack@suse.cz>]

On Thu 08-04-10 11:57:48, Jan Kara wrote:
> On Fri 26-03-10 14:07:00, Francis Moreau wrote:
> > This patch removes a useless call to brelse(bitmap_bh) since at that
> > point bitmap_bh is NULL.
> > 
> > It also converts the last brelse(bitmap_bh) into a __brelse(bitmap_bh)
> > since at that point bitmap_bh is no more NULL.
>   The cleanup looks fine. Only I'd use normal brelse instead of __brelse
> at the end. The compiler should figure out itself that the check is
> unnecessary (if not, the cost of extra test is negligible in that path
> anyway) and you don't have to think twice when looking at the function.
> Also you don't have to initialize bitmap_bh to NULL after your cleanups.
> So I've taken your patch with these two minor changes.
  Just for reference, here's the patch I currently carry.

								Honza

>From 97afb5cae1f853e0dc20d5eaba14e36748ed6121 Mon Sep 17 00:00:00 2001
From: Francis Moreau <francis.moro@gmail.com>
Date: Thu, 8 Apr 2010 11:35:17 +0200
Subject: [PATCH] ext2: remove useless call to brelse() in ext2_free_inode()

This patch removes a useless call to brelse(bitmap_bh) since at that
point bitmap_bh is NULL and slightly cleans up bitmap_bh handling.

Signed-off-by: Francis Moreau <francis.moro@gmail.com>
Signed-off-by: Jan Kara <jack@suse.cz>
---
 fs/ext2/ialloc.c |    9 ++++-----
 1 files changed, 4 insertions(+), 5 deletions(-)

diff --git a/fs/ext2/ialloc.c b/fs/ext2/ialloc.c
index ad7d572..f0c5286 100644
--- a/fs/ext2/ialloc.c
+++ b/fs/ext2/ialloc.c
@@ -106,7 +106,7 @@ void ext2_free_inode (struct inode * inode)
 	struct super_block * sb = inode->i_sb;
 	int is_directory;
 	unsigned long ino;
-	struct buffer_head *bitmap_bh = NULL;
+	struct buffer_head *bitmap_bh;
 	unsigned long block_group;
 	unsigned long bit;
 	struct ext2_super_block * es;
@@ -135,14 +135,13 @@ void ext2_free_inode (struct inode * inode)
 	    ino > le32_to_cpu(es->s_inodes_count)) {
 		ext2_error (sb, "ext2_free_inode",
 			    "reserved or nonexistent inode %lu", ino);
-		goto error_return;
+		return;
 	}
 	block_group = (ino - 1) / EXT2_INODES_PER_GROUP(sb);
 	bit = (ino - 1) % EXT2_INODES_PER_GROUP(sb);
-	brelse(bitmap_bh);
 	bitmap_bh = read_inode_bitmap(sb, block_group);
 	if (!bitmap_bh)
-		goto error_return;
+		return;
 
 	/* Ok, now we can actually update the inode bitmaps.. */
 	if (!ext2_clear_bit_atomic(sb_bgl_lock(EXT2_SB(sb), block_group),
@@ -154,7 +153,7 @@ void ext2_free_inode (struct inode * inode)
 	mark_buffer_dirty(bitmap_bh);
 	if (sb->s_flags & MS_SYNCHRONOUS)
 		sync_dirty_buffer(bitmap_bh);
-error_return:
+
 	brelse(bitmap_bh);
 }
 
-- 
1.6.4.2