linux-ext4.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Darrick J. Wong" <darrick.wong@oracle.com>
To: "Theodore Ts'o" <tytso@mit.edu>
Cc: Ext4 Developers List <linux-ext4@vger.kernel.org>
Subject: Re: [PATCH 06/12] libext2fs: further clean up and rename check_block_uninit
Date: Mon, 20 Jan 2014 12:17:36 -0800	[thread overview]
Message-ID: <20140120201736.GN9229@birch.djwong.org> (raw)
In-Reply-To: <1390197254-14583-7-git-send-email-tytso@mit.edu>

On Mon, Jan 20, 2014 at 12:54:08AM -0500, Theodore Ts'o wrote:
> Commit 8e44eb64bb (libext2fs: mark group data blocks when loading
> block bitmap) simplified check_block_uninit since we are now
> initializing the bitmap when it is loaded from disk.  It left some
> variables which were being set but never used, however.  In addition,
> since we only need check_block_uninit() to clear the block bitmap's
> uninit flag, rename it to clear_block_uninit(), and only call it once
> we have found a free block in ext2fs_new_blocks2().
> 
> This cleans up the code some and optimizes things if we need to search
> multiple block groups trying to find a free block.
> 
> Signed-off-by: "Theodore Ts'o" <tytso@mit.edu>
> Cc: Darrick J. Wong <darrick.wong@oracle.com>

Looks reasonable, so you can:
Acked-by: Darrick J. Wong <darrick.wong@oracle.com>

--D
> ---
>  lib/ext2fs/alloc.c | 33 +++++----------------------------
>  1 file changed, 5 insertions(+), 28 deletions(-)
> 
> diff --git a/lib/ext2fs/alloc.c b/lib/ext2fs/alloc.c
> index 42096a4..91637dd 100644
> --- a/lib/ext2fs/alloc.c
> +++ b/lib/ext2fs/alloc.c
> @@ -27,31 +27,15 @@
>  #include "ext2fs.h"
>  
>  /*
> - * Check for uninit block bitmaps and deal with them appropriately
> + * Clear the uninit block bitmap flag if necessary
>   */
> -static void check_block_uninit(ext2_filsys fs, ext2fs_block_bitmap map,
> -			       dgrp_t group)
> +static void clear_block_uninit(ext2_filsys fs, dgrp_t group)
>  {
> -	blk_t		i;
> -	blk64_t		blk, super_blk, old_desc_blk, new_desc_blk;
> -	int		old_desc_blocks;
> -
>  	if (!(EXT2_HAS_RO_COMPAT_FEATURE(fs->super,
>  					 EXT4_FEATURE_RO_COMPAT_GDT_CSUM)) ||
>  	    !(ext2fs_bg_flags_test(fs, group, EXT2_BG_BLOCK_UNINIT)))
>  		return;
>  
> -	blk = ext2fs_group_first_block2(fs, group);
> -
> -	ext2fs_super_and_bgd_loc2(fs, group, &super_blk,
> -				  &old_desc_blk, &new_desc_blk, 0);
> -
> -	if (fs->super->s_feature_incompat &
> -	    EXT2_FEATURE_INCOMPAT_META_BG)
> -		old_desc_blocks = fs->super->s_first_meta_bg;
> -	else
> -		old_desc_blocks = fs->desc_blocks + fs->super->s_reserved_gdt_blocks;
> -
>  	/* uninit block bitmaps are now initialized in read_bitmaps() */
>  
>  	ext2fs_bg_flags_clear(fs, group, EXT2_BG_BLOCK_UNINIT);
> @@ -78,10 +62,11 @@ static void check_inode_uninit(ext2_filsys fs, ext2fs_inode_bitmap map,
>  		ext2fs_fast_unmark_inode_bitmap2(map, ino);
>  
>  	ext2fs_bg_flags_clear(fs, group, EXT2_BG_INODE_UNINIT);
> +	/* Mimics what the kernel does */
> +	ext2fs_bg_flags_clear(fs, group, EXT2_BG_BLOCK_UNINIT);
>  	ext2fs_group_desc_csum_set(fs, group);
>  	ext2fs_mark_ib_dirty(fs);
>  	ext2fs_mark_super_dirty(fs);
> -	check_block_uninit(fs, fs->block_map, group);
>  }
>  
>  /*
> @@ -167,17 +152,9 @@ errcode_t ext2fs_new_block2(ext2_filsys fs, blk64_t goal,
>  	c_ratio = 1 << ext2fs_get_bitmap_granularity(map);
>  	if (c_ratio > 1)
>  		goal &= ~EXT2FS_CLUSTER_MASK(fs);
> -	check_block_uninit(fs, map,
> -			   (i - fs->super->s_first_data_block) /
> -			   EXT2_BLOCKS_PER_GROUP(fs->super));
>  	do {
> -		if (((i - fs->super->s_first_data_block) %
> -		     EXT2_BLOCKS_PER_GROUP(fs->super)) == 0)
> -			check_block_uninit(fs, map,
> -					   (i - fs->super->s_first_data_block) /
> -					   EXT2_BLOCKS_PER_GROUP(fs->super));
> -
>  		if (!ext2fs_fast_test_block_bitmap2(map, i)) {
> +			clear_block_uninit(fs, ext2fs_group_of_blk2(fs, i));
>  			*ret = i;
>  			return 0;
>  		}
> -- 
> 1.8.5.rc3.362.gdf10213
> 

  reply	other threads:[~2014-01-20 20:17 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-01-20  5:54 [PATCH 00/12] e2fsprogs mke2fs optimizations and new features Theodore Ts'o
2014-01-20  5:54 ` [PATCH 01/12] libext2fs: fix off-by-one bug in ext2fs_extent_insert() Theodore Ts'o
2014-01-20  5:54 ` [PATCH 02/12] libext2fs: clean up generic handling of ext2fs_find_first_{set,zero}_*() Theodore Ts'o
2014-01-20  5:54 ` [PATCH 03/12] libext2fs: build tst_bitmaps with rep invariants checking enabled Theodore Ts'o
2014-01-20  5:54 ` [PATCH 04/12] libext: optimize find_first_set() for bitarray-based bitmaps Theodore Ts'o
2014-01-20  5:54 ` [PATCH 05/12] libext2fs: optimize find_first_{zero,set}() for red-black tree based bitmaps Theodore Ts'o
2014-01-20  5:54 ` [PATCH 06/12] libext2fs: further clean up and rename check_block_uninit Theodore Ts'o
2014-01-20 20:17   ` Darrick J. Wong [this message]
2014-01-20  5:54 ` [PATCH 07/12] libext2fs: add ext2fs_block_alloc_stats_range() Theodore Ts'o
2014-02-13 21:50   ` Darrick J. Wong
2014-01-20  5:54 ` [PATCH 08/12] libext2fs: optimize ext2fs_allocate_group_table() Theodore Ts'o
2014-01-20  5:54 ` [PATCH 09/12] libext2: optimize ext2fs_new_block2() Theodore Ts'o
2014-01-20 21:52   ` Andreas Dilger
2014-01-21 15:54     ` Theodore Ts'o
2014-01-20  5:54 ` [PATCH 10/12] mke2fs: optimize fix_cluster_bg_counts() Theodore Ts'o
2014-01-20  5:54 ` [PATCH 11/12] Add support for new compat feature "sparse_super2" Theodore Ts'o
2014-01-20 21:49   ` Andreas Dilger
     [not found]   ` <alpine.DEB.2.10.1402051559390.16807@tglase.lan.tarent.de>
2014-02-05 16:17     ` Theodore Ts'o
2014-01-20  5:54 ` [PATCH 12/12] mke2fs: allow metadata blocks to be at the beginning of the file system Theodore Ts'o
2014-01-20 16:30   ` Theodore Ts'o
2014-01-20 23:25   ` Andreas Dilger
2014-01-21  6:23     ` Theodore Ts'o
2014-01-23 21:28       ` Andreas Dilger
2014-01-20 16:30 ` [PATCH 00/12] e2fsprogs mke2fs optimizations and new features Theodore Ts'o

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20140120201736.GN9229@birch.djwong.org \
    --to=darrick.wong@oracle.com \
    --cc=linux-ext4@vger.kernel.org \
    --cc=tytso@mit.edu \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).