Re: [PATCH 2/5] xfs: fix sparse warning in xfs_extent_busy_clear

Christoph Hellwig
2024-04-03T04:32:55Z

So while this works it looks pretty ugly. I'd rewrite the nested loop
as what it is: a nested loop. Something like the patch below. Only
compile tested so far, but I'll kick off a real test later.

diff --git a/fs/xfs/xfs_extent_busy.c b/fs/xfs/xfs_extent_busy.c
index 2ccde32c9a9e97..1334360128662a 100644
--- a/fs/xfs/xfs_extent_busy.c
+++ b/fs/xfs/xfs_extent_busy.c
@@ -533,21 +533,6 @@ xfs_extent_busy_clear_one(
kmem_free(busyp);
}

-static void
-xfs_extent_busy_put_pag(
- struct xfs_perag *pag,
- bool wakeup)
- __releases(pag->pagb_lock)
-{
- if (wakeup) {
- pag->pagb_gen++;
- wake_up_all(&pag->pagb_wait);
- }
-
- spin_unlock(&pag->pagb_lock);
- xfs_perag_put(pag);
-}
-
/*
* Remove all extents on the passed in list from the busy extents tree.
* If do_discard is set skip extents that need to be discarded, and mark
@@ -559,32 +544,43 @@ xfs_extent_busy_clear(
struct list_head *list,
bool do_discard)
{
- struct xfs_extent_busy *busyp, *n;
- struct xfs_perag *pag = NULL;
- xfs_agnumber_t agno = NULLAGNUMBER;
- bool wakeup = false;
-
- list_for_each_entry_safe(busyp, n, list, list) {
- if (busyp->agno != agno) {
- if (pag)
- xfs_extent_busy_put_pag(pag, wakeup);
- agno = busyp->agno;
- pag = xfs_perag_get(mp, agno);
- spin_lock(&pag->pagb_lock);
- wakeup = false;
- }
+ struct xfs_extent_busy *busyp;

- if (do_discard && busyp->length &&
- !(busyp->flags & XFS_EXTENT_BUSY_SKIP_DISCARD)) {
- busyp->flags = XFS_EXTENT_BUSY_DISCARDED;
- } else {
- xfs_extent_busy_clear_one(mp, pag, busyp);
- wakeup = true;
- }
- }
+ busyp = list_first_entry_or_null(list, typeof(*busyp), list);
+ if (!busyp)
+ return;
+
+ do {
+ struct xfs_perag *pag = xfs_perag_get(mp, busyp->agno);
+ bool wakeup = false;
+ struct xfs_extent_busy *next;

- if (pag)
- xfs_extent_busy_put_pag(pag, wakeup);
+ spin_lock(&pag->pagb_lock);
+ for (;;) {
+ next = list_next_entry(busyp, list);
+
+ if (do_discard && busyp->length &&
+ !(busyp->flags & XFS_EXTENT_BUSY_SKIP_DISCARD)) {
+ busyp->flags = XFS_EXTENT_BUSY_DISCARDED;
+ } else {
+ xfs_extent_busy_clear_one(mp, pag, busyp);
+ wakeup = true;
+ }
+
+ if (list_entry_is_head(next, list, list) ||
+ next->agno != pag->pag_agno)
+ break;
+ busyp = next;
+ }
+ if (wakeup) {
+ pag->pagb_gen++;
+ wake_up_all(&pag->pagb_wait);
+ }
+ spin_unlock(&pag->pagb_lock);
+ xfs_perag_put(pag);
+
+ busyp = next;
+ } while (!list_entry_is_head(busyp, list, list));
}

/*