[Cluster-devel] [PATCH 28/32] gfs2: Eliminate GFS2_RDF_UPTODATE flag in favor of buffer existence

[Steven Whitehouse <swhiteho@redhat.com>]

Hi,

On 13/11/2019 21:30, Bob Peterson wrote:
> Before this patch, the rgrp code used two different methods to check
> if the rgrp information was up-to-date: (1) The GFS2_RDF_UPTODATE flag
> in the rgrp and (2) the existence (or not) of valid buffer_head
> pointers in the first bitmap. When the buffer_heads are read in from
> media, the rgrp is, by definition, up to date. When the rgrp glock is
> invalidated, the buffer_heads are released, thereby indicating the
> rgrp is no longer up to date (another node may have changed it).
> So we don't need both of these flags. This patch eliminates the flag
> in favor of simply checking if the buffer_head pointers exist.
> This simplifies the code. It also makes it more bullet-proof:
> if there are two methods, they can possibly get out of sync. With
> one method, there's no way to get out of sync, and debugging is
> easier.
>
> Signed-off-by: Bob Peterson <rpeterso@redhat.com>

These are two different things... the buffer_head flags signal whether 
the buffer head is up to date with respect to what is on disk. The 
GFS2_RDF_UPTODATE flag is there to indicate whether the internal copy of 
the various fields in the resource group is up to date.

These might match depending on how the rgrp's internal copy of the 
fields is maintained, but not sure that this is guaranteed. Has this 
been tested with the rgrplvb option? We should make sure that is all 
still working correctly,

Steve.


> ---
>   fs/gfs2/glops.c  |  3 ---
>   fs/gfs2/incore.h |  1 -
>   fs/gfs2/rgrp.c   | 22 +++++++++++-----------
>   3 files changed, 11 insertions(+), 15 deletions(-)
>
> diff --git a/fs/gfs2/glops.c b/fs/gfs2/glops.c
> index 4072f37e4278..183fd7cbdbc1 100644
> --- a/fs/gfs2/glops.c
> +++ b/fs/gfs2/glops.c
> @@ -213,9 +213,6 @@ static void rgrp_go_inval(struct gfs2_glock *gl, int flags)
>   
>   	WARN_ON_ONCE(!(flags & DIO_METADATA));
>   	truncate_inode_pages_range(mapping, gl->gl_vm.start, gl->gl_vm.end);
> -
> -	if (rgd)
> -		rgd->rd_flags &= ~GFS2_RDF_UPTODATE;
>   }
>   
>   static struct gfs2_inode *gfs2_glock2inode(struct gfs2_glock *gl)
> diff --git a/fs/gfs2/incore.h b/fs/gfs2/incore.h
> index a15ddd2f9bf4..61be366a2fa7 100644
> --- a/fs/gfs2/incore.h
> +++ b/fs/gfs2/incore.h
> @@ -113,7 +113,6 @@ struct gfs2_rgrpd {
>   	u32 rd_flags;
>   	u32 rd_extfail_pt;		/* extent failure point */
>   #define GFS2_RDF_CHECK		0x10000000 /* check for unlinked inodes */
> -#define GFS2_RDF_UPTODATE	0x20000000 /* rg is up to date */
>   #define GFS2_RDF_ERROR		0x40000000 /* error in rg */
>   #define GFS2_RDF_PREFERRED	0x80000000 /* This rgrp is preferred */
>   #define GFS2_RDF_MASK		0xf0000000 /* mask for internal flags */
> diff --git a/fs/gfs2/rgrp.c b/fs/gfs2/rgrp.c
> index 10d3397ed3cd..e5eba83a1a42 100644
> --- a/fs/gfs2/rgrp.c
> +++ b/fs/gfs2/rgrp.c
> @@ -939,7 +939,7 @@ static int read_rindex_entry(struct gfs2_inode *ip)
>   		goto fail;
>   
>   	rgd->rd_rgl = (struct gfs2_rgrp_lvb *)rgd->rd_gl->gl_lksb.sb_lvbptr;
> -	rgd->rd_flags &= ~(GFS2_RDF_UPTODATE | GFS2_RDF_PREFERRED);
> +	rgd->rd_flags &= ~GFS2_RDF_PREFERRED;
>   	if (rgd->rd_data > sdp->sd_max_rg_data)
>   		sdp->sd_max_rg_data = rgd->rd_data;
>   	spin_lock(&sdp->sd_rindex_spin);
> @@ -1214,15 +1214,15 @@ static int gfs2_rgrp_bh_get(struct gfs2_rgrpd *rgd)
>   		}
>   	}
>   
> -	if (!(rgd->rd_flags & GFS2_RDF_UPTODATE)) {
> -		for (x = 0; x < length; x++)
> -			clear_bit(GBF_FULL, &rgd->rd_bits[x].bi_flags);
> -		gfs2_rgrp_in(rgd, (rgd->rd_bits[0].bi_bh)->b_data);
> -		rgd->rd_flags |= (GFS2_RDF_UPTODATE | GFS2_RDF_CHECK);
> -		rgd->rd_free_clone = rgd->rd_free;
> -		/* max out the rgrp allocation failure point */
> -		rgd->rd_extfail_pt = rgd->rd_free;
> -	}
> +	for (x = 0; x < length; x++)
> +		clear_bit(GBF_FULL, &rgd->rd_bits[x].bi_flags);
> +
> +	gfs2_rgrp_in(rgd, (rgd->rd_bits[0].bi_bh)->b_data);
> +	rgd->rd_flags |= GFS2_RDF_CHECK;
> +	rgd->rd_free_clone = rgd->rd_free;
> +	/* max out the rgrp allocation failure point */
> +	rgd->rd_extfail_pt = rgd->rd_free;
> +
>   	if (cpu_to_be32(GFS2_MAGIC) != rgd->rd_rgl->rl_magic) {
>   		rgd->rd_rgl->rl_unlinked = cpu_to_be32(count_unlinked(rgd));
>   		gfs2_rgrp_ondisk2lvb(rgd->rd_rgl,
> @@ -1254,7 +1254,7 @@ static int update_rgrp_lvb(struct gfs2_rgrpd *rgd)
>   {
>   	u32 rl_flags;
>   
> -	if (rgd->rd_flags & GFS2_RDF_UPTODATE)
> +	if (rgd->rd_bits[0].bi_bh)
>   		return 0;
>   
>   	if (cpu_to_be32(GFS2_MAGIC) != rgd->rd_rgl->rl_magic)