[Cluster-devel] [GFS2 PATCH] gfs2: Don't set GFS2_RDF_UPTODATE when the lvb is updated

[Steven Whitehouse <swhiteho@redhat.com>]

Acked-by: Steven Whitehouse <swhiteho@redhat.com>


On 17/08/18 16:51, Bob Peterson wrote:
> Hi,
>
> The GFS2_RDF_UPTODATE flag in the rgrp is used to determine when
> a rgrp buffer is valid. It's cleared when the glock is invalidated,
> signifying that the buffer data is now invalid. But before this
> patch, function update_rgrp_lvb was setting the flag when it
> determined it had a valid lvb. But that's an invalid assumption:
> just because you have a valid lvb doesn't mean you have valid
> buffers. After all, another node may have made the lvb valid,
> and this node just fetched it from the glock via dlm.
>
> Consider this scenario:
> 1. The file system is mounted with RGRPLVB option.
> 2. In gfs2_inplace_reserve it locks the rgrp glock EX, but thanks
>     to GL_SKIP, it skips the gfs2_rgrp_bh_get.
> 3. Since loops == 0 and the allocation target (ap->target) is
>     bigger than the largest known chunk of blocks in the rgrp
>     (rs->rs_rbm.rgd->rd_extfail_pt) it skips that rgrp and bypasses
>     the call to gfs2_rgrp_bh_get there as well.
> 4. update_rgrp_lvb sees the lvb MAGIC number is valid, so bypasses
>     gfs2_rgrp_bh_get, but it still sets sets GFS2_RDF_UPTODATE due
>     to this invalid assumption.
> 5. The next time update_rgrp_lvb is called, it sees the bit is set
>     and just returns 0, assuming both the lvb and rgrp are both
>     uptodate. But since this is a smaller allocation, or space has
>     been freed by another node, thus adjusting the lvb values,
>     it decides to use the rgrp for allocations, with invalid rd_free
>     due to the fact it was never updated.
>
> This patch changes update_rgrp_lvb so it doesn't set the UPTODATE
> flag anymore. That way, it has no choice but to fetch the latest
> values.
>
> Signed-off-by: Bob Peterson <rpeterso@redhat.com>
> ---
>   fs/gfs2/rgrp.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/fs/gfs2/rgrp.c b/fs/gfs2/rgrp.c
> index 44a4cbc043dd..fc181c81cca2 100644
> --- a/fs/gfs2/rgrp.c
> +++ b/fs/gfs2/rgrp.c
> @@ -1249,7 +1249,7 @@ static int update_rgrp_lvb(struct gfs2_rgrpd *rgd)
>   	rl_flags = be32_to_cpu(rgd->rd_rgl->rl_flags);
>   	rl_flags &= ~GFS2_RDF_MASK;
>   	rgd->rd_flags &= GFS2_RDF_MASK;
> -	rgd->rd_flags |= (rl_flags | GFS2_RDF_UPTODATE | GFS2_RDF_CHECK);
> +	rgd->rd_flags |= (rl_flags | GFS2_RDF_CHECK);
>   	if (rgd->rd_rgl->rl_unlinked == 0)
>   		rgd->rd_flags &= ~GFS2_RDF_CHECK;
>   	rgd->rd_free = be32_to_cpu(rgd->rd_rgl->rl_free);
>