libgfs2: Remove sdp argument from compute_heightsize

PATCH New Andrew Price 3 participants 13 messages Jan 27, 2014

lore

Share & tools

lore

Andrew Price

author

2014-01-27 14:17 UTC #0 lore raw

PATCH 1/8 libgfs2: Remove sdp argument from compute_heightsize

compute_heightsize only uses the bsize member of sdp so just accept a bsize argument instead.

Signed-off-by Andrew Price <[email protected]>

---
 gfs2/convert/gfs2_convert.c | 8 ++++----
 gfs2/libgfs2/libgfs2.h      | 2 +-
 gfs2/libgfs2/misc.c         | 8 ++++----
 3 files changed, 9 insertions(+), 9 deletions(-)

gfs2/convert/gfs2_convert.c +4 -4

index 719ba9c..e9dd0da 100644
@@ -1611,13 +1611,13 @@ static int init(struct gfs2_sbd *sbp)
 		sizeof(uint64_t);
 	sbp->sd_jbsize = sbp->bsize - sizeof(struct gfs2_meta_header);
 	brelse(bh);
-	if (compute_heightsize(sbp, sbp->sd_heightsize, &sbp->sd_max_height,
+	if (compute_heightsize(sbp->bsize, sbp->sd_heightsize, &sbp->sd_max_height,
 				sbp->bsize, sbp->sd_diptrs, sbp->sd_inptrs)) {
 		log_crit(_("Error: Bad constants (1)\n"));
 		exit(-1);
 	}
 
-	if (compute_heightsize(sbp, sbp->sd_jheightsize, &sbp->sd_max_jheight,
+	if (compute_heightsize(sbp->bsize, sbp->sd_jheightsize, &sbp->sd_max_jheight,
 				sbp->sd_jbsize, sbp->sd_diptrs, sbp->sd_inptrs)) {
 		log_crit(_("Error: Bad constants (1)\n"));
 		exit(-1);

@@ -1628,13 +1628,13 @@ static int init(struct gfs2_sbd *sbp)
 	gfs2_inptrs = (sbp->bsize - sizeof(struct gfs2_meta_header)) /
                 sizeof(uint64_t); /* How many ptrs can we fit on a block? */
 	memset(gfs2_heightsize, 0, sizeof(gfs2_heightsize));
-	if (compute_heightsize(sbp, gfs2_heightsize, &gfs2_max_height,
+	if (compute_heightsize(sbp->bsize, gfs2_heightsize, &gfs2_max_height,
 				sbp->bsize, sbp->sd_diptrs, gfs2_inptrs)) {
 		log_crit(_("Error: Bad constants (1)\n"));
 		exit(-1);
 	}
 	memset(gfs2_jheightsize, 0, sizeof(gfs2_jheightsize));
-	if (compute_heightsize(sbp, gfs2_jheightsize, &gfs2_max_jheight,
+	if (compute_heightsize(sbp->bsize, gfs2_jheightsize, &gfs2_max_jheight,
 				sbp->sd_jbsize, sbp->sd_diptrs, gfs2_inptrs)) {
 		log_crit(_("Error: Bad constants (1)\n"));
 		exit(-1);

gfs2/libgfs2/libgfs2.h +1 -1

index e785017..3119f39 100644
@@ -712,7 +712,7 @@ extern void decrease_verbosity(void);
 
 extern int metafs_interrupted;
 
-extern int compute_heightsize(struct gfs2_sbd *sdp, uint64_t *heightsize,
+extern int compute_heightsize(unsigned bsize, uint64_t *heightsize,
 		uint32_t *maxheight, uint32_t bsize1, int diptrs, int inptrs);
 extern int compute_constants(struct gfs2_sbd *sdp);
 extern int lgfs2_open_mnt(const char *path, int dirflags, int *dirfd, int devflags, int *devfd, struct mntent **mnt);

gfs2/libgfs2/misc.c +4 -4

index c4ed722..8ee8324 100644
@@ -28,10 +28,10 @@
 
 int metafs_interrupted = 0;
 
-int compute_heightsize(struct gfs2_sbd *sdp, uint64_t *heightsize,
+int compute_heightsize(unsigned bsize, uint64_t *heightsize,
 	uint32_t *maxheight, uint32_t bsize1, int diptrs, int inptrs)
 {
-	heightsize[0] = sdp->bsize - sizeof(struct gfs2_dinode);
+	heightsize[0] = bsize - sizeof(struct gfs2_dinode);
 	heightsize[1] = bsize1 * diptrs;
 	for (*maxheight = 2;; (*maxheight)++) {
 		uint64_t space, d;

@@ -91,11 +91,11 @@ int compute_constants(struct gfs2_sbd *sdp)
 
 	sdp->sd_max_dirres = hash_blocks + ind_blocks + leaf_blocks;
 
-	if (compute_heightsize(sdp, sdp->sd_heightsize, &sdp->sd_max_height,
+	if (compute_heightsize(sdp->bsize, sdp->sd_heightsize, &sdp->sd_max_height,
 				sdp->bsize, sdp->sd_diptrs, sdp->sd_inptrs)) {
 		return -1;
 	}
-	if (compute_heightsize(sdp, sdp->sd_jheightsize, &sdp->sd_max_jheight,
+	if (compute_heightsize(sdp->bsize, sdp->sd_jheightsize, &sdp->sd_max_jheight,
 				sdp->sd_jbsize, sdp->sd_diptrs, sdp->sd_inptrs)) {
 		return -1;
 	}

Signature · 1 line

1.8.3.1

Andrew Price

author

2014-01-27 14:17 UTC #1 lore raw

PATCH 2/8 libgfs2: Remove sdp and j arguments from write_journal

write_journal accepts an sdp argument which it only used to look up the current journal (using j as the index) and the bsize. Replace these arguments with the journal inode itself and the block size.

Signed-off-by Andrew Price <[email protected]>

---
 gfs2/convert/gfs2_convert.c |  2 +-
 gfs2/fsck/fs_recovery.c     |  2 +-
 gfs2/libgfs2/libgfs2.h      |  3 +--
 gfs2/libgfs2/structures.c   | 21 +++++++--------------
 4 files changed, 10 insertions(+), 18 deletions(-)

gfs2/convert/gfs2_convert.c +1 -1

index e9dd0da..8ab3981 100644
@@ -2006,7 +2006,7 @@ static int conv_build_jindex(struct gfs2_sbd *sdp)
 		sprintf(name, "journal%u", j);
 		sdp->md.journal[j] = createi(sdp->md.jiinode, name, S_IFREG |
 					     0600, GFS2_DIF_SYSTEM);
-		write_journal(sdp, j,
+		write_journal(sdp->md.journal[j], sdp->bsize,
 			      sdp->jsize << 20 >> sdp->sd_sb.sb_bsize_shift);
 		inode_put(&sdp->md.journal[j]);
 		printf(_("done.\n"));

gfs2/fsck/fs_recovery.c +1 -1

index 41d575a..54a8c5f 100644
@@ -552,7 +552,7 @@ out:
 	log_info( _("jid=%u: Failed\n"), j);
 reinit:
 	if (query( _("Do you want to clear the journal instead? (y/n)")))
-		error = write_journal(sdp, j,
+		error = write_journal(sdp->md.journal[j], sdp->bsize,
 				      sdp->md.journal[j]->i_di.di_size /
 				      sdp->sd_sb.sb_bsize);
 	else

gfs2/libgfs2/libgfs2.h +1 -2

index 3119f39..42f9d44 100644
@@ -496,8 +496,7 @@ extern int gfs2_dirent_next(struct gfs2_inode *dip, struct gfs2_buffer_head *bh,
 extern void build_height(struct gfs2_inode *ip, int height);
 extern void unstuff_dinode(struct gfs2_inode *ip);
 extern unsigned int calc_tree_height(struct gfs2_inode *ip, uint64_t size);
-extern int write_journal(struct gfs2_sbd *sdp, unsigned int j,
-			 unsigned int blocks);
+extern int write_journal(struct gfs2_inode *jnl, unsigned bsize, unsigned blocks);
 
 /* gfs1.c - GFS1 backward compatibility structures and functions */

gfs2/libgfs2/structures.c +7 ~~-14~~

index e888f1e..030974b 100644
@@ -80,7 +80,7 @@ void build_sb(struct gfs2_sbd *sdp, const unsigned char *uuid)
 	}
 }
 
-int write_journal(struct gfs2_sbd *sdp, unsigned int j, unsigned int blocks)
+int write_journal(struct gfs2_inode *jnl, unsigned bsize, unsigned int blocks)
 {
 	struct gfs2_log_header lh;
 	unsigned int x;

@@ -90,9 +90,8 @@ int write_journal(struct gfs2_sbd *sdp, unsigned int j, unsigned int blocks)
 
 	/* Build the height up so our journal blocks will be contiguous and */
 	/* not broken up by indirect block pages.                           */
-	height = calc_tree_height(sdp->md.journal[j],
-				  (blocks + 1) * sdp->bsize);
-	build_height(sdp->md.journal[j], height);
+	height = calc_tree_height(jnl, (blocks + 1) * bsize);
+	build_height(jnl, height);
 
 	memset(&lh, 0, sizeof(struct gfs2_log_header));
 	lh.lh_header.mh_magic = GFS2_MAGIC;

@@ -101,20 +100,18 @@ int write_journal(struct gfs2_sbd *sdp, unsigned int j, unsigned int blocks)
 	lh.lh_flags = GFS2_LOG_HEAD_UNMOUNT;
 
 	for (x = 0; x < blocks; x++) {
-		struct gfs2_buffer_head *bh = get_file_buf(sdp->md.journal[j],
-							   x, TRUE);
+		struct gfs2_buffer_head *bh = get_file_buf(jnl, x, TRUE);
 		if (!bh)
 			return -1;
 		bmodified(bh);
 		brelse(bh);
 	}
 	for (x = 0; x < blocks; x++) {
-		struct gfs2_buffer_head *bh = get_file_buf(sdp->md.journal[j],
-							   x, FALSE);
+		struct gfs2_buffer_head *bh = get_file_buf(jnl, x, FALSE);
 		if (!bh)
 			return -1;
 
-		memset(bh->b_data, 0, sdp->bsize);
+		memset(bh->b_data, 0, bsize);
 		lh.lh_sequence = seq;
 		lh.lh_blkno = x;
 		gfs2_log_header_out(&lh, bh);

@@ -128,10 +125,6 @@ int write_journal(struct gfs2_sbd *sdp, unsigned int j, unsigned int blocks)
 			seq = 0;
 	}
 
-	if (sdp->debug) {
-		printf("\nJournal %u:\n", j);
-		gfs2_dinode_print(&sdp->md.journal[j]->i_di);
-	}
 	return 0;
 }

@@ -146,7 +139,7 @@ int build_journal(struct gfs2_sbd *sdp, int j, struct gfs2_inode *jindex)
 	if (sdp->md.journal[j] == NULL) {
 		return errno;
 	}
-	ret = write_journal(sdp, j,
+	ret = write_journal(sdp->md.journal[j], sdp->bsize,
 			    sdp->jsize << 20 >> sdp->sd_sb.sb_bsize_shift);
 	return ret;
 }

Signature · 1 line

1.8.3.1

Andrew Price

author

2014-01-27 14:17 UTC #2 lore raw

PATCH 3/8 libgfs2: Rework find_metapath

In reply to Andrew Price

Rework find_metapath to receive a pointer to a metapath structure instead of allocating memory and exiting on allocation failure. Also simplify some other aspects of the function and update its callers.

Signed-off-by Andrew Price <[email protected]>

---
 gfs2/libgfs2/fs_ops.c  | 39 ++++++++++++++-------------------------
 gfs2/libgfs2/gfs1.c    | 17 +++++++----------
 gfs2/libgfs2/libgfs2.h |  2 +-
 3 files changed, 22 insertions(+), 36 deletions(-)

gfs2/libgfs2/fs_ops.c +14 ~~-25~~

index 4a84687..03b1c41 100644
@@ -393,24 +393,16 @@ void build_height(struct gfs2_inode *ip, int height)
 	}
 }
 
-struct metapath *find_metapath(struct gfs2_inode *ip, uint64_t block)
+void find_metapath(struct gfs2_inode *ip, uint64_t block, struct metapath *mp)
 {
-	struct gfs2_sbd *sdp = ip->i_sbd;
-	struct metapath *mp;
-	uint64_t b = block;
-	unsigned int i;
+	const uint32_t inptrs = ip->i_sbd->sd_inptrs;
+	unsigned int i = ip->i_di.di_height;
 
-	mp = calloc(1, sizeof(struct metapath));
-	if (mp == NULL) {
-		fprintf(stderr, "Out of memory in %s\n", __FUNCTION__);
-		exit(-1);
-	}
-	for (i = ip->i_di.di_height; i--;) {
-		mp->mp_list[i] = b % sdp->sd_inptrs;
-		b /= sdp->sd_inptrs;
+	memset(mp, 0, sizeof(struct metapath));
+	while (i--) {
+		mp->mp_list[i] = block % inptrs;
+		block /= inptrs;
 	}
-
-	return mp;
 }
 
 void lookup_block(struct gfs2_inode *ip, struct gfs2_buffer_head *bh,

@@ -448,7 +440,7 @@ void block_map(struct gfs2_inode *ip, uint64_t lblock, int *new,
 {
 	struct gfs2_sbd *sdp = ip->i_sbd;
 	struct gfs2_buffer_head *bh;
-	struct metapath *mp;
+	struct metapath mp;
 	int create = *new;
 	unsigned int bsize;
 	unsigned int height;

@@ -479,17 +471,17 @@ void block_map(struct gfs2_inode *ip, uint64_t lblock, int *new,
 		build_height(ip, height);
 	}
 
-	mp = find_metapath(ip, lblock);
+	find_metapath(ip, lblock, &mp);
 	end_of_metadata = ip->i_di.di_height - 1;
 
 	bh = ip->i_bh;
 
 	for (x = 0; x < end_of_metadata; x++) {
-		lookup_block(ip, bh, x, mp, create, new, dblock);
+		lookup_block(ip, bh, x, &mp, create, new, dblock);
 		if (bh != ip->i_bh)
 			brelse(bh);
 		if (!*dblock)
-			goto out;
+			return;
 
 		if (*new) {
 			struct gfs2_meta_header mh;

@@ -507,7 +499,7 @@ void block_map(struct gfs2_inode *ip, uint64_t lblock, int *new,
 	}
 
 	if (!prealloc)
-		lookup_block(ip, bh, end_of_metadata, mp, create, new, dblock);
+		lookup_block(ip, bh, end_of_metadata, &mp, create, new, dblock);
 
 	if (extlen && *dblock) {
 		*extlen = 1;

@@ -519,8 +511,8 @@ void block_map(struct gfs2_inode *ip, uint64_t lblock, int *new,
 
 			nptrs = (end_of_metadata) ? sdp->sd_inptrs : sdp->sd_diptrs;
 
-			while (++mp->mp_list[end_of_metadata] < nptrs) {
-				lookup_block(ip, bh, end_of_metadata, mp, FALSE, &tmp_new,
+			while (++mp.mp_list[end_of_metadata] < nptrs) {
+				lookup_block(ip, bh, end_of_metadata, &mp, FALSE, &tmp_new,
 							 &tmp_dblock);
 
 				if (*dblock + *extlen != tmp_dblock)

@@ -533,9 +525,6 @@ void block_map(struct gfs2_inode *ip, uint64_t lblock, int *new,
 
 	if (bh != ip->i_bh)
 		brelse(bh);
-
- out:
-	free(mp);
 }
 
 static void

gfs2/libgfs2/gfs1.c +7 ~~-10~~

index 89fb898..619542c 100644
@@ -76,7 +76,7 @@ void gfs1_block_map(struct gfs2_inode *ip, uint64_t lblock, int *new,
 {
 	struct gfs2_sbd *sdp = ip->i_sbd;
 	struct gfs2_buffer_head *bh;
-	struct metapath *mp;
+	struct metapath mp;
 	int create = *new;
 	unsigned int bsize;
 	unsigned int height;

@@ -107,17 +107,17 @@ void gfs1_block_map(struct gfs2_inode *ip, uint64_t lblock, int *new,
 		build_height(ip, height);
 	}
 
-	mp = find_metapath(ip, lblock);
+	find_metapath(ip, lblock, &mp);
 	end_of_metadata = ip->i_di.di_height - 1;
 
 	bh = ip->i_bh;
 
 	for (x = 0; x < end_of_metadata; x++) {
-		gfs1_lookup_block(ip, bh, x, mp, create, new, dblock);
+		gfs1_lookup_block(ip, bh, x, &mp, create, new, dblock);
 		if (bh != ip->i_bh)
 			brelse(bh);
 		if (!*dblock)
-			goto out;
+			return;
 
 		if (*new) {
 			struct gfs2_meta_header mh;

@@ -136,7 +136,7 @@ void gfs1_block_map(struct gfs2_inode *ip, uint64_t lblock, int *new,
 	}
 
 	if (!prealloc)
-		gfs1_lookup_block(ip, bh, end_of_metadata, mp, create, new,
+		gfs1_lookup_block(ip, bh, end_of_metadata, &mp, create, new,
 				  dblock);
 
 	if (extlen && *dblock) {

@@ -149,8 +149,8 @@ void gfs1_block_map(struct gfs2_inode *ip, uint64_t lblock, int *new,
 
 			nptrs = (end_of_metadata) ? sdp->sd_inptrs : sdp->sd_diptrs;
 
-			while (++mp->mp_list[end_of_metadata] < nptrs) {
-				gfs1_lookup_block(ip, bh, end_of_metadata, mp,
+			while (++mp.mp_list[end_of_metadata] < nptrs) {
+				gfs1_lookup_block(ip, bh, end_of_metadata, &mp,
 						  FALSE, &tmp_new,
 						  &tmp_dblock);

@@ -164,9 +164,6 @@ void gfs1_block_map(struct gfs2_inode *ip, uint64_t lblock, int *new,
 
 	if (bh != ip->i_bh)
 		brelse(bh);
-
- out:
-	free(mp);
 }
 
 int gfs1_writei(struct gfs2_inode *ip, char *buf, uint64_t offset,

gfs2/libgfs2/libgfs2.h +1 -1

index 42f9d44..07cb221 100644
@@ -437,7 +437,7 @@ extern void build_rgrps(struct gfs2_sbd *sdp, int write);
 #define IS_LEAF     (1)
 #define IS_DINODE   (2)
 
-extern struct metapath *find_metapath(struct gfs2_inode *ip, uint64_t block);
+extern void find_metapath(struct gfs2_inode *ip, uint64_t block, struct metapath *mp);
 extern void lookup_block(struct gfs2_inode *ip, struct gfs2_buffer_head *bh,
 			 unsigned int height, struct metapath *mp,
 			 int create, int *new, uint64_t *block);

Signature · 1 line

1.8.3.1

Andrew Price

author

2014-01-27 14:17 UTC #3 lore raw

PATCH 4/8 libgfs2: Improve and simplify blk_alloc_in_rg

In reply to Andrew Price

This function included naive implementations of gfs2_setbit and gfs2_bitfit so these have been replaced with calls to those functions. The 'type' parameter has been replaced with 'state' and the type defines removed in favour of the GFS2_BLKST_* values. As these were the same for both meta and data block allocations, data_alloc() has been removed and its callers updated to use meta_alloc().

Signed-off-by Andrew Price <[email protected]>

---
 gfs2/libgfs2/fs_ops.c | 117 +++++++++++++++-----------------------------------
 gfs2/libgfs2/gfs1.c   |   7 +--
 2 files changed, 35 insertions(+), 89 deletions(-)

gfs2/libgfs2/fs_ops.c +34 ~~-83~~

index 03b1c41..1d6fb4a 100644
@@ -14,10 +14,6 @@
 #include <linux/types.h>
 #include "libgfs2.h"
 
-#define DATA (1)
-#define META (2)
-#define DINODE (3)
-
 static __inline__ uint64_t *metapointer(struct gfs2_buffer_head *bh,
 					unsigned int height,
 					struct metapath *mp)

@@ -116,88 +112,57 @@ void inode_put(struct gfs2_inode **ip_in)
 	*ip_in = NULL; /* make sure the memory isn't accessed again */
 }
 
-static int blk_alloc_in_rg(struct gfs2_sbd *sdp, unsigned int type, struct rgrp_tree *rl, uint64_t *blkno)
+static int blk_alloc_in_rg(struct gfs2_sbd *sdp, unsigned state, struct rgrp_tree *rgd, uint64_t *blkno)
 {
-	struct gfs2_rindex *ri;
-	struct gfs2_rgrp *rg;
-	unsigned int block, bn = 0, x = 0, y = 0;
-	unsigned int state;
 	unsigned int release = 0;
-	struct gfs2_buffer_head *bh;
+	unsigned int bm;
+	int error = -1;
 
-	if (rl == NULL || rl->rg.rg_free == 0) {
+	if (rgd == NULL || rgd->rg.rg_free == 0) {
 		errno = ENOSPC;
 		return -1;
 	}
 
-	if (rl->bh[0] == NULL) {
-		if (gfs2_rgrp_read(sdp, rl) != 0)
+	if (rgd->bh[0] == NULL) {
+		if (gfs2_rgrp_read(sdp, rgd))
 			return -1;
 		release = 1;
 	}
 
-	ri = &rl->ri;
-	rg = &rl->rg;
-	for (block = 0; block < ri->ri_length; block++) {
-		bh = rl->bh[block];
-		x = (block) ? sizeof(struct gfs2_meta_header) : sizeof(struct gfs2_rgrp);
-
-		for (; x < sdp->bsize; x++)
-			for (y = 0; y < GFS2_NBBY; y++) {
-				state = (bh->b_data[x] >> (GFS2_BIT_SIZE * y)) & 0x03;
-				if (state == GFS2_BLKST_FREE)
-					goto found;
-				bn++;
-			}
-	}
+	*blkno = 0;
+	for (bm = 0; bm < rgd->ri.ri_length; bm++) {
+		unsigned long blk = 0;
+		struct gfs2_bitmap *bits = &rgd->bits[bm];
 
-	fprintf(stderr, "allocation is broken (1): %"PRIu64" %u\n",
-	    (uint64_t)rl->ri.ri_addr, rl->rg.rg_free);
-	goto out_err;
-
-found:
-	if (bn >= ri->ri_bitbytes * GFS2_NBBY) {
-		fprintf(stderr, "allocation is broken (2): bn: %u %u rgrp: %"PRIu64
-		    " (0x%" PRIx64 ") Free:%u\n",
-		    bn, ri->ri_bitbytes * GFS2_NBBY, (uint64_t)rl->ri.ri_addr,
-		    (uint64_t)rl->ri.ri_addr, rl->rg.rg_free);
-		goto out_err;
+		blk = gfs2_bitfit((unsigned char *)rgd->bh[bm]->b_data + bits->bi_offset,
+		                  bits->bi_len, blk, GFS2_BLKST_FREE);
+		if (blk != BFITNOENT) {
+			*blkno = blk + (bits->bi_start * GFS2_NBBY) + rgd->ri.ri_data0;
+			break;
+		}
 	}
 
-	switch (type) {
-	case DATA:
-	case META:
-		state = GFS2_BLKST_USED;
-		break;
-	case DINODE:
-		state = GFS2_BLKST_DINODE;
-		rg->rg_dinodes++;
-		break;
-	default:
-		fprintf(stderr, "bad state\n");
-		goto out_err;
-	}
+	if (*blkno == 0)
+		goto out;
 
-	bh->b_data[x] &= ~(0x03 << (GFS2_BIT_SIZE * y));
-	bh->b_data[x] |= state << (GFS2_BIT_SIZE * y);
-	rg->rg_free--;
+	error = gfs2_set_bitmap(sdp, *blkno, state);
+	if (error)
+		goto out;
 
-	bmodified(bh);
+	if (state == GFS2_BLKST_DINODE)
+		rgd->rg.rg_dinodes++;
+
+	rgd->rg.rg_free--;
 	if (sdp->gfs1)
-		gfs_rgrp_out((struct gfs_rgrp *)rg, rl->bh[0]);
+		gfs_rgrp_out((struct gfs_rgrp *)&rgd->rg, rgd->bh[0]);
 	else
-		gfs2_rgrp_out_bh(rg, rl->bh[0]);
+		gfs2_rgrp_out_bh(&rgd->rg, rgd->bh[0]);
 
 	sdp->blks_alloced++;
-	*blkno = ri->ri_data0 + bn;
-	if (release)
-		gfs2_rgrp_relse(rl);
-	return 0;
-out_err:
+out:
 	if (release)
-		gfs2_rgrp_relse(rl);
-	return -1;
-
+		gfs2_rgrp_relse(rgd);
+	return error;
 }
 
 /**

@@ -219,19 +184,10 @@ static uint64_t blk_alloc_i(struct gfs2_sbd *sdp, unsigned int type)
 	return blkno;
 }
 
-uint64_t data_alloc(struct gfs2_inode *ip)
-{
-	uint64_t x;
-	x = blk_alloc_i(ip->i_sbd, DATA);
-	ip->i_di.di_goal_data = x;
-	bmodified(ip->i_bh);
-	return x;
-}
-
 uint64_t meta_alloc(struct gfs2_inode *ip)
 {
 	uint64_t x;
-	x = blk_alloc_i(ip->i_sbd, META);
+	x = blk_alloc_i(ip->i_sbd, GFS2_BLKST_USED);
 	ip->i_di.di_goal_meta = x;
 	bmodified(ip->i_bh);
 	return x;

@@ -255,7 +211,7 @@ int lgfs2_dinode_alloc(struct gfs2_sbd *sdp, const uint64_t blksreq, uint64_t *b
 	if (rgt == NULL)
 		return -1;
 
-	ret = blk_alloc_in_rg(sdp, DINODE, rgt, blkno);
+	ret = blk_alloc_in_rg(sdp, GFS2_BLKST_DINODE, rgt, blkno);
 	if (ret == 0)
 		sdp->dinodes_alloced++;

@@ -305,7 +261,7 @@ void unstuff_dinode(struct gfs2_inode *ip)
 
 			brelse(bh);
 		} else {
-			block = data_alloc(ip);
+			block = meta_alloc(ip);
 			bh = bget(sdp, block);
 
 			buffer_copy_tail(sdp, bh, 0,

@@ -421,12 +377,7 @@ void lookup_block(struct gfs2_inode *ip, struct gfs2_buffer_head *bh,
 	if (!create)
 		return;
 
-	if (height == ip->i_di.di_height - 1&&
-	    !(S_ISDIR(ip->i_di.di_mode)))
-		*block = data_alloc(ip);
-	else
-		*block = meta_alloc(ip);
-
+	*block = meta_alloc(ip);
 	*ptr = cpu_to_be64(*block);
 	bmodified(bh);
 	ip->i_di.di_blocks++;

gfs2/libgfs2/gfs1.c +1 -6

index 619542c..00e18e6 100644
@@ -57,12 +57,7 @@ void gfs1_lookup_block(struct gfs2_inode *ip, struct gfs2_buffer_head *bh,
 	if (!create)
 		return;
 
-	if (height == ip->i_di.di_height - 1&&
-	    !(S_ISDIR(ip->i_di.di_mode)))
-		*block = data_alloc(ip);
-	else
-		*block = meta_alloc(ip);
-
+	*block = meta_alloc(ip);
 	*ptr = cpu_to_be64(*block);
 	bmodified(bh);
 	ip->i_di.di_blocks++;

Signature · 1 line

1.8.3.1

Bob Peterson

2014-01-27 15:46 UTC #4 lore raw

PATCH 4/8 libgfs2: Improve and simplify blk_alloc_in_rg

----- Original Message -----

| This function included naive implementations of gfs2_setbit and
| gfs2_bitfit so these have been replaced with calls to those functions.
| The 'type' parameter has been replaced with 'state' and the type
| defines removed in favour of the GFS2_BLKST_* values. As these were the
| same for both meta and data block allocations, data_alloc() has been
| removed and its callers updated to use meta_alloc().
| 
| Signed-off-by: Andrew Price <[email protected]>

Hi,

I've been staring at this patch for a long time now. The patch looks correct. However, my concern is this:

The fsck.gfs2 tool is designed to repair either GFS2 or GFS1 file systems, and GFS1 uses bitmaps differently from GFS2. An indirect block, for example, will be marked as a data block in a GFS2 file system, but "meta" in GFS1, (which translates to unlinked dinode in a GFS2 bitmap).

Of course, since this is upstream code, it's tempting to say that there won't be any GFS1 file systems hanging around. However, in theory, users might take a GFS1 file system from a legacy system and try migrate it to a newer system, upstream, RHEL7, Fedora, whatever, in which case they want to use gfs2_convert. But gfs2_convert doesn't do any error checking, so we recommend running fsck before the convert. However, on newer OSes, there isn't a gfs_fsck, there's only fsck.gfs2, which should handle both.

So we need to ask some hard questions:

(1) Does it matter? If the fsck is only being run for the sake of sanity for gfs2_convert, allocating those "metadata" blocks as "data" blocks will likely be all right, since gfs2_convert will need to convert them anyway.
(2) From the patch, it looks like "data" and "meta" are being treated the same anyway, so is there a bug in today's fsck.gfs2? This might show up if there's a GFS1 file system that has enough damage as to push a pile of directory entries into lost+found, thus increasing its size enough to add indirect blocks. Due to that same bug, fsck.gfs2 might not catch the discrepancy, so we probably should check it by hand using gfs2_edit.
(3) If there is a bug with how fsck.gfs2 handles GFS1 bitmaps, do we need to fix it? Or is it too much work for too little gain?
(4) I've got a test case I run called fsck.gfs2.nightmare2.sh, which tests fsck against my entire collection of metadata sets, both GFS and GFS2. It can run for days, depending on the hardware. Do the GFS1 metadata sets still pass with this patch?

Regards,

Bob Peterson
Red Hat File Systems

Andrew Price

author

2014-01-27 17:09 UTC #5 lore raw

PATCH 4/8 libgfs2: Improve and simplify blk_alloc_in_rg

Hi Bob,

Quoted 20 lines · Bob Peterson

----- Original Message -----
| This function included naive implementations of gfs2_setbit and
| gfs2_bitfit so these have been replaced with calls to those functions.
| The 'type' parameter has been replaced with 'state' and the type
| defines removed in favour of the GFS2_BLKST_* values. As these were the
| same for both meta and data block allocations, data_alloc() has been
| removed and its callers updated to use meta_alloc().
|
| Signed-off-by: Andrew Price <[email protected]>
Hi,
I've been staring at this patch for a long time now. The patch looks correct. However, my concern is this:
The fsck.gfs2 tool is designed to repair either GFS2 or GFS1 file systems, and GFS1 uses bitmaps differently from GFS2. An indirect block, for example, will be marked as a data block in a GFS2 file system, but "meta" in GFS1, (which translates to unlinked dinode in a GFS2 bitmap).

Quoted 7 lines

Of course, since this is upstream code, it's tempting to say that there won't be any GFS1 file systems hanging around. However, in theory, users might take a GFS1 file system from a legacy system and try migrate it to a newer system, upstream, RHEL7, Fedora, whatever, in which case they want to use gfs2_convert. But gfs2_convert doesn't do any error checking, so we recommend running fsck before the convert. However, on newer OSes, there isn't a gfs_fsck, there's only fsck.gfs2, which should handle both.

I'm certainly not suggesting dropping gfs1 support in fsck.gfs2 with this patch, and it really shouldn't be semantically different to before these changes anyway since it's just a code reorganisation.

Quoted 6 lines

So we need to ask some hard questions:
(1) Does it matter? If the fsck is only being run for the sake of sanity for gfs2_convert, allocating those "metadata" blocks as "data" blocks will likely be all right, since gfs2_convert will need to convert them anyway.

I'm not sure where the problem you're describing might be. Before this patch both meta_alloc() and data_alloc() allocated blocks using the GFS2_BLKST_USED state:

- case DATA:
- case META:
- state = GFS2_BLKST_USED;
- break;

so their effects were identical and that's why I felt it ok to combine those functions. I've not got my hands very dirty with gfs1 code but I understand that, whereas gfs2 has _UNLINKED and _DINODE, gfs1 has _FREEMETA and _USEDMETA, so perhaps we just need to look at whether the gfs1 code is calling the correct allocation function for the block state it requires.

Quoted 9 lines

(2) From the patch, it looks like "data" and "meta" are being treated the same anyway, so is there a bug in today's fsck.gfs2? This might show up if there's a GFS1 file system that has enough damage as to push a pile of directory entries into lost+found, thus increasing its size enough to add indirect blocks. Due to that same bug, fsck.gfs2 might not catch the discrepancy, so we probably should check it by hand using gfs2_edit.
(3) If there is a bug with how fsck.gfs2 handles GFS1 bitmaps, do we need to fix it? Or is it too much work for too little gain?

If we do find a bug here I think we should fix it if at all possible. If we ever do drop support for gfs1 in fsck.gfs2 it should be done in a more considered way than letting the gfs1 bits go rotten :)

Quoted 4 lines

(4) I've got a test case I run called fsck.gfs2.nightmare2.sh, which tests fsck against my entire collection of metadata sets, both GFS and GFS2. It can run for days, depending on the hardware. Do the GFS1 metadata sets still pass with this patch?

Well I would like to subject these patches to those tests in any case (and add some of them to the in-tree test suite where possible), but the intention here was to better organise the code rather than change any functionality so I would be surprised if this patch introduced the bug that you're theorizing rather than simply highlighting it.

Andy

Andrew Price

author

2014-01-27 14:17 UTC #6 lore raw

PATCH 5/8 mkfs.gfs2 tests: Enable debug output

In reply to Andrew Price

Enable debug output of mkfs.gfs2 tests and define the base options as a shell variable to make them easier to maintain.

Signed-off-by Andrew Price <[email protected]>

---
 tests/atlocal.in |  1 +
 tests/mkfs.at    | 46 +++++++++++++++++++++++-----------------------
 2 files changed, 24 insertions(+), 23 deletions(-)

tests/atlocal.in +1 -0

index 660c87a..b021b13 100644
@@ -1,5 +1,6 @@
 GFS_TGT="$abs_builddir/testvol"
 GFS_TGT_SZ=10
+GFS_MKFS="mkfs.gfs2 -O -D"
 
 gfs_max_blocks()
 {

tests/mkfs.at +23 ~~-23~~

93 lines not on this page · raw message

Signature · 1 line

1.8.3.1

Andrew Price

author

2014-01-27 14:17 UTC #7 lore raw

PATCH 6/8 libgfs2: Refactor block allocation functions

In reply to Andrew Price

Split the part of blk_alloc_in_rg which finds a free block out into a separate find_free_block function. This will allow callers to have more control over where blocks are allocated in future.

Also combine the old blk_alloc_i and lgfs2_dinode_alloc and meta_alloc functions into a single block_alloc function which now acts as a backing function for lgfs2_dinode_alloc and meta_alloc, which is renamed lgfs2_meta_alloc. An exit() call from blk_alloc_i has been pushed down into its callers and replaced with error returns where possible.

Signed-off-by Andrew Price <[email protected]>

---
 gfs2/fsck/metawalk.c   |   7 ++-
 gfs2/libgfs2/fs_ops.c  | 119 ++++++++++++++++++++++++-------------------------
 gfs2/libgfs2/gfs1.c    |   6 ++-
 gfs2/libgfs2/libgfs2.h |   2 +-
 4 files changed, 70 insertions(+), 64 deletions(-)

gfs2/fsck/metawalk.c +6 -1

index b8cef87..b9b4154 100644
@@ -1912,7 +1912,12 @@ int write_new_leaf(struct gfs2_inode *dip, int start_lindex, int num_copies,
 	}
 
 	/* allocate and write out a new leaf block */
-	*bn = meta_alloc(dip);
+	if (lgfs2_meta_alloc(dip, bn)) {
+		log_err( _("Error: allocation failed while fixing directory leaf "
+			   "pointers.\n"));
+		free(padbuf);
+		return -1;
+	}
 	fsck_blockmap_set(dip, *bn, _("directory leaf"), gfs2_leaf_blk);
 	log_err(_("A new directory leaf was allocated at block %lld "
 		  "(0x%llx) to fill the %d (0x%x) pointer gap %s the existing "

gfs2/libgfs2/fs_ops.c +58 ~~-61~~

211 lines not on this page · raw message

gfs2/libgfs2/gfs1.c +5 -1

index 00e18e6..8c8ca91 100644
@@ -57,7 +57,11 @@ void gfs1_lookup_block(struct gfs2_inode *ip, struct gfs2_buffer_head *bh,
 	if (!create)
 		return;
 
-	*block = meta_alloc(ip);
+	if (lgfs2_meta_alloc(ip, block)) {
+		*block = 0;
+		return;
+	}
+
 	*ptr = cpu_to_be64(*block);
 	bmodified(bh);
 	ip->i_di.di_blocks++;

gfs2/libgfs2/libgfs2.h +1 -1

index 07cb221..084ef41 100644
@@ -448,7 +448,7 @@ extern struct gfs2_inode *is_system_inode(struct gfs2_sbd *sdp,
 					  uint64_t block);
 extern void inode_put(struct gfs2_inode **ip);
 extern uint64_t data_alloc(struct gfs2_inode *ip);
-extern uint64_t meta_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 int gfs2_readi(struct gfs2_inode *ip, void *buf, uint64_t offset,
 		      unsigned int size);

Signature · 1 line

1.8.3.1

Andrew Price

author

2014-01-27 14:17 UTC #8 lore raw

PATCH 7/8 gfs2-utils: Clean up unused functions

In reply to Andrew Price

Remove unused functions, some of which call exit() from libgfs2.

Signed-off-by Andrew Price <[email protected]>

---
 gfs2/fsck/Makefile.am  |  4 ++--
 gfs2/fsck/eattr.c      | 63 --------------------------------------------------
 gfs2/fsck/eattr.h      | 18 ---------------
 gfs2/fsck/pass2.c      |  1 -
 gfs2/libgfs2/fs_ops.c  | 18 ---------------
 gfs2/libgfs2/gfs1.c    | 14 -----------
 gfs2/libgfs2/libgfs2.h |  4 ----
 gfs2/libgfs2/misc.c    | 28 ----------------------
 8 files changed, 2 insertions(+), 148 deletions(-)
 delete mode 100644 gfs2/fsck/eattr.c
 delete mode 100644 gfs2/fsck/eattr.h

gfs2/fsck/Makefile.am +2 -2

index b92c745..92d56f3 100644
@@ -10,10 +10,10 @@ sbindir := $(shell rpl=0; test '$(exec_prefix):$(sbindir)' = /usr:/usr/sbin \
 
 sbin_PROGRAMS		= fsck.gfs2
 
-noinst_HEADERS		= eattr.h fsck.h fs_recovery.h \
+noinst_HEADERS		= fsck.h fs_recovery.h \
 			  inode_hash.h link.h lost_n_found.h metawalk.h util.h
 
-fsck_gfs2_SOURCES	= eattr.c fs_recovery.c initialize.c \
+fsck_gfs2_SOURCES	= fs_recovery.c initialize.c \
 			  inode_hash.c link.c lost_n_found.c main.c metawalk.c \
 			  pass1b.c pass1.c pass1c.c pass2.c pass3.c pass4.c \
 			  pass5.c rgrepair.c util.c

gfs2/fsck/eattr.c deleted +0 ~~-63~~

63 lines not on this page · raw message

gfs2/fsck/eattr.h deleted +0 ~~-18~~

deleted file mode 100644
index f4264b0..0000000
@@ -1,18 +0,0 @@
-#ifndef _EATTR_H
-#define _EATTR_H
-
-int clear_eattr_indir(struct gfs2_inode *ip, uint64_t block, uint64_t parent,
-		      struct gfs2_buffer_head **bh, void *private);
-int clear_eattr_leaf(struct gfs2_inode *ip, uint64_t block, uint64_t parent,
-		     struct gfs2_buffer_head **bh, void *private);
-int clear_eattr_entry (struct gfs2_inode *ip,
-					   struct gfs2_buffer_head *leaf_bh,
-					   struct gfs2_ea_header *ea_hdr,
-					   struct gfs2_ea_header *ea_hdr_prev,
-					   void *private);
-int clear_eattr_extentry(struct gfs2_inode *ip, uint64_t *ea_data_ptr,
-			 struct gfs2_buffer_head *leaf_bh,
-			 struct gfs2_ea_header *ea_hdr,
-			 struct gfs2_ea_header *ea_hdr_prev, void *private);
-
-#endif /* _EATTR_H */

gfs2/fsck/pass2.c +0 -1

index 26f7d48..d3b56fc 100644
@@ -12,7 +12,6 @@
 #include "libgfs2.h"
 #include "fsck.h"
 #include "util.h"
-#include "eattr.h"
 #include "metawalk.h"
 #include "link.h"
 #include "lost_n_found.h"

gfs2/libgfs2/fs_ops.c +0 ~~-18~~

index 7a6f57a..e00bd1f 100644
@@ -835,24 +835,6 @@ void gfs2_get_leaf_nr(struct gfs2_inode *dip, uint32_t lindex,
 	*leaf_out = be64_to_cpu(leaf_no);
 }
 
-void gfs2_put_leaf_nr(struct gfs2_inode *dip, uint32_t inx, uint64_t leaf_out)
-{
-	uint64_t leaf_no;
-	int count;
-
-	if (dip->i_sbd->gfs1) {
-		gfs_put_leaf_nr(dip, inx, leaf_out);
-		return;
-	}
-	leaf_no = cpu_to_be64(leaf_out);
-	count = gfs2_writei(dip, (char *)&leaf_no, inx * sizeof(uint64_t),
-			    sizeof(uint64_t));
-	if (count != sizeof(uint64_t)) {
-		fprintf(stderr, "gfs2_put_leaf_nr:  Bad internal write.\n");
-		exit(1);
-	}
-}
-
 void dir_split_leaf(struct gfs2_inode *dip, uint32_t start, uint64_t leaf_no,
 		    struct gfs2_buffer_head *obh)
 {

gfs2/libgfs2/gfs1.c +0 ~~-14~~

index 8c8ca91..c00d7be 100644
@@ -389,17 +389,3 @@ void gfs_rgrp_out(struct gfs_rgrp *rgrp, struct gfs2_buffer_head *rbh)
 	memcpy(str->rg_reserved, rgrp->rg_reserved, 64);
 	bmodified(rbh);
 }
-
-void gfs_put_leaf_nr(struct gfs2_inode *dip, uint32_t inx, uint64_t leaf_out)
-{
-	uint64_t leaf_no;
-	int count;
-
-	leaf_no = cpu_to_be64(leaf_out);
-	count = gfs1_writei(dip, (char *)&leaf_no, inx * sizeof(uint64_t),
-			   sizeof(uint64_t));
-	if (count != sizeof(uint64_t)) {
-		fprintf(stderr, "gfs_put_leaf_nr:  Bad internal write.\n");
-		exit(1);
-	}
-}

gfs2/libgfs2/libgfs2.h +0 -4

index 084ef41..71adb30 100644
@@ -481,7 +481,6 @@ extern void block_map(struct gfs2_inode *ip, uint64_t lblock, int *new,
 		      uint64_t *dblock, uint32_t *extlen, int prealloc);
 extern void gfs2_get_leaf_nr(struct gfs2_inode *dip, uint32_t index,
 			     uint64_t *leaf_out);
-extern void gfs2_put_leaf_nr(struct gfs2_inode *dip, uint32_t inx, uint64_t leaf_out);
 extern void dir_split_leaf(struct gfs2_inode *dip, uint32_t start,
 			   uint64_t leaf_no, struct gfs2_buffer_head *obh);
 extern void gfs2_free_block(struct gfs2_sbd *sdp, uint64_t block);

@@ -670,8 +669,6 @@ extern struct gfs2_inode *lgfs2_gfs_inode_read(struct gfs2_sbd *sdp,
 extern void gfs_jindex_in(struct gfs_jindex *jindex, char *buf);
 extern void gfs_rgrp_in(struct gfs_rgrp *rg, struct gfs2_buffer_head *bh);
 extern void gfs_rgrp_out(struct gfs_rgrp *rg, struct gfs2_buffer_head *bh);
-extern void gfs_put_leaf_nr(struct gfs2_inode *dip, uint32_t inx,
-			    uint64_t leaf_out);
 
 /* gfs2_log.c */

@@ -721,7 +718,6 @@ extern int find_gfs2_meta(struct gfs2_sbd *sdp);
 extern int dir_exists(const char *dir);
 extern int mount_gfs2_meta(struct gfs2_sbd *sdp);
 extern void cleanup_metafs(struct gfs2_sbd *sdp);
-extern int set_sysfs(const char *fsname, const char *filename, const char *val);
 extern int is_fsname(char *name);
 extern void get_random_bytes(void *buf, int nbytes);

gfs2/libgfs2/misc.c +0 ~~-28~~

index 8ee8324..d34b8a5 100644
@@ -23,7 +23,6 @@
 #include "libgfs2.h"
 
 #define PAGE_SIZE (4096)
-#define SYS_BASE "/sys/fs/gfs2" /* FIXME: Look in /proc/mounts to find this */
 #define DIV_RU(x, y) (((x) + (y) - 1) / (y))
 
 int metafs_interrupted = 0;

@@ -278,33 +277,6 @@ void cleanup_metafs(struct gfs2_sbd *sdp)
 	metafs_interrupted = 0;
 }
 
-int set_sysfs(const char *fsname, const char *filename, const char *val)
-{
-	char path[PATH_MAX];
-	int fd, rv, len;
-
-	len = strlen(val) + 1;
-	if (len > PAGE_SIZE) {
-		errno = EINVAL;
-		return -1;
-	}
-
-	memset(path, 0, PATH_MAX);
-	snprintf(path, PATH_MAX - 1, "%s/%s/%s", SYS_BASE, fsname, filename);
-
-	fd = open(path, O_WRONLY);
-	if (fd < 0)
-		return -1;
-
-	rv = write(fd, val, len);
-	if (rv != len) {
-		close(fd);
-		return -1;
-	}
-	close(fd);
-	return 0;
-}
-
 /*
  * get_random_bytes - Generate a series of random bytes using /dev/urandom.
  *

Signature · 1 line

1.8.3.1

Andrew Price

author

2014-01-27 14:17 UTC #9 lore raw

PATCH 8/8 libgfs2: Remove exit call from build_rgrps

In reply to Andrew Price

Change build_rgrps to return an error value instead of void and push the exit call down into fsck.gfs2 and gfs2_grow.

Signed-off-by Andrew Price <[email protected]>

---
 gfs2/fsck/rgrepair.c       |  5 ++++-
 gfs2/libgfs2/fs_geometry.c | 10 ++++------
 gfs2/libgfs2/libgfs2.h     |  2 +-
 gfs2/mkfs/main_grow.c      |  5 ++++-
 4 files changed, 13 insertions(+), 9 deletions(-)

gfs2/fsck/rgrepair.c +4 -1

index 1ebdc70..2c1d613 100644
@@ -654,7 +654,10 @@ static int gfs2_rindex_calculate(struct gfs2_sbd *sdp, int *num_rgs)
 	/* Compute the default resource group layout as mkfs would have done */
 	compute_rgrp_layout(sdp, &sdp->rgcalc, TRUE);
 	debug_print_rgrps(sdp, &sdp->rgcalc);
-	build_rgrps(sdp, FALSE); /* FALSE = calc but don't write to disk. */
+	if (build_rgrps(sdp, FALSE)) { /* FALSE = calc but don't write to disk. */
+		fprintf(stderr, _("Failed to build resource groups\n"));
+		exit(-1);
+	}
 	log_debug( _("fs_total_size = 0x%llx blocks.\n"),
 		  (unsigned long long)sdp->device.length);
 	log_warn( _("L3: number of rgs in the index = %d.\n"), *num_rgs);

gfs2/libgfs2/fs_geometry.c +4 -6

index 932a2e6..4bdb64a 100644
@@ -171,7 +171,7 @@ uint32_t rgblocks2bitblocks(const unsigned int bsize, const uint32_t rgblocks, u
  * If fd > 0, write the data to the given file handle.
  * Otherwise, use gfs2 buffering in buf.c.
  */
-void build_rgrps(struct gfs2_sbd *sdp, int do_write)
+int build_rgrps(struct gfs2_sbd *sdp, int do_write)
 {
 	struct osi_node *n, *next = NULL;
 	struct rgrp_tree *rl;

@@ -207,11 +207,8 @@ void build_rgrps(struct gfs2_sbd *sdp, int do_write)
 		rl->rg.rg_header.mh_format = GFS2_FORMAT_RG;
 		rl->rg.rg_free = rgblocks;
 
-		if (gfs2_compute_bitstructs(sdp->sd_sb.sb_bsize, rl)) {
-			fprintf(stderr, "%s: Unable to build resource groups "
-				"with these characteristics.\n", __FUNCTION__);
-			exit(-1);
-		}
+		if (gfs2_compute_bitstructs(sdp->sd_sb.sb_bsize, rl))
+			return -1;
 
 		if (do_write) {
 			for (x = 0; x < bitblocks; x++) {

@@ -231,4 +228,5 @@ void build_rgrps(struct gfs2_sbd *sdp, int do_write)
 		sdp->blks_total += rgblocks;
 		sdp->fssize = ri->ri_data0 + ri->ri_data;
 	}
+	return 0;
 }

gfs2/libgfs2/libgfs2.h +1 -1

index 71adb30..cc65003 100644
@@ -431,7 +431,7 @@ 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,
 				int rgsize_specified);
-extern void build_rgrps(struct gfs2_sbd *sdp, int write);
+extern int build_rgrps(struct gfs2_sbd *sdp, int write);
 
 /* fs_ops.c */
 #define IS_LEAF     (1)

gfs2/mkfs/main_grow.c +4 -1

index 541b0f2..718bb30 100644
@@ -181,7 +181,10 @@ static void initialize_new_portion(struct gfs2_sbd *sdp, int *old_rg_count)
 	discard_blocks(sdp->device_fd, rl->start * sdp->bsize,
 		       (sdp->device.length - rl->start) * sdp->bsize);
 	/* Build the remaining resource groups */
-	build_rgrps(sdp, !test);
+	if (build_rgrps(sdp, !test)) {
+		fprintf(stderr, _("Failed to build resource groups\n"));
+		exit(-1);
+	}
 
 	inode_put(&sdp->md.riinode);
 	inode_put(&sdp->master_dir);

Signature · 1 line

1.8.3.1

Steven Whitehouse

2014-01-27 14:47 UTC #10 lore raw

PATCH 1/8 libgfs2: Remove sdp argument from compute_heightsize

In reply to Andrew Price

Hi,

These look like a step in the right direction - however patch 5 seems to be missing in the set that I received,

Steve.

Quoted 90 lines · Andrew Price

compute_heightsize only uses the bsize member of sdp so just accept a bsize argument instead.

Signed-off-by Andrew Price <[email protected]>

---
 gfs2/convert/gfs2_convert.c | 8 ++++----
 gfs2/libgfs2/libgfs2.h      | 2 +-
 gfs2/libgfs2/misc.c         | 8 ++++----
 3 files changed, 9 insertions(+), 9 deletions(-)

gfs2/convert/gfs2_convert.c +4 -4

index 719ba9c..e9dd0da 100644
@@ -1611,13 +1611,13 @@ static int init(struct gfs2_sbd *sbp)
 		sizeof(uint64_t);
 	sbp->sd_jbsize = sbp->bsize - sizeof(struct gfs2_meta_header);
 	brelse(bh);
-	if (compute_heightsize(sbp, sbp->sd_heightsize, &sbp->sd_max_height,
+	if (compute_heightsize(sbp->bsize, sbp->sd_heightsize, &sbp->sd_max_height,
 				sbp->bsize, sbp->sd_diptrs, sbp->sd_inptrs)) {
 		log_crit(_("Error: Bad constants (1)\n"));
 		exit(-1);
 	}
 
-	if (compute_heightsize(sbp, sbp->sd_jheightsize, &sbp->sd_max_jheight,
+	if (compute_heightsize(sbp->bsize, sbp->sd_jheightsize, &sbp->sd_max_jheight,
 				sbp->sd_jbsize, sbp->sd_diptrs, sbp->sd_inptrs)) {
 		log_crit(_("Error: Bad constants (1)\n"));
 		exit(-1);

@@ -1628,13 +1628,13 @@ static int init(struct gfs2_sbd *sbp)
 	gfs2_inptrs = (sbp->bsize - sizeof(struct gfs2_meta_header)) /
                 sizeof(uint64_t); /* How many ptrs can we fit on a block? */
 	memset(gfs2_heightsize, 0, sizeof(gfs2_heightsize));
-	if (compute_heightsize(sbp, gfs2_heightsize, &gfs2_max_height,
+	if (compute_heightsize(sbp->bsize, gfs2_heightsize, &gfs2_max_height,
 				sbp->bsize, sbp->sd_diptrs, gfs2_inptrs)) {
 		log_crit(_("Error: Bad constants (1)\n"));
 		exit(-1);
 	}
 	memset(gfs2_jheightsize, 0, sizeof(gfs2_jheightsize));
-	if (compute_heightsize(sbp, gfs2_jheightsize, &gfs2_max_jheight,
+	if (compute_heightsize(sbp->bsize, gfs2_jheightsize, &gfs2_max_jheight,
 				sbp->sd_jbsize, sbp->sd_diptrs, gfs2_inptrs)) {
 		log_crit(_("Error: Bad constants (1)\n"));
 		exit(-1);

gfs2/libgfs2/libgfs2.h +1 -1

index e785017..3119f39 100644
@@ -712,7 +712,7 @@ extern void decrease_verbosity(void);
 
 extern int metafs_interrupted;
 
-extern int compute_heightsize(struct gfs2_sbd *sdp, uint64_t *heightsize,
+extern int compute_heightsize(unsigned bsize, uint64_t *heightsize,
 		uint32_t *maxheight, uint32_t bsize1, int diptrs, int inptrs);
 extern int compute_constants(struct gfs2_sbd *sdp);
 extern int lgfs2_open_mnt(const char *path, int dirflags, int *dirfd, int devflags, int *devfd, struct mntent **mnt);

gfs2/libgfs2/misc.c +4 -4

index c4ed722..8ee8324 100644
@@ -28,10 +28,10 @@
 
 int metafs_interrupted = 0;
 
-int compute_heightsize(struct gfs2_sbd *sdp, uint64_t *heightsize,
+int compute_heightsize(unsigned bsize, uint64_t *heightsize,
 	uint32_t *maxheight, uint32_t bsize1, int diptrs, int inptrs)
 {
-	heightsize[0] = sdp->bsize - sizeof(struct gfs2_dinode);
+	heightsize[0] = bsize - sizeof(struct gfs2_dinode);
 	heightsize[1] = bsize1 * diptrs;
 	for (*maxheight = 2;; (*maxheight)++) {
 		uint64_t space, d;

@@ -91,11 +91,11 @@ int compute_constants(struct gfs2_sbd *sdp)
 
 	sdp->sd_max_dirres = hash_blocks + ind_blocks + leaf_blocks;
 
-	if (compute_heightsize(sdp, sdp->sd_heightsize, &sdp->sd_max_height,
+	if (compute_heightsize(sdp->bsize, sdp->sd_heightsize, &sdp->sd_max_height,
 				sdp->bsize, sdp->sd_diptrs, sdp->sd_inptrs)) {
 		return -1;
 	}
-	if (compute_heightsize(sdp, sdp->sd_jheightsize, &sdp->sd_max_jheight,
+	if (compute_heightsize(sdp->bsize, sdp->sd_jheightsize, &sdp->sd_max_jheight,
 				sdp->sd_jbsize, sdp->sd_diptrs, sdp->sd_inptrs)) {
 		return -1;
 	}

Andrew Price

author

2014-01-27 15:03 UTC #11 lore raw

PATCH 1/8 libgfs2: Remove sdp argument from compute_heightsize

Quoted 4 lines · Steven Whitehouse

Hi,
These look like a step in the right direction - however patch 5 seems to be missing in the set that I received,

Hm strange, I didn't receive it either but it hit the archives:

https://www.redhat.com/archives/cluster-devel/2014-January/msg00070.html

It's fairly trivial so probably not worth re-sending it.

Cheers,
Andy

Steven Whitehouse

2014-01-27 15:04 UTC #12 lore raw

PATCH 1/8 libgfs2: Remove sdp argument from compute_heightsize

Hi,

Quoted 6 lines · Andrew Price

Quoted 4 lines · Steven Whitehouse
Hi,
These look like a step in the right direction - however patch 5 seems to be missing in the set that I received,

Quoted 8 lines

Hm strange, I didn't receive it either but it hit the archives:
https://www.redhat.com/archives/cluster-devel/2014-January/msg00070.html
It's fairly trivial so probably not worth re-sending it.
Cheers,
Andy

Yes, I think they look ok to me. Although Bob is the real expert so far as fsck goes,

Steve.