gfs2: Minor cleanups and fixes

PATCH New Andreas Gruenbacher 3 participants 15 messages Oct 11, 2018 – Oct 12, 2018 GFS2 file system

Share & tools

Andreas Gruenbacher

author maintainer

2018-10-11 19:20 UTC #0 lore raw

cover gfs2: Minor cleanups and fixes

Here are some minor cleanups and fixes that resulted from looking into the resource group glock sharing patches (mostly updated versions of the patches I've previously posted on October 5). These should be ready to go into the next merge window.

Thanks,
Andreas

Andreas Gruenbacher (8):
gfs2: Always check the result of gfs2_rbm_from_block
gfs2: Clean up out-of-bounds check in gfs2_rbm_from_block
gfs2: Move rs_{sizehint, rgd_gh} fields into the inode
gfs2: Remove unused RGRP_RSRV_MINBYTES definition
gfs2: Rename bitmap.bi_{len => bytes}
gfs2: Fix some minor typos
gfs2: Fix marking bitmaps non-full
gfs2: Pass resource group to rgblk_free

Bob Peterson (1):
gfs2: Remove unnecessary gfs2_rlist_alloc parameter

 fs/gfs2/bmap.c   |   4 +-
 fs/gfs2/dir.c    |   7 ++-
 fs/gfs2/file.c   |   4 +-
 fs/gfs2/incore.h |   8 ++-
 fs/gfs2/lops.c   |   2 +-
 fs/gfs2/main.c   |   2 +
 fs/gfs2/quota.c  |   2 +-
 fs/gfs2/rgrp.c   | 134 ++++++++++++++++++++++-------------------------
 fs/gfs2/rgrp.h   |  11 ++--
 fs/gfs2/xattr.c  |  18 ++++---
 10 files changed, 96 insertions(+), 96 deletions(-)

Signature · 1 line

2.17.1

Andreas Gruenbacher

author maintainer

2018-10-11 19:20 UTC #1 lore raw

PATCH 1/9 gfs2: Always check the result of gfs2_rbm_from_block

When gfs2_rbm_from_block fails, the rbm it returns is undefined, so we always want to make sure gfs2_rbm_from_block has succeeded before looking at the rbm.

Signed-off-by Andreas Gruenbacher <[email protected]>

---
 fs/gfs2/rgrp.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

fs/gfs2/rgrp.c +8 -3

index b445ae15f87e..7f8b562d1cbe 100644
@@ -2233,7 +2233,8 @@ static struct gfs2_rgrpd *rgblk_free(struct gfs2_sbd *sdp, u64 bstart,
 		return NULL;
 	}
 
-	gfs2_rbm_from_block(&rbm, bstart);
+	if (WARN_ON_ONCE(gfs2_rbm_from_block(&rbm, bstart)))
+		return NULL;
 	while (blen--) {
 		bi = rbm_bi(&rbm);
 		if (bi != bi_prev) {

@@ -2366,7 +2367,10 @@ static void gfs2_set_alloc_start(struct gfs2_rbm *rbm,
 	else
 		goal = rbm->rgd->rd_last_alloc + rbm->rgd->rd_data0;
 
-	gfs2_rbm_from_block(rbm, goal);
+	if (WARN_ON_ONCE(gfs2_rbm_from_block(rbm, goal))) {
+		rbm->bii = 0;
+		rbm->offset = 0;
+	}
 }
 
 /**

@@ -2575,7 +2579,8 @@ int gfs2_check_blk_type(struct gfs2_sbd *sdp, u64 no_addr, unsigned int type)
 
 	rbm.rgd = rgd;
 	error = gfs2_rbm_from_block(&rbm, no_addr);
-	WARN_ON_ONCE(error != 0);
+	if (WARN_ON_ONCE(error))
+		goto fail;
 
 	if (gfs2_testbit(&rbm, false) != type)
 		error = -ESTALE;

Signature · 1 line

2.17.1

Andreas Gruenbacher

author maintainer

2018-10-11 19:20 UTC #2 lore raw

PATCH 2/9 gfs2: Clean up out-of-bounds check in gfs2_rbm_from_block

In reply to Andreas Gruenbacher

We already have a function that checks if a block is within a resource group, so use that in gfs2_rbm_from_block as well.

Signed-off-by Andreas Gruenbacher <[email protected]>

---
 fs/gfs2/rgrp.c | 9 ++-------
 1 file changed, 2 insertions(+), 7 deletions(-)

fs/gfs2/rgrp.c +2 -7

index 7f8b562d1cbe..6eb2addcbff5 100644
@@ -273,15 +273,10 @@ static u32 gfs2_bitfit(const u8 *buf, const unsigned int len,
 
 static int gfs2_rbm_from_block(struct gfs2_rbm *rbm, u64 block)
 {
-	u64 rblock = block - rbm->rgd->rd_data0;
-
-	if (WARN_ON_ONCE(rblock > UINT_MAX))
-		return -EINVAL;
-	if (block >= rbm->rgd->rd_data0 + rbm->rgd->rd_data)
+	if (!rgrp_contains_block(rbm->rgd, block))
 		return -E2BIG;
-
 	rbm->bii = 0;
-	rbm->offset = (u32)(rblock);
+	rbm->offset = block - rbm->rgd->rd_data0;
 	/* Check if the block is within the first block */
 	if (rbm->offset < rbm_bi(rbm)->bi_blocks)
 		return 0;

Signature · 1 line

2.17.1

Andreas Gruenbacher

author maintainer

2018-10-11 19:20 UTC #3 lore raw

PATCH 3/9 gfs2: Move rs_{sizehint, rgd_gh} fields into the inode

In reply to Andreas Gruenbacher

Move the rs_sizehint and rs_rgd_gh fields from struct gfs2_blkreserv into the inode: they are more closely related to the inode than to a particular reservation.

Signed-off-by Andreas Gruenbacher <[email protected]>

---
 fs/gfs2/file.c   |  4 ++--
 fs/gfs2/incore.h |  6 ++----
 fs/gfs2/main.c   |  2 ++
 fs/gfs2/rgrp.c   | 16 +++++++---------
 4 files changed, 13 insertions(+), 15 deletions(-)

fs/gfs2/file.c +2 -2

index 6510f4e07d0e..45a17b770d97 100644
@@ -361,8 +361,8 @@ static void gfs2_size_hint(struct file *filep, loff_t offset, size_t size)
 	size_t blks = (size + sdp->sd_sb.sb_bsize - 1) >> sdp->sd_sb.sb_bsize_shift;
 	int hint = min_t(size_t, INT_MAX, blks);
 
-	if (hint > atomic_read(&ip->i_res.rs_sizehint))
-		atomic_set(&ip->i_res.rs_sizehint, hint);
+	if (hint > atomic_read(&ip->i_sizehint))
+		atomic_set(&ip->i_sizehint, hint);
 }
 
 /**

fs/gfs2/incore.h +2 -4

index 5d72e8b66a26..997a3a19f77d 100644
@@ -309,10 +309,6 @@ struct gfs2_qadata { /* quota allocation data */
 */
 
 struct gfs2_blkreserv {
-	/* components used during write (step 1): */
-	atomic_t rs_sizehint;         /* hint of the write size */
-
-	struct gfs2_holder rs_rgd_gh; /* Filled in by get_local_rgrp */
 	struct rb_node rs_node;       /* link to other block reservations */
 	struct gfs2_rbm rs_rbm;       /* Start of reservation */
 	u32 rs_free;                  /* how many blocks are still free */

@@ -417,8 +413,10 @@ struct gfs2_inode {
 	struct gfs2_holder i_iopen_gh;
 	struct gfs2_holder i_gh; /* for prepare/commit_write only */
 	struct gfs2_qadata *i_qadata; /* quota allocation data */
+	struct gfs2_holder i_rgd_gh;
 	struct gfs2_blkreserv i_res; /* rgrp multi-block reservation */
 	u64 i_goal;	/* goal block for allocations */
+	atomic_t i_sizehint;  /* hint of the write size */
 	struct rw_semaphore i_rw_mutex;
 	struct list_head i_ordered;
 	struct list_head i_trunc_list;

fs/gfs2/main.c +2 -0

index 2d55e2c3333c..c7603063f861 100644
@@ -39,9 +39,11 @@ static void gfs2_init_inode_once(void *foo)
 	struct gfs2_inode *ip = foo;
 
 	inode_init_once(&ip->i_inode);
+	atomic_set(&ip->i_sizehint, 0);
 	init_rwsem(&ip->i_rw_mutex);
 	INIT_LIST_HEAD(&ip->i_trunc_list);
 	ip->i_qadata = NULL;
+	gfs2_holder_mark_uninitialized(&ip->i_rgd_gh);
 	memset(&ip->i_res, 0, sizeof(ip->i_res));
 	RB_CLEAR_NODE(&ip->i_res.rs_node);
 	ip->i_hash_cache = NULL;

fs/gfs2/rgrp.c +7 -9

index 6eb2addcbff5..3b17a4e77b39 100644
@@ -1565,7 +1565,7 @@ static void rg_mblk_search(struct gfs2_rgrpd *rgd, struct gfs2_inode *ip,
 	if (S_ISDIR(inode->i_mode))
 		extlen = 1;
 	else {
-		extlen = max_t(u32, atomic_read(&rs->rs_sizehint), ap->target);
+		extlen = max_t(u32, atomic_read(&ip->i_sizehint), ap->target);
 		extlen = clamp(extlen, RGRP_RSRV_MINBLKS, free_blocks);
 	}
 	if ((rgd->rd_free_clone < rgd->rd_reserved) || (free_blocks < extlen))

@@ -2077,7 +2077,7 @@ int gfs2_inplace_reserve(struct gfs2_inode *ip, struct gfs2_alloc_parms *ap)
 			}
 			error = gfs2_glock_nq_init(rs->rs_rbm.rgd->rd_gl,
 						   LM_ST_EXCLUSIVE, flags,
-						   &rs->rs_rgd_gh);
+						   &ip->i_rgd_gh);
 			if (unlikely(error))
 				return error;
 			if (!gfs2_rs_active(rs) && (loops < 2) &&

@@ -2086,7 +2086,7 @@ int gfs2_inplace_reserve(struct gfs2_inode *ip, struct gfs2_alloc_parms *ap)
 			if (sdp->sd_args.ar_rgrplvb) {
 				error = update_rgrp_lvb(rs->rs_rbm.rgd);
 				if (unlikely(error)) {
-					gfs2_glock_dq_uninit(&rs->rs_rgd_gh);
+					gfs2_glock_dq_uninit(&ip->i_rgd_gh);
 					return error;
 				}
 			}

@@ -2129,7 +2129,7 @@ int gfs2_inplace_reserve(struct gfs2_inode *ip, struct gfs2_alloc_parms *ap)
 
 		/* Unlock rgrp if required */
 		if (!rg_locked)
-			gfs2_glock_dq_uninit(&rs->rs_rgd_gh);
+			gfs2_glock_dq_uninit(&ip->i_rgd_gh);
 next_rgrp:
 		/* Find the next rgrp, and continue looking */
 		if (gfs2_select_rgrp(&rs->rs_rbm.rgd, begin))

@@ -2166,10 +2166,8 @@ int gfs2_inplace_reserve(struct gfs2_inode *ip, struct gfs2_alloc_parms *ap)
 
 void gfs2_inplace_release(struct gfs2_inode *ip)
 {
-	struct gfs2_blkreserv *rs = &ip->i_res;
-
-	if (gfs2_holder_initialized(&rs->rs_rgd_gh))
-		gfs2_glock_dq_uninit(&rs->rs_rgd_gh);
+	if (gfs2_holder_initialized(&ip->i_rgd_gh))
+		gfs2_glock_dq_uninit(&ip->i_rgd_gh);
 }
 
 /**

@@ -2328,7 +2326,7 @@ static void gfs2_adjust_reservation(struct gfs2_inode *ip,
 				goto out;
 			/* We used up our block reservation, so we should
 			   reserve more blocks next time. */
-			atomic_add(RGRP_RSRV_ADDBLKS, &rs->rs_sizehint);
+			atomic_add(RGRP_RSRV_ADDBLKS, &ip->i_sizehint);
 		}
 		__rs_deltree(rs);
 	}

Signature · 1 line

2.17.1

Andreas Gruenbacher

author maintainer

2018-10-11 19:20 UTC #4 lore raw

PATCH 4/9 gfs2: Remove unused RGRP_RSRV_MINBYTES definition

In reply to Andreas Gruenbacher

This definition is only used to define RGRP_RSRV_MINBLKS, with no benefit over defining RGRP_RSRV_MINBLKS directly.

In addition, instead of forcing RGRP_RSRV_MINBLKS to be of type u32, cast it to that type where that type is required.

Signed-off-by Andreas Gruenbacher <[email protected]>

---
 fs/gfs2/rgrp.c | 2 +-
 fs/gfs2/rgrp.h | 3 +--
 2 files changed, 2 insertions(+), 3 deletions(-)

fs/gfs2/rgrp.c +1 -1

index 3b17a4e77b39..22c73ced3edf 100644
@@ -1566,7 +1566,7 @@ static void rg_mblk_search(struct gfs2_rgrpd *rgd, struct gfs2_inode *ip,
 		extlen = 1;
 	else {
 		extlen = max_t(u32, atomic_read(&ip->i_sizehint), ap->target);
-		extlen = clamp(extlen, RGRP_RSRV_MINBLKS, free_blocks);
+		extlen = clamp(extlen, (u32)RGRP_RSRV_MINBLKS, free_blocks);
 	}
 	if ((rgd->rd_free_clone < rgd->rd_reserved) || (free_blocks < extlen))
 		return;

fs/gfs2/rgrp.h +1 -2

index e90478e2f545..6bb5ee112324 100644
@@ -18,8 +18,7 @@
  * By reserving 32 blocks at a time, we can optimize / shortcut how we search
  * through the bitmaps by looking a word@a time.
  */
-#define RGRP_RSRV_MINBYTES 8
-#define RGRP_RSRV_MINBLKS ((u32)(RGRP_RSRV_MINBYTES * GFS2_NBBY))
+#define RGRP_RSRV_MINBLKS 32
 #define RGRP_RSRV_ADDBLKS 64
 
 struct gfs2_rgrpd;

Signature · 1 line

2.17.1

Andreas Gruenbacher

author maintainer

2018-10-11 19:20 UTC #5 lore raw

PATCH 5/9 gfs2: Rename bitmap.bi_{len => bytes}

In reply to Andreas Gruenbacher

This field indicates the size of the bitmap in bytes, similar to how the bi_blocks field indicates the size of the bitmap in blocks.

In count_unlinked, replace an instance of bi_bytes * GFS2_NBBY by bi_blocks.

Signed-off-by Andreas Gruenbacher <[email protected]>

---
 fs/gfs2/incore.h |  2 +-
 fs/gfs2/lops.c   |  2 +-
 fs/gfs2/rgrp.c   | 32 ++++++++++++++++----------------
 3 files changed, 18 insertions(+), 18 deletions(-)

fs/gfs2/incore.h +1 -1

index 997a3a19f77d..888b62cfd6d1 100644
@@ -92,7 +92,7 @@ struct gfs2_bitmap {
 	unsigned long bi_flags;
 	u32 bi_offset;
 	u32 bi_start;
-	u32 bi_len;
+	u32 bi_bytes;
 	u32 bi_blocks;
 };

fs/gfs2/lops.c +1 -1

index f2567f958d00..4c7069b8f3c1 100644
@@ -81,7 +81,7 @@ static void maybe_release_space(struct gfs2_bufdata *bd)
 	if (sdp->sd_args.ar_discard)
 		gfs2_rgrp_send_discards(sdp, rgd->rd_data0, bd->bd_bh, bi, 1, NULL);
 	memcpy(bi->bi_clone + bi->bi_offset,
-	       bd->bd_bh->b_data + bi->bi_offset, bi->bi_len);
+	       bd->bd_bh->b_data + bi->bi_offset, bi->bi_bytes);
 	clear_bit(GBF_FULL, &bi->bi_flags);
 	rgd->rd_free_clone = rgd->rd_free;
 	rgd->rd_extfail_pt = rgd->rd_free;

fs/gfs2/rgrp.c +16 ~~-16~~

index 22c73ced3edf..cfe7f5a7639a 100644
@@ -90,7 +90,7 @@ static inline void gfs2_setbit(const struct gfs2_rbm *rbm, bool do_clone,
 {
 	unsigned char *byte1, *byte2, *end, cur_state;
 	struct gfs2_bitmap *bi = rbm_bi(rbm);
-	unsigned int buflen = bi->bi_len;
+	unsigned int buflen = bi->bi_bytes;
 	const unsigned int bit = (rbm->offset % GFS2_NBBY) * GFS2_BIT_SIZE;
 
 	byte1 = bi->bi_bh->b_data + bi->bi_offset + (rbm->offset / GFS2_NBBY);

@@ -108,8 +108,8 @@ static inline void gfs2_setbit(const struct gfs2_rbm *rbm, bool do_clone,
 		fs_warn(sdp, "rgrp=0x%llx bi_start=0x%x biblk: 0x%llx\n",
 			(unsigned long long)rbm->rgd->rd_addr, bi->bi_start,
 			(unsigned long long)bi->bi_bh->b_blocknr);
-		fs_warn(sdp, "bi_offset=0x%x bi_len=0x%x block=0x%llx\n",
-			bi->bi_offset, bi->bi_len,
+		fs_warn(sdp, "bi_offset=0x%x bi_bytes=0x%x block=0x%llx\n",
+			bi->bi_offset, bi->bi_bytes,
 			(unsigned long long)gfs2_rbm_to_block(rbm));
 		dump_stack();
 		gfs2_consist_rgrpd(rbm->rgd);

@@ -381,7 +381,7 @@ static u32 gfs2_free_extlen(const struct gfs2_rbm *rrbm, u32 len)
 		if (bi->bi_clone)
 			start = bi->bi_clone;
 		start += bi->bi_offset;
-		end = start + bi->bi_len;
+		end = start + bi->bi_bytes;
 		BUG_ON(rbm.offset & 3);
 		start += (rbm.offset / GFS2_NBBY);
 		bytes = min_t(u32, len / GFS2_NBBY, (end - start));

@@ -466,7 +466,7 @@ void gfs2_rgrp_verify(struct gfs2_rgrpd *rgd)
 			count[x] += gfs2_bitcount(rgd,
 						  bi->bi_bh->b_data +
 						  bi->bi_offset,
-						  bi->bi_len, x);
+						  bi->bi_bytes, x);
 	}
 
 	if (count[0] != rgd->rd_free) {

@@ -781,21 +781,21 @@ static int compute_bitstructs(struct gfs2_rgrpd *rgd)
 			bytes = bytes_left;
 			bi->bi_offset = sizeof(struct gfs2_rgrp);
 			bi->bi_start = 0;
-			bi->bi_len = bytes;
+			bi->bi_bytes = bytes;
 			bi->bi_blocks = bytes * GFS2_NBBY;
 		/* header block */
 		} else if (x == 0) {
 			bytes = sdp->sd_sb.sb_bsize - sizeof(struct gfs2_rgrp);
 			bi->bi_offset = sizeof(struct gfs2_rgrp);
 			bi->bi_start = 0;
-			bi->bi_len = bytes;
+			bi->bi_bytes = bytes;
 			bi->bi_blocks = bytes * GFS2_NBBY;
 		/* last block */
 		} else if (x + 1 == length) {
 			bytes = bytes_left;
 			bi->bi_offset = sizeof(struct gfs2_meta_header);
 			bi->bi_start = rgd->rd_bitbytes - bytes_left;
-			bi->bi_len = bytes;
+			bi->bi_bytes = bytes;
 			bi->bi_blocks = bytes * GFS2_NBBY;
 		/* other blocks */
 		} else {

@@ -803,7 +803,7 @@ static int compute_bitstructs(struct gfs2_rgrpd *rgd)
 				sizeof(struct gfs2_meta_header);
 			bi->bi_offset = sizeof(struct gfs2_meta_header);
 			bi->bi_start = rgd->rd_bitbytes - bytes_left;
-			bi->bi_len = bytes;
+			bi->bi_bytes = bytes;
 			bi->bi_blocks = bytes * GFS2_NBBY;
 		}

@@ -815,11 +815,11 @@ static int compute_bitstructs(struct gfs2_rgrpd *rgd)
 		return -EIO;
 	}
 	bi = rgd->rd_bits + (length - 1);
-	if ((bi->bi_start + bi->bi_len) * GFS2_NBBY != rgd->rd_data) {
+	if ((bi->bi_start + bi->bi_bytes) * GFS2_NBBY != rgd->rd_data) {
 		if (gfs2_consist_rgrpd(rgd)) {
 			gfs2_rindex_print(rgd);
 			fs_err(sdp, "start=%u len=%u offset=%u\n",
-			       bi->bi_start, bi->bi_len, bi->bi_offset);
+			       bi->bi_start, bi->bi_bytes, bi->bi_offset);
 		}
 		return -EIO;
 	}

@@ -1146,8 +1146,8 @@ static u32 count_unlinked(struct gfs2_rgrpd *rgd)
 		goal = 0;
 		buffer = bi->bi_bh->b_data + bi->bi_offset;
 		WARN_ON(!buffer_uptodate(bi->bi_bh));
-		while (goal < bi->bi_len * GFS2_NBBY) {
-			goal = gfs2_bitfit(buffer, bi->bi_len, goal,
+		while (goal < bi->bi_blocks) {
+			goal = gfs2_bitfit(buffer, bi->bi_bytes, goal,
 					   GFS2_BLKST_UNLINKED);
 			if (goal == BFITNOENT)
 				break;

@@ -1319,7 +1319,7 @@ int gfs2_rgrp_send_discards(struct gfs2_sbd *sdp, u64 offset,
 	u32 trimmed = 0;
 	u8 diff;
 
-	for (x = 0; x < bi->bi_len; x++) {
+	for (x = 0; x < bi->bi_bytes; x++) {
 		const u8 *clone = bi->bi_clone ? bi->bi_clone : bi->bi_bh->b_data;
 		clone += bi->bi_offset;
 		clone += x;

@@ -1752,7 +1752,7 @@ static int gfs2_rbm_find(struct gfs2_rbm *rbm, u8 state, u32 *minext,
 		if (state != GFS2_BLKST_UNLINKED && bi->bi_clone)
 			buffer = bi->bi_clone + bi->bi_offset;
 		initial_offset = rbm->offset;
-		offset = gfs2_bitfit(buffer, bi->bi_len, rbm->offset, state);
+		offset = gfs2_bitfit(buffer, bi->bi_bytes, rbm->offset, state);
 		if (offset == BFITNOENT)
 			goto bitmap_full;
 		rbm->offset = offset;

@@ -2236,7 +2236,7 @@ static struct gfs2_rgrpd *rgblk_free(struct gfs2_sbd *sdp, u64 bstart,
 						      GFP_NOFS | __GFP_NOFAIL);
 				memcpy(bi->bi_clone + bi->bi_offset,
 				       bi->bi_bh->b_data + bi->bi_offset,
-				       bi->bi_len);
+				       bi->bi_bytes);
 			}
 			gfs2_trans_add_meta(rbm.rgd->rd_gl, bi->bi_bh);
 			bi_prev = bi;

Signature · 1 line

2.17.1

Andreas Gruenbacher

author maintainer

2018-10-11 19:20 UTC #6 lore raw

PATCH 6/9 gfs2: Fix some minor typos

In reply to Andreas Gruenbacher

Signed-off-by Andreas Gruenbacher <[email protected]>

---
 fs/gfs2/quota.c | 2 +-
 fs/gfs2/rgrp.c  | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

fs/gfs2/quota.c +1 -1

index 0efae7a0ee80..2ae5a109eea7 100644
@@ -1183,7 +1183,7 @@ static int print_message(struct gfs2_quota_data *qd, char *type)
  *
  * Returns: 0 on success.
  *                  min_req = ap->min_target ? ap->min_target : ap->target;
- *                  quota must allow atleast min_req blks for success and
+ *                  quota must allow at least min_req blks for success and
  *                  ap->allowed is set to the number of blocks allowed
  *
  *          -EDQUOT otherwise, quota violation. ap->allowed is set to number

fs/gfs2/rgrp.c +2 -2

index cfe7f5a7639a..f47c76d9d9d0 100644
@@ -2023,7 +2023,7 @@ static inline int fast_to_acquire(struct gfs2_rgrpd *rgd)
  * We try our best to find an rgrp that has at least ap->target blocks
  * available. After a couple of passes (loops == 2), the prospects of finding
  * such an rgrp diminish. At this stage, we return the first rgrp that has
- * atleast ap->min_target blocks available. Either way, we set ap->allowed to
+ * at least ap->min_target blocks available. Either way, we set ap->allowed to
  * the number of blocks available in the chosen rgrp.
  *
  * Returns: 0 on success,

@@ -2092,7 +2092,7 @@ int gfs2_inplace_reserve(struct gfs2_inode *ip, struct gfs2_alloc_parms *ap)
 			}
 		}
 
-		/* Skip unuseable resource groups */
+		/* Skip unusable resource groups */
 		if ((rs->rs_rbm.rgd->rd_flags & (GFS2_RGF_NOALLOC |
 						 GFS2_RDF_ERROR)) ||
 		    (loops == 0 && ap->target > rs->rs_rbm.rgd->rd_extfail_pt))

Signature · 1 line

2.17.1

Andreas Gruenbacher

author maintainer

2018-10-11 19:20 UTC #7 lore raw

PATCH 7/9 gfs2: Fix marking bitmaps non-full

In reply to Andreas Gruenbacher

Reservations in gfs can span multiple gfs2_bitmaps (but they won't span multiple resource groups). When removing a reservation, we want to clear the GBF_FULL flags of all involved gfs2_bitmaps, not just that of the first bitmap.

Signed-off-by Andreas Gruenbacher <[email protected]>

---
 fs/gfs2/rgrp.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

fs/gfs2/rgrp.c +11 -2

index f47c76d9d9d0..7c5904c49a6a 100644
@@ -641,7 +641,10 @@ static void __rs_deltree(struct gfs2_blkreserv *rs)
 	RB_CLEAR_NODE(&rs->rs_node);
 
 	if (rs->rs_free) {
-		struct gfs2_bitmap *bi = rbm_bi(&rs->rs_rbm);
+		u64 last_block = gfs2_rbm_to_block(&rs->rs_rbm) +
+				 rs->rs_free - 1;
+		struct gfs2_rbm last_rbm = { .rgd = rs->rs_rbm.rgd, };
+		struct gfs2_bitmap *start, *last;
 
 		/* return reserved blocks to the rgrp */
 		BUG_ON(rs->rs_rbm.rgd->rd_reserved < rs->rs_free);

@@ -652,7 +655,13 @@ static void __rs_deltree(struct gfs2_blkreserv *rs)
 		   it will force the number to be recalculated later. */
 		rgd->rd_extfail_pt += rs->rs_free;
 		rs->rs_free = 0;
-		clear_bit(GBF_FULL, &bi->bi_flags);
+		if (gfs2_rbm_from_block(&last_rbm, last_block))
+			return;
+		start = rbm_bi(&rs->rs_rbm);
+		last = rbm_bi(&last_rbm);
+		do
+			clear_bit(GBF_FULL, &start->bi_flags);
+		while (start++ != last);
 	}
 }

Signature · 1 line

2.17.1

Steven Whitehouse

2018-10-12 09:15 UTC #8 lore raw

PATCH 7/9 gfs2: Fix marking bitmaps non-full

Hi,

Quoted 9 lines · Andreas Gruenbacher

Reservations in gfs can span multiple gfs2_bitmaps (but they won't span multiple resource groups). When removing a reservation, we want to clear the GBF_FULL flags of all involved gfs2_bitmaps, not just that of the first bitmap.
Signed-off-by Andreas Gruenbacher <[email protected]>
---
  fs/gfs2/rgrp.c | 13 +++++++++++--
  1 file changed, 11 insertions(+), 2 deletions(-)

The series looks good I think. This one though looks like a bug fix and should probably go to -stable too?

Steve.

Quoted 32 lines

fs/gfs2/rgrp.c +11 -2

index f47c76d9d9d0..7c5904c49a6a 100644
@@ -641,7 +641,10 @@ static void __rs_deltree(struct gfs2_blkreserv *rs)
  	RB_CLEAR_NODE(&rs->rs_node);
 
  	if (rs->rs_free) {
-		struct gfs2_bitmap *bi = rbm_bi(&rs->rs_rbm);
+		u64 last_block = gfs2_rbm_to_block(&rs->rs_rbm) +
+				 rs->rs_free - 1;
+		struct gfs2_rbm last_rbm = { .rgd = rs->rs_rbm.rgd, };
+		struct gfs2_bitmap *start, *last;
 
  		/* return reserved blocks to the rgrp */
  		BUG_ON(rs->rs_rbm.rgd->rd_reserved < rs->rs_free);

@@ -652,7 +655,13 @@ static void __rs_deltree(struct gfs2_blkreserv *rs)
  		   it will force the number to be recalculated later. */
  		rgd->rd_extfail_pt += rs->rs_free;
  		rs->rs_free = 0;
-		clear_bit(GBF_FULL, &bi->bi_flags);
+		if (gfs2_rbm_from_block(&last_rbm, last_block))
+			return;
+		start = rbm_bi(&rs->rs_rbm);
+		last = rbm_bi(&last_rbm);
+		do
+			clear_bit(GBF_FULL, &start->bi_flags);
+		while (start++ != last);
  	}
  }

Andreas Gruenbacher

author maintainer

2018-10-12 11:40 UTC #9 lore raw

PATCH 7/9 gfs2: Fix marking bitmaps non-full

Quoted 2 lines · Steven Whitehouse

The series looks good I think. This one though looks like a bug fix and should probably go to -stable too?

Yes, that's a good idea. The fix applies all the way back to v4.4 or even further.

Thanks,
Andreas

Bob Peterson

2018-10-12 12:06 UTC #10 lore raw

PATCH 7/9 gfs2: Fix marking bitmaps non-full

In reply to Steven Whitehouse

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

Quoted 5 lines

Hi,
The series looks good I think. This one though looks like a bug fix and should probably go to -stable too?
Steve.

I concur. So can I add your reviewed-by before I push?

Bob Peterson

Steven Whitehouse

2018-10-12 12:07 UTC #11 lore raw

PATCH 7/9 gfs2: Fix marking bitmaps non-full

Hi,

Quoted 9 lines · Bob Peterson

----- Original Message -----
Quoted 5 lines
Hi,
The series looks good I think. This one though looks like a bug fix and should probably go to -stable too?
Steve.
I concur. So can I add your reviewed-by before I push?
Bob Peterson

Yes, please do,

Steve.

Andreas Gruenbacher

author maintainer

2018-10-11 19:20 UTC #12 lore raw

PATCH 8/9 gfs2: Remove unnecessary gfs2_rlist_alloc parameter

In reply to Andreas Gruenbacher

From: Bob Peterson <[email protected]>

The state parameter of gfs2_rlist_alloc is set to LM_ST_EXCLUSIVE in all calls, so remove it and hardcode that state in gfs2_rlist_alloc instead.

Signed-off-by Andreas Gruenbacher <[email protected]>

---
 fs/gfs2/dir.c   | 2 +-
 fs/gfs2/rgrp.c  | 5 ++---
 fs/gfs2/rgrp.h  | 2 +-
 fs/gfs2/xattr.c | 2 +-
 4 files changed, 5 insertions(+), 6 deletions(-)

fs/gfs2/dir.c +1 -1

index 2e28fc947f7f..87a6dee88a62 100644
@@ -2021,7 +2021,7 @@ static int leaf_dealloc(struct gfs2_inode *dip, u32 index, u32 len,
 		l_blocks++;
 	}
 
-	gfs2_rlist_alloc(&rlist, LM_ST_EXCLUSIVE);
+	gfs2_rlist_alloc(&rlist);
 
 	for (x = 0; x < rlist.rl_rgrps; x++) {
 		struct gfs2_rgrpd *rgd = gfs2_glock2rgrp(rlist.rl_ghs[x].gh_gl);

fs/gfs2/rgrp.c +2 -3

index 7c5904c49a6a..7fef6789fb92 100644
@@ -2668,13 +2668,12 @@ void gfs2_rlist_add(struct gfs2_inode *ip, struct gfs2_rgrp_list *rlist,
  * gfs2_rlist_alloc - all RGs have been added to the rlist, now allocate
  *      and initialize an array of glock holders for them
  * @rlist: the list of resource groups
- * @state: the lock state to acquire the RG lock in
  *
  * FIXME: Don't use NOFAIL
  *
  */
 
-void gfs2_rlist_alloc(struct gfs2_rgrp_list *rlist, unsigned int state)
+void gfs2_rlist_alloc(struct gfs2_rgrp_list *rlist)
 {
 	unsigned int x;

@@ -2683,7 +2682,7 @@ void gfs2_rlist_alloc(struct gfs2_rgrp_list *rlist, unsigned int state)
 				      GFP_NOFS | __GFP_NOFAIL);
 	for (x = 0; x < rlist->rl_rgrps; x++)
 		gfs2_holder_init(rlist->rl_rgd[x]->rd_gl,
-				state, 0,
+				LM_ST_EXCLUSIVE, 0,
 				&rlist->rl_ghs[x]);
 }

fs/gfs2/rgrp.h +1 -1

index 6bb5ee112324..09519ae10fb6 100644
@@ -67,7 +67,7 @@ struct gfs2_rgrp_list {
 
 extern void gfs2_rlist_add(struct gfs2_inode *ip, struct gfs2_rgrp_list *rlist,
 			   u64 block);
-extern void gfs2_rlist_alloc(struct gfs2_rgrp_list *rlist, unsigned int state);
+extern void gfs2_rlist_alloc(struct gfs2_rgrp_list *rlist);
 extern void gfs2_rlist_free(struct gfs2_rgrp_list *rlist);
 extern u64 gfs2_ri_total(struct gfs2_sbd *sdp);
 extern void gfs2_rgrp_dump(struct seq_file *seq, const struct gfs2_glock *gl);

fs/gfs2/xattr.c +1 -1

index 38515988aaf7..e11f77f080a0 100644
@@ -1299,7 +1299,7 @@ static int ea_dealloc_indirect(struct gfs2_inode *ip)
 	else
 		goto out;
 
-	gfs2_rlist_alloc(&rlist, LM_ST_EXCLUSIVE);
+	gfs2_rlist_alloc(&rlist);
 
 	for (x = 0; x < rlist.rl_rgrps; x++) {
 		struct gfs2_rgrpd *rgd = gfs2_glock2rgrp(rlist.rl_ghs[x].gh_gl);

Signature · 1 line

2.17.1

Andreas Gruenbacher

author maintainer

2018-10-11 19:20 UTC #13 lore raw

PATCH 9/9 gfs2: Pass resource group to rgblk_free

In reply to Andreas Gruenbacher

Function rgblk_free can only deal with one resource group at a time, so pass that resource group is as a parameter. Several of the callers already have the resource group at hand, so we only need additional lookup code in a few places.

Signed-off-by Andreas Gruenbacher <[email protected]>

---
 fs/gfs2/bmap.c  |  4 ++--
 fs/gfs2/dir.c   |  5 ++++-
 fs/gfs2/rgrp.c  | 44 ++++++++++++++++----------------------------
 fs/gfs2/rgrp.h  |  6 ++++--
 fs/gfs2/xattr.c | 16 +++++++++-------
 5 files changed, 35 insertions(+), 40 deletions(-)

fs/gfs2/bmap.c +2 -2

index 03128ed1f34e..5f3ea07ef5e2 100644
@@ -1566,7 +1566,7 @@ static int sweep_bh_for_rgrps(struct gfs2_inode *ip, struct gfs2_holder *rd_gh,
 			continue;
 		}
 		if (bstart) {
-			__gfs2_free_blocks(ip, bstart, (u32)blen, meta);
+			__gfs2_free_blocks(ip, rgd, bstart, (u32)blen, meta);
 			(*btotal) += blen;
 			gfs2_add_inode_blocks(&ip->i_inode, -blen);
 		}

@@ -1574,7 +1574,7 @@ static int sweep_bh_for_rgrps(struct gfs2_inode *ip, struct gfs2_holder *rd_gh,
 		blen = 1;
 	}
 	if (bstart) {
-		__gfs2_free_blocks(ip, bstart, (u32)blen, meta);
+		__gfs2_free_blocks(ip, rgd, bstart, (u32)blen, meta);
 		(*btotal) += blen;
 		gfs2_add_inode_blocks(&ip->i_inode, -blen);
 	}

fs/gfs2/dir.c +4 -1

index 87a6dee88a62..daa14ab4e31b 100644
@@ -2042,6 +2042,8 @@ static int leaf_dealloc(struct gfs2_inode *dip, u32 index, u32 len,
 	bh = leaf_bh;
 
 	for (blk = leaf_no; blk; blk = nblk) {
+		struct gfs2_rgrpd *rgd;
+
 		if (blk != leaf_no) {
 			error = get_leaf(dip, blk, &bh);
 			if (error)

@@ -2052,7 +2054,8 @@ static int leaf_dealloc(struct gfs2_inode *dip, u32 index, u32 len,
 		if (blk != leaf_no)
 			brelse(bh);
 
-		gfs2_free_meta(dip, blk, 1);
+		rgd = gfs2_blk2rgrpd(sdp, blk, true);
+		gfs2_free_meta(dip, rgd, blk, 1);
 		gfs2_add_inode_blocks(&dip->i_inode, -1);
 	}

fs/gfs2/rgrp.c +16 ~~-28~~

110 lines not on this page · raw message

fs/gfs2/rgrp.h +4 -2

index 09519ae10fb6..b596c3d17988 100644
@@ -51,8 +51,10 @@ extern int gfs2_alloc_blocks(struct gfs2_inode *ip, u64 *bn, unsigned int *n,
 extern int gfs2_rsqa_alloc(struct gfs2_inode *ip);
 extern void gfs2_rs_deltree(struct gfs2_blkreserv *rs);
 extern void gfs2_rsqa_delete(struct gfs2_inode *ip, atomic_t *wcount);
-extern void __gfs2_free_blocks(struct gfs2_inode *ip, u64 bstart, u32 blen, int meta);
-extern void gfs2_free_meta(struct gfs2_inode *ip, u64 bstart, u32 blen);
+extern void __gfs2_free_blocks(struct gfs2_inode *ip, struct gfs2_rgrpd *rgd,
+			       u64 bstart, u32 blen, int meta);
+extern void gfs2_free_meta(struct gfs2_inode *ip, struct gfs2_rgrpd *rgd,
+			   u64 bstart, u32 blen);
 extern void gfs2_free_di(struct gfs2_rgrpd *rgd, struct gfs2_inode *ip);
 extern void gfs2_unlink_di(struct inode *inode);
 extern int gfs2_check_blk_type(struct gfs2_sbd *sdp, u64 no_addr,

fs/gfs2/xattr.c +9 -7

index e11f77f080a0..996c915a9c97 100644
@@ -283,7 +283,7 @@ static int ea_dealloc_unstuffed(struct gfs2_inode *ip, struct buffer_head *bh,
 			blen++;
 		else {
 			if (bstart)
-				gfs2_free_meta(ip, bstart, blen);
+				gfs2_free_meta(ip, rgd, bstart, blen);
 			bstart = bn;
 			blen = 1;
 		}

@@ -292,7 +292,7 @@ static int ea_dealloc_unstuffed(struct gfs2_inode *ip, struct buffer_head *bh,
 		gfs2_add_inode_blocks(&ip->i_inode, -1);
 	}
 	if (bstart)
-		gfs2_free_meta(ip, bstart, blen);
+		gfs2_free_meta(ip, rgd, bstart, blen);
 
 	if (prev && !leave) {
 		u32 len;

@@ -1250,6 +1250,7 @@ static int ea_dealloc_indirect(struct gfs2_inode *ip)
 {
 	struct gfs2_sbd *sdp = GFS2_SB(&ip->i_inode);
 	struct gfs2_rgrp_list rlist;
+	struct gfs2_rgrpd *rgd;
 	struct buffer_head *indbh, *dibh;
 	__be64 *eablk, *end;
 	unsigned int rg_blocks = 0;

@@ -1302,8 +1303,7 @@ static int ea_dealloc_indirect(struct gfs2_inode *ip)
 	gfs2_rlist_alloc(&rlist);
 
 	for (x = 0; x < rlist.rl_rgrps; x++) {
-		struct gfs2_rgrpd *rgd = gfs2_glock2rgrp(rlist.rl_ghs[x].gh_gl);
-
+		rgd = gfs2_glock2rgrp(rlist.rl_ghs[x].gh_gl);
 		rg_blocks += rgd->rd_length;
 	}

@@ -1320,6 +1320,7 @@ static int ea_dealloc_indirect(struct gfs2_inode *ip)
 
 	eablk = (__be64 *)(indbh->b_data + sizeof(struct gfs2_meta_header));
 	bstart = 0;
+	rgd = NULL;
 	blen = 0;
 
 	for (; eablk < end; eablk++) {

@@ -1333,8 +1334,9 @@ static int ea_dealloc_indirect(struct gfs2_inode *ip)
 			blen++;
 		else {
 			if (bstart)
-				gfs2_free_meta(ip, bstart, blen);
+				gfs2_free_meta(ip, rgd, bstart, blen);
 			bstart = bn;
+			rgd = gfs2_blk2rgrpd(sdp, bstart, true);
 			blen = 1;
 		}

@@ -1342,7 +1344,7 @@ static int ea_dealloc_indirect(struct gfs2_inode *ip)
 		gfs2_add_inode_blocks(&ip->i_inode, -1);
 	}
 	if (bstart)
-		gfs2_free_meta(ip, bstart, blen);
+		gfs2_free_meta(ip, rgd, bstart, blen);
 
 	ip->i_diskflags &= ~GFS2_DIF_EA_INDIRECT;

@@ -1391,7 +1393,7 @@ static int ea_dealloc_block(struct gfs2_inode *ip)
 	if (error)
 		goto out_gunlock;
 
-	gfs2_free_meta(ip, ip->i_eattr, 1);
+	gfs2_free_meta(ip, rgd, ip->i_eattr, 1);
 
 	ip->i_eattr = 0;
 	gfs2_add_inode_blocks(&ip->i_inode, -1);

Signature · 1 line

2.17.1

Bob Peterson

2018-10-12 12:48 UTC #14 lore raw

cover gfs2: Minor cleanups and fixes

In reply to Andreas Gruenbacher

Hi,

Thanks. This patch set is now pushed to the for-next branch of the linux-gfs2 tree:

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

Quoted 10 lines

Here are some minor cleanups and fixes that resulted from looking into the resource group glock sharing patches (mostly updated versions of the patches I've previously posted on October 5). These should be ready to go into the next merge window.
Thanks,
Andreas
Andreas Gruenbacher (8):
gfs2: Always check the result of gfs2_rbm_from_block

https://git.kernel.org/pub/scm/linux/kernel/git/gfs2/linux-gfs2.git/commit/fs/gfs2?h=for-next&id=f654683dae0d6c4e02eb7126b14f19fd945c3569

Quoted 1 line