[PATCH 3/3] xfs: clean up xfs_dialloc() by introducing __xfs_dialloc()

Gao Xiang
2020-11-24T15:52:34Z

Move the main loop out into a separated function, so we can save
many extra xfs_perag_put()s and gotoes to make the logic cleaner.

Also it can make the modification of perag protection by some lock
for shrinking in the future somewhat easier.

Signed-off-by: Gao Xiang <hsiangkao@redhat.com>
---
It tries to kill multiple goto exits... which makes the logic hard
to read and modify.

not quite sure the name of __xfs_dialloc(), cannot think of some
better name since xfs_dialloc_ag is used...

fs/xfs/libxfs/xfs_ialloc.c | 166 ++++++++++++++++++++-----------------
1 file changed, 88 insertions(+), 78 deletions(-)

diff --git a/fs/xfs/libxfs/xfs_ialloc.c b/fs/xfs/libxfs/xfs_ialloc.c
index 5c8b0210aad3..937455c50570 100644
--- a/fs/xfs/libxfs/xfs_ialloc.c
+++ b/fs/xfs/libxfs/xfs_ialloc.c
@@ -1681,6 +1681,83 @@ xfs_dialloc_ag(
return error;
}

+/*
+ * Return 0 for successfully allocating some inodes in this AG;
+ * 1 for skipping to allocating in the next AG;
+ * < 0 for error code.
+ */
+static int
+__xfs_dialloc(
+ struct xfs_trans *tp,
+ xfs_ino_t parent,
+ struct xfs_perag *pag,
+ struct xfs_buf **IO_agbp,
+ xfs_ino_t *inop,
+ bool okalloc)
+{
+ struct xfs_mount *mp = tp->t_mountp;
+ xfs_agnumber_t agno = pag->pag_agno;
+ struct xfs_buf *agbp;
+ int error;
+
+ if (!pag->pagi_inodeok) {
+ xfs_ialloc_next_ag(mp);
+ return 1;
+ }
+
+ if (!pag->pagi_init) {
+ error = xfs_ialloc_pagi_init(mp, tp, agno);
+ if (error)
+ return error;
+ }
+
+ /*
+ * Do a first racy fast path check if this AG is usable.
+ */
+ if (!pag->pagi_freecount && !okalloc)
+ return 1;
+
+ /*
+ * Then read in the AGI buffer and recheck with the AGI buffer
+ * lock held.
+ */
+ error = xfs_ialloc_read_agi(mp, tp, agno, &agbp);
+ if (error)
+ return error;
+
+ if (pag->pagi_freecount) {
+ *IO_agbp = NULL;
+ return xfs_dialloc_ag(tp, agbp, parent, inop);
+ }
+
+ if (okalloc) {
+ error = xfs_ialloc_ag_alloc(tp, agbp);
+ if (error < 0) {
+ xfs_trans_brelse(tp, agbp);
+ if (error != -ENOSPC)
+ return error;
+
+ *inop = NULLFSINO;
+ return 0;
+ }
+
+ if (!error) {
+ /*
+ * We successfully allocated some inodes, return
+ * the current context to the caller so that it
+ * can commit the current transaction and call
+ * us again where we left off.
+ */
+ ASSERT(pag->pagi_freecount > 0);
+ *IO_agbp = agbp;
+ *inop = NULLFSINO;
+ return 0;
+ }
+ }
+ xfs_trans_brelse(tp, agbp);
+ return 1;
+}
+
/*
* Allocate an inode on disk.
*
@@ -1711,7 +1788,6 @@ xfs_dialloc(
xfs_ino_t *inop)
{
struct xfs_mount *mp = tp->t_mountp;
- struct xfs_buf *agbp;
xfs_agnumber_t agno;
int error;
bool noroom = false;
@@ -1726,8 +1802,9 @@ xfs_dialloc(
* continue where we left off before. In this case, we
* know that the allocation group has free inodes.
*/
- agbp = *IO_agbp;
- goto out_alloc;
+ error = xfs_dialloc_ag(tp, *IO_agbp, parent, inop);
+ *IO_agbp = NULL;
+ return error;
}

/*
@@ -1761,87 +1838,20 @@ xfs_dialloc(
* allocation groups upward, wrapping at the end.
*/
agno = start_agno;
- for (;;) {
+ do {
pag = xfs_perag_get(mp, agno);
- if (!pag->pagi_inodeok) {
- xfs_ialloc_next_ag(mp);
- goto nextag;
- }
-
- if (!pag->pagi_init) {
- error = xfs_ialloc_pagi_init(mp, tp, agno);
- if (error)
- goto out_error;
- }
-
- /*
- * Do a first racy fast path check if this AG is usable.
- */
- if (!pag->pagi_freecount && !okalloc)
- goto nextag;
-
- /*
- * Then read in the AGI buffer and recheck with the AGI buffer
- * lock held.
- */
- error = xfs_ialloc_read_agi(mp, tp, agno, &agbp);
- if (error)
- goto out_error;
-
- if (pag->pagi_freecount) {
- xfs_perag_put(pag);
- goto out_alloc;
- }
-
- if (!okalloc)
- goto nextag_relse_buffer;
-
-
- error = xfs_ialloc_ag_alloc(tp, agbp);
- if (error < 0) {
- xfs_trans_brelse(tp, agbp);
-
- if (error != -ENOSPC)
- goto out_error;
-
- xfs_perag_put(pag);
- *inop = NULLFSINO;
- return 0;
- }
-
- if (!error) {
- /*
- * We successfully allocated some inodes, return
- * the current context to the caller so that it
- * can commit the current transaction and call
- * us again where we left off.
- */
- ASSERT(pag->pagi_freecount > 0);
- xfs_perag_put(pag);
+ error = __xfs_dialloc(tp, parent, pag, IO_agbp, inop, okalloc);
+ xfs_perag_put(pag);

- *IO_agbp = agbp;
- *inop = NULLFSINO;
- return 0;
- }
+ if (error <= 0)
+ return error;

-nextag_relse_buffer:
- xfs_trans_brelse(tp, agbp);
-nextag:
- xfs_perag_put(pag);
if (++agno == mp->m_sb.sb_agcount)
agno = 0;
- if (agno == start_agno) {
- *inop = NULLFSINO;
- return noroom ? -ENOSPC : 0;
- }
- }
+ } while (agno != start_agno);

-out_alloc:
- *IO_agbp = NULL;
- return xfs_dialloc_ag(tp, agbp, parent, inop);
-out_error:
- xfs_perag_put(pag);
- return error;
+ *inop = NULLFSINO;
+ return noroom ? -ENOSPC : 0;
}

/*
--
2.18.4