FAQ Search Today's Posts Mark Forums Read
» Video Reviews

» Linux Archive

Linux-archive is a website aiming to archive linux email lists and to make them easily accessible for linux users/developers.


» Sponsor

» Partners

» Sponsor

Go Back   Linux Archive > Redhat > Cluster Development

 
 
LinkBack Thread Tools
 
Old 11-18-2011, 07:30 PM
Andrew Price
 
Default libgfs2: Improve rgblocks2bitblocks()

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,
--
1.7.6.4
 
Old 11-18-2011, 07:52 PM
Bob Peterson
 
Default libgfs2: Improve rgblocks2bitblocks()

----- Original Message -----
| 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>
| + while (blks_rgrp + blks_meta * bitblocks < ((rgblocks - bitblocks)

Hi,

The patch looks good. There's only thing I'd do differently:
I know the implied arithmetic operator order is correct, but
I still prefer to see parens around statements like the above
just for clarity. e.g.

+ while (blks_rgrp + (blks_meta * bitblocks) < ((rgblocks - bitblocks)

I guess that's more of a style thing.

Regards,

Bob Peterson
Red Hat File Systems
 
Old 11-18-2011, 08:28 PM
Andrew Price
 
Default libgfs2: Improve rgblocks2bitblocks()

On 18/11/11 20:52, Bob Peterson wrote:

----- Original Message -----
| 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>
| + while (blks_rgrp + blks_meta * bitblocks< ((rgblocks - bitblocks)

Hi,

The patch looks good. There's only thing I'd do differently:
I know the implied arithmetic operator order is correct, but
I still prefer to see parens around statements like the above
just for clarity. e.g.

+ while (blks_rgrp + (blks_meta * bitblocks)< ((rgblocks - bitblocks)

I guess that's more of a style thing.


Yeah, it is more readable that way, I'll change it.

Andy
 
Old 11-21-2011, 09:11 AM
Steven Whitehouse
 
Default libgfs2: Improve rgblocks2bitblocks()

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,
 

Thread Tools




All times are GMT. The time now is 01:43 PM.

VBulletin, Copyright ©2000 - 2014, Jelsoft Enterprises Ltd.
Content Relevant URLs by vBSEO ©2007, Crawlability, Inc.
Copyright 2007 - 2008, www.linux-archive.org