From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andrew Price Date: Thu, 23 Jun 2016 13:45:54 +0100 Subject: [Cluster-devel] [gfs2-utils PATCH 1/7] fsck.gfs2: Don't bother to pass bl blockmap pointer In-Reply-To: References: Message-ID: <1078cc88-b3a5-2d06-d1d7-2643a036a1a2@redhat.com> List-Id: To: cluster-devel.redhat.com MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit On 22/06/16 20:26, Bob Peterson wrote: > Since the blockmap pointer, bl, is a global variable of pass1, we > don't need to pass it around needlessly. Make this more efficient. Is it measurably more efficient? I'd rather make bl's scope narrower instead of widening it from a maintainability standpoint, but it could also let the optimizer make better decisions with more limited scope, e.g. when inlining. In general I don't think we need to optimize at this small scale at this point, especially without profiling to make sure the trade-offs are worthwhile. Andy > Signed-off-by: Bob Peterson > --- > gfs2/fsck/pass1.c | 25 ++++++++++++------------- > 1 file changed, 12 insertions(+), 13 deletions(-) > > diff --git a/gfs2/fsck/pass1.c b/gfs2/fsck/pass1.c > index 6f04109..112b68e 100644 > --- a/gfs2/fsck/pass1.c > +++ b/gfs2/fsck/pass1.c > @@ -90,17 +90,17 @@ static int delete_block(struct gfs2_inode *ip, uint64_t block, > struct gfs2_buffer_head **bh, const char *btype, > void *private); > > -static int gfs2_blockmap_set(struct gfs2_bmap *bmap, uint64_t bblock, int mark) > +static int gfs2_blockmap_set(uint64_t bblock, int mark) > { > static unsigned char *byte; > static uint64_t b; > > - if (!bmap) > + if (bl == NULL) > return 0; > - if (bblock > bmap->size) > + if (bblock > bl->size) > return -1; > > - byte = bmap->map + BLOCKMAP_SIZE2(bblock); > + byte = bl->map + BLOCKMAP_SIZE2(bblock); > b = BLOCKMAP_BYTE_OFFSET2(bblock); > *byte &= ~(BLOCKMAP_MASK2 << b); > *byte |= (mark & BLOCKMAP_MASK2) << b; > @@ -120,7 +120,7 @@ static int _fsck_blockmap_set(struct gfs2_inode *ip, uint64_t bblock, > if (error) > return error; > > - return gfs2_blockmap_set(bl, bblock, mark); > + return gfs2_blockmap_set(bblock, mark); > } > > #define fsck_blockmap_set(ip, b, bt, m) \ > @@ -1336,7 +1336,7 @@ static int alloc_metalist(struct gfs2_inode *ip, uint64_t block, > "%lld (0x%llx) is now marked as indirect.\n"), > desc, (unsigned long long)block, > (unsigned long long)block); > - gfs2_blockmap_set(bl, block, ip->i_sbd->gfs1 ? > + gfs2_blockmap_set(block, ip->i_sbd->gfs1 ? > GFS2_BLKST_DINODE : GFS2_BLKST_USED); > } > return meta_is_good; > @@ -1358,7 +1358,7 @@ static int alloc_data(struct gfs2_inode *ip, uint64_t metablock, > "%lld (0x%llx) is now marked as data.\n"), > desc, (unsigned long long)block, > (unsigned long long)block); > - gfs2_blockmap_set(bl, block, GFS2_BLKST_USED); > + gfs2_blockmap_set(block, GFS2_BLKST_USED); > } > return 0; > } > @@ -1407,8 +1407,7 @@ static int pass1_check_metatree(struct gfs2_inode *ip, > > error = check_metatree(ip, pass); > if (error) > - gfs2_blockmap_set(bl, ip->i_di.di_num.no_addr, > - GFS2_BLKST_FREE); > + gfs2_blockmap_set(ip->i_di.di_num.no_addr, GFS2_BLKST_FREE); > return error; > } > > @@ -1671,7 +1670,7 @@ static int check_system_inode(struct gfs2_sbd *sdp, > "%llu (0x%llx)\n"), > (unsigned long long)iblock, > (unsigned long long)iblock); > - gfs2_blockmap_set(bl, iblock, GFS2_BLKST_FREE); > + gfs2_blockmap_set(iblock, GFS2_BLKST_FREE); > check_n_fix_bitmap(sdp, (*sysinode)->i_rgd, iblock, 0, > GFS2_BLKST_FREE); > inode_put(sysinode); > @@ -1706,7 +1705,7 @@ static int check_system_inode(struct gfs2_sbd *sdp, > /* Set the blockmap (but not bitmap) back to > 'free' so that it gets checked like any > normal dinode. */ > - gfs2_blockmap_set(bl, iblock, GFS2_BLKST_FREE); > + gfs2_blockmap_set(iblock, GFS2_BLKST_FREE); > log_err( _("Removed system inode \"%s\".\n"), > filename); > } > @@ -2044,7 +2043,7 @@ static int pass1_process_rgrp(struct gfs2_sbd *sdp, struct rgrp_tree *rgd) > > n = lgfs2_bm_scan(rgd, k, ibuf, GFS2_BLKST_UNLINKED); > for (i = 0; i < n; i++) { > - gfs2_blockmap_set(bl, ibuf[i], GFS2_BLKST_UNLINKED); > + gfs2_blockmap_set(ibuf[i], GFS2_BLKST_UNLINKED); > if (fsck_abort) > goto out; > } > @@ -2200,7 +2199,7 @@ int pass1(struct gfs2_sbd *sdp) > log_debug( _("rgrp block %lld (0x%llx) " > "is now marked as 'rgrp data'\n"), > rgd->ri.ri_addr + i, rgd->ri.ri_addr + i); > - if (gfs2_blockmap_set(bl, rgd->ri.ri_addr + i, > + if (gfs2_blockmap_set(rgd->ri.ri_addr + i, > GFS2_BLKST_USED)) { > stack; > gfs2_special_free(&gfs1_rindex_blks); >