cluster-devel.redhat.com archive mirror
 help / color / mirror / Atom feed
* [Cluster-devel] [PATCH] gfs2: Fix loop in gfs2_rbm_find (2)
@ 2019-01-24 15:57 Andreas Gruenbacher
  2019-01-24 17:43 ` Bob Peterson
  2019-01-29  0:35 ` Sasha Levin
  0 siblings, 2 replies; 7+ messages in thread
From: Andreas Gruenbacher @ 2019-01-24 15:57 UTC (permalink / raw)
  To: cluster-devel.redhat.com

The fix from commit 2d29f6b96d8f introduced an unexpected performance
regression when allocating blocks.  Fix by rewriting the overly
complicated wrap-around logic in gfs2_rbm_find in a more reasonable way.
Discovered and verified with iozone.

Fixes: 2d29f6b96d8f ("gfs2: Fix loop in gfs2_rbm_find")
Cc: stable at vger.kernel.org # v3.13+
Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com>
---
 fs/gfs2/rgrp.c | 40 +++++++++++++---------------------------
 1 file changed, 13 insertions(+), 27 deletions(-)

diff --git a/fs/gfs2/rgrp.c b/fs/gfs2/rgrp.c
index 831d7cb5a49c4..3f16411d885d9 100644
--- a/fs/gfs2/rgrp.c
+++ b/fs/gfs2/rgrp.c
@@ -1730,25 +1730,15 @@ static int gfs2_rbm_find(struct gfs2_rbm *rbm, u8 state, u32 *minext,
 			 const struct gfs2_inode *ip, bool nowrap)
 {
 	struct buffer_head *bh;
-	int initial_bii;
-	u32 initial_offset;
 	int first_bii = rbm->bii;
 	u32 first_offset = rbm->offset;
 	u32 offset;
 	u8 *buffer;
-	int n = 0;
-	int iters = rbm->rgd->rd_length;
+	bool wrapped = false;
 	int ret;
 	struct gfs2_bitmap *bi;
 	struct gfs2_extent maxext = { .rbm.rgd = rbm->rgd, };
 
-	/* If we are not starting at the beginning of a bitmap, then we
-	 * need to add one to the bitmap count to ensure that we search
-	 * the starting bitmap twice.
-	 */
-	if (rbm->offset != 0)
-		iters++;
-
 	while(1) {
 		bi = rbm_bi(rbm);
 		if ((ip == NULL || !gfs2_rs_active(&ip->i_res)) &&
@@ -1761,47 +1751,43 @@ static int gfs2_rbm_find(struct gfs2_rbm *rbm, u8 state, u32 *minext,
 		WARN_ON(!buffer_uptodate(bh));
 		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_bytes, rbm->offset, state);
-		if (offset == BFITNOENT)
-			goto bitmap_full;
+		if (offset == BFITNOENT) {
+			if (state == GFS2_BLKST_FREE && rbm->offset == 0)
+				set_bit(GBF_FULL, &bi->bi_flags);
+			goto next_bitmap;
+		}
 		rbm->offset = offset;
 		if (ip == NULL)
 			return 0;
 
-		initial_bii = rbm->bii;
 		ret = gfs2_reservation_check_and_update(rbm, ip,
 							minext ? *minext : 0,
 							&maxext);
 		if (ret == 0)
 			return 0;
-		if (ret > 0) {
-			n += (rbm->bii - initial_bii);
+		if (ret > 0)
 			goto next_iter;
-		}
 		if (ret == -E2BIG) {
-			n += rbm->bii - initial_bii;
 			rbm->bii = 0;
 			rbm->offset = 0;
 			goto res_covered_end_of_rgrp;
 		}
 		return ret;
 
-bitmap_full:	/* Mark bitmap as full and fall through */
-		if ((state == GFS2_BLKST_FREE) && initial_offset == 0)
-			set_bit(GBF_FULL, &bi->bi_flags);
-
 next_bitmap:	/* Find next bitmap in the rgrp */
 		rbm->offset = 0;
 		rbm->bii++;
 		if (rbm->bii == rbm->rgd->rd_length)
 			rbm->bii = 0;
 res_covered_end_of_rgrp:
-		if ((rbm->bii == 0) && nowrap)
-			break;
-		n++;
+		if (rbm->bii == 0) {
+			if (nowrap)
+				break;
+			wrapped = true;
+		}
 next_iter:
-		if (n >= iters)
+		if (wrapped && rbm->bii > first_bii)
 			break;
 	}
 
-- 
2.20.1



^ permalink raw reply related	[flat|nested] 7+ messages in thread
* [Cluster-devel] [PATCH] gfs2: Fix loop in gfs2_rbm_find (2)
@ 2019-01-31 14:42 Andreas Gruenbacher
  2019-02-02 14:38 ` Sasha Levin
  0 siblings, 1 reply; 7+ messages in thread
From: Andreas Gruenbacher @ 2019-01-31 14:42 UTC (permalink / raw)
  To: cluster-devel.redhat.com

This is an updated version of the patch previously posted on January 24.

Changes since then:

 - The previous version didn't take filesystems with a single bitmap
   block per filesystem into account, and could still loop endlessly
   in that case.

 - When starting a scan at the beginning of a bitmap block and we wrap
   around, there is no need to rescan that bitmap block again.  This
   revised version takes that into account as well.

 - At the end of gfs2_rbm_find, we update rd_extfail_pt when the scan
   has started at the beginning of the resource group.  However, we
   should update rd_extfail_pt whenever we have scanned the entire
   resource group, including when we have wrapped around.

--

The fix from commit 2d29f6b96d8f introduced an unexpected performance
regression when allocating blocks.  Fix by rewriting the overly complicated
wrap-around logic in gfs2_rbm_find.  Discovered and verified with iozone.

Fixes: 2d29f6b96d8f ("gfs2: Fix loop in gfs2_rbm_find")
Cc: stable at vger.kernel.org # v3.13+
Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com>
---
 fs/gfs2/rgrp.c | 54 +++++++++++++++++++++++---------------------------
 1 file changed, 25 insertions(+), 29 deletions(-)

diff --git a/fs/gfs2/rgrp.c b/fs/gfs2/rgrp.c
index 831d7cb5a49c4..52a4f340a8672 100644
--- a/fs/gfs2/rgrp.c
+++ b/fs/gfs2/rgrp.c
@@ -1729,25 +1729,22 @@ static int gfs2_reservation_check_and_update(struct gfs2_rbm *rbm,
 static int gfs2_rbm_find(struct gfs2_rbm *rbm, u8 state, u32 *minext,
 			 const struct gfs2_inode *ip, bool nowrap)
 {
+	bool scan_from_start = rbm->bii == 0 && rbm->offset == 0;
 	struct buffer_head *bh;
-	int initial_bii;
-	u32 initial_offset;
-	int first_bii = rbm->bii;
-	u32 first_offset = rbm->offset;
+	int last_bii;
 	u32 offset;
 	u8 *buffer;
-	int n = 0;
-	int iters = rbm->rgd->rd_length;
+	bool wrapped = false;
 	int ret;
 	struct gfs2_bitmap *bi;
 	struct gfs2_extent maxext = { .rbm.rgd = rbm->rgd, };
 
-	/* If we are not starting at the beginning of a bitmap, then we
-	 * need to add one to the bitmap count to ensure that we search
-	 * the starting bitmap twice.
+	/*
+	 * Determine the last bitmap to search.  If we're not starting at the
+	 * beginning of a bitmap, we need to search that bitmap twice to scan
+	 * the entire resource group.
 	 */
-	if (rbm->offset != 0)
-		iters++;
+	last_bii = rbm->bii - (rbm->offset == 0);
 
 	while(1) {
 		bi = rbm_bi(rbm);
@@ -1761,47 +1758,46 @@ static int gfs2_rbm_find(struct gfs2_rbm *rbm, u8 state, u32 *minext,
 		WARN_ON(!buffer_uptodate(bh));
 		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_bytes, rbm->offset, state);
-		if (offset == BFITNOENT)
-			goto bitmap_full;
+		if (offset == BFITNOENT) {
+			if (state == GFS2_BLKST_FREE && rbm->offset == 0)
+				set_bit(GBF_FULL, &bi->bi_flags);
+			goto next_bitmap;
+		}
 		rbm->offset = offset;
 		if (ip == NULL)
 			return 0;
 
-		initial_bii = rbm->bii;
 		ret = gfs2_reservation_check_and_update(rbm, ip,
 							minext ? *minext : 0,
 							&maxext);
 		if (ret == 0)
 			return 0;
-		if (ret > 0) {
-			n += (rbm->bii - initial_bii);
+		if (ret > 0)
 			goto next_iter;
-		}
 		if (ret == -E2BIG) {
-			n += rbm->bii - initial_bii;
 			rbm->bii = 0;
 			rbm->offset = 0;
 			goto res_covered_end_of_rgrp;
 		}
 		return ret;
 
-bitmap_full:	/* Mark bitmap as full and fall through */
-		if ((state == GFS2_BLKST_FREE) && initial_offset == 0)
-			set_bit(GBF_FULL, &bi->bi_flags);
-
 next_bitmap:	/* Find next bitmap in the rgrp */
 		rbm->offset = 0;
 		rbm->bii++;
 		if (rbm->bii == rbm->rgd->rd_length)
 			rbm->bii = 0;
 res_covered_end_of_rgrp:
-		if ((rbm->bii == 0) && nowrap)
-			break;
-		n++;
+		if (rbm->bii == 0) {
+			if (wrapped)
+				break;
+			wrapped = true;
+			if (nowrap)
+				break;
+		}
 next_iter:
-		if (n >= iters)
+		/* Have we scanned the entire resource group? */
+		if (wrapped && rbm->bii > last_bii)
 			break;
 	}
 
@@ -1811,8 +1807,8 @@ static int gfs2_rbm_find(struct gfs2_rbm *rbm, u8 state, u32 *minext,
 	/* If the extent was too small, and it's smaller than the smallest
 	   to have failed before, remember for future reference that it's
 	   useless to search this rgrp again for this amount or more. */
-	if ((first_offset == 0) && (first_bii == 0) &&
-	    (*minext < rbm->rgd->rd_extfail_pt))
+	if (wrapped && (scan_from_start || rbm->bii > last_bii) &&
+	    *minext < rbm->rgd->rd_extfail_pt)
 		rbm->rgd->rd_extfail_pt = *minext;
 
 	/* If the maximum extent we found is big enough to fulfill the
-- 
2.20.1



^ permalink raw reply related	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2019-02-02 14:38 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-01-24 15:57 [Cluster-devel] [PATCH] gfs2: Fix loop in gfs2_rbm_find (2) Andreas Gruenbacher
2019-01-24 17:43 ` Bob Peterson
2019-01-24 18:32   ` Bob Peterson
2019-01-29  0:35 ` Sasha Levin
2019-01-30 20:39   ` Bob Peterson
  -- strict thread matches above, loose matches on Subject: below --
2019-01-31 14:42 Andreas Gruenbacher
2019-02-02 14:38 ` Sasha Levin

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).