From: Dave Chinner <david@fromorbit.com>
To: linux-xfs@vger.kernel.org
Subject: [PATCH 2/5] xfs: fix sparse warning in xfs_extent_busy_clear
Date: Wed, 3 Apr 2024 08:28:29 +1100 [thread overview]
Message-ID: <20240402213541.1199959-3-david@fromorbit.com> (raw)
In-Reply-To: <20240402213541.1199959-1-david@fromorbit.com>
From: Dave Chinner <dchinner@redhat.com>
Sparse reports:
fs/xfs/xfs_extent_busy.c:588:17: warning: context imbalance in 'xfs_extent_busy_clear' - unexpected unlock
But there is no locking bug here. Sparse simply doesn't understand
the logic and locking in the busy extent processing loop.
xfs_extent_busy_put_pag() has an annotation to suppresses an
unexpected unlock warning, but that isn't sufficient.
If we move the pag existence check into xfs_extent_busy_put_pag() and
annotate that with a __release() so that this function always
appears to release the pag->pagb_lock, sparse now thinks the loop
locking is balanced (one unlock, one lock per loop) but still throws
an unexpected unlock warning after loop cleanup.
i.e. it does not understand that we enter the loop without any locks
held and exit it with the last lock still held. Whilst the locking
within the loop is inow balanced, we need to add an __acquire() to
xfs_extent_busy_clear() to set the initial lock context needed to
avoid false warnings.
Signed-off-by: Dave Chinner <dchinner@redhat.com>
---
fs/xfs/xfs_extent_busy.c | 27 +++++++++++++++++++++++----
1 file changed, 23 insertions(+), 4 deletions(-)
diff --git a/fs/xfs/xfs_extent_busy.c b/fs/xfs/xfs_extent_busy.c
index 56cfa1498571..686b67372030 100644
--- a/fs/xfs/xfs_extent_busy.c
+++ b/fs/xfs/xfs_extent_busy.c
@@ -534,12 +534,24 @@ xfs_extent_busy_clear_one(
kfree(busyp);
}
+/*
+ * Sparse has real trouble with the structure of xfs_extent_busy_clear() and it
+ * is impossible to annotate it correctly if we leave the 'if (pag)' conditional
+ * in xfs_extent_busy_clear(). Hence we always "release" the lock in
+ * xfs_extent_busy_put_pag() so sparse only ever sees one possible path to
+ * drop the lock.
+ */
static void
xfs_extent_busy_put_pag(
struct xfs_perag *pag,
bool wakeup)
__releases(pag->pagb_lock)
{
+ if (!pag) {
+ __release(pag->pagb_lock);
+ return;
+ }
+
if (wakeup) {
pag->pagb_gen++;
wake_up_all(&pag->pagb_wait);
@@ -565,10 +577,18 @@ xfs_extent_busy_clear(
xfs_agnumber_t agno = NULLAGNUMBER;
bool wakeup = false;
+ /*
+ * Sparse thinks the locking in the loop below is balanced (one unlock,
+ * one lock per loop iteration) and doesn't understand that we enter
+ * with no lock held and exit with a lock held. Hence we need to
+ * "acquire" the lock to create the correct initial condition for the
+ * cleanup after loop termination to avoid an unexpected unlock warning.
+ */
+ __acquire(pag->pagb_lock);
+
list_for_each_entry_safe(busyp, n, list, list) {
if (busyp->agno != agno) {
- if (pag)
- xfs_extent_busy_put_pag(pag, wakeup);
+ xfs_extent_busy_put_pag(pag, wakeup);
agno = busyp->agno;
pag = xfs_perag_get(mp, agno);
spin_lock(&pag->pagb_lock);
@@ -584,8 +604,7 @@ xfs_extent_busy_clear(
}
}
- if (pag)
- xfs_extent_busy_put_pag(pag, wakeup);
+ xfs_extent_busy_put_pag(pag, wakeup);
}
/*
--
2.43.0
next prev parent reply other threads:[~2024-04-02 21:35 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-04-02 21:28 [PATCH 0/5] xfs: sparse warning fixes Dave Chinner
2024-04-02 21:28 ` [PATCH 1/5] xfs: fix CIL sparse lock context warnings Dave Chinner
2024-04-03 3:53 ` Christoph Hellwig
2024-04-02 21:28 ` Dave Chinner [this message]
2024-04-03 4:04 ` [PATCH 2/5] xfs: fix sparse warning in xfs_extent_busy_clear Darrick J. Wong
2024-04-03 4:32 ` Christoph Hellwig
2024-04-02 21:28 ` [PATCH 3/5] xfs: silence sparse warning when checking version number Dave Chinner
2024-04-03 3:57 ` Darrick J. Wong
2024-04-03 4:35 ` Christoph Hellwig
2024-04-02 21:28 ` [PATCH 4/5] xfs: remove unused is_rt_data_fork() function Dave Chinner
2024-04-03 3:54 ` Darrick J. Wong
2024-04-03 4:35 ` Christoph Hellwig
2024-04-02 21:28 ` [PATCH 5/5] xfs: fix sparse warnings about unused interval tree functions Dave Chinner
2024-04-03 3:53 ` Darrick J. Wong
2024-04-03 4:39 ` Christoph Hellwig
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=20240402213541.1199959-3-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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.