gfs2.lists.linux.dev archive mirror
 help / color / mirror / Atom feed
From: Andreas Gruenbacher <agruenba@redhat.com>
To: gfs2@lists.linux.dev
Cc: Andreas Gruenbacher <agruenba@redhat.com>
Subject: [PATCH 12/13] gfs2: Be more careful with the quota sync generation
Date: Tue, 18 Jun 2024 18:05:16 +0200	[thread overview]
Message-ID: <20240618160517.901589-13-agruenba@redhat.com> (raw)
In-Reply-To: <20240618160517.901589-1-agruenba@redhat.com>

The quota sync generation is only ever updated under sd_quota_sync_mutex
by gfs2_quota_sync(), but its current value is fetched ouside of that
mutex, so use WRITE_ONCE() and READ_ONCE() when accessing it without
holding that mutex.

Pass the current sync generation to do_sync() from its callers to ensure
that we're not recording the wrong generation when the syncing is
done.  Also, make sure that qd->qd_sync_gen only ever moves forward.

In gfs2_quota_sync(), only write the new sync generation when we know
that there are changes.  This eliminates the need for function
sd_changed(), which we will remove in the next commit.

Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com>
---
 fs/gfs2/quota.c | 27 +++++++++++++++++++--------
 1 file changed, 19 insertions(+), 8 deletions(-)

diff --git a/fs/gfs2/quota.c b/fs/gfs2/quota.c
index 2490117afb96..e27fceee9c63 100644
--- a/fs/gfs2/quota.c
+++ b/fs/gfs2/quota.c
@@ -893,7 +893,8 @@ static int gfs2_adjust_quota(struct gfs2_sbd *sdp, loff_t loc,
 	return err;
 }
 
-static int do_sync(unsigned int num_qd, struct gfs2_quota_data **qda)
+static int do_sync(unsigned int num_qd, struct gfs2_quota_data **qda,
+		   u64 sync_gen)
 {
 	struct gfs2_sbd *sdp = (*qda)->qd_sbd;
 	struct gfs2_inode *ip = GFS2_I(sdp->sd_quota_inode);
@@ -984,8 +985,13 @@ static int do_sync(unsigned int num_qd, struct gfs2_quota_data **qda)
 	gfs2_log_flush(ip->i_gl->gl_name.ln_sbd, ip->i_gl,
 		       GFS2_LOG_HEAD_FLUSH_NORMAL | GFS2_LFC_DO_SYNC);
 	if (!error) {
-		for (x = 0; x < num_qd; x++)
-			qda[x]->qd_sync_gen = sdp->sd_quota_sync_gen;
+		for (x = 0; x < num_qd; x++) {
+			qd = qda[x];
+			spin_lock(&qd->qd_lockref.lock);
+			if (qd->qd_sync_gen < sync_gen)
+				qd->qd_sync_gen = sync_gen;
+			spin_unlock(&qd->qd_lockref.lock);
+		}
 	}
 	return error;
 }
@@ -1179,7 +1185,9 @@ void gfs2_quota_unlock(struct gfs2_inode *ip)
 	}
 
 	if (count) {
-		do_sync(count, qda);
+		u64 sync_gen = READ_ONCE(sdp->sd_quota_sync_gen);
+
+		do_sync(count, qda, sync_gen);
 		for (x = 0; x < count; x++)
 			qd_unlock(qda[x]);
 	}
@@ -1325,6 +1333,7 @@ int gfs2_quota_sync(struct super_block *sb, int type)
 	struct gfs2_sbd *sdp = sb->s_fs_info;
 	struct gfs2_quota_data **qda;
 	unsigned int max_qd = PAGE_SIZE / sizeof(struct gfs2_holder);
+	u64 sync_gen;
 	int error = 0;
 
 	if (sb_rdonly(sdp->sd_vfs))
@@ -1337,7 +1346,7 @@ int gfs2_quota_sync(struct super_block *sb, int type)
 		return -ENOMEM;
 
 	mutex_lock(&sdp->sd_quota_sync_mutex);
-	sdp->sd_quota_sync_gen++;
+	sync_gen = sdp->sd_quota_sync_gen + 1;
 
 	do {
 		struct gfs2_quota_data *iter;
@@ -1346,7 +1355,7 @@ int gfs2_quota_sync(struct super_block *sb, int type)
 
 		spin_lock(&qd_lock);
 		list_for_each_entry(iter, &sdp->sd_quota_list, qd_list) {
-			if (qd_grab_sync(sdp, iter, sdp->sd_quota_sync_gen)) {
+			if (qd_grab_sync(sdp, iter, sync_gen)) {
 				qda[num_qd++] = iter;
 				if (num_qd == max_qd)
 					break;
@@ -1367,8 +1376,10 @@ int gfs2_quota_sync(struct super_block *sb, int type)
 			break;
 		}
 
-		if (!error)
-			error = do_sync(num_qd, qda);
+		if (!error) {
+			WRITE_ONCE(sdp->sd_quota_sync_gen, sync_gen);
+			error = do_sync(num_qd, qda, sync_gen);
+		}
 
 		for (x = 0; x < num_qd; x++)
 			qd_unlock(qda[x]);
-- 
2.45.1


  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 ` [PATCH 07/13] gfs2: Fix and clean up function do_qc Andreas Gruenbacher
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 ` Andreas Gruenbacher [this message]
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-13-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).