[Cluster-devel] Corrections to patch d1475c07f7 "GFS2: rgrp free blocks used incorrectly"

[Bob Peterson <rpeterso@redhat.com>]

Hi,

Just a heads up:

There's a problem with patch d1475c07f7ce737d06560c24bf025533b7969e3b,
"GFS2: rgrp free blocks used incorrectly".

In today's code, block reservations are currently a "hint" about
where to reserve blocks. As long as the resource group (rgrp) glock
is held in EX on the local node, they are more than just hints:
they're skipped by other allocations. But if the rgrp is moved from
one cluster node to another, the reservations are never returned
(at least not until the patch I posted on 13 July is pushed: see
https://www.redhat.com/archives/cluster-devel/2018-July/msg00059.html)

Until that gets pushed, we can have lots of reservations accumulate
on an rgrp, but they won't necessarily reflect the accurate bitmaps
of the rgrp, which means rd_free (and therefore rd_free_clone)
can have fewer free blocks available than the total reservations.

For that reason, the following line from patch d1475c07f7 is incorrect:

+	BUG_ON(rgd->rd_free_clone < tot_reserved);

That's because tot_reserved can exceed rd_free_clone in today's gfs2.

I've had some discussions with Andreas Gruenbacher and Steve
Whitehouse about trying to enforce gfs2 block reservations as more
than just a hint, but until we resolve that, the patch should be
revised. I'm therefore planning to revise that patch like this:

diff --git a/fs/gfs2/rgrp.c b/fs/gfs2/rgrp.c
index 3821b3ab04fa..3632a9a78e3c 100644
--- a/fs/gfs2/rgrp.c
+++ b/fs/gfs2/rgrp.c
@@ -1508,7 +1508,9 @@ static inline u32 rgd_free(struct gfs2_rgrpd *rgd, struct gfs2_blkreserv *rs)
 	BUG_ON(rgd->rd_reserved < rs->rs_free);
 	tot_reserved = rgd->rd_reserved - rs->rs_free;
 
-	BUG_ON(rgd->rd_free_clone < tot_reserved);
+	if (rgd->rd_free_clone < tot_reserved)
+		tot_reserved = 0;
+
 	tot_free = rgd->rd_free_clone - tot_reserved;
 
 	return tot_free;

In other words, if the number of blocks reserved exceeds the number
of free blocks, just return rd_free_clone like we did before.
When those blocks are allocated, function gfs2_adjust_reservation
should then remove the reservation from the rgrp tree and adjust its
count of reserved blocks accordingly.

Later, if we tighten the enforcement of block reservations,
we should change it back into a BUG_ON.

Regards,

Bob Peterson
Red Hat File Systems