gfs2: Add quota_change type
authorBob Peterson <rpeterso@redhat.com>
Wed, 28 Jun 2023 14:54:41 +0000 (09:54 -0500)
committerAndreas Gruenbacher <agruenba@redhat.com>
Mon, 3 Jul 2023 20:30:48 +0000 (22:30 +0200)
Function do_qc has two main uses: (1) to re-sync the local quota changes
(qd) to the master quotas, and (2) normal quota changes. In the case of
normal quota changes, the change can be positive or negative, as the
quota usage goes up and down.

Before this patch function do_qc was distinguishing one from another by
whether the resulting value is or isn't zero: In the case of a re-sync
(called do_sync) the quota value is moved from the temporary value to a
master value, so the amount is added to one and subtracted from the
other. The problem is that since the values can be positive or negative
we can occasionally run into situations where we are not doing a re-sync
but the quota change just happens to cancel out the previous value.

In the case of a re-sync extra references and locks are taken, and so
do_qc needs to release them. In the case of a normal quota change, no
extra references and locks are taken, so it must not try to release
them.

The problem is: if the quota change is not a re-sync but the value just
happens to cancel out the original quota change, the resulting zero
value fools do_qc into thinking this is a re-sync and therefore it must
release the extra references. This results in problems, mainly having to
do with slot reference numbers going smaller than zero.

This patch introduces new constants, QC_SYNC and QC_CHANGE so do_qc can
really tell the difference. For QC_SYNC calls it must release the extra
references acquired by gfs2_quota_unlock's call to qd_check_sync. For
QC_CHANGE calls it does not have extra references to put.

Note that this allows quota changes back to a value of zero, and so I
removed an assert warning related to that.

Signed-off-by: Bob Peterson <rpeterso@redhat.com>
Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com>
fs/gfs2/quota.c
fs/gfs2/util.c

index 7765346..704192b 100644 (file)
@@ -75,6 +75,9 @@
 #define GFS2_QD_HASH_SIZE       BIT(GFS2_QD_HASH_SHIFT)
 #define GFS2_QD_HASH_MASK       (GFS2_QD_HASH_SIZE - 1)
 
+#define QC_CHANGE 0
+#define QC_SYNC 1
+
 /* Lock order: qd_lock -> bucket lock -> qd->lockref.lock -> lru lock */
 /*                     -> sd_bitmap_lock                              */
 static DEFINE_SPINLOCK(qd_lock);
@@ -470,7 +473,6 @@ static int qd_fish(struct gfs2_sbd *sdp, struct gfs2_quota_data **qdp)
        spin_unlock(&qd_lock);
 
        if (qd) {
-               gfs2_assert_warn(sdp, qd->qd_change_sync);
                error = bh_get(qd);
                if (error) {
                        clear_bit(QDF_LOCKED, &qd->qd_flags);
@@ -662,7 +664,7 @@ static int sort_qd(const void *a, const void *b)
        return 0;
 }
 
-static void do_qc(struct gfs2_quota_data *qd, s64 change)
+static void do_qc(struct gfs2_quota_data *qd, s64 change, int qc_type)
 {
        struct gfs2_sbd *sdp = qd->qd_gl->gl_name.ln_sbd;
        struct gfs2_inode *ip = GFS2_I(sdp->sd_qc_inode);
@@ -687,16 +689,18 @@ static void do_qc(struct gfs2_quota_data *qd, s64 change)
        qd->qd_change = x;
        spin_unlock(&qd_lock);
 
-       if (!x) {
+       if (qc_type == QC_CHANGE) {
+               if (!test_and_set_bit(QDF_CHANGE, &qd->qd_flags)) {
+                       qd_hold(qd);
+                       slot_hold(qd);
+               }
+       } else {
                gfs2_assert_warn(sdp, test_bit(QDF_CHANGE, &qd->qd_flags));
                clear_bit(QDF_CHANGE, &qd->qd_flags);
                qc->qc_flags = 0;
                qc->qc_id = 0;
                slot_put(qd);
                qd_put(qd);
-       } else if (!test_and_set_bit(QDF_CHANGE, &qd->qd_flags)) {
-               qd_hold(qd);
-               slot_hold(qd);
        }
 
        if (change < 0) /* Reset quiet flag if we freed some blocks */
@@ -953,7 +957,7 @@ static int do_sync(unsigned int num_qd, struct gfs2_quota_data **qda)
                if (error)
                        goto out_end_trans;
 
-               do_qc(qd, -qd->qd_change_sync);
+               do_qc(qd, -qd->qd_change_sync, QC_SYNC);
                set_bit(QDF_REFRESH, &qd->qd_flags);
        }
 
@@ -1279,7 +1283,7 @@ void gfs2_quota_change(struct gfs2_inode *ip, s64 change,
 
                if (qid_eq(qd->qd_id, make_kqid_uid(uid)) ||
                    qid_eq(qd->qd_id, make_kqid_gid(gid))) {
-                       do_qc(qd, change);
+                       do_qc(qd, change, QC_CHANGE);
                }
        }
 }
index b9db034..dac22b1 100644 (file)
@@ -98,12 +98,12 @@ out_unlock:
  */
 int gfs2_freeze_lock_shared(struct gfs2_sbd *sdp)
 {
-       int flags = LM_FLAG_NOEXP | GL_EXACT;
        int error;
 
-       error = gfs2_glock_nq_init(sdp->sd_freeze_gl, LM_ST_SHARED, flags,
+       error = gfs2_glock_nq_init(sdp->sd_freeze_gl, LM_ST_SHARED,
+                                  LM_FLAG_NOEXP | GL_EXACT,
                                   &sdp->sd_freeze_gh);
-       if (error && error != GLR_TRYFAILED)
+       if (error)
                fs_err(sdp, "can't lock the freeze glock: %d\n", error);
        return error;
 }