[Cluster-devel] [PATCH 1/5] mkfs.gfs2: Make dev a member of mkfs_opts

[Cluster-devel] [PATCH 1/5] mkfs.gfs2: Make dev a member of mkfs_opts

[Andrew Price]

As most of the functions which accept a struct mkfs_dev* also require a
struct mkfs_opts* it makes sense to move the device path string into the
mkfs_dev and add a struct mkfs_dev member to the struct mkfs_opts.

Nothing much to see here, just refactoring.

Signed-off-by: Andrew Price <anprice@redhat.com>
---
 gfs2/mkfs/main_mkfs.c | 125 +++++++++++++++++++++++++-------------------------
 1 file changed, 62 insertions(+), 63 deletions(-)

diff --git a/gfs2/mkfs/main_mkfs.c b/gfs2/mkfs/main_mkfs.c
index e58de68..40f4766 100644
--- a/gfs2/mkfs/main_mkfs.c
+++ b/gfs2/mkfs/main_mkfs.c
@@ -96,6 +96,28 @@ static void print_ext_opts(void)
 	}
 }
 
+/**
+ * Values probed by libblkid:
+ *  alignment_offset: offset, in bytes, of the start of the dev from its natural alignment
+ *  logical_sector_size: smallest addressable unit
+ *  minimum_io_size: device's preferred unit of I/O. RAID stripe unit.
+ *  optimal_io_size: biggest I/O we can submit without incurring a penalty. RAID stripe width.
+ *  physical_sector_size: the smallest unit we can write atomically
+ */
+struct mkfs_dev {
+	int fd;
+	const char *path;
+	struct stat stat;
+	uint64_t size;
+	unsigned long alignment_offset;
+	unsigned long logical_sector_size;
+	unsigned long minimum_io_size;
+	unsigned long optimal_io_size;
+	unsigned long physical_sector_size;
+
+	unsigned int got_topol:1;
+};
+
 struct mkfs_opts {
 	unsigned bsize;
 	unsigned qcsize;
@@ -107,7 +129,7 @@ struct mkfs_opts {
 	uint32_t journals;
 	const char *lockproto;
 	const char *locktable;
-	const char *device;
+	struct mkfs_dev dev;
 	unsigned discard:1;
 
 	unsigned got_bsize:1;
@@ -130,27 +152,6 @@ struct mkfs_opts {
 	unsigned align:1;
 };
 
-/**
- * Values probed by libblkid:
- *  alignment_offset: offset, in bytes, of the start of the dev from its natural alignment
- *  logical_sector_size: smallest addressable unit
- *  minimum_io_size: device's preferred unit of I/O. RAID stripe unit.
- *  optimal_io_size: biggest I/O we can submit without incurring a penalty. RAID stripe width.
- *  physical_sector_size: the smallest unit we can write atomically
- */
-struct mkfs_dev {
-	int fd;
-	struct stat stat;
-	uint64_t size;
-	unsigned long alignment_offset;
-	unsigned long logical_sector_size;
-	unsigned long minimum_io_size;
-	unsigned long optimal_io_size;
-	unsigned long physical_sector_size;
-
-	unsigned int got_topol:1;
-};
-
 static void opts_init(struct mkfs_opts *opts)
 {
 	memset(opts, 0, sizeof(*opts));
@@ -362,7 +363,7 @@ static void opts_get(int argc, char *argv[], struct mkfs_opts *opts)
 			if (strcmp(optarg, "gfs2") == 0)
 				continue;
 			if (!opts->got_device) {
-				opts->device = optarg;
+				opts->dev.path = optarg;
 				opts->got_device = 1;
 			} else if (!opts->got_fssize && isdigit(optarg[0])) {
 				opts->fssize = atol(optarg);
@@ -457,10 +458,11 @@ static void are_you_sure(void)
 		free(line);
 }
 
-static unsigned choose_blocksize(struct mkfs_opts *opts, const struct mkfs_dev *dev)
+static unsigned choose_blocksize(struct mkfs_opts *opts)
 {
 	unsigned int x;
 	unsigned int bsize = opts->bsize;
+	struct mkfs_dev *dev = &opts->dev;
 
 	if (dev->got_topol && opts->debug) {
 		printf("alignment_offset: %lu\n", dev->alignment_offset);
@@ -536,15 +538,14 @@ static void opts_check(struct mkfs_opts *opts)
 	}
 }
 
-static void print_results(struct gfs2_sb *sb, struct mkfs_dev *dev, struct mkfs_opts *opts,
-                          uint64_t rgrps, uint64_t fssize)
+static void print_results(struct gfs2_sb *sb, struct mkfs_opts *opts, uint64_t rgrps, uint64_t fssize)
 {
-	printf("%-27s%s\n", _("Device:"), opts->device);
+	printf("%-27s%s\n", _("Device:"), opts->dev.path);
 	printf("%-27s%u\n", _("Block size:"), sb->sb_bsize);
 	printf("%-27s%.2f %s (%"PRIu64" %s)\n", _("Device size:"),
 	       /* Translators: "GB" here means "gigabytes" */
-	       (dev->size / ((float)(1 << 30))), _("GB"),
-	       (dev->size / sb->sb_bsize), _("blocks"));
+	       (opts->dev.size / ((float)(1 << 30))), _("GB"),
+	       (opts->dev.size / sb->sb_bsize), _("blocks"));
 	printf("%-27s%.2f %s (%"PRIu64" %s)\n", _("Filesystem size:"),
 	       (fssize / ((float)(1 << 30)) * sb->sb_bsize), _("GB"), fssize, _("blocks"));
 	printf("%-27s%u\n", _("Journals:"), opts->journals);
@@ -580,7 +581,7 @@ static void warn_of_destruction(const char *path)
 	free(abspath);
 }
 
-static lgfs2_rgrps_t rgs_init(struct mkfs_opts *opts, struct mkfs_dev *dev, struct gfs2_sbd *sdp)
+static lgfs2_rgrps_t rgs_init(struct mkfs_opts *opts, struct gfs2_sbd *sdp)
 {
 	uint64_t rgsize = (opts->rgsize << 20) / sdp->bsize;
 	struct lgfs2_rgrp_align align = {.base = 0, .offset = 0};
@@ -600,10 +601,10 @@ static lgfs2_rgrps_t rgs_init(struct mkfs_opts *opts, struct mkfs_dev *dev, stru
 			align.offset = opts->sunit / sdp->bsize;
 		}
 	} else if (opts->align) {
-		if ((dev->minimum_io_size > dev->physical_sector_size) &&
-		    (dev->optimal_io_size > dev->physical_sector_size)) {
-			align.base = dev->optimal_io_size / sdp->bsize;
-			align.offset = dev->minimum_io_size / sdp->bsize;
+		if ((opts->dev.minimum_io_size > opts->dev.physical_sector_size) &&
+		    (opts->dev.optimal_io_size > opts->dev.physical_sector_size)) {
+			align.base = opts->dev.optimal_io_size / sdp->bsize;
+			align.offset = opts->dev.minimum_io_size / sdp->bsize;
 		}
 	}
 
@@ -624,7 +625,7 @@ static lgfs2_rgrps_t rgs_init(struct mkfs_opts *opts, struct mkfs_dev *dev, stru
 	return rgs;
 }
 
-static uint64_t place_rgrps(struct gfs2_sbd *sdp, lgfs2_rgrps_t rgs, struct mkfs_opts *opts, const struct mkfs_dev *dev)
+static uint64_t place_rgrps(struct gfs2_sbd *sdp, lgfs2_rgrps_t rgs, struct mkfs_opts *opts)
 {
 	int err = 0;
 	lgfs2_rgrp_t rg = NULL;
@@ -656,7 +657,7 @@ static uint64_t place_rgrps(struct gfs2_sbd *sdp, lgfs2_rgrps_t rgs, struct mkfs
 		return ri->ri_data0 + ri->ri_data;
 }
 
-static void sbd_init(struct gfs2_sbd *sdp, struct mkfs_opts *opts, struct mkfs_dev *dev, unsigned bsize)
+static void sbd_init(struct gfs2_sbd *sdp, struct mkfs_opts *opts, unsigned bsize)
 {
 	memset(sdp, 0, sizeof(struct gfs2_sbd));
 	sdp->time = time(NULL);
@@ -665,20 +666,20 @@ static void sbd_init(struct gfs2_sbd *sdp, struct mkfs_opts *opts, struct mkfs_d
 	sdp->qcsize = opts->qcsize;
 	sdp->jsize = opts->jsize;
 	sdp->md.journals = opts->journals;
-	sdp->device_fd = dev->fd;
+	sdp->device_fd = opts->dev.fd;
 	sdp->bsize = bsize;
 
 	if (compute_constants(sdp)) {
 		perror(_("Failed to compute file system constants"));
 		exit(1);
 	}
-	sdp->device.length = dev->size / sdp->bsize;
+	sdp->device.length = opts->dev.size / sdp->bsize;
 	if (opts->got_fssize) {
 		if (opts->fssize > sdp->device.length) {
 			fprintf(stderr, _("Specified size is bigger than the device."));
 			die("%s %.2f %s (%"PRIu64" %s)\n", _("Device size:"),
-			       dev->size / ((float)(1 << 30)), _("GB"),
-			       dev->size / sdp->bsize, _("blocks"));
+			       opts->dev.size / ((float)(1 << 30)), _("GB"),
+			       opts->dev.size / sdp->bsize, _("blocks"));
 		}
 		/* TODO: Check if the fssize is too small, somehow */
 		sdp->device.length = opts->fssize;
@@ -733,20 +734,19 @@ static int probe_contents(struct mkfs_dev *dev)
 	return 0;
 }
 
-static void open_dev(const char *path, struct mkfs_dev *dev)
+static void open_dev(struct mkfs_dev *dev)
 {
 	int error;
 
-	memset(dev, 0, sizeof(*dev));
-	dev->fd = open(path, O_RDWR | O_CLOEXEC);
+	dev->fd = open(dev->path, O_RDWR | O_CLOEXEC);
 	if (dev->fd < 0) {
-		perror(path);
+		perror(dev->path);
 		exit(1);
 	}
 
 	error = fstat(dev->fd, &dev->stat);
 	if (error < 0) {
-		perror(path);
+		perror(dev->path);
 		exit(1);
 	}
 
@@ -755,11 +755,11 @@ static void open_dev(const char *path, struct mkfs_dev *dev)
 	} else if (S_ISBLK(dev->stat.st_mode)) {
 		dev->size = lseek(dev->fd, 0, SEEK_END);
 		if (dev->size < 1) {
-			fprintf(stderr, _("Device '%s' is too small\n"), path);
+			fprintf(stderr, _("Device '%s' is too small\n"), dev->path);
 			exit(1);
 		}
 	} else {
-		fprintf(stderr, _("'%s' is not a block device or regular file\n"), path);
+		fprintf(stderr, _("'%s' is not a block device or regular file\n"), dev->path);
 		exit(1);
 	}
 
@@ -773,7 +773,6 @@ void main_mkfs(int argc, char *argv[])
 	struct gfs2_sbd sbd;
 	struct gfs2_sb sb;
 	struct mkfs_opts opts;
-	struct mkfs_dev dev;
 	lgfs2_rgrps_t rgs;
 	int error;
 	unsigned bsize;
@@ -782,14 +781,14 @@ void main_mkfs(int argc, char *argv[])
 	opts_get(argc, argv, &opts);
 	opts_check(&opts);
 
-	open_dev(opts.device, &dev);
-	bsize = choose_blocksize(&opts, &dev);
+	open_dev(&opts.dev);
+	bsize = choose_blocksize(&opts);
 
-	if (S_ISREG(dev.stat.st_mode)) {
+	if (S_ISREG(opts.dev.stat.st_mode)) {
 		opts.got_bsize = 1; /* Use default block size for regular files */
 	}
 
-	sbd_init(&sbd, &opts, &dev, bsize);
+	sbd_init(&sbd, &opts, bsize);
 	lgfs2_sb_init(&sb, bsize);
 	if (opts.debug) {
 		printf(_("File system options:\n"));
@@ -804,16 +803,16 @@ void main_mkfs(int argc, char *argv[])
 		printf("  sunit = %lu\n", opts.sunit);
 		printf("  swidth = %lu\n", opts.swidth);
 	}
-	rgs = rgs_init(&opts, &dev, &sbd);
-	warn_of_destruction(opts.device);
+	rgs = rgs_init(&opts, &sbd);
+	warn_of_destruction(opts.dev.path);
 
 	if (opts.confirm && !opts.override)
 		are_you_sure();
 
-	if (!S_ISREG(dev.stat.st_mode) && opts.discard)
-		discard_blocks(dev.fd, dev.size, opts.debug);
+	if (!S_ISREG(opts.dev.stat.st_mode) && opts.discard)
+		discard_blocks(opts.dev.fd, opts.dev.size, opts.debug);
 
-	sbd.fssize = place_rgrps(&sbd, rgs, &opts, &dev);
+	sbd.fssize = place_rgrps(&sbd, rgs, &opts);
 	if (sbd.fssize == 0) {
 		fprintf(stderr, _("Failed to build resource groups\n"));
 		exit(1);
@@ -872,24 +871,24 @@ void main_mkfs(int argc, char *argv[])
 
 	gfs2_rgrp_free(&sbd.rgtree);
 
-	error = lgfs2_sb_write(&sb, dev.fd, sbd.bsize);
+	error = lgfs2_sb_write(&sb, opts.dev.fd, sbd.bsize);
 	if (error) {
 		perror(_("Failed to write superblock\n"));
 		exit(EXIT_FAILURE);
 	}
 
-	error = fsync(dev.fd);
+	error = fsync(opts.dev.fd);
 	if (error){
-		perror(opts.device);
+		perror(opts.dev.path);
 		exit(EXIT_FAILURE);
 	}
 
-	error = close(dev.fd);
+	error = close(opts.dev.fd);
 	if (error){
-		perror(opts.device);
+		perror(opts.dev.path);
 		exit(EXIT_FAILURE);
 	}
 
 	if (!opts.quiet)
-		print_results(&sb, &dev, &opts, sbd.rgrps, sbd.fssize);
+		print_results(&sb, &opts, sbd.rgrps, sbd.fssize);
 }
-- 
1.8.5.3

[Cluster-devel] [PATCH 2/5] libgfs2: Add lgfs2_space_for_data()

[Andrew Price]

Add a function which calculates the total number of blocks required for
a file given the size of its contents.

Signed-off-by: Andrew Price <anprice@redhat.com>
---
 gfs2/libgfs2/fs_ops.c  | 18 ++++++++++++++++++
 gfs2/libgfs2/libgfs2.h |  2 ++
 2 files changed, 20 insertions(+)

diff --git a/gfs2/libgfs2/fs_ops.c b/gfs2/libgfs2/fs_ops.c
index 198f6eb..b95f2ed 100644
--- a/gfs2/libgfs2/fs_ops.c
+++ b/gfs2/libgfs2/fs_ops.c
@@ -273,6 +273,24 @@ void unstuff_dinode(struct gfs2_inode *ip)
 	ip->i_di.di_height = 1;
 }
 
+/**
+ * Calculate the total number of blocks required by a file containing 'bytes' bytes of data.
+ */
+uint64_t lgfs2_space_for_data(const struct gfs2_sbd *sdp, const unsigned bsize, const uint64_t bytes)
+{
+	uint64_t blks = (bytes + bsize - 1) / bsize;
+	uint64_t ptrs = blks;
+
+	if (bytes <= bsize - sizeof(struct gfs2_dinode))
+		return 1;
+
+	while (ptrs > sdp->sd_diptrs) {
+		ptrs = (ptrs + sdp->sd_inptrs - 1) / sdp->sd_inptrs;
+		blks += ptrs;
+	}
+	return blks + 1;
+}
+
 unsigned int calc_tree_height(struct gfs2_inode *ip, uint64_t size)
 {
 	struct gfs2_sbd *sdp = ip->i_sbd;
diff --git a/gfs2/libgfs2/libgfs2.h b/gfs2/libgfs2/libgfs2.h
index 6248e4b..ce51e8c 100644
--- a/gfs2/libgfs2/libgfs2.h
+++ b/gfs2/libgfs2/libgfs2.h
@@ -450,6 +450,8 @@ extern void inode_put(struct gfs2_inode **ip);
 extern uint64_t data_alloc(struct gfs2_inode *ip);
 extern int lgfs2_meta_alloc(struct gfs2_inode *ip, uint64_t *blkno);
 extern int lgfs2_dinode_alloc(struct gfs2_sbd *sdp, const uint64_t blksreq, uint64_t *blkno);
+extern uint64_t lgfs2_space_for_data(const struct gfs2_sbd *sdp, unsigned bsize, uint64_t bytes);
+
 extern int gfs2_readi(struct gfs2_inode *ip, void *buf, uint64_t offset,
 		      unsigned int size);
 #define gfs2_writei(ip, buf, offset, size) \
-- 
1.8.5.3

[Cluster-devel] [PATCH 3/5] libgfs2: Don't try to read more than IOV_MAX iovecs

[Andrew Price]

Reading large collections of blocks, as gfs2_rgrp_read may do with large
rgrps and small block sizes, can cause preadv() to fail with EINVAL in
breadm(). This patch splits large reads into IOV_MAX chunks to avoid
reads failing in that way. It also includes a test to exercise these
changes.

Signed-off-by: Andrew Price <anprice@redhat.com>
---
 gfs2/libgfs2/buf.c | 57 ++++++++++++++++++++++++++++++++----------------------
 tests/mkfs.at      |  4 ++++
 2 files changed, 38 insertions(+), 23 deletions(-)

diff --git a/gfs2/libgfs2/buf.c b/gfs2/libgfs2/buf.c
index 6fcdd17..92cd393 100644
--- a/gfs2/libgfs2/buf.c
+++ b/gfs2/libgfs2/buf.c
@@ -15,6 +15,14 @@
 
 #include "libgfs2.h"
 
+#ifndef IOV_MAX
+  #ifdef UIO_MAXIOV
+    #define IOV_MAX UIO_MAXIOV
+  #else
+    #define IOV_MAX (1024)
+  #endif
+#endif
+
 struct gfs2_buffer_head *bget(struct gfs2_sbd *sdp, uint64_t num)
 {
 	struct gfs2_buffer_head *bh;
@@ -34,31 +42,34 @@ struct gfs2_buffer_head *bget(struct gfs2_sbd *sdp, uint64_t num)
 int __breadm(struct gfs2_sbd *sdp, struct gfs2_buffer_head **bhs, size_t n,
 	     uint64_t block, int line, const char *caller)
 {
-	struct iovec *iov = alloca(n * sizeof(struct iovec));
+	size_t v = (n < IOV_MAX) ? n : IOV_MAX;
+	struct iovec *iov = alloca(v * sizeof(struct iovec));
 	struct iovec *iovbase = iov;
-	uint64_t b = block;
-	size_t size = 0;
-	size_t i;
-	int ret;
-
-	for (i = 0; i < n; i++) {
-		bhs[i] = bget(sdp, b++);
-		if (bhs[i] == NULL)
-			return -1;
-		*iov++ = bhs[i]->iov;
-		size += bhs[i]->iov.iov_len;
+	size_t i = 0;
+
+	while (i < n) {
+		int j;
+		ssize_t ret;
+		ssize_t size = 0;
+
+		for (j = 0; (i + j < n) && (j < IOV_MAX); j++) {
+			bhs[i + j] = bget(sdp, block + i + j);
+			if (bhs[i + j] == NULL)
+				return -1;
+			iov[j] = bhs[i + j]->iov;
+			size += bhs[i + j]->iov.iov_len;
+		}
+
+		ret = preadv(sdp->device_fd, iovbase, j, (block + i) * sdp->bsize);
+		if (ret != size) {
+			fprintf(stderr, "bad read: %s from %s:%d: block %llu (0x%llx) "
+					"count: %d size: %zd ret: %zd\n", strerror(errno),
+					caller, line, (unsigned long long)block,
+					(unsigned long long)block, j, size, ret);
+			exit(-1);
+		}
+		i += j;
 	}
-
-	ret = preadv(sdp->device_fd, iovbase, n, block * sdp->bsize);
-
-	if (ret != size) {
-		fprintf(stderr, "bad read: %s from %s:%d: block "
-				"%llu (0x%llx)\n", strerror(errno),
-				caller, line, (unsigned long long)block,
-				(unsigned long long)block);
-		exit(-1);
-	}
-
 	return 0;
 }
 
diff --git a/tests/mkfs.at b/tests/mkfs.at
index 2616109..ff99bb1 100644
--- a/tests/mkfs.at
+++ b/tests/mkfs.at
@@ -54,6 +54,10 @@ AT_SETUP([Min. resource group size])
 GFS_FSCK_CHECK([$GFS_MKFS -p lock_nolock -r 32 $GFS_TGT])
 AT_CLEANUP
 
+AT_SETUP([Max. resource group size, min. block size])
+GFS_FSCK_CHECK([$GFS_MKFS -p lock_nolock -r 2048 -b 512 $GFS_TGT])
+AT_CLEANUP
+
 AT_SETUP([Max. journal size])
 GFS_FSCK_CHECK([$GFS_MKFS -p lock_nolock -J 1024 $GFS_TGT])
 AT_CLEANUP
-- 
1.8.5.3

[Cluster-devel] [PATCH 4/5] mkfs.gfs2: Fix the resource group layout strategy, again

[Andrew Price]

The previous attempt at improving the resource group layout and
alignment fell somewhat short of the mark and left some issues, such as
the possibility of leaving a small resource group at the end of the
device which gfs2_grow could wrongly use as the file system's resource
group size.

The core of this patch is the new lgfs2_rgrps_plan() function which
calculates a sensible resource group size given a target maximum (which
we now default to 2GB instead of 256MB). In order to avoid leaving a gap
or a small rgrp at the end of the device, we adjust the rgrp length down
until a further adjustment would leave a gap, then apply a constant
adjustment to the size of a subset of the resource groups.

The rest of the patch aims to clean up libgfs2's resource group API and
give more control to the application rather than storing a lot of the
resource group layout parameters in the lgfs2_rgrps_t. This should make
it easier to use the same functions in gfs2_grow, fsck.gfs2 and any
other tools which might need to manipulate resource groups.

Signed-off-by: Andrew Price <anprice@redhat.com>
---
 gfs2/libgfs2/libgfs2.h |  16 ++--
 gfs2/libgfs2/rgrp.c    | 206 ++++++++++++++++++++++++++++++++++---------------
 gfs2/mkfs/main_mkfs.c  | 104 +++++++++++++++++--------
 3 files changed, 218 insertions(+), 108 deletions(-)

diff --git a/gfs2/libgfs2/libgfs2.h b/gfs2/libgfs2/libgfs2.h
index ce51e8c..24947c2 100644
--- a/gfs2/libgfs2/libgfs2.h
+++ b/gfs2/libgfs2/libgfs2.h
@@ -186,19 +186,16 @@ struct rgrp_tree {
 	struct gfs2_buffer_head **bh;
 };
 
-struct lgfs2_rgrp_align {
-	uint64_t base;
-	uint64_t offset;
-};
-
 typedef struct rgrp_tree *lgfs2_rgrp_t;
 typedef struct _lgfs2_rgrps *lgfs2_rgrps_t;
 
-extern lgfs2_rgrps_t lgfs2_rgrps_init(unsigned bsize, uint64_t start, uint64_t devlen, uint32_t rglen, struct lgfs2_rgrp_align *al);
+extern lgfs2_rgrps_t lgfs2_rgrps_init(unsigned bsize, uint64_t devlen, uint64_t align, uint64_t offset);
+extern uint64_t lgfs2_rgrp_align_addr(const lgfs2_rgrps_t rgs, uint64_t addr);
+extern uint32_t lgfs2_rgrp_align_len(const lgfs2_rgrps_t rgs, uint32_t len);
 extern unsigned lgfs2_rgsize_for_data(uint64_t blksreq, unsigned bsize);
-extern lgfs2_rgrp_t lgfs2_rgrp_append(lgfs2_rgrps_t rgs, uint32_t rglen, int expand);
-extern int lgfs2_rgrp_write(int fd, lgfs2_rgrp_t rg, unsigned bsize);
-extern int lgfs2_rgrps_end(lgfs2_rgrps_t rgs);
+extern uint32_t lgfs2_rgrps_plan(const lgfs2_rgrps_t rgs, uint64_t space, uint32_t tgtsize);
+extern lgfs2_rgrp_t lgfs2_rgrp_append(lgfs2_rgrps_t rgs, uint64_t addr, uint32_t rglen, uint64_t *nextaddr);
+extern int lgfs2_rgrp_write(lgfs2_rgrps_t rgs, int fd, lgfs2_rgrp_t rg);
 extern struct gfs2_rindex *lgfs2_rgrp_index(lgfs2_rgrp_t rg);
 // Temporary function to aid API migration
 extern struct osi_node *lgfs2_rgrps_root(lgfs2_rgrps_t rgs) __attribute__((deprecated));
@@ -350,7 +347,6 @@ struct metapath {
 
 #define GFS2_EXP_MIN_RGSIZE         (1)
 #define GFS2_MIN_RGSIZE             (32)
-/* Look at this!  Why can't we go bigger than 2GB? */
 #define GFS2_MAX_RGSIZE             (2048)
 
 /* meta.c */
diff --git a/gfs2/libgfs2/rgrp.c b/gfs2/libgfs2/rgrp.c
index 0752772..1242385 100644
--- a/gfs2/libgfs2/rgrp.c
+++ b/gfs2/libgfs2/rgrp.c
@@ -223,6 +223,11 @@ void gfs2_rgrp_free(struct osi_root *rgrp_tree)
 	}
 }
 
+struct rgplan {
+	uint32_t num;
+	uint32_t len;
+};
+
 /**
  * This structure is defined in libgfs2.h as an opaque type. It stores the
  * constants and context required for creating resource groups from any point
@@ -230,17 +235,11 @@ void gfs2_rgrp_free(struct osi_root *rgrp_tree)
  */
 struct _lgfs2_rgrps {
 	struct osi_root root;
-	uint64_t nextaddr;
+	struct rgplan plan[2];
 	unsigned bsize;
 	unsigned long align;
 	unsigned long align_off;
-	unsigned long curr_offset;
-	uint64_t maxrgsz;
-	uint64_t minrgsz;
 	uint64_t devlen;
-	uint64_t count;
-	uint64_t blks_total;
-	uint32_t rgsize;
 };
 
 static uint64_t align_block(const uint64_t base, const uint64_t align)
@@ -251,29 +250,121 @@ static uint64_t align_block(const uint64_t base, const uint64_t align)
 }
 
 /**
+ * Calculate the aligned block address of a resource group.
+ * rgs: The resource groups handle
+ * base: The base address of the first resource group address, in blocks
+ * Returns the aligned address of the first resource group.
+ */
+uint64_t lgfs2_rgrp_align_addr(const lgfs2_rgrps_t rgs, uint64_t addr)
+{
+	return align_block(addr, rgs->align);
+}
+
+/**
+ * Calculate the aligned relative address of the next resource group (and thus
+ * the aligned length of this one).
+ * rgs: The resource groups handle
+ * base: The base length of the current resource group, in blocks
+ * Returns the length of the resource group (the aligned relative address of
+ * the next one)
+ */
+uint32_t lgfs2_rgrp_align_len(const lgfs2_rgrps_t rgs, uint32_t len)
+{
+	return align_block(len, rgs->align) + rgs->align_off;
+}
+
+/**
+ * Plan the sizes of resource groups for remaining free space, based on a
+ * target maximum size. In order to make best use of the space while keeping
+ * the resource groups aligned appropriately we need to either reduce the
+ * length of every resource group or of a subset of the resource groups, so
+ * we're left with either one or two resource group sizes. We keep track of
+ * both of these and the numbers of each size of resource group inside the
+ * resource groups descriptor.
+ * rgs: The resource groups descriptor
+ * space: The number of remaining blocks to be allocated
+ * tgtsize: The target resource group size in blocks
+ * Returns the larger of the calculated resource group sizes or 0 if the
+ * smaller would be less than GFS2_MIN_RGSIZE.
+ */
+uint32_t lgfs2_rgrps_plan(const lgfs2_rgrps_t rgs, uint64_t space, uint32_t tgtsize)
+{
+	uint32_t maxlen = (GFS2_MAX_RGSIZE << 20) / rgs->bsize;
+	uint32_t minlen = (GFS2_MIN_RGSIZE << 20) / rgs->bsize;
+
+	/* Apps should already have checked that the rg size is <=
+	   GFS2_MAX_RGSIZE but just in case alignment pushes it over we clamp
+	   it back down while calculating the initial rgrp length.  */
+	do {
+		rgs->plan[0].len = lgfs2_rgrp_align_len(rgs, tgtsize);
+		tgtsize -= (rgs->align + 1);
+	} while (rgs->plan[0].len > maxlen);
+
+	rgs->plan[0].num = space / rgs->plan[0].len;
+
+	if ((space - (rgs->plan[0].num * rgs->plan[0].len)) > rgs->align) {
+		unsigned adj = (rgs->align > 0) ? rgs->align : 1;
+
+		/* Spread the adjustment required to fit a new rgrp at the end
+		   over all of the rgrps so that we don't end with a single
+		   tiny one.  */
+		while (((rgs->plan[0].len - adj) * (rgs->plan[0].num + 1)) >= space)
+			rgs->plan[0].len -= adj;
+
+		/* We've adjusted the size of the rgrps down as far as we can
+		   without leaving a large gap at the end of the device now,
+		   but we still need to reduce the size of some rgrps in order
+		   to make everything fit, so we use the second rgplan to
+		   specify a second length for a subset of the resource groups.
+		   If plan[0].len already divides the space with no remainder,
+		   plan[1].num will stay 0 and it won't be used.  */
+		rgs->plan[1].len = rgs->plan[0].len - adj;
+		rgs->plan[1].num = 0;
+
+		while (((rgs->plan[0].len * rgs->plan[0].num) +
+		        (rgs->plan[1].len * rgs->plan[1].num)) > space) {
+			/* Total number of rgrps stays constant now. We just
+			   need to shift some weight around */
+			rgs->plan[0].num--;
+			rgs->plan[1].num++;
+		}
+	}
+
+	/* Once we've reached this point,
+	   (plan[0].num * plan[0].len) + (plan[1].num * plan[1].len)
+	   will be less than one adjustment smaller than 'space'.  */
+
+	if (rgs->plan[0].len < minlen)
+		return 0;
+
+	return rgs->plan[0].len;
+}
+
+/**
  * Create and initialise an empty set of resource groups
  * bsize: The block size of the fs
- * start: The block address of the first resource group
  * devlen: The length of the device, in fs blocks
- * rglen: Default rg size, in blocks
- * al: The required alignment of the resource groups
+ * align: The required stripe alignment of the resource groups. Must be a multiple of 'offset'.
+ * offset: The required stripe offset of the resource groups
  * Returns an initialised lgfs2_rgrps_t or NULL if unsuccessful with errno set
  */
-lgfs2_rgrps_t lgfs2_rgrps_init(unsigned bsize, uint64_t start, uint64_t devlen, uint32_t rglen, struct lgfs2_rgrp_align *al)
+lgfs2_rgrps_t lgfs2_rgrps_init(unsigned bsize, uint64_t devlen, uint64_t align, uint64_t offset)
 {
-	lgfs2_rgrps_t rgs = calloc(1, sizeof(*rgs));
+	lgfs2_rgrps_t rgs;
+
+	errno = EINVAL;
+	if (offset != 0 && (align % offset) != 0)
+		return NULL;
+
+	rgs = calloc(1, sizeof(*rgs));
 	if (rgs == NULL)
 		return NULL;
 
 	rgs->bsize = bsize;
-	rgs->maxrgsz = (GFS2_MAX_RGSIZE << 20) / bsize;
-	rgs->minrgsz = (GFS2_MIN_RGSIZE << 20) / bsize;
-	rgs->rgsize = rglen;
 	rgs->devlen = devlen;
-	rgs->align = al->base;
-	rgs->align_off = al->offset;
+	rgs->align = align;
+	rgs->align_off = offset;
 	memset(&rgs->root, 0, sizeof(rgs->root));
-	rgs->nextaddr = align_block(start, rgs->align);
 
 	return rgs;
 }
@@ -287,14 +378,6 @@ struct gfs2_rindex *lgfs2_rgrp_index(lgfs2_rgrp_t rg)
 }
 
 /**
- * Return non-zero if there is space left for more resource groups or zero if not
- */
-int lgfs2_rgrps_end(lgfs2_rgrps_t rgs)
-{
-	return (rgs->nextaddr == 0);
-}
-
-/**
  * Returns the total resource group size, in blocks, required to give blksreq data blocks
  */
 unsigned lgfs2_rgsize_for_data(uint64_t blksreq, unsigned bsize)
@@ -316,43 +399,38 @@ struct osi_node *lgfs2_rgrps_root(lgfs2_rgrps_t rgs)
 /**
  * Create a new resource group after the last resource group in a set.
  * rgs: The set of resource groups
- * rglen: The required length of the resource group. If its is 0 the default rgsize
- *        passed to lgfs2_rgrps_init() is used.
- * expand: Whether to expand the resource group when alignment would leave a gap.
- * Returns the new resource group on success or NULL on failure.
+ * addr: The address at which to place this resource group
+ * rglen: The required length of the resource group, in fs blocks.
+ * Returns the new resource group on success or NULL on failure with errno set.
+ * If errno is ENOSPC on a NULL return from this function, it could be
+ * interpreted as 'finished' unless you expected there to be enough space on
+ * the device for the resource group.
  */
-lgfs2_rgrp_t lgfs2_rgrp_append(lgfs2_rgrps_t rgs, uint32_t rglen, int expand)
+lgfs2_rgrp_t lgfs2_rgrp_append(lgfs2_rgrps_t rgs, uint64_t addr, uint32_t rglen, uint64_t *nextaddr)
 {
 	int err = 0;
-	lgfs2_rgrp_t rg = rgrp_insert(&rgs->root, rgs->nextaddr);
-	if (rg == NULL)
-		return NULL;
-
-	rgs->curr_offset += rgs->align_off;
-	if (rgs->curr_offset >= rgs->align)
-		rgs->curr_offset = 0;
-
-	if (rgs->rgsize > rglen)
-		rglen = rgs->rgsize;
-
-	rgs->nextaddr = align_block(rg->ri.ri_addr + rgs->rgsize, rgs->align) + rgs->curr_offset;
-	/* Use up gap left by alignment if possible */
-	if (expand && ((rgs->nextaddr - rg->ri.ri_addr) <= rgs->maxrgsz))
-		rglen = rgs->nextaddr - rg->ri.ri_addr;
-
-	if ((rgs->nextaddr + rgs->rgsize) > rgs->devlen) {
-		/* Squeeze the last 1 or 2 rgs into the remaining space */
-		if ((rgs->nextaddr < rgs->devlen) && ((rgs->devlen - rgs->nextaddr) >= rgs->minrgsz)) {
-			rgs->rgsize = rgs->devlen - rgs->nextaddr;
+	lgfs2_rgrp_t rg;
+
+	if (rglen == 0) {
+		if (rgs->plan[0].num > 0) {
+			rglen = rgs->plan[0].len;
+			rgs->plan[0].num--;
+		} else if (rgs->plan[1].num > 0) {
+			rglen = rgs->plan[1].len;
+			rgs->plan[1].num--;
 		} else {
-			if (rgs->devlen - rg->ri.ri_addr <= rgs->maxrgsz)
-				rglen = rgs->devlen - rg->ri.ri_addr;
-			else
-				rglen = rgs->maxrgsz;
-			/* This is the last rg */
-			rgs->nextaddr = 0;
+			errno = ENOSPC;
+			return NULL;
 		}
 	}
+	if (addr + rglen > rgs->devlen) {
+		errno = ENOSPC;
+		return NULL;
+	}
+
+	rg = rgrp_insert(&rgs->root, addr);
+	if (rg == NULL)
+		return NULL;
 
 	rg->ri.ri_length = rgblocks2bitblocks(rgs->bsize, rglen, &rg->ri.ri_data);
 	rg->ri.ri_data0 = rg->ri.ri_addr + rg->ri.ri_length;
@@ -361,12 +439,12 @@ lgfs2_rgrp_t lgfs2_rgrp_append(lgfs2_rgrps_t rgs, uint32_t rglen, int expand)
 	rg->rg.rg_header.mh_type = GFS2_METATYPE_RG;
 	rg->rg.rg_header.mh_format = GFS2_FORMAT_RG;
 	rg->rg.rg_free = rg->ri.ri_data;
-
 	err = gfs2_compute_bitstructs(rgs->bsize, rg);
 	if (err != 0)
 		return NULL;
-	rgs->blks_total += rg->ri.ri_data;
-	rgs->count++;
+
+	if (nextaddr)
+		*nextaddr = rg->ri.ri_addr + rglen;
 	return rg;
 }
 
@@ -374,10 +452,10 @@ lgfs2_rgrp_t lgfs2_rgrp_append(lgfs2_rgrps_t rgs, uint32_t rglen, int expand)
  * Write a resource group to a file descriptor.
  * Returns 0 on success or non-zero on failure with errno set
  */
-int lgfs2_rgrp_write(int fd, lgfs2_rgrp_t rg, unsigned bsize)
+int lgfs2_rgrp_write(const lgfs2_rgrps_t rgs, int fd, const lgfs2_rgrp_t rg)
 {
 	ssize_t ret = 0;
-	size_t len = rg->ri.ri_length * bsize;
+	size_t len = rg->ri.ri_length * rgs->bsize;
 	unsigned int i;
 	const struct gfs2_meta_header bmh = {
 		.mh_magic = GFS2_MAGIC,
@@ -390,9 +468,9 @@ int lgfs2_rgrp_write(int fd, lgfs2_rgrp_t rg, unsigned bsize)
 
 	gfs2_rgrp_out(&rg->rg, buff);
 	for (i = 1; i < rg->ri.ri_length; i++)
-		gfs2_meta_header_out(&bmh, buff + (i * bsize));
+		gfs2_meta_header_out(&bmh, buff + (i * rgs->bsize));
 
-	ret = pwrite(fd, buff, len, rg->ri.ri_addr * bsize);
+	ret = pwrite(fd, buff, len, rg->ri.ri_addr * rgs->bsize);
 	if (ret != len) {
 		free(buff);
 		return -1;
diff --git a/gfs2/mkfs/main_mkfs.c b/gfs2/mkfs/main_mkfs.c
index 40f4766..ae82c9f 100644
--- a/gfs2/mkfs/main_mkfs.c
+++ b/gfs2/mkfs/main_mkfs.c
@@ -160,7 +160,7 @@ static void opts_init(struct mkfs_opts *opts)
 	opts->bsize = GFS2_DEFAULT_BSIZE;
 	opts->jsize = GFS2_DEFAULT_JSIZE;
 	opts->qcsize = GFS2_DEFAULT_QCSIZE;
-	opts->rgsize = GFS2_DEFAULT_RGSIZE;
+	opts->rgsize = GFS2_MAX_RGSIZE;
 	opts->lockproto = "lock_dlm";
 	opts->locktable = "";
 	opts->confirm = 1;
@@ -583,9 +583,9 @@ static void warn_of_destruction(const char *path)
 
 static lgfs2_rgrps_t rgs_init(struct mkfs_opts *opts, struct gfs2_sbd *sdp)
 {
-	uint64_t rgsize = (opts->rgsize << 20) / sdp->bsize;
-	struct lgfs2_rgrp_align align = {.base = 0, .offset = 0};
 	lgfs2_rgrps_t rgs;
+	uint64_t al_base = 0;
+	uint64_t al_off = 0;
 
 	if (opts->align && opts->got_sunit) {
 		if ((opts->sunit % sdp->bsize) != 0) {
@@ -597,18 +597,18 @@ static lgfs2_rgrps_t rgs_init(struct mkfs_opts *opts, struct gfs2_sbd *sdp)
 			        opts->swidth, opts->sunit);
 			exit(1);
 		} else {
-			align.base = opts->swidth / sdp->bsize;
-			align.offset = opts->sunit / sdp->bsize;
+			al_base = opts->swidth / sdp->bsize;
+			al_off = opts->sunit / sdp->bsize;
 		}
 	} else if (opts->align) {
 		if ((opts->dev.minimum_io_size > opts->dev.physical_sector_size) &&
 		    (opts->dev.optimal_io_size > opts->dev.physical_sector_size)) {
-			align.base = opts->dev.optimal_io_size / sdp->bsize;
-			align.offset = opts->dev.minimum_io_size / sdp->bsize;
+			al_base = opts->dev.optimal_io_size / sdp->bsize;
+			al_off = opts->dev.minimum_io_size / sdp->bsize;
 		}
 	}
 
-	rgs = lgfs2_rgrps_init(sdp->bsize, sdp->sb_addr + 1, sdp->device.length, rgsize, &align);
+	rgs = lgfs2_rgrps_init(sdp->bsize, sdp->device.length, al_base, al_off);
 	if (rgs == NULL) {
 		perror(_("Could not initialise resource groups"));
 		exit(-1);
@@ -617,7 +617,7 @@ static lgfs2_rgrps_t rgs_init(struct mkfs_opts *opts, struct gfs2_sbd *sdp)
 	if (opts->debug) {
 		printf("  rgrp align = ");
 		if (opts->align)
-			printf("%lu+%lu blocks\n", align.base, align.offset);
+			printf("%lu+%lu blocks\n", al_base, al_off);
 		else
 			printf("(disabled)\n");
 	}
@@ -625,36 +625,71 @@ static lgfs2_rgrps_t rgs_init(struct mkfs_opts *opts, struct gfs2_sbd *sdp)
 	return rgs;
 }
 
-static uint64_t place_rgrps(struct gfs2_sbd *sdp, lgfs2_rgrps_t rgs, struct mkfs_opts *opts)
+static int place_rgrp(struct gfs2_sbd *sdp, lgfs2_rgrps_t rgs, uint64_t rgaddr, uint32_t len, uint64_t *next)
 {
 	int err = 0;
 	lgfs2_rgrp_t rg = NULL;
 	struct gfs2_rindex *ri = NULL;
 
-	while (!lgfs2_rgrps_end(rgs)) {
-		rg = lgfs2_rgrp_append(rgs, 0, !opts->got_rgsize);
-		if (rg == NULL) {
-			perror(_("Failed to create resource group"));
-			return 0;
-		}
-		err = lgfs2_rgrp_write(sdp->device_fd, rg, sdp->bsize);
-		if (err != 0) {
-			perror(_("Failed to write resource group"));
-			return 0;
-		}
-		ri = lgfs2_rgrp_index(rg);
-		if (opts->debug) {
-			gfs2_rindex_print(ri);
-			printf("\n");
-		}
-		sdp->blks_total += ri->ri_data;
-		sdp->rgrps++;
+	rg = lgfs2_rgrp_append(rgs, rgaddr, len, next);
+	if (rg == NULL) {
+		if (errno == ENOSPC)
+			return 1;
+		perror(_("Failed to create resource group"));
+		return -1;
+	}
+	err = lgfs2_rgrp_write(rgs, sdp->device_fd, rg);
+	if (err != 0) {
+		perror(_("Failed to write resource group"));
+		return -1;
+	}
+	ri = lgfs2_rgrp_index(rg);
+	if (sdp->debug) {
+		gfs2_rindex_print(ri);
+		printf("\n");
 	}
+	sdp->blks_total += ri->ri_data;
+	sdp->fssize = ri->ri_data0 + ri->ri_data;
+	sdp->rgrps++;
+	return 0;
+}
 
-	if (ri == NULL)
-		return 0;
-	else
-		return ri->ri_data0 + ri->ri_data;
+static int place_rgrps(struct gfs2_sbd *sdp, lgfs2_rgrps_t rgs, struct mkfs_opts *opts)
+{
+	uint64_t jfsize = lgfs2_space_for_data(sdp, sdp->bsize, opts->jsize << 20);
+	uint32_t jrgsize = lgfs2_rgsize_for_data(jfsize, sdp->bsize);
+	uint64_t rgaddr = lgfs2_rgrp_align_addr(rgs, sdp->sb_addr + 1);
+	uint32_t rgsize = lgfs2_rgrps_plan(rgs, sdp->device.length - rgaddr, ((opts->rgsize << 20) / sdp->bsize));
+	unsigned j;
+
+	if (rgsize >= jrgsize)
+		jrgsize = rgsize;
+
+	if (rgsize < ((GFS2_MIN_RGSIZE << 20) / sdp->bsize)) {
+		fprintf(stderr, _("Resource group size is too small\n"));
+		return -1;
+	} else if (rgsize < ((GFS2_DEFAULT_RGSIZE << 20) / sdp->bsize)) {
+		fprintf(stderr, _("Warning: small resource group size could impact performance\n"));
+	}
+
+	for (j = 0; j < opts->journals; j++) {
+		int result = place_rgrp(sdp, rgs, rgaddr, jrgsize, NULL);
+		if (result != 0)
+			return result;
+		rgaddr = rgaddr + jrgsize;
+	}
+
+	if (rgsize != jrgsize)
+		lgfs2_rgrps_plan(rgs, sdp->device.length - rgaddr, ((opts->rgsize << 20) / sdp->bsize));
+
+	while (1) {
+		int result = place_rgrp(sdp, rgs, rgaddr, 0, &rgaddr);
+		if (result < 0)
+			return result;
+		if (result > 0)
+			break; /* Done */
+	}
+	return 0;
 }
 
 static void sbd_init(struct gfs2_sbd *sdp, struct mkfs_opts *opts, unsigned bsize)
@@ -668,6 +703,7 @@ static void sbd_init(struct gfs2_sbd *sdp, struct mkfs_opts *opts, unsigned bsiz
 	sdp->md.journals = opts->journals;
 	sdp->device_fd = opts->dev.fd;
 	sdp->bsize = bsize;
+	sdp->debug = opts->debug;
 
 	if (compute_constants(sdp)) {
 		perror(_("Failed to compute file system constants"));
@@ -812,8 +848,8 @@ void main_mkfs(int argc, char *argv[])
 	if (!S_ISREG(opts.dev.stat.st_mode) && opts.discard)
 		discard_blocks(opts.dev.fd, opts.dev.size, opts.debug);
 
-	sbd.fssize = place_rgrps(&sbd, rgs, &opts);
-	if (sbd.fssize == 0) {
+	error = place_rgrps(&sbd, rgs, &opts);
+	if (error) {
 		fprintf(stderr, _("Failed to build resource groups\n"));
 		exit(1);
 	}
-- 
1.8.5.3

[Cluster-devel] [PATCH 5/5] libgfs2: Don't call gfs2_blk2rgrpd in gfs2_set_bitmap

[Andrew Price]

gfs2_set_bitmap was calling gfs2_blk2rgrpd even when callers already
have the resource group to hand. This changes that function to accept
the resource group instead and calls to gfs2_blk2rgrpd have been moved
outside.

Signed-off-by: Andrew Price <anprice@redhat.com>
---
 gfs2/edit/hexedit.c    |  2 +-
 gfs2/fsck/metawalk.c   |  2 +-
 gfs2/fsck/pass5.c      |  6 ++++--
 gfs2/libgfs2/fs_bits.c |  5 +----
 gfs2/libgfs2/fs_ops.c  | 12 +++++-------
 gfs2/libgfs2/libgfs2.h |  2 +-
 6 files changed, 13 insertions(+), 16 deletions(-)

diff --git a/gfs2/edit/hexedit.c b/gfs2/edit/hexedit.c
index bc3ca35..53a816d 100644
--- a/gfs2/edit/hexedit.c
+++ b/gfs2/edit/hexedit.c
@@ -2045,7 +2045,7 @@ static void find_change_block_alloc(int *newval)
 		if (rgd) {
 			gfs2_rgrp_read(&sbd, rgd);
 			if (newval) {
-				if (gfs2_set_bitmap(&sbd, ablock, *newval))
+				if (gfs2_set_bitmap(rgd, ablock, *newval))
 					printf("-1 (block invalid or part of an rgrp).\n");
 				else
 					printf("%d\n", *newval);
diff --git a/gfs2/fsck/metawalk.c b/gfs2/fsck/metawalk.c
index 15cba56..594fbfa 100644
--- a/gfs2/fsck/metawalk.c
+++ b/gfs2/fsck/metawalk.c
@@ -73,7 +73,7 @@ int check_n_fix_bitmap(struct gfs2_sbd *sdp, uint64_t blk, int error_on_dinode,
 			   subtract to the free space.  If the type changed
 			   from dinode to data or data to dinode, no change in
 			   free space. */
-			gfs2_set_bitmap(sdp, blk, new_bitmap_state);
+			gfs2_set_bitmap(rgd, blk, new_bitmap_state);
 			if (new_bitmap_state == GFS2_BLKST_FREE) {
 				/* If we're freeing a dinode, get rid of
 				   the hash table entries for it. */
diff --git a/gfs2/fsck/pass5.c b/gfs2/fsck/pass5.c
index 92861a1..49ab682 100644
--- a/gfs2/fsck/pass5.c
+++ b/gfs2/fsck/pass5.c
@@ -148,7 +148,8 @@ static int check_block_status(struct gfs2_sbd *sdp, char *buffer,
 				 (unsigned long long)block);
 			if (query(_("Do you want to reclaim the block? "
 				   "(y/n) "))) {
-				if (gfs2_set_bitmap(sdp, block, block_status))
+				lgfs2_rgrp_t rg = gfs2_blk2rgrpd(sdp, block);
+				if (gfs2_set_bitmap(rg, block, block_status))
 					log_err(_("Unlinked block %llu "
 						  "(0x%llx) bitmap not fixed."
 						  "\n"),
@@ -182,7 +183,8 @@ static int check_block_status(struct gfs2_sbd *sdp, char *buffer,
 			if (query(_("Fix bitmap for block %llu (0x%llx) ? (y/n) "),
 				 (unsigned long long)block,
 				 (unsigned long long)block)) {
-				if (gfs2_set_bitmap(sdp, block, block_status))
+				lgfs2_rgrp_t rg = gfs2_blk2rgrpd(sdp, block);
+				if (gfs2_set_bitmap(rg, block, block_status))
 					log_err( _("Repair failed.\n"));
 				else
 					log_err( _("Fixed.\n"));
diff --git a/gfs2/libgfs2/fs_bits.c b/gfs2/libgfs2/fs_bits.c
index e4b5505..7194949 100644
--- a/gfs2/libgfs2/fs_bits.c
+++ b/gfs2/libgfs2/fs_bits.c
@@ -124,12 +124,11 @@ int gfs2_check_range(struct gfs2_sbd *sdp, uint64_t blkno)
  *
  * Returns: 0 on success, -1 on error
  */
-int gfs2_set_bitmap(struct gfs2_sbd *sdp, uint64_t blkno, int state)
+int gfs2_set_bitmap(lgfs2_rgrp_t rgd, uint64_t blkno, int state)
 {
 	int           buf;
 	uint32_t        rgrp_block;
 	struct gfs2_bitmap *bits = NULL;
-	struct rgrp_tree *rgd;
 	unsigned char *byte, cur_state;
 	unsigned int bit;
 
@@ -137,8 +136,6 @@ int gfs2_set_bitmap(struct gfs2_sbd *sdp, uint64_t blkno, int state)
 	if ((state < GFS2_BLKST_FREE) || (state > GFS2_BLKST_DINODE))
 		return -1;
 
-	rgd = gfs2_blk2rgrpd(sdp, blkno);
-
 	if(!rgd || blkno < rgd->ri.ri_data0)
 		return -1;
 
diff --git a/gfs2/libgfs2/fs_ops.c b/gfs2/libgfs2/fs_ops.c
index b95f2ed..fdd4438 100644
--- a/gfs2/libgfs2/fs_ops.c
+++ b/gfs2/libgfs2/fs_ops.c
@@ -141,7 +141,7 @@ static int blk_alloc_in_rg(struct gfs2_sbd *sdp, unsigned state, struct rgrp_tre
 	if (blkno == 0)
 		return -1;
 
-	if (gfs2_set_bitmap(sdp, blkno, state))
+	if (gfs2_set_bitmap(rgd, blkno, state))
 		return -1;
 
 	if (state == GFS2_BLKST_DINODE)
@@ -1763,7 +1763,7 @@ void gfs2_free_block(struct gfs2_sbd *sdp, uint64_t block)
 	/* Adjust the free space count for the freed block */
 	rgd = gfs2_blk2rgrpd(sdp, block); /* find the rg for indir block */
 	if (rgd) {
-		gfs2_set_bitmap(sdp, block, GFS2_BLKST_FREE);
+		gfs2_set_bitmap(rgd, block, GFS2_BLKST_FREE);
 		rgd->rg.rg_free++; /* adjust the free count */
 		if (sdp->gfs1)
 			gfs_rgrp_out((struct gfs_rgrp *)&rgd->rg, rgd->bh[0]);
@@ -1826,16 +1826,14 @@ int gfs2_freedi(struct gfs2_sbd *sdp, uint64_t diblock)
 			}
 		}
 	}
-	/* Set the bitmap type for inode to free space: */
-	gfs2_set_bitmap(sdp, ip->i_di.di_num.no_addr, GFS2_BLKST_FREE);
+	rgd = gfs2_blk2rgrpd(sdp, diblock);
+	gfs2_set_bitmap(rgd, diblock, GFS2_BLKST_FREE);
 	inode_put(&ip);
 	/* inode_put deallocated the extra block used by the disk inode, */
 	/* so adjust it in the superblock struct */
 	sdp->blks_alloced--;
-	/* Now we have to adjust the rg freespace count and inode count: */
-	rgd = gfs2_blk2rgrpd(sdp, diblock);
 	rgd->rg.rg_free++;
-	rgd->rg.rg_dinodes--; /* one less inode in use */
+	rgd->rg.rg_dinodes--;
 	if (sdp->gfs1)
 		gfs_rgrp_out((struct gfs_rgrp *)&rgd->rg, rgd->bh[0]);
 	else
diff --git a/gfs2/libgfs2/libgfs2.h b/gfs2/libgfs2/libgfs2.h
index 24947c2..b2f6bc3 100644
--- a/gfs2/libgfs2/libgfs2.h
+++ b/gfs2/libgfs2/libgfs2.h
@@ -418,7 +418,7 @@ extern int gfs2_check_range(struct gfs2_sbd *sdp, uint64_t blkno);
 
 /* functions with blk #'s that are file system relative */
 extern int lgfs2_get_bitmap(struct gfs2_sbd *sdp, uint64_t blkno, struct rgrp_tree *rgd);
-extern int gfs2_set_bitmap(struct gfs2_sbd *sdp, uint64_t blkno, int state);
+extern int gfs2_set_bitmap(lgfs2_rgrp_t rg, uint64_t blkno, int state);
 
 /* fs_geometry.c */
 extern uint32_t rgblocks2bitblocks(const unsigned int bsize, const uint32_t rgblocks,
-- 
1.8.5.3