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

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

[Francis Moreau]

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.

Signed-off-by: Francis Moreau <francis.moro@gmail.com>
---
 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..5addd35 100644
--- a/fs/ext2/ialloc.c
+++ b/fs/ext2/ialloc.c
@@ -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,8 +153,8 @@ 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);
+
+	__brelse(bitmap_bh);
 }

 /*
-- 
1.6.6

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

[Jan Kara]

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.

								Honza
-- 
Jan Kara <jack@suse.cz>
SUSE Labs, CR

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

[Jan Kara]

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

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

[Francis Moreau]

On Thu, Apr 8, 2010 at 12:01 PM, Jan Kara <jack@suse.cz> wrote:
> 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.
>

ok,


thanks
-- 
Francis
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html