From: Andreas Gruenbacher <agruenba@redhat.com>
To: gfs2@lists.linux.dev
Cc: Andreas Gruenbacher <agruenba@redhat.com>
Subject: [PATCH 07/13] gfs2: Fix and clean up function do_qc
Date: Tue, 18 Jun 2024 18:05:11 +0200 [thread overview]
Message-ID: <20240618160517.901589-8-agruenba@redhat.com> (raw)
In-Reply-To: <20240618160517.901589-1-agruenba@redhat.com>
Function do_qc() is supposed to be conceptually simple: it alters the
current in-memory and on-disk quota change values for a given uid/gid by
a given delta. If the on-disk record isn't defined yet, a new record is
created. If the on-disk record exists and the resulting change value is
zero, there no longer is a need for that record and so the record is
deleted. On top of that, some reference counting is involved when
creating and deleting records.
Currently, instead of doing the above, do_qc() alters the on-disk value
and then it sets the in-memory value to the on-disk value. This is
incorrect when the on-disk value differs from the in-memory value. The
two values are allowed to differ when quota changes are synced to the
global quota file. Fix by changing both values by the same amount.
In addition, do_qc() currently gets confused when the delta value is 0.
It isn't supposed to be called that way, but that assumption isn't
mentioned and it makes the code harder to read. Make the code more
explicit.
Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com>
---
fs/gfs2/quota.c | 34 +++++++++++++++++++++-------------
1 file changed, 21 insertions(+), 13 deletions(-)
diff --git a/fs/gfs2/quota.c b/fs/gfs2/quota.c
index 371fad7d64e7..29a69756cc32 100644
--- a/fs/gfs2/quota.c
+++ b/fs/gfs2/quota.c
@@ -702,32 +702,40 @@ static void do_qc(struct gfs2_quota_data *qd, s64 change)
mutex_lock(&sdp->sd_quota_mutex);
gfs2_trans_add_meta(ip->i_gl, qd->qd_bh);
- if (!test_bit(QDF_CHANGE, &qd->qd_flags)) {
- qc->qc_change = 0;
- qc->qc_flags = 0;
- if (qd->qd_id.type == USRQUOTA)
- qc->qc_flags = cpu_to_be32(GFS2_QCF_USER);
- qc->qc_id = cpu_to_be32(from_kqid(&init_user_ns, qd->qd_id));
- }
+ /*
+ * The QDF_CHANGE flag indicates that the slot in the quota change file
+ * is used. Here, we use the value of qc->qc_change when the slot is
+ * used, and we assume a value of 0 otherwise.
+ */
- x = be64_to_cpu(qc->qc_change) + change;
- qc->qc_change = cpu_to_be64(x);
+ x = 0;
+ if (test_bit(QDF_CHANGE, &qd->qd_flags))
+ x = be64_to_cpu(qc->qc_change);
+ x += change;
spin_lock(&qd_lock);
- qd->qd_change = x;
+ qd->qd_change += change;
spin_unlock(&qd_lock);
- if (!x) {
- gfs2_assert_warn(sdp, test_bit(QDF_CHANGE, &qd->qd_flags));
+ if (!x && test_bit(QDF_CHANGE, &qd->qd_flags)) {
+ /* The slot in the quota change file becomes unused. */
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)) {
+ } else if (x && !test_bit(QDF_CHANGE, &qd->qd_flags)) {
+ /* The slot in the quota change file becomes used. */
+ set_bit(QDF_CHANGE, &qd->qd_flags);
qd_hold(qd);
slot_hold(qd);
+
+ qc->qc_flags = 0;
+ if (qd->qd_id.type == USRQUOTA)
+ qc->qc_flags = cpu_to_be32(GFS2_QCF_USER);
+ qc->qc_id = cpu_to_be32(from_kqid(&init_user_ns, qd->qd_id));
}
+ qc->qc_change = cpu_to_be64(x);
if (change < 0) /* Reset quiet flag if we freed some blocks */
clear_bit(QDF_QMSG_QUIET, &qd->qd_flags);
--
2.45.1
next prev parent reply other threads:[~2024-06-18 16:05 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-06-18 16:05 [PATCH 00/13] gfs2: quota changes Andreas Gruenbacher
2024-06-18 16:05 ` [PATCH 01/13] gfs2: Minor gfs2_quota_init error path cleanup Andreas Gruenbacher
2024-06-18 16:05 ` [PATCH 02/13] gfs2: Check quota consistency on mount Andreas Gruenbacher
2024-06-18 16:05 ` [PATCH 03/13] gfs2: Revert "introduce qd_bh_get_or_undo" Andreas Gruenbacher
2024-06-18 16:05 ` [PATCH 04/13] gfs2: qd_check_sync cleanups Andreas Gruenbacher
2024-06-18 16:05 ` [PATCH 05/13] gfs2: Revert "ignore negated quota changes" Andreas Gruenbacher
2024-06-18 16:05 ` [PATCH 06/13] gfs2: Revert "Add quota_change type" Andreas Gruenbacher
2024-06-18 16:05 ` Andreas Gruenbacher [this message]
2024-06-18 16:05 ` [PATCH 08/13] gfs2: quota need_sync cleanup Andreas Gruenbacher
2024-06-18 16:05 ` [PATCH 09/13] gfs2: Fold qd_fish into gfs2_quota_sync Andreas Gruenbacher
2024-06-18 16:05 ` [PATCH 10/13] gfs2: Add some missing quota locking Andreas Gruenbacher
2024-06-18 16:05 ` [PATCH 11/13] gfs2: Get rid of some unnecessary " Andreas Gruenbacher
2024-06-20 14:22 ` Alexander Aring
2024-06-20 14:45 ` Andreas Gruenbacher
2024-06-18 16:05 ` [PATCH 12/13] gfs2: Be more careful with the quota sync generation Andreas Gruenbacher
2024-06-18 16:05 ` [PATCH 13/13] gfs2: Revert "check for no eligible quota changes" Andreas Gruenbacher
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20240618160517.901589-8-agruenba@redhat.com \
--to=agruenba@redhat.com \
--cc=gfs2@lists.linux.dev \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).