On 30 June 2017 at 18:40, Bob Peterson <[email protected]> wrote:
> Hi,
>
> Before this patch, GFS2 remembered the last rgrp it used by way of
> a variable i_rgd. However, block allocations are made by way of a
> reservations structure, ip->i_res, which also has a rgrp pointers.
> These two values are at best redundant, and at worse, confuse the
> logic and make GFS2 maintain and use two possibly opposing values.
> The proper solution is to only use a single value. Since new block
> allocations should be kept close to the last reservation, it makes
> sense to only use the value in the reservations structure.
> Therefore, this patch removes i_rgd.
>
> Signed-off-by: Bob Peterson <[email protected]>
> ---
> diff --git a/fs/gfs2/incore.h b/fs/gfs2/incore.h
> index de42384..2ad003b 100644
> --- a/fs/gfs2/incore.h
> +++ b/fs/gfs2/incore.h
> @@ -400,7 +400,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 7c9b6d2..ad62bfb 100644
> --- a/fs/gfs2/rgrp.c
> +++ b/fs/gfs2/rgrp.c
> @@ -1971,8 +1971,9 @@ 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);
> @@ -2033,8 +2034,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:
> @@ -2311,7 +2311,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;
> @@ -2540,15 +2540,16 @@ 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;
> + if (ip->i_res.rs_rbm.rgd && rgrp_contains_block(ip->i_res.rs_rbm.rgd,
> + block))
> + rgd = ip->i_res.rs_rbm.rgd;
> else
> 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;
> + ip->i_res.rs_rbm.rgd = rgd;
>
> for (x = 0; x < rlist->rl_rgrps; x++)
> if (rlist->rl_rgd[x] == rgd)
> diff --git a/fs/gfs2/super.c b/fs/gfs2/super.c
> index fdedec3..c83fd9c 100644
> --- a/fs/gfs2/super.c
> +++ b/fs/gfs2/super.c
> @@ -1660,7 +1660,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;
Oops, we want to initialize ip->i_res.rs_rbm.rgd here:
+ ip->i_res.rs_rbm.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 1e6e7da..ec0a68d 100644
> --- a/fs/gfs2/trans.h
> +++ b/fs/gfs2/trans.h
> @@ -30,9 +30,9 @@ 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)
> + if (requested < ip->i_res.rs_rbm.rgd->rd_length)
> return requested + 1;
> - return ip->i_rgd->rd_length;
> + return ip->i_res.rs_rbm.rgd->rd_length;
> }
>
> extern int gfs2_trans_begin(struct gfs2_sbd *sdp, unsigned int blocks,
Andreas
