gfs2: Prepare for resource group glock sharing

PATCH New Andreas Gruenbacher 2 participants 10 messages Dec 1, 2018 – Dec 4, 2018 GFS2 file system

Share & tools

Andreas Gruenbacher

author maintainer

2018-12-01 11:10 UTC #0 lore raw

v2 cover gfs2: Prepare for resource group glock sharing

This is an update of the resource group glock sharing patch queue by Bob and myself. Some of the cleanups and fixes posted previously [*] have been integrated upstream in the meantime.

Patch "gfs2: Add local resource group locking" is now built on top of the existing rgrp locking scheme; I'll get back to cleaning that up later.

Steve was questioning the performance impact of patch "gfs2: Only use struct gfs2_rbm for bitmap manipulations". I still think it's a non-issue, but some analysis will be useful.

This updated patch queue has passed initial testing.

Thanks,
Andreas

[*] https://www.redhat.com/archives/cluster-devel/2018-October/msg00006.html

Andreas Gruenbacher (3):
gfs2: Only use struct gfs2_rbm for bitmap manipulations
gfs2: Clean up gfs2_adjust_reservation
gfs2: Add per-reservation reserved block accounting

Bob Peterson (3):
gfs2: Add local resource group locking
gfs2: Allow node-wide exclusive glock sharing
gfs2: Introduce resource group sharing

 fs/gfs2/bmap.c       |   4 +-
 fs/gfs2/file.c       |   4 +-
 fs/gfs2/glock.c      |  26 +++-
 fs/gfs2/glock.h      |   6 +
 fs/gfs2/incore.h     |  33 +----
 fs/gfs2/inode.c      |   5 +-
 fs/gfs2/lops.c       |   5 +-
 fs/gfs2/rgrp.c       | 322 ++++++++++++++++++++++++++++---------------
 fs/gfs2/rgrp.h       |   4 +
 fs/gfs2/super.c      |   3 +-
 fs/gfs2/trace_gfs2.h |  18 ++-
 fs/gfs2/trans.h      |   2 +-
 fs/gfs2/xattr.c      |   6 +-
 13 files changed, 279 insertions(+), 159 deletions(-)

Signature · 1 line

2.19.1.546.g028f9c799.dirty

Andreas Gruenbacher

author maintainer

2018-12-01 11:10 UTC #1 lore raw

PATCH v2 1/6 gfs2: Only use struct gfs2_rbm for bitmap manipulations

GFS2 uses struct gfs2_rbm to represent a filesystem block number as a bit position within a resource group. This representation is used in the bitmap manipulation code to prevent excessive conversions between block numbers and bit positions, but also in struct gfs2_blkreserv which is part of struct gfs2_inode, to mark the start of a reservation. In the inode, the bit position representation makes less sense: first, the start position is used as a block number about as often as a bit position; second, the bit position representation makes the code unnecessarily complicated and difficult to read.

Therefore, change struct gfs2_blkreserv to represent the start of a reservation as a block number instead of a bit position. (This requires keeping track of the resource group in gfs2_blkreserv separately.) With that change, various things can be slightly simplified, and struct gfs2_rbm can be moved to rgrp.c.

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

---
 fs/gfs2/bmap.c       |   2 +-
 fs/gfs2/incore.h     |  30 +--------
 fs/gfs2/rgrp.c       | 153 +++++++++++++++++++++++++------------------
 fs/gfs2/trace_gfs2.h |  10 +--
 fs/gfs2/trans.h      |   2 +-
 5 files changed, 100 insertions(+), 97 deletions(-)

fs/gfs2/bmap.c +1 -1

index 9a4a15d646ebb..7851540bd8916 100644
@@ -1497,7 +1497,7 @@ static int sweep_bh_for_rgrps(struct gfs2_inode *ip, struct gfs2_holder *rd_gh,
 
 			/* Must be done with the rgrp glock held: */
 			if (gfs2_rs_active(&ip->i_res) &&
-			    rgd == ip->i_res.rs_rbm.rgd)
+			    rgd == ip->i_res.rs_rgd)
 				gfs2_rs_deltree(&ip->i_res);
 		}

fs/gfs2/incore.h +3 ~~-27~~

index 888b62cfd6d1a..aaab2af6a2d98 100644
@@ -124,31 +124,6 @@ struct gfs2_rgrpd {
 	struct rb_root rd_rstree;       /* multi-block reservation tree */
 };
 
-struct gfs2_rbm {
-	struct gfs2_rgrpd *rgd;
-	u32 offset;		/* The offset is bitmap relative */
-	int bii;		/* Bitmap index */
-};
-
-static inline struct gfs2_bitmap *rbm_bi(const struct gfs2_rbm *rbm)
-{
-	return rbm->rgd->rd_bits + rbm->bii;
-}
-
-static inline u64 gfs2_rbm_to_block(const struct gfs2_rbm *rbm)
-{
-	BUG_ON(rbm->offset >= rbm->rgd->rd_data);
-	return rbm->rgd->rd_data0 + (rbm_bi(rbm)->bi_start * GFS2_NBBY) +
-		rbm->offset;
-}
-
-static inline bool gfs2_rbm_eq(const struct gfs2_rbm *rbm1,
-			       const struct gfs2_rbm *rbm2)
-{
-	return (rbm1->rgd == rbm2->rgd) && (rbm1->bii == rbm2->bii) &&
-	       (rbm1->offset == rbm2->offset);
-}
-
 enum gfs2_state_bits {
 	BH_Pinned = BH_PrivateStart,
 	BH_Escaped = BH_PrivateStart + 1,

@@ -309,8 +284,9 @@ struct gfs2_qadata { /* quota allocation data */
 */
 
 struct gfs2_blkreserv {
-	struct rb_node rs_node;       /* link to other block reservations */
-	struct gfs2_rbm rs_rbm;       /* Start of reservation */
+	struct rb_node rs_node;       /* node within rd_rstree */
+	struct gfs2_rgrpd *rs_rgd;
+	u64 rs_start;		      /* start of reservation */
 	u32 rs_free;                  /* how many blocks are still free */
 };

fs/gfs2/rgrp.c +90 ~~-63~~

345 lines not on this page · raw message

fs/gfs2/trace_gfs2.h +5 -5

index e0025258107a8..7586c7629497f 100644
@@ -602,13 +602,13 @@ TRACE_EVENT(gfs2_rs,
 	),
 
 	TP_fast_assign(
-		__entry->dev		= rs->rs_rbm.rgd->rd_sbd->sd_vfs->s_dev;
-		__entry->rd_addr	= rs->rs_rbm.rgd->rd_addr;
-		__entry->rd_free_clone	= rs->rs_rbm.rgd->rd_free_clone;
-		__entry->rd_reserved	= rs->rs_rbm.rgd->rd_reserved;
+		__entry->dev		= rs->rs_rgd->rd_sbd->sd_vfs->s_dev;
+		__entry->rd_addr	= rs->rs_rgd->rd_addr;
+		__entry->rd_free_clone	= rs->rs_rgd->rd_free_clone;
+		__entry->rd_reserved	= rs->rs_rgd->rd_reserved;
 		__entry->inum		= container_of(rs, struct gfs2_inode,
 						       i_res)->i_no_addr;
-		__entry->start		= gfs2_rbm_to_block(&rs->rs_rbm);
+		__entry->start		= rs->rs_start;
 		__entry->free		= rs->rs_free;
 		__entry->func		= func;
 	),

fs/gfs2/trans.h +1 -1

index ad70087d05971..a27078f98e7a9 100644
@@ -30,7 +30,7 @@ struct gfs2_glock;
  * block, or all of the blocks in the rg, whichever is smaller */
 static inline unsigned int gfs2_rg_blocks(const struct gfs2_inode *ip, unsigned requested)
 {
-	struct gfs2_rgrpd *rgd = ip->i_res.rs_rbm.rgd;
+	struct gfs2_rgrpd *rgd = ip->i_res.rs_rgd;
 
 	if (requested < rgd->rd_length)
 		return requested + 1;

Signature · 1 line

2.19.1.546.g028f9c799.dirty

Andreas Gruenbacher

author maintainer

2018-12-01 11:10 UTC #2 lore raw

PATCH v2 2/6 gfs2: Clean up gfs2_adjust_reservation

In reply to Andreas Gruenbacher

Unconditionally call gfs2_adjust_reservation in gfs2_alloc_blocks. Move the code for updating rd_free and rd_free_clone from gfs2_alloc_blocks into gfs2_adjust_reservation.

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

---
 fs/gfs2/rgrp.c | 26 ++++++++++++++++----------
 1 file changed, 16 insertions(+), 10 deletions(-)

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

index bbcf8b8b2597f..733e21cd4cf25 100644
@@ -2333,13 +2333,23 @@ static void gfs2_rgrp_error(struct gfs2_rgrpd *rgd)
  * then it is removed.
  */
 
-static void gfs2_adjust_reservation(struct gfs2_inode *ip,
-				    const struct gfs2_rbm *rbm, unsigned len)
+static int gfs2_adjust_reservation(struct gfs2_inode *ip,
+				   const struct gfs2_rbm *rbm, unsigned len)
 {
+	struct gfs2_sbd *sdp = GFS2_SB(&ip->i_inode);
 	struct gfs2_blkreserv *rs = &ip->i_res;
 	struct gfs2_rgrpd *rgd = rbm->rgd;
+	int error = 0;
 
 	spin_lock(&rgd->rd_rsspin);
+	if (unlikely(rgd->rd_free < len || rgd->rd_free_clone < len)) {
+		fs_warn(sdp, "rgrp free block accounting error (%u %u %u)\n",
+			rgd->rd_free, rgd->rd_free_clone, len);
+		error = -EIO;
+		goto out;
+	}
+	rgd->rd_free -= len;
+	rgd->rd_free_clone -= len;
 	if (gfs2_rs_active(rs)) {
 		u64 start = gfs2_rbm_to_block(rbm);

@@ -2362,6 +2372,7 @@ static void gfs2_adjust_reservation(struct gfs2_inode *ip,
 	}
 out:
 	spin_unlock(&rgd->rd_rsspin);
+	return error;
 }
 
 /**

@@ -2435,8 +2446,9 @@ int gfs2_alloc_blocks(struct gfs2_inode *ip, u64 *bn, unsigned int *nblocks,
 	gfs2_alloc_extent(&rbm, dinode, nblocks);
 	block = gfs2_rbm_to_block(&rbm);
 	rbm.rgd->rd_last_alloc = block - rbm.rgd->rd_data0;
-	if (gfs2_rs_active(&ip->i_res))
-		gfs2_adjust_reservation(ip, &rbm, *nblocks);
+	error = gfs2_adjust_reservation(ip, &rbm, *nblocks);
+	if (error)
+		goto rgrp_error;
 	ndata = *nblocks;
 	if (dinode)
 		ndata--;

@@ -2453,12 +2465,7 @@ int gfs2_alloc_blocks(struct gfs2_inode *ip, u64 *bn, unsigned int *nblocks,
 			brelse(dibh);
 		}
 	}
-	if (rbm.rgd->rd_free < *nblocks) {
-		fs_warn(sdp, "nblocks=%u\n", *nblocks);
-		goto rgrp_error;
-	}
 
-	rbm.rgd->rd_free -= *nblocks;
 	if (dinode) {
 		rbm.rgd->rd_dinodes++;
 		*generation = rbm.rgd->rd_igeneration++;

@@ -2475,7 +2482,6 @@ int gfs2_alloc_blocks(struct gfs2_inode *ip, u64 *bn, unsigned int *nblocks,
 
 	gfs2_quota_change(ip, *nblocks, ip->i_inode.i_uid, ip->i_inode.i_gid);
 
-	rbm.rgd->rd_free_clone -= *nblocks;
 	trace_gfs2_block_alloc(ip, rbm.rgd, block, *nblocks,
 			       dinode ? GFS2_BLKST_DINODE : GFS2_BLKST_USED);
 	*bn = block;

Signature · 1 line

2.19.1.546.g028f9c799.dirty

Bob Peterson

2018-12-03 14:32 UTC #3 lore raw

PATCH v2 2/6 gfs2: Clean up gfs2_adjust_reservation

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

Quoted 13 lines

Unconditionally call gfs2_adjust_reservation in gfs2_alloc_blocks. Move the code for updating rd_free and rd_free_clone from gfs2_alloc_blocks into gfs2_adjust_reservation.
Signed-off-by Andreas Gruenbacher <[email protected]>
---
 fs/gfs2/rgrp.c | 26 ++++++++++++++++----------
 1 file changed, 16 insertions(+), 10 deletions(-)
fs/gfs2/rgrp.c +0 -0
index bbcf8b8b2597f..733e21cd4cf25 100644

(snip)

fs/gfs2/rgrp.c int gfs2_alloc_blocks(struct gfs2_inode *ip, u64 *bn, unsigned int *nblocks, · line 2449

Quoted 3 lines

fs/gfs2/rgrp.c +1 -2

-	if (gfs2_rs_active(&ip->i_res))
-		gfs2_adjust_reservation(ip, &rbm, *nblocks);
+	error = gfs2_adjust_reservation(ip, &rbm, *nblocks);

Wait. At what point do we require a reservation for all block allocations? In today's code, a reservation is not required, so the check for "if (gfs2_rs_active(&ip->i_res))" is necessary because you can't adjust a reservation that's not active (in the tree)...being within the tree determines whether the reservation is active or not.

Yes, I'm looking forward to the day when we require a reservation for all block allocations as per my original version (this needs to happen to allow rgrp sharing between processes.) Perhaps a future patch will add the requirement, but at this point, there's no such requirement, which means this patch will probably break during a git bisect, no?

We only get into this situation when all bitmaps are nearly full or fragmented and therefore no reservation will fit the minimum requirements, but we still need to allow small one-shot block allocations. So we need to do lots of full-bitmap testing. Or else we need to add this patch only after the patch that makes reservations mandatory.

Regards,

Bob Peterson

Andreas Gruenbacher

author maintainer

2018-12-03 14:58 UTC #4 lore raw

PATCH v2 2/6 gfs2: Clean up gfs2_adjust_reservation

Quoted 26 lines · Bob Peterson

----- Original Message -----
Quoted 13 lines
Unconditionally call gfs2_adjust_reservation in gfs2_alloc_blocks. Move the code for updating rd_free and rd_free_clone from gfs2_alloc_blocks into gfs2_adjust_reservation.
Signed-off-by Andreas Gruenbacher <[email protected]>
---
 fs/gfs2/rgrp.c | 26 ++++++++++++++++----------
 1 file changed, 16 insertions(+), 10 deletions(-)
fs/gfs2/rgrp.c +0 -0
index bbcf8b8b2597f..733e21cd4cf25 100644
(snip)
fs/gfs2/rgrp.c int gfs2_alloc_blocks(struct gfs2_inode *ip, u64 *bn, unsigned int *nblocks, · line 2449
Quoted 3 lines
fs/gfs2/rgrp.c +1 -2
-     if (gfs2_rs_active(&ip->i_res))
-             gfs2_adjust_reservation(ip, &rbm, *nblocks);
+     error = gfs2_adjust_reservation(ip, &rbm, *nblocks);
Wait. At what point do we require a reservation for all block allocations? In today's code, a reservation is not required, so the check for "if (gfs2_rs_active(&ip->i_res))" is necessary because you can't adjust a reservation that's not active (in the tree)...being within the tree determines whether the reservation is active or not.

Quoted 5 lines

Yes, I'm looking forward to the day when we require a reservation for all block allocations as per my original version (this needs to happen to allow rgrp sharing between processes.) Perhaps a future patch will add the requirement, but at this point, there's no such requirement, which means this patch will probably break during a git bisect, no?

gfs2_adjust_reservation already checks if there is an active reservation, so the duplicate check here is unnecessary.

Quoted 8 lines

We only get into this situation when all bitmaps are nearly full or fragmented and therefore no reservation will fit the minimum requirements, but we still need to allow small one-shot block allocations. So we need to do lots of full-bitmap testing. Or else we need to add this patch only after the patch that makes reservations mandatory.
Regards,
Bob Peterson

Andreas

Andreas Gruenbacher

author maintainer

2018-12-04 22:02 UTC #5 lore raw

PATCH v2 2/6 gfs2: Clean up gfs2_adjust_reservation

In reply to Andreas Gruenbacher

Quoted 3 lines · Andreas Gruenbacher

Unconditionally call gfs2_adjust_reservation in gfs2_alloc_blocks. Move the code for updating rd_free and rd_free_clone from gfs2_alloc_blocks into gfs2_adjust_reservation.

Let me change that description to:

Prepare gfs2_adjust_reservation for resource group glock sharing: Call it from gfs2_alloc_blocks unconditionally. Update rd_free and rd_free_clone in gfs2_adjust_reservation under rd_rsspin: holding the resource group glock alone will no longer be enough to keep the balance of the resoruce group counters straight.

Thanks,
Andreas

Andreas Gruenbacher

author maintainer

2018-12-01 11:10 UTC #6 lore raw

PATCH v2 3/6 gfs2: Add per-reservation reserved block accounting

In reply to Andreas Gruenbacher

Add a rs_reserved field to struct gfs2_blkreserv to keep track of the number of blocks reserved by this particular reservation. When making a reservation with gfs2_inplace_reserve, this field is set to somewhere between ap->min_target and ap->target depending on the number of free blocks in the resource group. When allocating blocks with gfs2_alloc_blocks, rs_reserved is decremented accordingly. Eventually, any reserved but not consumed blocks are returned to the resource group by gfs2_inplace_release (via gfs2_adjust_reservation).

The reservation tree (rd_rstree) is unaffected by this change: the reservations it tracks are still advisory, and the sizes of those reservations (rs_free) are still determined by the tentative allocation sizes (i_sizehint). Since rd_reserved now tracks the number of reserved blocks rather than the number of tentatively rd_reserved blocks, we may end up with slightly different allocation patterns, though. The rd_extfail_pt optimization will still cause ill-suited resource groups to be skipped quickly.

We expect to augment this with a patch that will reserve an extent of blocks rather than just reserving a number of blocks in gfs2_inplace_reserve. gfs2_alloc_blocks will then be able to consume that reserved extent before scanning for additional available blocks; this should eliminate double bitmap scanning in most cases.

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

---
 fs/gfs2/file.c       |  4 +--
 fs/gfs2/incore.h     |  1 +
 fs/gfs2/rgrp.c       | 73 ++++++++++++++++++++++++++------------------
 fs/gfs2/trace_gfs2.h |  8 +++--
 4 files changed, 52 insertions(+), 34 deletions(-)

fs/gfs2/file.c +2 -2

index 45a17b770d97d..0106abe8c6ee0 100644
@@ -1022,8 +1022,8 @@ static long __gfs2_fallocate(struct file *file, int mode, loff_t offset, loff_t
 			goto out_qunlock;
 
 		/* check if the selected rgrp limits our max_blks further */
-		if (ap.allowed && ap.allowed < max_blks)
-			max_blks = ap.allowed;
+		if (ip->i_res.rs_reserved < max_blks)
+			max_blks = ip->i_res.rs_reserved;
 
 		/* Almost done. Calculate bytes that can be written using
 		 * max_blks. We also recompute max_bytes, data_blocks and

fs/gfs2/incore.h +1 -0

index aaab2af6a2d98..ca25043fc26df 100644
@@ -288,6 +288,7 @@ struct gfs2_blkreserv {
 	struct gfs2_rgrpd *rs_rgd;
 	u64 rs_start;		      /* start of reservation */
 	u32 rs_free;                  /* how many blocks are still free */
+	u32 rs_reserved;              /* number of reserved blocks */
 };
 
 /*

fs/gfs2/rgrp.c +43 ~~-30~~

150 lines not on this page · raw message

fs/gfs2/trace_gfs2.h +6 -2

index 7586c7629497f..282fcb1a242f9 100644
@@ -598,6 +598,7 @@ TRACE_EVENT(gfs2_rs,
 		__field(	u64,	inum			)
 		__field(	u64,	start			)
 		__field(	u32,	free			)
+		__field(	u32,	reserved		)
 		__field(	u8,	func			)
 	),

@@ -610,17 +611,20 @@ TRACE_EVENT(gfs2_rs,
 						       i_res)->i_no_addr;
 		__entry->start		= rs->rs_start;
 		__entry->free		= rs->rs_free;
+		__entry->reserved	= rs->rs_reserved;
 		__entry->func		= func;
 	),
 
-	TP_printk("%u,%u bmap %llu resrv %llu rg:%llu rf:%lu rr:%lu %s f:%lu",
+	TP_printk("%u,%u bmap %llu resrv %llu rg:%llu rf:%lu rr:%lu %s f:%lu r:%lu",
 		  MAJOR(__entry->dev), MINOR(__entry->dev),
 		  (unsigned long long)__entry->inum,
 		  (unsigned long long)__entry->start,
 		  (unsigned long long)__entry->rd_addr,
 		  (unsigned long)__entry->rd_free_clone,
 		  (unsigned long)__entry->rd_reserved,
-		  rs_func_name(__entry->func), (unsigned long)__entry->free)
+		  rs_func_name(__entry->func),
+		  (unsigned long)__entry->free,
+		  (unsigned long)__entry->reserved)
 );
 
 #endif /* _TRACE_GFS2_H */

Signature · 1 line

2.19.1.546.g028f9c799.dirty

Andreas Gruenbacher

author maintainer

2018-12-01 11:10 UTC #7 lore raw

PATCH v2 4/6 gfs2: Add local resource group locking

In reply to Andreas Gruenbacher

From: Bob Peterson <[email protected]>

Prepare for treating resource group glocks as exclusive among nodes but shared among all tasks running on a node: introduce another layer of node-specific locking that the local tasks can use to coordinate their accesses.

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

---
 fs/gfs2/incore.h |  2 ++
 fs/gfs2/lops.c   |  5 +++-
 fs/gfs2/rgrp.c   | 74 ++++++++++++++++++++++++++++++++++++++++++------
 fs/gfs2/rgrp.h   |  4 +++
 4 files changed, 75 insertions(+), 10 deletions(-)

fs/gfs2/incore.h +2 -0

index ca25043fc26df..feba57a2a6bab 100644
@@ -23,6 +23,7 @@
 #include <linux/percpu.h>
 #include <linux/lockref.h>
 #include <linux/rhashtable.h>
+#include <linux/mutex.h>
 
 #define DIO_WAIT	0x00000010
 #define DIO_METADATA	0x00000020

@@ -121,6 +122,7 @@ struct gfs2_rgrpd {
 #define GFS2_RDF_PREFERRED	0x80000000 /* This rgrp is preferred */
 #define GFS2_RDF_MASK		0xf0000000 /* mask for internal flags */
 	spinlock_t rd_rsspin;           /* protects reservation related vars */
+	struct mutex rd_mutex;
 	struct rb_root rd_rstree;       /* multi-block reservation tree */
 };

fs/gfs2/lops.c +4 -1

index 4c7069b8f3c1d..a9e858e01c97f 100644
@@ -76,8 +76,9 @@ static void maybe_release_space(struct gfs2_bufdata *bd)
 	unsigned int index = bd->bd_bh->b_blocknr - gl->gl_name.ln_number;
 	struct gfs2_bitmap *bi = rgd->rd_bits + index;
 
+	rgrp_lock_local(rgd);
 	if (bi->bi_clone == NULL)
-		return;
+		goto out;
 	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,

@@ -85,6 +86,8 @@ static void maybe_release_space(struct gfs2_bufdata *bd)
 	clear_bit(GBF_FULL, &bi->bi_flags);
 	rgd->rd_free_clone = rgd->rd_free;
 	rgd->rd_extfail_pt = rgd->rd_free;
+out:
+	rgrp_unlock_local(rgd);
 }
 
 /**

fs/gfs2/rgrp.c +65 -9

228 lines not on this page · raw message

fs/gfs2/rgrp.h +4 -0

index b596c3d179888..33e52dab76efa 100644
@@ -92,4 +92,8 @@ static inline int rgrp_contains_block(struct gfs2_rgrpd *rgd, u64 block)
 }
 
 extern void check_and_update_goal(struct gfs2_inode *ip);
+
+extern void rgrp_lock_local(struct gfs2_rgrpd *rgd);
+extern void rgrp_unlock_local(struct gfs2_rgrpd *rgd);
+
 #endif /* __RGRP_DOT_H__ */

Signature · 1 line

2.19.1.546.g028f9c799.dirty

Andreas Gruenbacher

author maintainer

2018-12-01 11:10 UTC #8 lore raw

PATCH v2 5/6 gfs2: Allow node-wide exclusive glock sharing

In reply to Andreas Gruenbacher

From: Bob Peterson <[email protected]>

Introduce a new LM_FLAG_NODE_SCOPE glock holder flag: when taking a glock in LM_ST_EXCLUSIVE (EX) mode and with the LM_FLAG_NODE_SCOPE flag set, the exclusive lock is shared among all local processes who are holding the glock in EX mode and have the LM_FLAG_NODE_SCOPE flag set as well. (From the point of view of other nodes, the lock is still held exclusively.)

A future patch will start using this flag to improve performance with rgrp sharing.

Signed-off-by Bob Peterson <[email protected]>

---
 fs/gfs2/glock.c | 26 ++++++++++++++++++++++----
 fs/gfs2/glock.h |  6 ++++++
 2 files changed, 28 insertions(+), 4 deletions(-)

fs/gfs2/glock.c +22 -4

index 05431324b262b..1c07a1819c1b0 100644
@@ -279,10 +279,26 @@ void gfs2_glock_put(struct gfs2_glock *gl)
 
 static inline int may_grant(const struct gfs2_glock *gl, const struct gfs2_holder *gh)
 {
-	const struct gfs2_holder *gh_head = list_entry(gl->gl_holders.next, const struct gfs2_holder, gh_list);
-	if ((gh->gh_state == LM_ST_EXCLUSIVE ||
-	     gh_head->gh_state == LM_ST_EXCLUSIVE) && gh != gh_head)
-		return 0;
+	const struct gfs2_holder *gh_head = list_entry(gl->gl_holders.next,
+						       const struct gfs2_holder,
+						       gh_list);
+
+	if (gh != gh_head) {
+		/**
+		 * Here we make a special exception to grant holders who agree
+		 * to share the EX lock with other holders who also have the
+		 * bit set. If the original holder has the LM_FLAG_NODE_SCOPE bit
+		 * is set, we grant more holders with the bit set.
+		 */
+		if (gh_head->gh_state == LM_ST_EXCLUSIVE &&
+		    (gh_head->gh_flags & LM_FLAG_NODE_SCOPE) &&
+		    gh->gh_state == LM_ST_EXCLUSIVE &&
+		    (gh->gh_flags & LM_FLAG_NODE_SCOPE))
+			return 1;
+		if ((gh->gh_state == LM_ST_EXCLUSIVE ||
+		     gh_head->gh_state == LM_ST_EXCLUSIVE))
+			return 0;
+	}
 	if (gl->gl_state == gh->gh_state)
 		return 1;
 	if (gh->gh_flags & GL_EXACT)

@@ -1682,6 +1698,8 @@ static const char *hflags2str(char *buf, u16 flags, unsigned long iflags)
 		*p++ = 'A';
 	if (flags & LM_FLAG_PRIORITY)
 		*p++ = 'p';
+	if (flags & LM_FLAG_NODE_SCOPE)
+		*p++ = 'n';
 	if (flags & GL_ASYNC)
 		*p++ = 'a';
 	if (flags & GL_EXACT)

fs/gfs2/glock.h +6 -0

index 5e12220cc0c24..3a3c1bd5b2d89 100644
@@ -78,6 +78,11 @@ enum {
  * request and directly join the other shared lock.  A shared lock request
  * without the priority flag might be forced to wait until the deferred
  * requested had acquired and released the lock.
+ *
+ * LM_FLAG_NODE_SCOPE
+ * This holder agrees to share the lock within this node. In other words,
+ * the glock is held in EX mode according to DLM, but local holders on the
+ * same node can share it.
  */
 
 #define LM_FLAG_TRY		0x0001

@@ -85,6 +90,7 @@ enum {
 #define LM_FLAG_NOEXP		0x0004
 #define LM_FLAG_ANY		0x0008
 #define LM_FLAG_PRIORITY	0x0010
+#define LM_FLAG_NODE_SCOPE	0x0020
 #define GL_ASYNC		0x0040
 #define GL_EXACT		0x0080
 #define GL_SKIP			0x0100

Signature · 1 line

2.19.1.546.g028f9c799.dirty

Andreas Gruenbacher

author maintainer

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

PATCH v2 6/6 gfs2: Introduce resource group sharing

In reply to Andreas Gruenbacher

From: Bob Peterson <[email protected]>

This patch takes advantage of the new glock holder sharing feature for resource groups. We have already introduced local resource group locking in a previous patch, so competing accesses of local processes are already under control.

Signed-off-by Bob Peterson <[email protected]>

---
 fs/gfs2/bmap.c  |  2 +-
 fs/gfs2/inode.c |  5 +++--
 fs/gfs2/rgrp.c  | 10 +++++-----
 fs/gfs2/super.c |  3 ++-
 fs/gfs2/xattr.c |  6 ++++--
 5 files changed, 15 insertions(+), 11 deletions(-)

fs/gfs2/bmap.c +1 -1

index 7851540bd8916..f15d58975b62e 100644
@@ -1491,7 +1491,7 @@ static int sweep_bh_for_rgrps(struct gfs2_inode *ip, struct gfs2_holder *rd_gh,
 				goto out;
 			}
 			ret = gfs2_glock_nq_init(rgd->rd_gl, LM_ST_EXCLUSIVE,
-						 0, rd_gh);
+						 LM_FLAG_NODE_SCOPE, rd_gh);
 			if (ret)
 				goto out;

fs/gfs2/inode.c +3 -2

index 648f0ca1ad57e..f757c88b1e4c0 100644
@@ -1120,7 +1120,7 @@ static int gfs2_unlink(struct inode *dir, struct dentry *dentry)
 	if (!rgd)
 		goto out_inodes;
 
-	gfs2_holder_init(rgd->rd_gl, LM_ST_EXCLUSIVE, 0, ghs + 2);
+	gfs2_holder_init(rgd->rd_gl, LM_ST_EXCLUSIVE, LM_FLAG_NODE_SCOPE, ghs + 2);
 
 
 	error = gfs2_glock_nq(ghs); /* parent */

@@ -1407,7 +1407,8 @@ static int gfs2_rename(struct inode *odir, struct dentry *odentry,
 		 */
 		nrgd = gfs2_blk2rgrpd(sdp, nip->i_no_addr, 1);
 		if (nrgd)
-			gfs2_holder_init(nrgd->rd_gl, LM_ST_EXCLUSIVE, 0, ghs + num_gh++);
+			gfs2_holder_init(nrgd->rd_gl, LM_ST_EXCLUSIVE,
+					 LM_FLAG_NODE_SCOPE, ghs + num_gh++);
 	}
 
 	for (x = 0; x < num_gh; x++) {

fs/gfs2/rgrp.c +5 -5

index 95ecd81e61e70..86fa25f1756b3 100644
@@ -1465,7 +1465,8 @@ int gfs2_fitrim(struct file *filp, void __user *argp)
 
 	while (1) {
 
-		ret = gfs2_glock_nq_init(rgd->rd_gl, LM_ST_EXCLUSIVE, 0, &gh);
+		ret = gfs2_glock_nq_init(rgd->rd_gl, LM_ST_EXCLUSIVE,
+					 LM_FLAG_NODE_SCOPE, &gh);
 		if (ret)
 			goto out;

@@ -2082,7 +2083,7 @@ int gfs2_inplace_reserve(struct gfs2_inode *ip, struct gfs2_alloc_parms *ap)
 	struct gfs2_sbd *sdp = GFS2_SB(&ip->i_inode);
 	struct gfs2_rgrpd *begin = NULL;
 	struct gfs2_blkreserv *rs = &ip->i_res;
-	int error = 0, flags = 0;
+	int error = 0, flags = LM_FLAG_NODE_SCOPE;
 	bool rg_locked;
 	u64 last_unlinked = NO_BLOCK;
 	int loops = 0;

@@ -2761,9 +2762,8 @@ void gfs2_rlist_alloc(struct gfs2_rgrp_list *rlist)
 				      sizeof(struct gfs2_holder),
 				      GFP_NOFS | __GFP_NOFAIL);
 	for (x = 0; x < rlist->rl_rgrps; x++)
-		gfs2_holder_init(rlist->rl_rgd[x]->rd_gl,
-				LM_ST_EXCLUSIVE, 0,
-				&rlist->rl_ghs[x]);
+		gfs2_holder_init(rlist->rl_rgd[x]->rd_gl, LM_ST_EXCLUSIVE,
+				 LM_FLAG_NODE_SCOPE, &rlist->rl_ghs[x]);
 }
 
 /**

fs/gfs2/super.c +2 -1

index ca71163ff7cfd..3f43ac36b1f49 100644
@@ -1507,7 +1507,8 @@ static int gfs2_dinode_dealloc(struct gfs2_inode *ip)
 		goto out_qs;
 	}
 
-	error = gfs2_glock_nq_init(rgd->rd_gl, LM_ST_EXCLUSIVE, 0, &gh);
+	error = gfs2_glock_nq_init(rgd->rd_gl, LM_ST_EXCLUSIVE,
+				   LM_FLAG_NODE_SCOPE, &gh);
 	if (error)
 		goto out_qs;

fs/gfs2/xattr.c +4 -2

index 996c915a9c97e..dc45b0e13e08a 100644
@@ -262,7 +262,8 @@ static int ea_dealloc_unstuffed(struct gfs2_inode *ip, struct buffer_head *bh,
 		return -EIO;
 	}
 
-	error = gfs2_glock_nq_init(rgd->rd_gl, LM_ST_EXCLUSIVE, 0, &rg_gh);
+	error = gfs2_glock_nq_init(rgd->rd_gl, LM_ST_EXCLUSIVE,
+				   LM_FLAG_NODE_SCOPE, &rg_gh);
 	if (error)
 		return error;

@@ -1384,7 +1385,8 @@ static int ea_dealloc_block(struct gfs2_inode *ip)
 		return -EIO;
 	}
 
-	error = gfs2_glock_nq_init(rgd->rd_gl, LM_ST_EXCLUSIVE, 0, &gh);
+	error = gfs2_glock_nq_init(rgd->rd_gl, LM_ST_EXCLUSIVE,
+				   LM_FLAG_NODE_SCOPE, &gh);
 	if (error)
 		return error;

Signature · 1 line

2.19.1.546.g028f9c799.dirty