[Cluster-devel] [PATCH v2 1/2] gfs2: Stop messing with ip->i_rgd in the rlist code

[Cluster-devel] [PATCH v2 1/2] gfs2: Stop messing with ip->i_rgd in the rlist code

[Andreas Gruenbacher]

In the resource group list code, keep the last resource group added in
the last position in the array.  Check against that instead of messing
with ip->i_rgd.

Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com>
---
 fs/gfs2/rgrp.c | 25 +++++++++++++++++++------
 1 file changed, 19 insertions(+), 6 deletions(-)

diff --git a/fs/gfs2/rgrp.c b/fs/gfs2/rgrp.c
index 33abcf29bc05..015177974ff8 100644
--- a/fs/gfs2/rgrp.c
+++ b/fs/gfs2/rgrp.c
@@ -2558,19 +2558,32 @@ void gfs2_rlist_add(struct gfs2_inode *ip, struct gfs2_rgrp_list *rlist,
 	if (gfs2_assert_warn(sdp, !rlist->rl_ghs))
 		return;
 
-	if (ip->i_rgd && rgrp_contains_block(ip->i_rgd, block))
-		rgd = ip->i_rgd;
-	else
+	/*
+	 * The resource group last accessed is kept in the last position.
+	 */
+
+	if (rlist->rl_rgrps) {
+		rgd = rlist->rl_rgd[rlist->rl_rgrps - 1];
+		if (rgrp_contains_block(rgd, block))
+			return;
 		rgd = gfs2_blk2rgrpd(sdp, block, 1);
+	} else {
+		rgd = ip->i_rgd;
+		if (!rgd || !rgrp_contains_block(rgd, block))
+			rgd = gfs2_blk2rgrpd(sdp, block, 1);
+	}
+
 	if (!rgd) {
 		fs_err(sdp, "rlist_add: no rgrp for block %llu\n", (unsigned long long)block);
 		return;
 	}
-	ip->i_rgd = rgd;
 
-	for (x = 0; x < rlist->rl_rgrps; x++)
-		if (rlist->rl_rgd[x] == rgd)
+	for (x = 0; x < rlist->rl_rgrps; x++) {
+		if (rlist->rl_rgd[x] == rgd) {
+			swap(rlist->rl_rgd[x], rlist->rl_rgd[rlist->rl_rgrps - 1]);
 			return;
+		}
+	}
 
 	if (rlist->rl_rgrps == rlist->rl_space) {
 		new_space = rlist->rl_space + 10;
-- 
2.17.1

[Cluster-devel] [PATCH v2 2/2] gfs2: Eliminate redundant ip->i_rgd

[Andreas Gruenbacher]

GFS2 remembers the last rgrp used for allocations in ip->i_rgd.
However, block allocations are made by way of a reservations structure,
ip->i_res, which keeps the last rgrp in ip->i_res.rs_rgd, and ip->i_res
is kept in sync with ip->i_res.rs_rgd, so it's redundant.  Get rid of
ip->i_rgd and just use ip->i_res.rs_rgd in its place.

Based on patches by Robert Peterson.

Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com>
---
 fs/gfs2/incore.h |  1 -
 fs/gfs2/rgrp.c   | 11 +++++------
 fs/gfs2/super.c  |  1 -
 fs/gfs2/trans.h  |  6 ++++--
 4 files changed, 9 insertions(+), 10 deletions(-)

diff --git a/fs/gfs2/incore.h b/fs/gfs2/incore.h
index d2ad817e089f..f491577a7de2 100644
--- a/fs/gfs2/incore.h
+++ b/fs/gfs2/incore.h
@@ -398,7 +398,6 @@ struct gfs2_inode {
 	struct gfs2_holder i_gh; /* for prepare/commit_write only */
 	struct gfs2_qadata *i_qadata; /* quota allocation data */
 	struct gfs2_blkreserv i_res; /* rgrp multi-block reservation */
-	struct gfs2_rgrpd *i_rgd;
 	u64 i_goal;	/* goal block for allocations */
 	struct rw_semaphore i_rw_mutex;
 	struct list_head i_ordered;
diff --git a/fs/gfs2/rgrp.c b/fs/gfs2/rgrp.c
index 015177974ff8..97d11fd5e3d7 100644
--- a/fs/gfs2/rgrp.c
+++ b/fs/gfs2/rgrp.c
@@ -1991,8 +1991,8 @@ int gfs2_inplace_reserve(struct gfs2_inode *ip, struct gfs2_alloc_parms *ap)
 		return -EINVAL;
 	if (gfs2_rs_active(rs)) {
 		begin = rs->rs_rbm.rgd;
-	} else if (ip->i_rgd && rgrp_contains_block(ip->i_rgd, ip->i_goal)) {
-		rs->rs_rbm.rgd = begin = ip->i_rgd;
+	} else if (rs->rs_rbm.rgd && rgrp_contains_block(rs->rs_rbm.rgd, ip->i_goal)) {
+		begin = rs->rs_rbm.rgd;
 	} else {
 		check_and_update_goal(ip);
 		rs->rs_rbm.rgd = begin = gfs2_blk2rgrpd(sdp, ip->i_goal, 1);
@@ -2056,8 +2056,7 @@ int gfs2_inplace_reserve(struct gfs2_inode *ip, struct gfs2_alloc_parms *ap)
 		if (rs->rs_rbm.rgd->rd_free_clone >= ap->target ||
 		    (loops == 2 && ap->min_target &&
 		     rs->rs_rbm.rgd->rd_free_clone >= ap->min_target)) {
-			ip->i_rgd = rs->rs_rbm.rgd;
-			ap->allowed = ip->i_rgd->rd_free_clone;
+			ap->allowed = rs->rs_rbm.rgd->rd_free_clone;
 			return 0;
 		}
 check_rgrp:
@@ -2335,7 +2334,7 @@ int gfs2_alloc_blocks(struct gfs2_inode *ip, u64 *bn, unsigned int *nblocks,
 {
 	struct gfs2_sbd *sdp = GFS2_SB(&ip->i_inode);
 	struct buffer_head *dibh;
-	struct gfs2_rbm rbm = { .rgd = ip->i_rgd, };
+	struct gfs2_rbm rbm = { .rgd = ip->i_res.rs_rbm.rgd, };
 	unsigned int ndata;
 	u64 block; /* block, within the file system scope */
 	int error;
@@ -2568,7 +2567,7 @@ void gfs2_rlist_add(struct gfs2_inode *ip, struct gfs2_rgrp_list *rlist,
 			return;
 		rgd = gfs2_blk2rgrpd(sdp, block, 1);
 	} else {
-		rgd = ip->i_rgd;
+		rgd = ip->i_res.rs_rbm.rgd;
 		if (!rgd || !rgrp_contains_block(rgd, block))
 			rgd = gfs2_blk2rgrpd(sdp, block, 1);
 	}
diff --git a/fs/gfs2/super.c b/fs/gfs2/super.c
index af0d5b01cf0b..c212893534ed 100644
--- a/fs/gfs2/super.c
+++ b/fs/gfs2/super.c
@@ -1729,7 +1729,6 @@ static struct inode *gfs2_alloc_inode(struct super_block *sb)
 	if (ip) {
 		ip->i_flags = 0;
 		ip->i_gl = NULL;
-		ip->i_rgd = NULL;
 		memset(&ip->i_res, 0, sizeof(ip->i_res));
 		RB_CLEAR_NODE(&ip->i_res.rs_node);
 		ip->i_rahead = 0;
diff --git a/fs/gfs2/trans.h b/fs/gfs2/trans.h
index 1e6e7da25a17..ad70087d0597 100644
--- a/fs/gfs2/trans.h
+++ b/fs/gfs2/trans.h
@@ -30,9 +30,11 @@ 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)
 {
-	if (requested < ip->i_rgd->rd_length)
+	struct gfs2_rgrpd *rgd = ip->i_res.rs_rbm.rgd;
+
+	if (requested < rgd->rd_length)
 		return requested + 1;
-	return ip->i_rgd->rd_length;
+	return rgd->rd_length;
 }
 
 extern int gfs2_trans_begin(struct gfs2_sbd *sdp, unsigned int blocks,
-- 
2.17.1

[Cluster-devel] [PATCH v2 1/2] gfs2: Stop messing with ip->i_rgd in the rlist code

[Andreas Gruenbacher]

On 20 June 2018 at 23:39, Andreas Gruenbacher <agruenba@redhat.com> wrote:
> In the resource group list code, keep the last resource group added in
> the last position in the array.  Check against that instead of messing
> with ip->i_rgd.
>
> Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com>
> ---
>  fs/gfs2/rgrp.c | 25 +++++++++++++++++++------
>  1 file changed, 19 insertions(+), 6 deletions(-)
>
> diff --git a/fs/gfs2/rgrp.c b/fs/gfs2/rgrp.c
> index 33abcf29bc05..015177974ff8 100644
> --- a/fs/gfs2/rgrp.c
> +++ b/fs/gfs2/rgrp.c
> @@ -2558,19 +2558,32 @@ void gfs2_rlist_add(struct gfs2_inode *ip, struct gfs2_rgrp_list *rlist,
>         if (gfs2_assert_warn(sdp, !rlist->rl_ghs))
>                 return;
>
> -       if (ip->i_rgd && rgrp_contains_block(ip->i_rgd, block))
> -               rgd = ip->i_rgd;
> -       else
> +       /*
> +        * The resource group last accessed is kept in the last position.
> +        */
> +
> +       if (rlist->rl_rgrps) {
> +               rgd = rlist->rl_rgd[rlist->rl_rgrps - 1];
> +               if (rgrp_contains_block(rgd, block))
> +                       return;
>                 rgd = gfs2_blk2rgrpd(sdp, block, 1);
> +       } else {
> +               rgd = ip->i_rgd;
> +               if (!rgd || !rgrp_contains_block(rgd, block))
> +                       rgd = gfs2_blk2rgrpd(sdp, block, 1);
> +       }
> +
>         if (!rgd) {
>                 fs_err(sdp, "rlist_add: no rgrp for block %llu\n", (unsigned long long)block);
>                 return;
>         }
> -       ip->i_rgd = rgd;

We want to keep the above assignment to ip->i_rgd in this patch; it
should only be removed in the follow-up patch.

>
> -       for (x = 0; x < rlist->rl_rgrps; x++)
> -               if (rlist->rl_rgd[x] == rgd)
> +       for (x = 0; x < rlist->rl_rgrps; x++) {
> +               if (rlist->rl_rgd[x] == rgd) {
> +                       swap(rlist->rl_rgd[x], rlist->rl_rgd[rlist->rl_rgrps - 1]);
>                         return;
> +               }
> +       }
>
>         if (rlist->rl_rgrps == rlist->rl_space) {
>                 new_space = rlist->rl_space + 10;
> --
> 2.17.1
>

Andreas