[Cluster-devel] [fsck.gfs2 PATCH] fsck.gfs2: Speed up fsck.gfs2 with a new inode rgrp pointer
Andrew Price
anprice at redhat.com
Thu Jun 2 09:39:47 UTC 2016
On 01/06/16 19:17, Bob Peterson wrote:
> Hi,
>
> This patch tries to speed up fsck.gfs2
I assume it succeeds :)
> by adding a new field in the
> in-core inode which points to the inode's rgrp. Most blocks will be
> contained within the rgrp, so we save time looking up the rgrp by
> first checking if the desired inode block falls within the inode's
> rgrp.
>
> Signed-off-by: Bob Peterson <rpeterso at redhat.com>
>
> diff --git a/gfs2/fsck/afterpass1_common.c b/gfs2/fsck/afterpass1_common.c
> index 95fdbf8..0632695 100644
> --- a/gfs2/fsck/afterpass1_common.c
> +++ b/gfs2/fsck/afterpass1_common.c
> @@ -108,7 +108,8 @@ static int delete_block_if_notdup(struct gfs2_inode *ip, uint64_t block,
> (unsigned long long)ip->i_di.di_num.no_addr,
> (unsigned long long)block, (unsigned long long)block);
> } else {
> - check_n_fix_bitmap(ip->i_sbd, block, 0, GFS2_BLKST_FREE);
> + check_n_fix_bitmap(ip->i_sbd, ip->i_rgd, block, 0,
> + GFS2_BLKST_FREE);
> }
> return meta_is_good;
> }
> diff --git a/gfs2/fsck/fsck.h b/gfs2/fsck/fsck.h
> index 09b96ed..ee7c07d 100644
> --- a/gfs2/fsck/fsck.h
> +++ b/gfs2/fsck/fsck.h
> @@ -108,7 +108,8 @@ enum rgindex_trust_level { /* how far can we trust our RG index? */
>
> extern struct gfs2_inode *fsck_load_inode(struct gfs2_sbd *sdp, uint64_t block);
> extern struct gfs2_inode *fsck_inode_get(struct gfs2_sbd *sdp,
> - struct gfs2_buffer_head *bh);
> + struct rgrp_tree *rgd,
> + struct gfs2_buffer_head *bh);
> extern void fsck_inode_put(struct gfs2_inode **ip);
>
> extern int initialize(struct gfs2_sbd *sdp, int force_check, int preen,
> diff --git a/gfs2/fsck/initialize.c b/gfs2/fsck/initialize.c
> index 652eec3..75f050b 100644
> --- a/gfs2/fsck/initialize.c
> +++ b/gfs2/fsck/initialize.c
> @@ -300,7 +300,7 @@ static void check_rgrp_integrity(struct gfs2_sbd *sdp, struct rgrp_tree *rgd,
> bh = bread(sdp, diblock);
> if (!gfs2_check_meta(bh, GFS2_METATYPE_DI)) {
> struct gfs2_inode *ip =
> - fsck_inode_get(sdp, bh);
> + fsck_inode_get(sdp, rgd, bh);
> if (ip->i_di.di_blocks > 1) {
> blks_2free +=
> ip->i_di.di_blocks - 1;
> diff --git a/gfs2/fsck/metawalk.c b/gfs2/fsck/metawalk.c
> index 0e279e5..be54cef 100644
> --- a/gfs2/fsck/metawalk.c
> +++ b/gfs2/fsck/metawalk.c
> @@ -23,17 +23,25 @@
>
> #define COMFORTABLE_BLKS 5242880 /* 20GB in 4K blocks */
>
> +static inline int rgrp_contains_block(struct rgrp_tree *rgd, uint64_t blk)
> +{
> + if (blk < rgd->ri.ri_addr)
> + return 0;
> + if (blk >= rgd->ri.ri_data0 + rgd->ri.ri_data)
> + return 0;
> + return 1;
Dangling whitespace ^
Otherwise it looks pretty straightforward. ACK to both patches.
Thanks,
Andy
> +}
> +
> /* There are two bitmaps: (1) The "blockmap" that fsck uses to keep track of
> what block type has been discovered, and (2) The rgrp bitmap. Function
> gfs2_blockmap_set is used to set the former and gfs2_set_bitmap
> is used to set the latter. The two must be kept in sync, otherwise
> you'll get bitmap mismatches. This function checks the status of the
> bitmap whenever the blockmap changes, and fixes it accordingly. */
> -int check_n_fix_bitmap(struct gfs2_sbd *sdp, uint64_t blk, int error_on_dinode,
> - int new_state)
> +int check_n_fix_bitmap(struct gfs2_sbd *sdp, struct rgrp_tree *rgd,
> + uint64_t blk, int error_on_dinode, int new_state)
> {
> int old_state;
> - struct rgrp_tree *rgd;
> int treat_as_inode = 0;
> int rewrite_rgrp = 0;
> struct gfs_rgrp *gfs1rg;
> @@ -42,7 +50,9 @@ int check_n_fix_bitmap(struct gfs2_sbd *sdp, uint64_t blk, int error_on_dinode,
> /* gfs1 descriptions: */
> {"free", "data", "free meta", "metadata", "reserved"}};
>
> - rgd = gfs2_blk2rgrpd(sdp, blk);
> + if (rgd == NULL || !rgrp_contains_block(rgd, blk))
> + rgd = gfs2_blk2rgrpd(sdp, blk);
> +
> gfs1rg = (struct gfs_rgrp *)&rgd->rg;
>
> old_state = lgfs2_get_bitmap(sdp, blk, rgd);
> @@ -208,7 +218,8 @@ int _fsck_bitmap_set(struct gfs2_inode *ip, uint64_t bblock,
> prev_mark = mark;
> prev_caller = caller;
> }
> - error = check_n_fix_bitmap(ip->i_sbd, bblock, error_on_dinode, mark);
> + error = check_n_fix_bitmap(ip->i_sbd, ip->i_rgd, bblock,
> + error_on_dinode, mark);
> if (error < 0)
> log_err(_("This block is not represented in the bitmap.\n"));
> return error;
> @@ -273,18 +284,23 @@ struct gfs2_inode *fsck_load_inode(struct gfs2_sbd *sdp, uint64_t block)
>
> /* fsck_inode_get - same as inode_get() in libgfs2 but system inodes
> get special treatment. */
> -struct gfs2_inode *fsck_inode_get(struct gfs2_sbd *sdp,
> +struct gfs2_inode *fsck_inode_get(struct gfs2_sbd *sdp, struct rgrp_tree *rgd,
> struct gfs2_buffer_head *bh)
> {
> struct gfs2_inode *sysip;
> + struct gfs2_inode *ip;
>
> sysip = fsck_system_inode(sdp, bh->b_blocknr);
> if (sysip)
> return sysip;
>
> if (sdp->gfs1)
> - return lgfs2_gfs_inode_get(sdp, bh);
> - return lgfs2_inode_get(sdp, bh);
> + ip = lgfs2_gfs_inode_get(sdp, bh);
> + else
> + ip = lgfs2_inode_get(sdp, bh);
> + if (ip)
> + ip->i_rgd = rgd;
> + return ip;
> }
>
> /* fsck_inode_put - same as inode_put() in libgfs2 but system inodes
> diff --git a/gfs2/fsck/metawalk.h b/gfs2/fsck/metawalk.h
> index b16e57d..119efee 100644
> --- a/gfs2/fsck/metawalk.h
> +++ b/gfs2/fsck/metawalk.h
> @@ -22,8 +22,9 @@ extern int check_leaf(struct gfs2_inode *ip, int lindex,
> extern int _fsck_bitmap_set(struct gfs2_inode *ip, uint64_t bblock,
> const char *btype, int mark, int error_on_dinode,
> const char *caller, int line);
> -extern int check_n_fix_bitmap(struct gfs2_sbd *sdp, uint64_t blk,
> - int error_on_dinode, int new_state);
> +extern int check_n_fix_bitmap(struct gfs2_sbd *sdp, struct rgrp_tree *rgd,
> + uint64_t blk, int error_on_dinode,
> + int new_state);
> extern struct duptree *dupfind(uint64_t block);
> extern struct gfs2_inode *fsck_system_inode(struct gfs2_sbd *sdp,
> uint64_t block);
> diff --git a/gfs2/fsck/pass1.c b/gfs2/fsck/pass1.c
> index 6908ac7..3a1931d 100644
> --- a/gfs2/fsck/pass1.c
> +++ b/gfs2/fsck/pass1.c
> @@ -248,8 +248,9 @@ static int resuscitate_metalist(struct gfs2_inode *ip, uint64_t block,
> ip->i_sbd->gfs1 ?
> GFS2_BLKST_DINODE : GFS2_BLKST_USED);
> else
> - check_n_fix_bitmap(ip->i_sbd, block, 0, ip->i_sbd->gfs1 ?
> - GFS2_BLKST_DINODE : GFS2_BLKST_USED);
> + check_n_fix_bitmap(ip->i_sbd, ip->i_rgd, block, 0,
> + ip->i_sbd->gfs1 ?
> + GFS2_BLKST_DINODE : GFS2_BLKST_USED);
> bc->indir_count++;
> return meta_is_good;
> }
> @@ -296,7 +297,8 @@ static int resuscitate_dentry(struct gfs2_inode *ip, struct gfs2_dirent *dent,
> fsck_blockmap_set(ip, block, _("system file"),
> GFS2_BLKST_DINODE);
> else
> - check_n_fix_bitmap(sdp, block, 0, GFS2_BLKST_DINODE);
> + check_n_fix_bitmap(sdp, ip->i_rgd, block, 0,
> + GFS2_BLKST_DINODE);
> /* Return the number of leaf entries so metawalk doesn't flag this
> leaf as having none. */
> *count = be16_to_cpu(((struct gfs2_leaf *)bh->b_data)->lf_entries);
> @@ -1594,13 +1596,14 @@ static void check_i_goal(struct gfs2_sbd *sdp, struct gfs2_inode *ip)
> * handle_di - This is now a wrapper function that takes a gfs2_buffer_head
> * and calls handle_ip, which takes an in-code dinode structure.
> */
> -static int handle_di(struct gfs2_sbd *sdp, struct gfs2_buffer_head *bh)
> +static int handle_di(struct gfs2_sbd *sdp, struct rgrp_tree *rgd,
> + struct gfs2_buffer_head *bh)
> {
> int error = 0;
> uint64_t block = bh->b_blocknr;
> struct gfs2_inode *ip;
>
> - ip = fsck_inode_get(sdp, bh);
> + ip = fsck_inode_get(sdp, rgd, bh);
>
> if (ip->i_di.di_num.no_addr != block) {
> log_err( _("Inode #%llu (0x%llx): Bad inode address found: %llu "
> @@ -1671,7 +1674,8 @@ static int check_system_inode(struct gfs2_sbd *sdp,
> (unsigned long long)iblock,
> (unsigned long long)iblock);
> gfs2_blockmap_set(bl, iblock, GFS2_BLKST_FREE);
> - check_n_fix_bitmap(sdp, iblock, 0, GFS2_BLKST_FREE);
> + check_n_fix_bitmap(sdp, (*sysinode)->i_rgd, iblock, 0,
> + GFS2_BLKST_FREE);
> inode_put(sysinode);
> }
> }
> @@ -1955,7 +1959,7 @@ static int pass1_process_bitmap(struct gfs2_sbd *sdp, struct rgrp_tree *rgd, uin
> (unsigned long long)block,
> (unsigned long long)block,
> block_type_string(q));
> - ip = fsck_inode_get(sdp, bh);
> + ip = fsck_inode_get(sdp, rgd, bh);
> if (is_inode && ip->i_di.di_num.no_addr == block)
> add_duplicate_ref(ip, block, ref_is_inode, 0,
> INODE_VALID);
> @@ -1991,8 +1995,9 @@ static int pass1_process_bitmap(struct gfs2_sbd *sdp, struct rgrp_tree *rgd, uin
> "%llu (0x%llx)\n"),
> (unsigned long long)block,
> (unsigned long long)block);
> - check_n_fix_bitmap(sdp, block, 0, GFS2_BLKST_FREE);
> - } else if (handle_di(sdp, bh) < 0) {
> + check_n_fix_bitmap(sdp, rgd, block, 0,
> + GFS2_BLKST_FREE);
> + } else if (handle_di(sdp, rgd, bh) < 0) {
> stack;
> brelse(bh);
> gfs2_special_free(&gfs1_rindex_blks);
> @@ -2206,7 +2211,7 @@ int pass1(struct gfs2_sbd *sdp)
> }
> /* rgrps and bitmaps don't have bits to represent
> their blocks, so don't do this:
> - check_n_fix_bitmap(sdp, rgd->ri.ri_addr + i, 0,
> + check_n_fix_bitmap(sdp, rgd, rgd->ri.ri_addr + i, 0,
> gfs2_meta_rgrp);*/
> }
>
> diff --git a/gfs2/fsck/pass1b.c b/gfs2/fsck/pass1b.c
> index 6dec193..62686fe 100644
> --- a/gfs2/fsck/pass1b.c
> +++ b/gfs2/fsck/pass1b.c
> @@ -775,7 +775,8 @@ static int handle_dup_blk(struct gfs2_sbd *sdp, struct duptree *dt)
> (unsigned long long)dup_blk);
> if (dh.dt)
> dup_delete(dh.dt);
> - check_n_fix_bitmap(sdp, dup_blk, 0, GFS2_BLKST_FREE);
> + check_n_fix_bitmap(sdp, NULL, dup_blk, 0,
> + GFS2_BLKST_FREE);
> }
> }
> return 0;
> diff --git a/gfs2/fsck/pass2.c b/gfs2/fsck/pass2.c
> index ffd58dc..d072634 100644
> --- a/gfs2/fsck/pass2.c
> +++ b/gfs2/fsck/pass2.c
> @@ -2036,7 +2036,7 @@ static int pass2_check_dir(struct gfs2_sbd *sdp, struct gfs2_inode *ip)
>
> log_debug(_("Directory block %lld (0x%llx) is now marked as 'invalid'\n"),
> (unsigned long long)dirblk, (unsigned long long)dirblk);
> - check_n_fix_bitmap(sdp, dirblk, 0, GFS2_BLKST_FREE);
> + check_n_fix_bitmap(sdp, ip->i_rgd, dirblk, 0, GFS2_BLKST_FREE);
> }
>
> if (!ds.dotdir) {
> diff --git a/gfs2/fsck/pass3.c b/gfs2/fsck/pass3.c
> index fbf8318..ec67a7b 100644
> --- a/gfs2/fsck/pass3.c
> +++ b/gfs2/fsck/pass3.c
> @@ -253,7 +253,8 @@ int pass3(struct gfs2_sbd *sdp)
> di->dinode.no_addr,
> (unsigned long long)
> di->dinode.no_addr);
> - check_n_fix_bitmap(sdp, di->dinode.no_addr,
> + check_n_fix_bitmap(sdp, ip->i_rgd,
> + di->dinode.no_addr,
> 0, GFS2_BLKST_FREE);
> fsck_inode_put(&ip);
> break;
> @@ -273,7 +274,8 @@ int pass3(struct gfs2_sbd *sdp)
> "marked as free\n"),
> (unsigned long long)di->dinode.no_addr,
> (unsigned long long)di->dinode.no_addr);
> - check_n_fix_bitmap(sdp, di->dinode.no_addr, 0,
> + check_n_fix_bitmap(sdp, ip->i_rgd,
> + di->dinode.no_addr, 0,
> GFS2_BLKST_FREE);
> log_err( _("The block was cleared\n"));
> fsck_inode_put(&ip);
> diff --git a/gfs2/libgfs2/libgfs2.h b/gfs2/libgfs2/libgfs2.h
> index d6c54b3..5414a20 100644
> --- a/gfs2/libgfs2/libgfs2.h
> +++ b/gfs2/libgfs2/libgfs2.h
> @@ -235,6 +235,7 @@ struct gfs2_inode {
> struct gfs2_dinode i_di;
> struct gfs2_buffer_head *i_bh;
> struct gfs2_sbd *i_sbd;
> + struct rgrp_tree *i_rgd; /* performance hint */
> };
>
> struct master_dir
>
More information about the Cluster-devel
mailing list