public inbox for linux-xfs@vger.kernel.org
 help / color / mirror / Atom feed
From: Dave Chinner <david@fromorbit.com>
To: linux-xfs@vger.kernel.org
Subject: [PATCH 1/4] xfs: don't nest icloglock inside ic_callback_lock
Date: Tue, 22 Jun 2021 14:06:01 +1000	[thread overview]
Message-ID: <20210622040604.1290539-2-david@fromorbit.com> (raw)
In-Reply-To: <20210622040604.1290539-1-david@fromorbit.com>

From: Dave Chinner <dchinner@redhat.com>

It's completely unnecessary because callbacks are added to iclogs
without holding the icloglock, hence no amount of ordering between
the icloglock and ic_callback_lock will order the removal of
callbacks from the iclog.

Signed-off-by: Dave Chinner <dchinner@redhat.com>
---
 fs/xfs/xfs_log.c | 18 ++++--------------
 1 file changed, 4 insertions(+), 14 deletions(-)

diff --git a/fs/xfs/xfs_log.c b/fs/xfs/xfs_log.c
index e93cac6b5378..bb4390942275 100644
--- a/fs/xfs/xfs_log.c
+++ b/fs/xfs/xfs_log.c
@@ -2773,11 +2773,8 @@ static void
 xlog_state_do_iclog_callbacks(
 	struct xlog		*log,
 	struct xlog_in_core	*iclog)
-		__releases(&log->l_icloglock)
-		__acquires(&log->l_icloglock)
 {
 	trace_xlog_iclog_callbacks_start(iclog, _RET_IP_);
-	spin_unlock(&log->l_icloglock);
 	spin_lock(&iclog->ic_callback_lock);
 	while (!list_empty(&iclog->ic_callbacks)) {
 		LIST_HEAD(tmp);
@@ -2789,12 +2786,6 @@ xlog_state_do_iclog_callbacks(
 		spin_lock(&iclog->ic_callback_lock);
 	}
 
-	/*
-	 * Pick up the icloglock while still holding the callback lock so we
-	 * serialise against anyone trying to add more callbacks to this iclog
-	 * now we've finished processing.
-	 */
-	spin_lock(&log->l_icloglock);
 	spin_unlock(&iclog->ic_callback_lock);
 	trace_xlog_iclog_callbacks_done(iclog, _RET_IP_);
 }
@@ -2836,13 +2827,12 @@ xlog_state_do_callback(
 				iclog = iclog->ic_next;
 				continue;
 			}
+			spin_unlock(&log->l_icloglock);
 
-			/*
-			 * Running callbacks will drop the icloglock which means
-			 * we'll have to run at least one more complete loop.
-			 */
-			cycled_icloglock = true;
 			xlog_state_do_iclog_callbacks(log, iclog);
+			cycled_icloglock = true;
+
+			spin_lock(&log->l_icloglock);
 			if (XLOG_FORCED_SHUTDOWN(log))
 				wake_up_all(&iclog->ic_force_wait);
 			else
-- 
2.31.1


  reply	other threads:[~2021-06-22  4:06 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-06-22  4:06 [PATCH 0/4] xfs: fix CIL shutdown UAF and shutdown hang Dave Chinner
2021-06-22  4:06 ` Dave Chinner [this message]
2021-06-22 12:38   ` [PATCH 1/4] xfs: don't nest icloglock inside ic_callback_lock Brian Foster
2021-06-22 22:42     ` Dave Chinner
2021-06-23 10:18       ` Brian Foster
2021-06-25 20:52   ` Darrick J. Wong
2021-06-22  4:06 ` [PATCH 2/4] xfs: remove callback dequeue loop from xlog_state_do_iclog_callbacks Dave Chinner
2021-06-22 12:39   ` Brian Foster
2021-06-22 22:56     ` Dave Chinner
2021-06-25 20:57   ` Darrick J. Wong
2021-06-22  4:06 ` [PATCH 3/4] xfs: Fix a CIL UAF by getting get rid of the iclog callback lock Dave Chinner
2021-06-22 12:41   ` Brian Foster
2021-06-25 21:02   ` Darrick J. Wong
2021-06-22  4:06 ` [PATCH 4/4] xfs: don't wait on future iclogs when pushing the CIL Dave Chinner
2021-06-22 12:41   ` Brian Foster
2021-06-25 21:02   ` Darrick J. Wong

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=20210622040604.1290539-2-david@fromorbit.com \
    --to=david@fromorbit.com \
    --cc=linux-xfs@vger.kernel.org \
    /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