Hi,
On 10/01/18 20:42, Bob Peterson wrote:
Function gfs2_rgrp_congested is called in two places: once before
we acquire the rgrp glock, and once after. The first time, it checks
to see if the rgrp was used recently because the usage statistics
are only valid in short-term. For the second call, that check was
not done, but should have been, for the same reason.
This patch simply moves function gfs2_rgrp_used_recently before
function gfs2_rgrp_congested, and it's now called from there so
it applies equally to both checks.
I don't think this makes sense. The purpose of the
gfs2_rgrp_used_recently test is that when we don't hold the glock we
might not have up to date stats to make decisions from, so we check that
we've held the lock recently enough that we can use historical
statistics. Once we hold the lock however, then we can be sure that the
stats are up to date, as the process of acquiring the lock will bring
them up to date, so why repeat the test when we already have the lock?
Steve.
Signed-off-by: Bob Peterson <[email protected]>
---
fs/gfs2/rgrp.c | 36 ++++++++++++++++++++----------------
1 file changed, 20 insertions(+), 16 deletions(-)
diff --git a/fs/gfs2/rgrp.c b/fs/gfs2/rgrp.c
index ea95d7c70e23..960fbc2d39d7 100644
--- a/fs/gfs2/rgrp.c
+++ b/fs/gfs2/rgrp.c
@@ -1819,6 +1819,21 @@ static void try_rgrp_unlink(struct gfs2_rgrpd *rgd, u64
*last_unlinked, u64 skip
return;
}
+/**
+ * gfs2_rgrp_used_recently - check if a rgrp has been used recently
+ * @rgd: The rgrp to test
+ * @msecs: The time limit in milliseconds
+ *
+ * Returns: True if the rgrp glock has been used within the time limit
+ */
+static inline bool gfs2_rgrp_used_recently(const struct gfs2_rgrpd *rgd,
+ u64 msecs)
+{
+ return (ktime_before(ktime_get_real(),
+ ktime_add(rgd->rd_gl->gl_dstamp,
+ ms_to_ktime(msecs))));
+}
+
/**
* gfs2_rgrp_congested - Use stats to figure out whether an rgrp is congested
* @rgd: The rgrp in question
@@ -1858,6 +1873,11 @@ static bool gfs2_rgrp_congested(const struct gfs2_rgrpd
*rgd, int loops)
u64 var;
int cpu, nonzero = 0;
+ /* If it hasn't been used recently we can't judge the statistics, so
+ assume it's not congested. */
+ if (!gfs2_rgrp_used_recently(rgd, HZ))
+ return false;
+
preempt_disable();
for_each_present_cpu(cpu) {
st = &per_cpu_ptr(sdp->sd_lkstats, cpu)->lkstats[LM_TYPE_RGRP];
@@ -1892,21 +1912,6 @@ static bool gfs2_rgrp_congested(const struct gfs2_rgrpd
*rgd, int loops)
return ((srttb_diff < 0) && (sqr_diff > var));
}
-/**
- * gfs2_rgrp_used_recently - check if a rgrp has been used recently
- * @rgd: The rgrp to test
- * @msecs: The time limit in milliseconds
- *
- * Returns: True if the rgrp glock has been used within the time limit
- */
-static inline bool gfs2_rgrp_used_recently(const struct gfs2_rgrpd *rgd,
- u64 msecs)
-{
- return (ktime_before(ktime_get_real(),
- ktime_add(rgd->rd_gl->gl_dstamp,
- ms_to_ktime(msecs))));
-}
-
static u32 gfs2_orlov_skip(const struct gfs2_inode *ip)
{
const struct gfs2_sbd *sdp = GFS2_SB(&ip->i_inode);
@@ -2004,7 +2009,6 @@ int gfs2_inplace_reserve(struct gfs2_inode *ip, struct
gfs2_alloc_parms *ap)
!fast_to_acquire(rs->rs_rbm.rgd))
goto next_rgrp;
if ((loops < 2) &&
- gfs2_rgrp_used_recently(rs->rs_rbm.rgd, HZ)
&&
gfs2_rgrp_congested(rs->rs_rbm.rgd, loops))
goto next_rgrp;
}
