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@xxxxxxxxx> 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@xxxxxxxxx> Signed-off-by: Jan Kara <jack@xxxxxxx> --- 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 -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html