Re: [PATCH] e2fsck: improve performance of region_allocate()

[Jaco Kroon <jaco@xxxxxxxxx>]

Hi Ted,

On 24/08/2017 01:21, Theodore Ts'o wrote:
> Jaco, here's the version of your patch I plan to use.  Please take a
> look.
>
> The only real changes are mostly whitespace and formatting.
Variable name change + formatting :).
> By the way, it would be nice if you could send me permission to add a
>
> Signed-off-by: Jaco Kroon <jaco@xxxxxxxxx>
Approved.

Kind Regards,
Jaco

> See http://elinux.org/Developer_Certificate_Of_Origin for an
> explanation of what this means.
>
> 						- Ted
>
> commit 320d436c006dc2550ebd01084b6e823f0cea8bc2
> Author: Jaco Kroon <jaco@xxxxxxxxx>
> Date:   Wed Aug 23 13:54:25 2017 -0400
>
>      e2fsck: optimize out the use region_t in scan_extent_node()
>      
>      Since extents have a guarantee of being monotonically increasing we
>      merely need to check that block n+1 starts after block n.  This is a
>      simple enough check and we can perform this by calculating the next
>      expected logical block instead of using the region usage tracking data
>      abstraction.
>      
>      Signed-off-by: Theodore Ts'o <tytso@xxxxxxx>
>
> diff --git a/e2fsck/pass1.c b/e2fsck/pass1.c
> index 8044beed6..bc26beb99 100644
> --- a/e2fsck/pass1.c
> +++ b/e2fsck/pass1.c
> @@ -96,7 +96,7 @@ struct process_block_struct {
>   	struct problem_context *pctx;
>   	ext2fs_block_bitmap fs_meta_blocks;
>   	e2fsck_t	ctx;
> -	region_t	region;
> +	blk64_t		next_lblock;
>   	struct extent_tree_info	eti;
>   };
>   
> @@ -2628,9 +2628,18 @@ static void scan_extent_node(e2fsck_t ctx, struct problem_context *pctx,
>   			  (1U << (21 - ctx->fs->super->s_log_block_size))))
>   			problem = PR_1_TOOBIG_DIR;
>   
> -		if (is_leaf && problem == 0 && extent.e_len > 0 &&
> -		    region_allocate(pb->region, extent.e_lblk, extent.e_len))
> -			problem = PR_1_EXTENT_COLLISION;
> +		if (is_leaf && problem == 0 && extent.e_len > 0) {
> +#if 0
> +			printf("extent_region(ino=%u, expect=%llu, "
> +			       "lblk=%llu, len=%u)\n",
> +			       pb->ino, pb->next_lblock,
> +			       extent.e_lblk, extent.e_len);
> +#endif
> +			if (extent.e_lblk < pb->next_lblock)
> +				problem = PR_1_EXTENT_COLLISION;
> +			else if (extent.e_lblk + extent.e_len > pb->next_lblock)
> +				pb->next_lblock = extent.e_lblk + extent.e_len;
> +		}
>   
>   		/*
>   		 * Uninitialized blocks in a directory?  Clear the flag and
> @@ -2963,13 +2972,7 @@ static void check_blocks_extents(e2fsck_t ctx, struct problem_context *pctx,
>   	memset(pb->eti.ext_info, 0, sizeof(pb->eti.ext_info));
>   	pb->eti.ino = pb->ino;
>   
> -	pb->region = region_create(0, info.max_lblk);
> -	if (!pb->region) {
> -		ext2fs_extent_free(ehandle);
> -		fix_problem(ctx, PR_1_EXTENT_ALLOC_REGION_ABORT, pctx);
> -		ctx->flags |= E2F_FLAG_ABORT;
> -		return;
> -	}
> +	pb->next_lblock = 0;
>   
>   	eof_lblk = ((EXT2_I_SIZE(inode) + fs->blocksize - 1) >>
>   		EXT2_BLOCK_SIZE_BITS(fs->super)) - 1;
> @@ -2982,8 +2985,6 @@ static void check_blocks_extents(e2fsck_t ctx, struct problem_context *pctx,
>   				   "check_blocks_extents");
>   		pctx->errcode = 0;
>   	}
> -	region_free(pb->region);
> -	pb->region = NULL;
>   	ext2fs_extent_free(ehandle);
>   
>   	/* Rebuild unless it's a dir and we're rehashing it */