From mboxrd@z Thu Jan 1 00:00:00 1970 From: Steven Whitehouse Date: Mon, 20 Aug 2018 09:47:43 +0100 Subject: [Cluster-devel] [GFS2 PATCH] gfs2: Don't set GFS2_RDF_UPTODATE when the lvb is updated In-Reply-To: <702883679.3490088.1534521078349.JavaMail.zimbra@redhat.com> References: <702883679.3490088.1534521078349.JavaMail.zimbra@redhat.com> Message-ID: <0796de99-7b37-e1a8-30fe-ddebe0ac33bf@redhat.com> List-Id: To: cluster-devel.redhat.com MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Acked-by: Steven Whitehouse 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 > --- > 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); >