Hi,
Looks good to me,
Steve.
On Fri, 2011-11-18 at 20:30 +0000, Andrew Price wrote:
> This patch reworks the rgblocks2bitblocks function which was
> inefficient, difficult to read and generally unwieldy.
>
> As this is core code from the days of yore and fsck.gfs2 depends on it,
> I made sure to test the new function extensively, comparing its outputs
> with the original function over a large range of values for rgblocks (up
> to 195312500) and valid block sizes between 512 and 4096.
>
> All call points have been updated and, as a nice side effect, the run
> time of the function is greatly reduced.
>
> Signed-off-by: Andrew Price <anprice@redhat.com>
> ---
> gfs2/fsck/rgrepair.c | 14 +++--------
> gfs2/libgfs2/fs_geometry.c | 54 +++++++++++++++++--------------------------
> gfs2/libgfs2/libgfs2.h | 4 +-
> 3 files changed, 27 insertions(+), 45 deletions(-)
>
> diff --git a/gfs2/fsck/rgrepair.c b/gfs2/fsck/rgrepair.c
> index 6394546..1ffa2c0 100644
> --- a/gfs2/fsck/rgrepair.c
> +++ b/gfs2/fsck/rgrepair.c
> @@ -507,12 +507,9 @@ static int gfs2_rindex_rebuild(struct gfs2_sbd *sdp, int *num_rgs,
> calc_rgd->ri.ri_data0 = calc_rgd->ri.ri_addr +
> calc_rgd->ri.ri_length;
> if (prev_rgd) {
> - uint32_t rgblocks, bitblocks;
> + uint32_t rgblocks;
>
> - rgblocks = block_bump;
> - rgblocks2bitblocks(sdp->bsize, &rgblocks, &bitblocks);
> -
> - prev_rgd->ri.ri_length = bitblocks;
> + prev_rgd->ri.ri_length = rgblocks2bitblocks(sdp->bsize, block_bump, &rgblocks);
> prev_rgd->ri.ri_data = rgblocks;
> prev_rgd->ri.ri_data0 = prev_rgd->ri.ri_addr +
> prev_rgd->ri.ri_length;
> @@ -566,12 +563,9 @@ static int gfs2_rindex_rebuild(struct gfs2_sbd *sdp, int *num_rgs,
> /* allocation information for the very last RG. */
> /* ----------------------------------------------------------------- */
> if (prev_rgd && !prev_rgd->ri.ri_data) {
> - uint32_t rgblocks, bitblocks;
> -
> - rgblocks = block_bump;
> - rgblocks2bitblocks(sdp->bsize, &rgblocks, &bitblocks);
> + uint32_t rgblocks;
>
> - prev_rgd->ri.ri_length = bitblocks;
> + prev_rgd->ri.ri_length = rgblocks2bitblocks(sdp->bsize, block_bump, &rgblocks);
> prev_rgd->ri.ri_data0 = prev_rgd->ri.ri_addr +
> prev_rgd->ri.ri_length;
> prev_rgd->ri.ri_data = rgblocks;
> diff --git a/gfs2/libgfs2/fs_geometry.c b/gfs2/libgfs2/fs_geometry.c
> index 130331a..891858e 100644
> --- a/gfs2/libgfs2/fs_geometry.c
> +++ b/gfs2/libgfs2/fs_geometry.c
> @@ -34,11 +34,10 @@ uint64_t how_many_rgrps(struct gfs2_sbd *sdp, struct device *dev, int rgsize_spe
> nrgrp = DIV_RU(dev->length, (sdp->rgsize << 20) / sdp->bsize);
>
> /* check to see if the rg length overflows max # bitblks */
> - rgblocksn = dev->length / nrgrp;
> - rgblocks2bitblocks(sdp->bsize, &rgblocksn, &bitblocksn);
> + bitblocksn = rgblocks2bitblocks(sdp->bsize, dev->length / nrgrp, &rgblocksn);
> /* calculate size of the first rgrp */
> - rgblocks1 = dev->length - (nrgrp - 1) * (dev->length / nrgrp);
> - rgblocks2bitblocks(sdp->bsize, &rgblocks1, &bitblocks1);
> + bitblocks1 = rgblocks2bitblocks(sdp->bsize, dev->length - (nrgrp - 1) * (dev->length / nrgrp),
> + &rgblocks1);
> if (bitblocks1 > 2149 || bitblocksn > 2149) {
> bitmap_overflow = 1;
> if (sdp->rgsize <= GFS2_DEFAULT_RGSIZE) {
> @@ -158,39 +157,29 @@ void compute_rgrp_layout(struct gfs2_sbd *sdp, struct osi_root *rgtree,
> }
>
> /**
> - * rgblocks2bitblocks -
> - * @bsize:
> - * @rgblocks:
> - * @bitblocks:
> - *
> - * Given a number of blocks in a RG, figure out the number of blocks
> - * needed for bitmaps.
> - *
> + * Given a number of blocks in a resource group, return the number of blocks
> + * needed for bitmaps. Also calculate the adjusted number of free data blocks
> + * in the resource group and store it in *ri_data.
> */
> -
> -void rgblocks2bitblocks(unsigned int bsize, uint32_t *rgblocks, uint32_t *bitblocks)
> +uint32_t rgblocks2bitblocks(const unsigned int bsize, const uint32_t rgblocks, uint32_t *ri_data)
> {
> - unsigned int bitbytes_provided, last = 0;
> - unsigned int bitbytes_needed;
> + uint32_t mappable = 0;
> + uint32_t bitblocks = 0;
> + /* Number of blocks mappable by bitmap blocks with these header types */
> + const uint32_t blks_rgrp = GFS2_NBBY * (bsize - sizeof(struct gfs2_rgrp));
> + const uint32_t blks_meta = GFS2_NBBY * (bsize - sizeof(struct gfs2_meta_header));
>
> - *bitblocks = 1;
> - bitbytes_provided = bsize - sizeof(struct gfs2_rgrp);
> + while (blks_rgrp + blks_meta * bitblocks < ((rgblocks - bitblocks) & ~(uint32_t)3))
> + bitblocks++;
>
> - for (;

{
> - bitbytes_needed = (*rgblocks - *bitblocks) / GFS2_NBBY;
> + if (bitblocks > 0)
> + mappable = blks_rgrp + blks_meta * (bitblocks - 1);
>
> - if (bitbytes_provided >= bitbytes_needed) {
> - if (last >= bitbytes_needed)
> - (*bitblocks)--;
> - break;
> - }
> -
> - last = bitbytes_provided;
> - (*bitblocks)++;
> - bitbytes_provided += bsize - sizeof(struct gfs2_meta_header);
> - }
> + *ri_data = (rgblocks - (bitblocks + 1)) & ~(uint32_t)3;
> + if (mappable < *ri_data)
> + bitblocks++;
>
> - *rgblocks = bitbytes_needed * GFS2_NBBY;
> + return bitblocks;
> }
>
> /**
> @@ -220,8 +209,7 @@ void build_rgrps(struct gfs2_sbd *sdp, int do_write)
> rl = (struct rgrp_tree *)n;
> ri = &rl->ri;
>
> - rgblocks = rl->length;
> - rgblocks2bitblocks(sdp->bsize, &rgblocks, &bitblocks);
> + bitblocks = rgblocks2bitblocks(sdp->bsize, rl->length, &rgblocks);
>
> ri->ri_addr = rl->start;
> ri->ri_length = bitblocks;
> diff --git a/gfs2/libgfs2/libgfs2.h b/gfs2/libgfs2/libgfs2.h
> index 77dfc29..95fd2b4 100644
> --- a/gfs2/libgfs2/libgfs2.h
> +++ b/gfs2/libgfs2/libgfs2.h
> @@ -338,8 +338,8 @@ extern int gfs2_get_bitmap(struct gfs2_sbd *sdp, uint64_t blkno,
> extern int gfs2_set_bitmap(struct gfs2_sbd *sdp, uint64_t blkno, int state);
>
> /* fs_geometry.c */
> -extern void rgblocks2bitblocks(unsigned int bsize, uint32_t *rgblocks,
> - uint32_t *bitblocks);
> +extern uint32_t rgblocks2bitblocks(const unsigned int bsize, const uint32_t rgblocks,
> + uint32_t *ri_data) __attribute__((nonnull(3)));
> extern uint64_t how_many_rgrps(struct gfs2_sbd *sdp, struct device *dev,
> int rgsize_specified);
> extern void compute_rgrp_layout(struct gfs2_sbd *sdp, struct osi_root *rgtree,