* [PATCH 1/5] xfs: remove the in-memory btree header block
2024-01-03 20:38 RFC: in-memory btree simplifications Christoph Hellwig
@ 2024-01-03 20:38 ` Christoph Hellwig
2024-01-04 1:24 ` Darrick J. Wong
2024-01-03 20:38 ` [PATCH 2/5] xfs: remove struct xfboff_bitmap Christoph Hellwig
` (3 subsequent siblings)
4 siblings, 1 reply; 19+ messages in thread
From: Christoph Hellwig @ 2024-01-03 20:38 UTC (permalink / raw)
To: Chandan Babu R, Darrick J. Wong; +Cc: linux-xfs
There is no need to allocate a whole block (aka page) for the fake btree
on-disk header as we can just stash away the nleves and root block in the
xfbtree structure.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
fs/xfs/libxfs/xfs_btree.h | 1 -
fs/xfs/libxfs/xfs_btree_mem.h | 7 --
fs/xfs/libxfs/xfs_rmap_btree.c | 4 +-
fs/xfs/libxfs/xfs_rmap_btree.h | 3 +-
fs/xfs/libxfs/xfs_rtrmap_btree.c | 4 +-
fs/xfs/libxfs/xfs_rtrmap_btree.h | 3 +-
fs/xfs/scrub/rcbag.c | 35 +-----
fs/xfs/scrub/rcbag_btree.c | 4 +-
fs/xfs/scrub/rcbag_btree.h | 3 +-
fs/xfs/scrub/rmap_repair.c | 41 +------
fs/xfs/scrub/rtrmap_repair.c | 35 +-----
fs/xfs/scrub/xfbtree.c | 188 ++-----------------------------
fs/xfs/scrub/xfbtree.h | 24 +---
13 files changed, 35 insertions(+), 317 deletions(-)
diff --git a/fs/xfs/libxfs/xfs_btree.h b/fs/xfs/libxfs/xfs_btree.h
index 64e37a0ffb78ea..503f51ef22f81e 100644
--- a/fs/xfs/libxfs/xfs_btree.h
+++ b/fs/xfs/libxfs/xfs_btree.h
@@ -271,7 +271,6 @@ struct xfbtree;
struct xfs_btree_cur_mem {
struct xfbtree *xfbtree;
- struct xfs_buf *head_bp;
struct xfs_perag *pag;
struct xfs_rtgroup *rtg;
};
diff --git a/fs/xfs/libxfs/xfs_btree_mem.h b/fs/xfs/libxfs/xfs_btree_mem.h
index cfb30cb1aabc69..eeb3340a22d201 100644
--- a/fs/xfs/libxfs/xfs_btree_mem.h
+++ b/fs/xfs/libxfs/xfs_btree_mem.h
@@ -26,8 +26,6 @@ struct xfbtree_config {
#define XFBTREE_DIRECT_MAP (1U << 0)
#ifdef CONFIG_XFS_BTREE_IN_XFILE
-unsigned int xfs_btree_mem_head_nlevels(struct xfs_buf *head_bp);
-
struct xfs_buftarg *xfbtree_target(struct xfbtree *xfbtree);
int xfbtree_check_ptr(struct xfs_btree_cur *cur,
const union xfs_btree_ptr *ptr, int index, int level);
@@ -63,11 +61,6 @@ int xfbtree_alloc_block(struct xfs_btree_cur *cur,
int *stat);
int xfbtree_free_block(struct xfs_btree_cur *cur, struct xfs_buf *bp);
#else
-static inline unsigned int xfs_btree_mem_head_nlevels(struct xfs_buf *head_bp)
-{
- return 0;
-}
-
static inline struct xfs_buftarg *
xfbtree_target(struct xfbtree *xfbtree)
{
diff --git a/fs/xfs/libxfs/xfs_rmap_btree.c b/fs/xfs/libxfs/xfs_rmap_btree.c
index 71887cc23e03f1..41f1b5fa863302 100644
--- a/fs/xfs/libxfs/xfs_rmap_btree.c
+++ b/fs/xfs/libxfs/xfs_rmap_btree.c
@@ -641,7 +641,6 @@ struct xfs_btree_cur *
xfs_rmapbt_mem_cursor(
struct xfs_perag *pag,
struct xfs_trans *tp,
- struct xfs_buf *head_bp,
struct xfbtree *xfbtree)
{
struct xfs_btree_cur *cur;
@@ -653,8 +652,7 @@ xfs_rmapbt_mem_cursor(
xfs_rmapbt_cur_cache);
cur->bc_statoff = XFS_STATS_CALC_INDEX(xs_rmap_2);
cur->bc_mem.xfbtree = xfbtree;
- cur->bc_mem.head_bp = head_bp;
- cur->bc_nlevels = xfs_btree_mem_head_nlevels(head_bp);
+ cur->bc_nlevels = xfbtree->nlevels;
cur->bc_mem.pag = xfs_perag_hold(pag);
return cur;
diff --git a/fs/xfs/libxfs/xfs_rmap_btree.h b/fs/xfs/libxfs/xfs_rmap_btree.h
index 415fad8dad73ed..dfe13b8cbb732d 100644
--- a/fs/xfs/libxfs/xfs_rmap_btree.h
+++ b/fs/xfs/libxfs/xfs_rmap_btree.h
@@ -68,8 +68,7 @@ void xfs_rmapbt_destroy_cur_cache(void);
#ifdef CONFIG_XFS_BTREE_IN_XFILE
struct xfbtree;
struct xfs_btree_cur *xfs_rmapbt_mem_cursor(struct xfs_perag *pag,
- struct xfs_trans *tp, struct xfs_buf *head_bp,
- struct xfbtree *xfbtree);
+ struct xfs_trans *tp, struct xfbtree *xfbtree);
int xfs_rmapbt_mem_create(struct xfs_mount *mp, xfs_agnumber_t agno,
struct xfs_buftarg *target, struct xfbtree **xfbtreep);
#endif /* CONFIG_XFS_BTREE_IN_XFILE */
diff --git a/fs/xfs/libxfs/xfs_rtrmap_btree.c b/fs/xfs/libxfs/xfs_rtrmap_btree.c
index 87ea5e3ca89375..3b105e2da8468d 100644
--- a/fs/xfs/libxfs/xfs_rtrmap_btree.c
+++ b/fs/xfs/libxfs/xfs_rtrmap_btree.c
@@ -643,7 +643,6 @@ struct xfs_btree_cur *
xfs_rtrmapbt_mem_cursor(
struct xfs_rtgroup *rtg,
struct xfs_trans *tp,
- struct xfs_buf *head_bp,
struct xfbtree *xfbtree)
{
struct xfs_btree_cur *cur;
@@ -655,8 +654,7 @@ xfs_rtrmapbt_mem_cursor(
xfs_rtrmapbt_cur_cache);
cur->bc_statoff = XFS_STATS_CALC_INDEX(xs_rmap_2);
cur->bc_mem.xfbtree = xfbtree;
- cur->bc_mem.head_bp = head_bp;
- cur->bc_nlevels = xfs_btree_mem_head_nlevels(head_bp);
+ cur->bc_nlevels = xfbtree->nlevels;
cur->bc_mem.rtg = xfs_rtgroup_hold(rtg);
return cur;
diff --git a/fs/xfs/libxfs/xfs_rtrmap_btree.h b/fs/xfs/libxfs/xfs_rtrmap_btree.h
index b0a8e8d89f9eb4..3347205846eb2e 100644
--- a/fs/xfs/libxfs/xfs_rtrmap_btree.h
+++ b/fs/xfs/libxfs/xfs_rtrmap_btree.h
@@ -208,8 +208,7 @@ unsigned long long xfs_rtrmapbt_calc_size(struct xfs_mount *mp,
#ifdef CONFIG_XFS_BTREE_IN_XFILE
struct xfbtree;
struct xfs_btree_cur *xfs_rtrmapbt_mem_cursor(struct xfs_rtgroup *rtg,
- struct xfs_trans *tp, struct xfs_buf *mhead_bp,
- struct xfbtree *xfbtree);
+ struct xfs_trans *tp, struct xfbtree *xfbtree);
int xfs_rtrmapbt_mem_create(struct xfs_mount *mp, xfs_rgnumber_t rgno,
struct xfs_buftarg *target, struct xfbtree **xfbtreep);
#endif /* CONFIG_XFS_BTREE_IN_XFILE */
diff --git a/fs/xfs/scrub/rcbag.c b/fs/xfs/scrub/rcbag.c
index 63f1b6e6488e15..f28ce02f961c7c 100644
--- a/fs/xfs/scrub/rcbag.c
+++ b/fs/xfs/scrub/rcbag.c
@@ -76,16 +76,11 @@ rcbag_add(
{
struct rcbag_rec bagrec;
struct xfs_mount *mp = bag->mp;
- struct xfs_buf *head_bp;
struct xfs_btree_cur *cur;
int has;
int error;
- error = xfbtree_head_read_buf(bag->xfbtree, tp, &head_bp);
- if (error)
- return error;
-
- cur = rcbagbt_mem_cursor(mp, tp, head_bp, bag->xfbtree);
+ cur = rcbagbt_mem_cursor(mp, tp, bag->xfbtree);
error = rcbagbt_lookup_eq(cur, rmap, &has);
if (error)
goto out_cur;
@@ -118,7 +113,6 @@ rcbag_add(
}
xfs_btree_del_cursor(cur, 0);
- xfs_trans_brelse(tp, head_bp);
error = xfbtree_trans_commit(bag->xfbtree, tp);
if (error)
@@ -129,7 +123,6 @@ rcbag_add(
out_cur:
xfs_btree_del_cursor(cur, error);
- xfs_trans_brelse(tp, head_bp);
xfbtree_trans_cancel(bag->xfbtree, tp);
return error;
}
@@ -157,7 +150,6 @@ rcbag_next_edge(
{
struct rcbag_rec bagrec;
struct xfs_mount *mp = bag->mp;
- struct xfs_buf *head_bp;
struct xfs_btree_cur *cur;
uint32_t next_bno = NULLAGBLOCK;
int has;
@@ -166,11 +158,7 @@ rcbag_next_edge(
if (next_valid)
next_bno = next_rmap->rm_startblock;
- error = xfbtree_head_read_buf(bag->xfbtree, tp, &head_bp);
- if (error)
- return error;
-
- cur = rcbagbt_mem_cursor(mp, tp, head_bp, bag->xfbtree);
+ cur = rcbagbt_mem_cursor(mp, tp, bag->xfbtree);
error = xfs_btree_goto_left_edge(cur);
if (error)
goto out_cur;
@@ -205,14 +193,12 @@ rcbag_next_edge(
}
xfs_btree_del_cursor(cur, 0);
- xfs_trans_brelse(tp, head_bp);
*next_bnop = next_bno;
return 0;
out_cur:
xfs_btree_del_cursor(cur, error);
- xfs_trans_brelse(tp, head_bp);
return error;
}
@@ -225,17 +211,12 @@ rcbag_remove_ending_at(
{
struct rcbag_rec bagrec;
struct xfs_mount *mp = bag->mp;
- struct xfs_buf *head_bp;
struct xfs_btree_cur *cur;
int has;
int error;
- error = xfbtree_head_read_buf(bag->xfbtree, tp, &head_bp);
- if (error)
- return error;
-
/* go to the right edge of the tree */
- cur = rcbagbt_mem_cursor(mp, tp, head_bp, bag->xfbtree);
+ cur = rcbagbt_mem_cursor(mp, tp, bag->xfbtree);
memset(&cur->bc_rec, 0xFF, sizeof(cur->bc_rec));
error = xfs_btree_lookup(cur, XFS_LOOKUP_GE, &has);
if (error)
@@ -271,11 +252,9 @@ rcbag_remove_ending_at(
}
xfs_btree_del_cursor(cur, 0);
- xfs_trans_brelse(tp, head_bp);
return xfbtree_trans_commit(bag->xfbtree, tp);
out_cur:
xfs_btree_del_cursor(cur, error);
- xfs_trans_brelse(tp, head_bp);
xfbtree_trans_cancel(bag->xfbtree, tp);
return error;
}
@@ -288,17 +267,12 @@ rcbag_dump(
{
struct rcbag_rec bagrec;
struct xfs_mount *mp = bag->mp;
- struct xfs_buf *head_bp;
struct xfs_btree_cur *cur;
unsigned long long nr = 0;
int has;
int error;
- error = xfbtree_head_read_buf(bag->xfbtree, tp, &head_bp);
- if (error)
- return;
-
- cur = rcbagbt_mem_cursor(mp, tp, head_bp, bag->xfbtree);
+ cur = rcbagbt_mem_cursor(mp, tp, bag->xfbtree);
error = xfs_btree_goto_left_edge(cur);
if (error)
goto out_cur;
@@ -327,5 +301,4 @@ rcbag_dump(
out_cur:
xfs_btree_del_cursor(cur, error);
- xfs_trans_brelse(tp, head_bp);
}
diff --git a/fs/xfs/scrub/rcbag_btree.c b/fs/xfs/scrub/rcbag_btree.c
index 9807e08129fe4a..6f0b48b5c37bbd 100644
--- a/fs/xfs/scrub/rcbag_btree.c
+++ b/fs/xfs/scrub/rcbag_btree.c
@@ -209,7 +209,6 @@ struct xfs_btree_cur *
rcbagbt_mem_cursor(
struct xfs_mount *mp,
struct xfs_trans *tp,
- struct xfs_buf *head_bp,
struct xfbtree *xfbtree)
{
struct xfs_btree_cur *cur;
@@ -218,8 +217,7 @@ rcbagbt_mem_cursor(
rcbagbt_maxlevels_possible(), rcbagbt_cur_cache);
cur->bc_mem.xfbtree = xfbtree;
- cur->bc_mem.head_bp = head_bp;
- cur->bc_nlevels = xfs_btree_mem_head_nlevels(head_bp);
+ cur->bc_nlevels = xfbtree->nlevels;
return cur;
}
diff --git a/fs/xfs/scrub/rcbag_btree.h b/fs/xfs/scrub/rcbag_btree.h
index 6486b6ae534096..59d81d707d32a5 100644
--- a/fs/xfs/scrub/rcbag_btree.h
+++ b/fs/xfs/scrub/rcbag_btree.h
@@ -63,8 +63,7 @@ void rcbagbt_destroy_cur_cache(void);
struct xfbtree;
struct xfs_btree_cur *rcbagbt_mem_cursor(struct xfs_mount *mp,
- struct xfs_trans *tp, struct xfs_buf *head_bp,
- struct xfbtree *xfbtree);
+ struct xfs_trans *tp, struct xfbtree *xfbtree);
int rcbagbt_mem_create(struct xfs_mount *mp, struct xfs_buftarg *target,
struct xfbtree **xfbtreep);
diff --git a/fs/xfs/scrub/rmap_repair.c b/fs/xfs/scrub/rmap_repair.c
index 0f3783aaaa9974..ab61f31868f841 100644
--- a/fs/xfs/scrub/rmap_repair.c
+++ b/fs/xfs/scrub/rmap_repair.c
@@ -226,7 +226,6 @@ xrep_rmap_stash(
};
struct xfs_scrub *sc = rr->sc;
struct xfs_btree_cur *mcur;
- struct xfs_buf *mhead_bp;
int error = 0;
if (xchk_should_terminate(sc, &error))
@@ -238,12 +237,7 @@ xrep_rmap_stash(
trace_xrep_rmap_found(sc->mp, sc->sa.pag->pag_agno, &rmap);
mutex_lock(&rr->lock);
- error = xfbtree_head_read_buf(rr->rmap_btree, sc->tp, &mhead_bp);
- if (error)
- goto out_abort;
-
- mcur = xfs_rmapbt_mem_cursor(sc->sa.pag, sc->tp, mhead_bp,
- rr->rmap_btree);
+ mcur = xfs_rmapbt_mem_cursor(sc->sa.pag, sc->tp, rr->rmap_btree);
error = xfs_rmap_map_raw(mcur, &rmap);
xfs_btree_del_cursor(mcur, error);
if (error)
@@ -924,7 +918,6 @@ xrep_rmap_find_rmaps(
struct xfs_scrub *sc = rr->sc;
struct xchk_ag *sa = &sc->sa;
struct xfs_inode *ip;
- struct xfs_buf *mhead_bp;
struct xfs_btree_cur *mcur;
int error;
@@ -1011,12 +1004,7 @@ xrep_rmap_find_rmaps(
* all our records before we start building a new btree, which requires
* a bnobt cursor.
*/
- error = xfbtree_head_read_buf(rr->rmap_btree, NULL, &mhead_bp);
- if (error)
- return error;
-
- mcur = xfs_rmapbt_mem_cursor(rr->sc->sa.pag, NULL, mhead_bp,
- rr->rmap_btree);
+ mcur = xfs_rmapbt_mem_cursor(rr->sc->sa.pag, NULL, rr->rmap_btree);
sc->sa.bno_cur = xfs_allocbt_init_cursor(sc->mp, sc->tp, sc->sa.agf_bp,
sc->sa.pag, XFS_BTNUM_BNO);
@@ -1026,7 +1014,6 @@ xrep_rmap_find_rmaps(
xfs_btree_del_cursor(sc->sa.bno_cur, error);
sc->sa.bno_cur = NULL;
xfs_btree_del_cursor(mcur, error);
- xfs_buf_relse(mhead_bp);
return error;
}
@@ -1364,7 +1351,6 @@ xrep_rmap_build_new_tree(
struct xfs_perag *pag = sc->sa.pag;
struct xfs_agf *agf = sc->sa.agf_bp->b_addr;
struct xfs_btree_cur *rmap_cur;
- struct xfs_buf *mhead_bp;
xfs_fsblock_t fsbno;
int error;
@@ -1403,12 +1389,7 @@ xrep_rmap_build_new_tree(
* Count the rmapbt records again, because the space reservation
* for the rmapbt itself probably added more records to the btree.
*/
- error = xfbtree_head_read_buf(rr->rmap_btree, NULL, &mhead_bp);
- if (error)
- goto err_cur;
-
- rr->mcur = xfs_rmapbt_mem_cursor(rr->sc->sa.pag, NULL, mhead_bp,
- rr->rmap_btree);
+ rr->mcur = xfs_rmapbt_mem_cursor(rr->sc->sa.pag, NULL, rr->rmap_btree);
error = xrep_rmap_count_records(rr->mcur, &rr->nr_records);
if (error)
@@ -1444,7 +1425,6 @@ xrep_rmap_build_new_tree(
xfs_btree_del_cursor(rmap_cur, 0);
xfs_btree_del_cursor(rr->mcur, 0);
rr->mcur = NULL;
- xfs_buf_relse(mhead_bp);
/*
* Now that we've written the new btree to disk, we don't need to keep
@@ -1476,7 +1456,6 @@ xrep_rmap_build_new_tree(
pag->pagf_repair_levels[XFS_BTNUM_RMAPi] = 0;
err_mcur:
xfs_btree_del_cursor(rr->mcur, error);
- xfs_buf_relse(mhead_bp);
err_cur:
xfs_btree_del_cursor(rmap_cur, error);
err_newbt:
@@ -1543,20 +1522,16 @@ xrep_rmap_remove_old_tree(
struct xfs_agf *agf = sc->sa.agf_bp->b_addr;
struct xfs_perag *pag = sc->sa.pag;
struct xfs_btree_cur *mcur;
- struct xfs_buf *mhead_bp;
xfs_agblock_t agend;
int error;
xagb_bitmap_init(&rfg.rmap_gaps);
/* Compute free space from the new rmapbt. */
- error = xfbtree_head_read_buf(rr->rmap_btree, NULL, &mhead_bp);
- mcur = xfs_rmapbt_mem_cursor(rr->sc->sa.pag, NULL, mhead_bp,
- rr->rmap_btree);
+ mcur = xfs_rmapbt_mem_cursor(rr->sc->sa.pag, NULL, rr->rmap_btree);
error = xfs_rmap_query_all(mcur, xrep_rmap_find_gaps, &rfg);
xfs_btree_del_cursor(mcur, error);
- xfs_buf_relse(mhead_bp);
if (error)
goto out_bitmap;
@@ -1646,7 +1621,6 @@ xrep_rmapbt_live_update(
struct xrep_rmap *rr;
struct xfs_mount *mp;
struct xfs_btree_cur *mcur;
- struct xfs_buf *mhead_bp;
struct xfs_trans *tp;
void *txcookie;
int error;
@@ -1664,12 +1638,7 @@ xrep_rmapbt_live_update(
goto out_abort;
mutex_lock(&rr->lock);
- error = xfbtree_head_read_buf(rr->rmap_btree, tp, &mhead_bp);
- if (error)
- goto out_cancel;
-
- mcur = xfs_rmapbt_mem_cursor(rr->sc->sa.pag, tp, mhead_bp,
- rr->rmap_btree);
+ mcur = xfs_rmapbt_mem_cursor(rr->sc->sa.pag, tp, rr->rmap_btree);
error = __xfs_rmap_finish_intent(mcur, action, p->startblock,
p->blockcount, &p->oinfo, p->unwritten);
xfs_btree_del_cursor(mcur, error);
diff --git a/fs/xfs/scrub/rtrmap_repair.c b/fs/xfs/scrub/rtrmap_repair.c
index c29e3982830183..885752c7436b45 100644
--- a/fs/xfs/scrub/rtrmap_repair.c
+++ b/fs/xfs/scrub/rtrmap_repair.c
@@ -159,7 +159,6 @@ xrep_rtrmap_stash(
};
struct xfs_scrub *sc = rr->sc;
struct xfs_btree_cur *mcur;
- struct xfs_buf *mhead_bp;
int error = 0;
if (xchk_should_terminate(sc, &error))
@@ -172,12 +171,7 @@ xrep_rtrmap_stash(
/* Add entry to in-memory btree. */
mutex_lock(&rr->lock);
- error = xfbtree_head_read_buf(rr->rtrmap_btree, sc->tp, &mhead_bp);
- if (error)
- goto out_abort;
-
- mcur = xfs_rtrmapbt_mem_cursor(sc->sr.rtg, sc->tp, mhead_bp,
- rr->rtrmap_btree);
+ mcur = xfs_rtrmapbt_mem_cursor(sc->sr.rtg, sc->tp, rr->rtrmap_btree);
error = xfs_rmap_map_raw(mcur, &rmap);
xfs_btree_del_cursor(mcur, error);
if (error)
@@ -569,7 +563,6 @@ xrep_rtrmap_find_rmaps(
struct xfs_scrub *sc = rr->sc;
struct xfs_perag *pag;
struct xfs_inode *ip;
- struct xfs_buf *mhead_bp;
struct xfs_btree_cur *mcur;
xfs_agnumber_t agno;
int error;
@@ -655,16 +648,10 @@ xrep_rtrmap_find_rmaps(
* check all our records before we start building a new btree, which
* requires the rtbitmap lock.
*/
- error = xfbtree_head_read_buf(rr->rtrmap_btree, NULL, &mhead_bp);
- if (error)
- return error;
-
- mcur = xfs_rtrmapbt_mem_cursor(rr->sc->sr.rtg, NULL, mhead_bp,
- rr->rtrmap_btree);
+ mcur = xfs_rtrmapbt_mem_cursor(rr->sc->sr.rtg, NULL, rr->rtrmap_btree);
rr->nr_records = 0;
error = xfs_rmap_query_all(mcur, xrep_rtrmap_check_record, rr);
xfs_btree_del_cursor(mcur, error);
- xfs_buf_relse(mhead_bp);
return error;
}
@@ -743,7 +730,6 @@ xrep_rtrmap_build_new_tree(
struct xfs_scrub *sc = rr->sc;
struct xfs_rtgroup *rtg = sc->sr.rtg;
struct xfs_btree_cur *rmap_cur;
- struct xfs_buf *mhead_bp;
int error;
/*
@@ -795,12 +781,7 @@ xrep_rtrmap_build_new_tree(
* Create a cursor to the in-memory btree so that we can bulk load the
* new btree.
*/
- error = xfbtree_head_read_buf(rr->rtrmap_btree, NULL, &mhead_bp);
- if (error)
- goto err_cur;
-
- rr->mcur = xfs_rtrmapbt_mem_cursor(sc->sr.rtg, NULL, mhead_bp,
- rr->rtrmap_btree);
+ rr->mcur = xfs_rtrmapbt_mem_cursor(sc->sr.rtg, NULL, rr->rtrmap_btree);
error = xfs_btree_goto_left_edge(rr->mcur);
if (error)
goto err_mcur;
@@ -821,7 +802,6 @@ xrep_rtrmap_build_new_tree(
xfs_btree_del_cursor(rmap_cur, 0);
xfs_btree_del_cursor(rr->mcur, 0);
rr->mcur = NULL;
- xfs_buf_relse(mhead_bp);
/*
* Now that we've written the new btree to disk, we don't need to keep
@@ -838,7 +818,6 @@ xrep_rtrmap_build_new_tree(
err_mcur:
xfs_btree_del_cursor(rr->mcur, error);
- xfs_buf_relse(mhead_bp);
err_cur:
xfs_btree_del_cursor(rmap_cur, error);
xrep_newbt_cancel(&rr->new_btree);
@@ -904,7 +883,6 @@ xrep_rtrmapbt_live_update(
struct xrep_rtrmap *rr;
struct xfs_mount *mp;
struct xfs_btree_cur *mcur;
- struct xfs_buf *mhead_bp;
struct xfs_trans *tp;
void *txcookie;
int error;
@@ -922,12 +900,7 @@ xrep_rtrmapbt_live_update(
goto out_abort;
mutex_lock(&rr->lock);
- error = xfbtree_head_read_buf(rr->rtrmap_btree, tp, &mhead_bp);
- if (error)
- goto out_cancel;
-
- mcur = xfs_rtrmapbt_mem_cursor(rr->sc->sr.rtg, tp, mhead_bp,
- rr->rtrmap_btree);
+ mcur = xfs_rtrmapbt_mem_cursor(rr->sc->sr.rtg, tp, rr->rtrmap_btree);
error = __xfs_rmap_finish_intent(mcur, action, p->startblock,
p->blockcount, &p->oinfo, p->unwritten);
xfs_btree_del_cursor(mcur, error);
diff --git a/fs/xfs/scrub/xfbtree.c b/fs/xfs/scrub/xfbtree.c
index 7c035ad1f696ac..3620acc008aa59 100644
--- a/fs/xfs/scrub/xfbtree.c
+++ b/fs/xfs/scrub/xfbtree.c
@@ -55,83 +55,6 @@ static inline int xfboff_bitmap_take_first_set(struct xfboff_bitmap *bitmap,
return 0;
}
-/* btree ops functions for in-memory btrees. */
-
-static xfs_failaddr_t
-xfs_btree_mem_head_verify(
- struct xfs_buf *bp)
-{
- struct xfs_btree_mem_head *mhead = bp->b_addr;
- struct xfs_mount *mp = bp->b_mount;
-
- if (!xfs_verify_magic(bp, mhead->mh_magic))
- return __this_address;
- if (be32_to_cpu(mhead->mh_nlevels) == 0)
- return __this_address;
- if (!uuid_equal(&mhead->mh_uuid, &mp->m_sb.sb_meta_uuid))
- return __this_address;
-
- return NULL;
-}
-
-static void
-xfs_btree_mem_head_read_verify(
- struct xfs_buf *bp)
-{
- xfs_failaddr_t fa = xfs_btree_mem_head_verify(bp);
-
- if (fa)
- xfs_verifier_error(bp, -EFSCORRUPTED, fa);
-}
-
-static void
-xfs_btree_mem_head_write_verify(
- struct xfs_buf *bp)
-{
- xfs_failaddr_t fa = xfs_btree_mem_head_verify(bp);
-
- if (fa)
- xfs_verifier_error(bp, -EFSCORRUPTED, fa);
-}
-
-static const struct xfs_buf_ops xfs_btree_mem_head_buf_ops = {
- .name = "xfs_btree_mem_head",
- .magic = { cpu_to_be32(XFS_BTREE_MEM_HEAD_MAGIC),
- cpu_to_be32(XFS_BTREE_MEM_HEAD_MAGIC) },
- .verify_read = xfs_btree_mem_head_read_verify,
- .verify_write = xfs_btree_mem_head_write_verify,
- .verify_struct = xfs_btree_mem_head_verify,
-};
-
-/* Initialize the header block for an in-memory btree. */
-static inline void
-xfs_btree_mem_head_init(
- struct xfs_buf *head_bp,
- unsigned long long owner,
- xfileoff_t leaf_xfoff)
-{
- struct xfs_btree_mem_head *mhead = head_bp->b_addr;
- struct xfs_mount *mp = head_bp->b_mount;
-
- mhead->mh_magic = cpu_to_be32(XFS_BTREE_MEM_HEAD_MAGIC);
- mhead->mh_nlevels = cpu_to_be32(1);
- mhead->mh_owner = cpu_to_be64(owner);
- mhead->mh_root = cpu_to_be64(leaf_xfoff);
- uuid_copy(&mhead->mh_uuid, &mp->m_sb.sb_meta_uuid);
-
- head_bp->b_ops = &xfs_btree_mem_head_buf_ops;
-}
-
-/* Return tree height from the in-memory btree head. */
-unsigned int
-xfs_btree_mem_head_nlevels(
- struct xfs_buf *head_bp)
-{
- struct xfs_btree_mem_head *mhead = head_bp->b_addr;
-
- return be32_to_cpu(mhead->mh_nlevels);
-}
-
/* Extract the buftarg target for this xfile btree. */
struct xfs_buftarg *
xfbtree_target(struct xfbtree *xfbtree)
@@ -170,7 +93,6 @@ xfbtree_check_ptr(
int level)
{
xfileoff_t bt_xfoff;
- xfs_failaddr_t fa = NULL;
ASSERT(cur->bc_flags & XFS_BTREE_IN_XFILE);
@@ -180,22 +102,10 @@ xfbtree_check_ptr(
bt_xfoff = be32_to_cpu(ptr->s);
if (!xfbtree_verify_xfileoff(cur, bt_xfoff)) {
- fa = __this_address;
- goto done;
- }
-
- /* Can't point to the head or anything before it */
- if (bt_xfoff < XFBTREE_INIT_LEAF_BLOCK) {
- fa = __this_address;
- goto done;
- }
-
-done:
- if (fa) {
xfs_err(cur->bc_mp,
"In-memory: Corrupt btree %d flags 0x%x pointer at level %d index %d fa %pS.",
cur->bc_btnum, cur->bc_flags, level, index,
- fa);
+ __this_address);
return -EFSCORRUPTED;
}
return 0;
@@ -244,20 +154,10 @@ xfbtree_set_root(
const union xfs_btree_ptr *ptr,
int inc)
{
- struct xfs_buf *head_bp = cur->bc_mem.head_bp;
- struct xfs_btree_mem_head *mhead = head_bp->b_addr;
-
ASSERT(cur->bc_flags & XFS_BTREE_IN_XFILE);
- if (cur->bc_flags & XFS_BTREE_LONG_PTRS) {
- mhead->mh_root = ptr->l;
- } else {
- uint32_t root = be32_to_cpu(ptr->s);
-
- mhead->mh_root = cpu_to_be64(root);
- }
- be32_add_cpu(&mhead->mh_nlevels, inc);
- xfs_trans_log_buf(cur->bc_tp, head_bp, 0, sizeof(*mhead) - 1);
+ cur->bc_mem.xfbtree->root = *ptr;
+ cur->bc_mem.xfbtree->nlevels += inc;
}
/* Initialize a pointer from the in-memory btree header. */
@@ -266,18 +166,9 @@ xfbtree_init_ptr_from_cur(
struct xfs_btree_cur *cur,
union xfs_btree_ptr *ptr)
{
- struct xfs_buf *head_bp = cur->bc_mem.head_bp;
- struct xfs_btree_mem_head *mhead = head_bp->b_addr;
-
ASSERT(cur->bc_flags & XFS_BTREE_IN_XFILE);
- if (cur->bc_flags & XFS_BTREE_LONG_PTRS) {
- ptr->l = mhead->mh_root;
- } else {
- uint64_t root = be64_to_cpu(mhead->mh_root);
-
- ptr->s = cpu_to_be32(root);
- }
+ *ptr = cur->bc_mem.xfbtree->root;
}
/* Duplicate an in-memory btree cursor. */
@@ -438,11 +329,11 @@ xfbtree_init_leaf_block(
const struct xfbtree_config *cfg)
{
struct xfs_buf *bp;
- xfs_daddr_t daddr;
+ xfileoff_t xfoff = xfbt->highest_offset++;
int error;
- daddr = xfo_to_daddr(XFBTREE_INIT_LEAF_BLOCK);
- error = xfs_buf_get(xfbt->target, daddr, xfbtree_bbsize(), &bp);
+ error = xfs_buf_get(xfbt->target, xfo_to_daddr(xfoff), xfbtree_bbsize(),
+ &bp);
if (error)
return error;
@@ -454,31 +345,10 @@ xfbtree_init_leaf_block(
if (error)
return error;
- xfbt->highest_offset++;
- return 0;
-}
-
-/* Initialize the in-memory btree header block. */
-STATIC int
-xfbtree_init_head(
- struct xfbtree *xfbt)
-{
- struct xfs_buf *bp;
- xfs_daddr_t daddr;
- int error;
-
- daddr = xfo_to_daddr(XFBTREE_HEAD_BLOCK);
- error = xfs_buf_get(xfbt->target, daddr, xfbtree_bbsize(), &bp);
- if (error)
- return error;
-
- xfs_btree_mem_head_init(bp, xfbt->owner, XFBTREE_INIT_LEAF_BLOCK);
- error = xfs_bwrite(bp);
- xfs_buf_relse(bp);
- if (error)
- return error;
-
- xfbt->highest_offset++;
+ if (cfg->btree_ops->geom_flags & XFS_BTREE_LONG_PTRS)
+ xfbt->root.l = xfoff;
+ else
+ xfbt->root.s = xfoff;
return 0;
}
@@ -519,16 +389,13 @@ xfbtree_create(
xfbt->minrecs[0] = xfbt->maxrecs[0] / 2;
xfbt->minrecs[1] = xfbt->maxrecs[1] / 2;
xfbt->owner = cfg->owner;
+ xfbt->nlevels = 1;
/* Initialize the empty btree. */
error = xfbtree_init_leaf_block(mp, xfbt, cfg);
if (error)
goto err_freesp;
- error = xfbtree_init_head(xfbt);
- if (error)
- goto err_freesp;
-
trace_xfbtree_create(mp, cfg, xfbt);
*xfbtreep = xfbt;
@@ -541,37 +408,6 @@ xfbtree_create(
return error;
}
-/* Read the in-memory btree head. */
-int
-xfbtree_head_read_buf(
- struct xfbtree *xfbt,
- struct xfs_trans *tp,
- struct xfs_buf **bpp)
-{
- struct xfs_buftarg *btp = xfbt->target;
- struct xfs_mount *mp = btp->bt_mount;
- struct xfs_btree_mem_head *mhead;
- struct xfs_buf *bp;
- xfs_daddr_t daddr;
- int error;
-
- daddr = xfo_to_daddr(XFBTREE_HEAD_BLOCK);
- error = xfs_trans_read_buf(mp, tp, btp, daddr, xfbtree_bbsize(), 0,
- &bp, &xfs_btree_mem_head_buf_ops);
- if (error)
- return error;
-
- mhead = bp->b_addr;
- if (be64_to_cpu(mhead->mh_owner) != xfbt->owner) {
- xfs_verifier_error(bp, -EFSCORRUPTED, __this_address);
- xfs_trans_brelse(tp, bp);
- return -EFSCORRUPTED;
- }
-
- *bpp = bp;
- return 0;
-}
-
static inline struct xfile *xfbtree_xfile(struct xfbtree *xfbt)
{
return xfbt->target->bt_xfile;
diff --git a/fs/xfs/scrub/xfbtree.h b/fs/xfs/scrub/xfbtree.h
index ed48981e6c347e..e98f9261464a06 100644
--- a/fs/xfs/scrub/xfbtree.h
+++ b/fs/xfs/scrub/xfbtree.h
@@ -10,17 +10,6 @@
#include "scrub/bitmap.h"
-/* Root block for an in-memory btree. */
-struct xfs_btree_mem_head {
- __be32 mh_magic;
- __be32 mh_nlevels;
- __be64 mh_owner;
- __be64 mh_root;
- uuid_t mh_uuid;
-};
-
-#define XFS_BTREE_MEM_HEAD_MAGIC 0x4341544D /* "CATM" */
-
/* xfile-backed in-memory btrees */
struct xfboff_bitmap {
@@ -34,6 +23,10 @@ struct xfbtree {
/* Bitmap of free space from pos to used */
struct xfboff_bitmap freespace;
+ /* Fake header block information */
+ union xfs_btree_ptr root;
+ uint32_t nlevels;
+
/* Highest xfile offset that has been written to. */
xfileoff_t highest_offset;
@@ -45,15 +38,6 @@ struct xfbtree {
unsigned int minrecs[2];
};
-/* The head of the in-memory btree is always at block 0 */
-#define XFBTREE_HEAD_BLOCK 0
-
-/* in-memory btrees are always created with an empty leaf block at block 1 */
-#define XFBTREE_INIT_LEAF_BLOCK 1
-
-int xfbtree_head_read_buf(struct xfbtree *xfbt, struct xfs_trans *tp,
- struct xfs_buf **bpp);
-
void xfbtree_destroy(struct xfbtree *xfbt);
int xfbtree_trans_commit(struct xfbtree *xfbt, struct xfs_trans *tp);
void xfbtree_trans_cancel(struct xfbtree *xfbt, struct xfs_trans *tp);
--
2.39.2
^ permalink raw reply related [flat|nested] 19+ messages in thread* Re: [PATCH 1/5] xfs: remove the in-memory btree header block
2024-01-03 20:38 ` [PATCH 1/5] xfs: remove the in-memory btree header block Christoph Hellwig
@ 2024-01-04 1:24 ` Darrick J. Wong
2024-01-04 6:27 ` Christoph Hellwig
0 siblings, 1 reply; 19+ messages in thread
From: Darrick J. Wong @ 2024-01-04 1:24 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: Chandan Babu R, linux-xfs
On Wed, Jan 03, 2024 at 09:38:32PM +0100, Christoph Hellwig wrote:
> There is no need to allocate a whole block (aka page) for the fake btree
> on-disk header as we can just stash away the nleves and root block in the
nlevels
> xfbtree structure.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
Originally it was kinda nice to be able to dump the xfbtree contents
including the root pointer. That said, it really /does/ complicate
things.
Reviewed-by: Darrick J. Wong <djwong@kernel.org>
I might just remove this tomorrow or whenever I'm travelling through the
xfbtree code again.
--D
> ---
> fs/xfs/libxfs/xfs_btree.h | 1 -
> fs/xfs/libxfs/xfs_btree_mem.h | 7 --
> fs/xfs/libxfs/xfs_rmap_btree.c | 4 +-
> fs/xfs/libxfs/xfs_rmap_btree.h | 3 +-
> fs/xfs/libxfs/xfs_rtrmap_btree.c | 4 +-
> fs/xfs/libxfs/xfs_rtrmap_btree.h | 3 +-
> fs/xfs/scrub/rcbag.c | 35 +-----
> fs/xfs/scrub/rcbag_btree.c | 4 +-
> fs/xfs/scrub/rcbag_btree.h | 3 +-
> fs/xfs/scrub/rmap_repair.c | 41 +------
> fs/xfs/scrub/rtrmap_repair.c | 35 +-----
> fs/xfs/scrub/xfbtree.c | 188 ++-----------------------------
> fs/xfs/scrub/xfbtree.h | 24 +---
> 13 files changed, 35 insertions(+), 317 deletions(-)
>
> diff --git a/fs/xfs/libxfs/xfs_btree.h b/fs/xfs/libxfs/xfs_btree.h
> index 64e37a0ffb78ea..503f51ef22f81e 100644
> --- a/fs/xfs/libxfs/xfs_btree.h
> +++ b/fs/xfs/libxfs/xfs_btree.h
> @@ -271,7 +271,6 @@ struct xfbtree;
>
> struct xfs_btree_cur_mem {
> struct xfbtree *xfbtree;
> - struct xfs_buf *head_bp;
> struct xfs_perag *pag;
> struct xfs_rtgroup *rtg;
> };
> diff --git a/fs/xfs/libxfs/xfs_btree_mem.h b/fs/xfs/libxfs/xfs_btree_mem.h
> index cfb30cb1aabc69..eeb3340a22d201 100644
> --- a/fs/xfs/libxfs/xfs_btree_mem.h
> +++ b/fs/xfs/libxfs/xfs_btree_mem.h
> @@ -26,8 +26,6 @@ struct xfbtree_config {
> #define XFBTREE_DIRECT_MAP (1U << 0)
>
> #ifdef CONFIG_XFS_BTREE_IN_XFILE
> -unsigned int xfs_btree_mem_head_nlevels(struct xfs_buf *head_bp);
> -
> struct xfs_buftarg *xfbtree_target(struct xfbtree *xfbtree);
> int xfbtree_check_ptr(struct xfs_btree_cur *cur,
> const union xfs_btree_ptr *ptr, int index, int level);
> @@ -63,11 +61,6 @@ int xfbtree_alloc_block(struct xfs_btree_cur *cur,
> int *stat);
> int xfbtree_free_block(struct xfs_btree_cur *cur, struct xfs_buf *bp);
> #else
> -static inline unsigned int xfs_btree_mem_head_nlevels(struct xfs_buf *head_bp)
> -{
> - return 0;
> -}
> -
> static inline struct xfs_buftarg *
> xfbtree_target(struct xfbtree *xfbtree)
> {
> diff --git a/fs/xfs/libxfs/xfs_rmap_btree.c b/fs/xfs/libxfs/xfs_rmap_btree.c
> index 71887cc23e03f1..41f1b5fa863302 100644
> --- a/fs/xfs/libxfs/xfs_rmap_btree.c
> +++ b/fs/xfs/libxfs/xfs_rmap_btree.c
> @@ -641,7 +641,6 @@ struct xfs_btree_cur *
> xfs_rmapbt_mem_cursor(
> struct xfs_perag *pag,
> struct xfs_trans *tp,
> - struct xfs_buf *head_bp,
> struct xfbtree *xfbtree)
> {
> struct xfs_btree_cur *cur;
> @@ -653,8 +652,7 @@ xfs_rmapbt_mem_cursor(
> xfs_rmapbt_cur_cache);
> cur->bc_statoff = XFS_STATS_CALC_INDEX(xs_rmap_2);
> cur->bc_mem.xfbtree = xfbtree;
> - cur->bc_mem.head_bp = head_bp;
> - cur->bc_nlevels = xfs_btree_mem_head_nlevels(head_bp);
> + cur->bc_nlevels = xfbtree->nlevels;
>
> cur->bc_mem.pag = xfs_perag_hold(pag);
> return cur;
> diff --git a/fs/xfs/libxfs/xfs_rmap_btree.h b/fs/xfs/libxfs/xfs_rmap_btree.h
> index 415fad8dad73ed..dfe13b8cbb732d 100644
> --- a/fs/xfs/libxfs/xfs_rmap_btree.h
> +++ b/fs/xfs/libxfs/xfs_rmap_btree.h
> @@ -68,8 +68,7 @@ void xfs_rmapbt_destroy_cur_cache(void);
> #ifdef CONFIG_XFS_BTREE_IN_XFILE
> struct xfbtree;
> struct xfs_btree_cur *xfs_rmapbt_mem_cursor(struct xfs_perag *pag,
> - struct xfs_trans *tp, struct xfs_buf *head_bp,
> - struct xfbtree *xfbtree);
> + struct xfs_trans *tp, struct xfbtree *xfbtree);
> int xfs_rmapbt_mem_create(struct xfs_mount *mp, xfs_agnumber_t agno,
> struct xfs_buftarg *target, struct xfbtree **xfbtreep);
> #endif /* CONFIG_XFS_BTREE_IN_XFILE */
> diff --git a/fs/xfs/libxfs/xfs_rtrmap_btree.c b/fs/xfs/libxfs/xfs_rtrmap_btree.c
> index 87ea5e3ca89375..3b105e2da8468d 100644
> --- a/fs/xfs/libxfs/xfs_rtrmap_btree.c
> +++ b/fs/xfs/libxfs/xfs_rtrmap_btree.c
> @@ -643,7 +643,6 @@ struct xfs_btree_cur *
> xfs_rtrmapbt_mem_cursor(
> struct xfs_rtgroup *rtg,
> struct xfs_trans *tp,
> - struct xfs_buf *head_bp,
> struct xfbtree *xfbtree)
> {
> struct xfs_btree_cur *cur;
> @@ -655,8 +654,7 @@ xfs_rtrmapbt_mem_cursor(
> xfs_rtrmapbt_cur_cache);
> cur->bc_statoff = XFS_STATS_CALC_INDEX(xs_rmap_2);
> cur->bc_mem.xfbtree = xfbtree;
> - cur->bc_mem.head_bp = head_bp;
> - cur->bc_nlevels = xfs_btree_mem_head_nlevels(head_bp);
> + cur->bc_nlevels = xfbtree->nlevels;
>
> cur->bc_mem.rtg = xfs_rtgroup_hold(rtg);
> return cur;
> diff --git a/fs/xfs/libxfs/xfs_rtrmap_btree.h b/fs/xfs/libxfs/xfs_rtrmap_btree.h
> index b0a8e8d89f9eb4..3347205846eb2e 100644
> --- a/fs/xfs/libxfs/xfs_rtrmap_btree.h
> +++ b/fs/xfs/libxfs/xfs_rtrmap_btree.h
> @@ -208,8 +208,7 @@ unsigned long long xfs_rtrmapbt_calc_size(struct xfs_mount *mp,
> #ifdef CONFIG_XFS_BTREE_IN_XFILE
> struct xfbtree;
> struct xfs_btree_cur *xfs_rtrmapbt_mem_cursor(struct xfs_rtgroup *rtg,
> - struct xfs_trans *tp, struct xfs_buf *mhead_bp,
> - struct xfbtree *xfbtree);
> + struct xfs_trans *tp, struct xfbtree *xfbtree);
> int xfs_rtrmapbt_mem_create(struct xfs_mount *mp, xfs_rgnumber_t rgno,
> struct xfs_buftarg *target, struct xfbtree **xfbtreep);
> #endif /* CONFIG_XFS_BTREE_IN_XFILE */
> diff --git a/fs/xfs/scrub/rcbag.c b/fs/xfs/scrub/rcbag.c
> index 63f1b6e6488e15..f28ce02f961c7c 100644
> --- a/fs/xfs/scrub/rcbag.c
> +++ b/fs/xfs/scrub/rcbag.c
> @@ -76,16 +76,11 @@ rcbag_add(
> {
> struct rcbag_rec bagrec;
> struct xfs_mount *mp = bag->mp;
> - struct xfs_buf *head_bp;
> struct xfs_btree_cur *cur;
> int has;
> int error;
>
> - error = xfbtree_head_read_buf(bag->xfbtree, tp, &head_bp);
> - if (error)
> - return error;
> -
> - cur = rcbagbt_mem_cursor(mp, tp, head_bp, bag->xfbtree);
> + cur = rcbagbt_mem_cursor(mp, tp, bag->xfbtree);
> error = rcbagbt_lookup_eq(cur, rmap, &has);
> if (error)
> goto out_cur;
> @@ -118,7 +113,6 @@ rcbag_add(
> }
>
> xfs_btree_del_cursor(cur, 0);
> - xfs_trans_brelse(tp, head_bp);
>
> error = xfbtree_trans_commit(bag->xfbtree, tp);
> if (error)
> @@ -129,7 +123,6 @@ rcbag_add(
>
> out_cur:
> xfs_btree_del_cursor(cur, error);
> - xfs_trans_brelse(tp, head_bp);
> xfbtree_trans_cancel(bag->xfbtree, tp);
> return error;
> }
> @@ -157,7 +150,6 @@ rcbag_next_edge(
> {
> struct rcbag_rec bagrec;
> struct xfs_mount *mp = bag->mp;
> - struct xfs_buf *head_bp;
> struct xfs_btree_cur *cur;
> uint32_t next_bno = NULLAGBLOCK;
> int has;
> @@ -166,11 +158,7 @@ rcbag_next_edge(
> if (next_valid)
> next_bno = next_rmap->rm_startblock;
>
> - error = xfbtree_head_read_buf(bag->xfbtree, tp, &head_bp);
> - if (error)
> - return error;
> -
> - cur = rcbagbt_mem_cursor(mp, tp, head_bp, bag->xfbtree);
> + cur = rcbagbt_mem_cursor(mp, tp, bag->xfbtree);
> error = xfs_btree_goto_left_edge(cur);
> if (error)
> goto out_cur;
> @@ -205,14 +193,12 @@ rcbag_next_edge(
> }
>
> xfs_btree_del_cursor(cur, 0);
> - xfs_trans_brelse(tp, head_bp);
>
> *next_bnop = next_bno;
> return 0;
>
> out_cur:
> xfs_btree_del_cursor(cur, error);
> - xfs_trans_brelse(tp, head_bp);
> return error;
> }
>
> @@ -225,17 +211,12 @@ rcbag_remove_ending_at(
> {
> struct rcbag_rec bagrec;
> struct xfs_mount *mp = bag->mp;
> - struct xfs_buf *head_bp;
> struct xfs_btree_cur *cur;
> int has;
> int error;
>
> - error = xfbtree_head_read_buf(bag->xfbtree, tp, &head_bp);
> - if (error)
> - return error;
> -
> /* go to the right edge of the tree */
> - cur = rcbagbt_mem_cursor(mp, tp, head_bp, bag->xfbtree);
> + cur = rcbagbt_mem_cursor(mp, tp, bag->xfbtree);
> memset(&cur->bc_rec, 0xFF, sizeof(cur->bc_rec));
> error = xfs_btree_lookup(cur, XFS_LOOKUP_GE, &has);
> if (error)
> @@ -271,11 +252,9 @@ rcbag_remove_ending_at(
> }
>
> xfs_btree_del_cursor(cur, 0);
> - xfs_trans_brelse(tp, head_bp);
> return xfbtree_trans_commit(bag->xfbtree, tp);
> out_cur:
> xfs_btree_del_cursor(cur, error);
> - xfs_trans_brelse(tp, head_bp);
> xfbtree_trans_cancel(bag->xfbtree, tp);
> return error;
> }
> @@ -288,17 +267,12 @@ rcbag_dump(
> {
> struct rcbag_rec bagrec;
> struct xfs_mount *mp = bag->mp;
> - struct xfs_buf *head_bp;
> struct xfs_btree_cur *cur;
> unsigned long long nr = 0;
> int has;
> int error;
>
> - error = xfbtree_head_read_buf(bag->xfbtree, tp, &head_bp);
> - if (error)
> - return;
> -
> - cur = rcbagbt_mem_cursor(mp, tp, head_bp, bag->xfbtree);
> + cur = rcbagbt_mem_cursor(mp, tp, bag->xfbtree);
> error = xfs_btree_goto_left_edge(cur);
> if (error)
> goto out_cur;
> @@ -327,5 +301,4 @@ rcbag_dump(
>
> out_cur:
> xfs_btree_del_cursor(cur, error);
> - xfs_trans_brelse(tp, head_bp);
> }
> diff --git a/fs/xfs/scrub/rcbag_btree.c b/fs/xfs/scrub/rcbag_btree.c
> index 9807e08129fe4a..6f0b48b5c37bbd 100644
> --- a/fs/xfs/scrub/rcbag_btree.c
> +++ b/fs/xfs/scrub/rcbag_btree.c
> @@ -209,7 +209,6 @@ struct xfs_btree_cur *
> rcbagbt_mem_cursor(
> struct xfs_mount *mp,
> struct xfs_trans *tp,
> - struct xfs_buf *head_bp,
> struct xfbtree *xfbtree)
> {
> struct xfs_btree_cur *cur;
> @@ -218,8 +217,7 @@ rcbagbt_mem_cursor(
> rcbagbt_maxlevels_possible(), rcbagbt_cur_cache);
>
> cur->bc_mem.xfbtree = xfbtree;
> - cur->bc_mem.head_bp = head_bp;
> - cur->bc_nlevels = xfs_btree_mem_head_nlevels(head_bp);
> + cur->bc_nlevels = xfbtree->nlevels;
> return cur;
> }
>
> diff --git a/fs/xfs/scrub/rcbag_btree.h b/fs/xfs/scrub/rcbag_btree.h
> index 6486b6ae534096..59d81d707d32a5 100644
> --- a/fs/xfs/scrub/rcbag_btree.h
> +++ b/fs/xfs/scrub/rcbag_btree.h
> @@ -63,8 +63,7 @@ void rcbagbt_destroy_cur_cache(void);
>
> struct xfbtree;
> struct xfs_btree_cur *rcbagbt_mem_cursor(struct xfs_mount *mp,
> - struct xfs_trans *tp, struct xfs_buf *head_bp,
> - struct xfbtree *xfbtree);
> + struct xfs_trans *tp, struct xfbtree *xfbtree);
> int rcbagbt_mem_create(struct xfs_mount *mp, struct xfs_buftarg *target,
> struct xfbtree **xfbtreep);
>
> diff --git a/fs/xfs/scrub/rmap_repair.c b/fs/xfs/scrub/rmap_repair.c
> index 0f3783aaaa9974..ab61f31868f841 100644
> --- a/fs/xfs/scrub/rmap_repair.c
> +++ b/fs/xfs/scrub/rmap_repair.c
> @@ -226,7 +226,6 @@ xrep_rmap_stash(
> };
> struct xfs_scrub *sc = rr->sc;
> struct xfs_btree_cur *mcur;
> - struct xfs_buf *mhead_bp;
> int error = 0;
>
> if (xchk_should_terminate(sc, &error))
> @@ -238,12 +237,7 @@ xrep_rmap_stash(
> trace_xrep_rmap_found(sc->mp, sc->sa.pag->pag_agno, &rmap);
>
> mutex_lock(&rr->lock);
> - error = xfbtree_head_read_buf(rr->rmap_btree, sc->tp, &mhead_bp);
> - if (error)
> - goto out_abort;
> -
> - mcur = xfs_rmapbt_mem_cursor(sc->sa.pag, sc->tp, mhead_bp,
> - rr->rmap_btree);
> + mcur = xfs_rmapbt_mem_cursor(sc->sa.pag, sc->tp, rr->rmap_btree);
> error = xfs_rmap_map_raw(mcur, &rmap);
> xfs_btree_del_cursor(mcur, error);
> if (error)
> @@ -924,7 +918,6 @@ xrep_rmap_find_rmaps(
> struct xfs_scrub *sc = rr->sc;
> struct xchk_ag *sa = &sc->sa;
> struct xfs_inode *ip;
> - struct xfs_buf *mhead_bp;
> struct xfs_btree_cur *mcur;
> int error;
>
> @@ -1011,12 +1004,7 @@ xrep_rmap_find_rmaps(
> * all our records before we start building a new btree, which requires
> * a bnobt cursor.
> */
> - error = xfbtree_head_read_buf(rr->rmap_btree, NULL, &mhead_bp);
> - if (error)
> - return error;
> -
> - mcur = xfs_rmapbt_mem_cursor(rr->sc->sa.pag, NULL, mhead_bp,
> - rr->rmap_btree);
> + mcur = xfs_rmapbt_mem_cursor(rr->sc->sa.pag, NULL, rr->rmap_btree);
> sc->sa.bno_cur = xfs_allocbt_init_cursor(sc->mp, sc->tp, sc->sa.agf_bp,
> sc->sa.pag, XFS_BTNUM_BNO);
>
> @@ -1026,7 +1014,6 @@ xrep_rmap_find_rmaps(
> xfs_btree_del_cursor(sc->sa.bno_cur, error);
> sc->sa.bno_cur = NULL;
> xfs_btree_del_cursor(mcur, error);
> - xfs_buf_relse(mhead_bp);
>
> return error;
> }
> @@ -1364,7 +1351,6 @@ xrep_rmap_build_new_tree(
> struct xfs_perag *pag = sc->sa.pag;
> struct xfs_agf *agf = sc->sa.agf_bp->b_addr;
> struct xfs_btree_cur *rmap_cur;
> - struct xfs_buf *mhead_bp;
> xfs_fsblock_t fsbno;
> int error;
>
> @@ -1403,12 +1389,7 @@ xrep_rmap_build_new_tree(
> * Count the rmapbt records again, because the space reservation
> * for the rmapbt itself probably added more records to the btree.
> */
> - error = xfbtree_head_read_buf(rr->rmap_btree, NULL, &mhead_bp);
> - if (error)
> - goto err_cur;
> -
> - rr->mcur = xfs_rmapbt_mem_cursor(rr->sc->sa.pag, NULL, mhead_bp,
> - rr->rmap_btree);
> + rr->mcur = xfs_rmapbt_mem_cursor(rr->sc->sa.pag, NULL, rr->rmap_btree);
>
> error = xrep_rmap_count_records(rr->mcur, &rr->nr_records);
> if (error)
> @@ -1444,7 +1425,6 @@ xrep_rmap_build_new_tree(
> xfs_btree_del_cursor(rmap_cur, 0);
> xfs_btree_del_cursor(rr->mcur, 0);
> rr->mcur = NULL;
> - xfs_buf_relse(mhead_bp);
>
> /*
> * Now that we've written the new btree to disk, we don't need to keep
> @@ -1476,7 +1456,6 @@ xrep_rmap_build_new_tree(
> pag->pagf_repair_levels[XFS_BTNUM_RMAPi] = 0;
> err_mcur:
> xfs_btree_del_cursor(rr->mcur, error);
> - xfs_buf_relse(mhead_bp);
> err_cur:
> xfs_btree_del_cursor(rmap_cur, error);
> err_newbt:
> @@ -1543,20 +1522,16 @@ xrep_rmap_remove_old_tree(
> struct xfs_agf *agf = sc->sa.agf_bp->b_addr;
> struct xfs_perag *pag = sc->sa.pag;
> struct xfs_btree_cur *mcur;
> - struct xfs_buf *mhead_bp;
> xfs_agblock_t agend;
> int error;
>
> xagb_bitmap_init(&rfg.rmap_gaps);
>
> /* Compute free space from the new rmapbt. */
> - error = xfbtree_head_read_buf(rr->rmap_btree, NULL, &mhead_bp);
> - mcur = xfs_rmapbt_mem_cursor(rr->sc->sa.pag, NULL, mhead_bp,
> - rr->rmap_btree);
> + mcur = xfs_rmapbt_mem_cursor(rr->sc->sa.pag, NULL, rr->rmap_btree);
>
> error = xfs_rmap_query_all(mcur, xrep_rmap_find_gaps, &rfg);
> xfs_btree_del_cursor(mcur, error);
> - xfs_buf_relse(mhead_bp);
> if (error)
> goto out_bitmap;
>
> @@ -1646,7 +1621,6 @@ xrep_rmapbt_live_update(
> struct xrep_rmap *rr;
> struct xfs_mount *mp;
> struct xfs_btree_cur *mcur;
> - struct xfs_buf *mhead_bp;
> struct xfs_trans *tp;
> void *txcookie;
> int error;
> @@ -1664,12 +1638,7 @@ xrep_rmapbt_live_update(
> goto out_abort;
>
> mutex_lock(&rr->lock);
> - error = xfbtree_head_read_buf(rr->rmap_btree, tp, &mhead_bp);
> - if (error)
> - goto out_cancel;
> -
> - mcur = xfs_rmapbt_mem_cursor(rr->sc->sa.pag, tp, mhead_bp,
> - rr->rmap_btree);
> + mcur = xfs_rmapbt_mem_cursor(rr->sc->sa.pag, tp, rr->rmap_btree);
> error = __xfs_rmap_finish_intent(mcur, action, p->startblock,
> p->blockcount, &p->oinfo, p->unwritten);
> xfs_btree_del_cursor(mcur, error);
> diff --git a/fs/xfs/scrub/rtrmap_repair.c b/fs/xfs/scrub/rtrmap_repair.c
> index c29e3982830183..885752c7436b45 100644
> --- a/fs/xfs/scrub/rtrmap_repair.c
> +++ b/fs/xfs/scrub/rtrmap_repair.c
> @@ -159,7 +159,6 @@ xrep_rtrmap_stash(
> };
> struct xfs_scrub *sc = rr->sc;
> struct xfs_btree_cur *mcur;
> - struct xfs_buf *mhead_bp;
> int error = 0;
>
> if (xchk_should_terminate(sc, &error))
> @@ -172,12 +171,7 @@ xrep_rtrmap_stash(
>
> /* Add entry to in-memory btree. */
> mutex_lock(&rr->lock);
> - error = xfbtree_head_read_buf(rr->rtrmap_btree, sc->tp, &mhead_bp);
> - if (error)
> - goto out_abort;
> -
> - mcur = xfs_rtrmapbt_mem_cursor(sc->sr.rtg, sc->tp, mhead_bp,
> - rr->rtrmap_btree);
> + mcur = xfs_rtrmapbt_mem_cursor(sc->sr.rtg, sc->tp, rr->rtrmap_btree);
> error = xfs_rmap_map_raw(mcur, &rmap);
> xfs_btree_del_cursor(mcur, error);
> if (error)
> @@ -569,7 +563,6 @@ xrep_rtrmap_find_rmaps(
> struct xfs_scrub *sc = rr->sc;
> struct xfs_perag *pag;
> struct xfs_inode *ip;
> - struct xfs_buf *mhead_bp;
> struct xfs_btree_cur *mcur;
> xfs_agnumber_t agno;
> int error;
> @@ -655,16 +648,10 @@ xrep_rtrmap_find_rmaps(
> * check all our records before we start building a new btree, which
> * requires the rtbitmap lock.
> */
> - error = xfbtree_head_read_buf(rr->rtrmap_btree, NULL, &mhead_bp);
> - if (error)
> - return error;
> -
> - mcur = xfs_rtrmapbt_mem_cursor(rr->sc->sr.rtg, NULL, mhead_bp,
> - rr->rtrmap_btree);
> + mcur = xfs_rtrmapbt_mem_cursor(rr->sc->sr.rtg, NULL, rr->rtrmap_btree);
> rr->nr_records = 0;
> error = xfs_rmap_query_all(mcur, xrep_rtrmap_check_record, rr);
> xfs_btree_del_cursor(mcur, error);
> - xfs_buf_relse(mhead_bp);
>
> return error;
> }
> @@ -743,7 +730,6 @@ xrep_rtrmap_build_new_tree(
> struct xfs_scrub *sc = rr->sc;
> struct xfs_rtgroup *rtg = sc->sr.rtg;
> struct xfs_btree_cur *rmap_cur;
> - struct xfs_buf *mhead_bp;
> int error;
>
> /*
> @@ -795,12 +781,7 @@ xrep_rtrmap_build_new_tree(
> * Create a cursor to the in-memory btree so that we can bulk load the
> * new btree.
> */
> - error = xfbtree_head_read_buf(rr->rtrmap_btree, NULL, &mhead_bp);
> - if (error)
> - goto err_cur;
> -
> - rr->mcur = xfs_rtrmapbt_mem_cursor(sc->sr.rtg, NULL, mhead_bp,
> - rr->rtrmap_btree);
> + rr->mcur = xfs_rtrmapbt_mem_cursor(sc->sr.rtg, NULL, rr->rtrmap_btree);
> error = xfs_btree_goto_left_edge(rr->mcur);
> if (error)
> goto err_mcur;
> @@ -821,7 +802,6 @@ xrep_rtrmap_build_new_tree(
> xfs_btree_del_cursor(rmap_cur, 0);
> xfs_btree_del_cursor(rr->mcur, 0);
> rr->mcur = NULL;
> - xfs_buf_relse(mhead_bp);
>
> /*
> * Now that we've written the new btree to disk, we don't need to keep
> @@ -838,7 +818,6 @@ xrep_rtrmap_build_new_tree(
>
> err_mcur:
> xfs_btree_del_cursor(rr->mcur, error);
> - xfs_buf_relse(mhead_bp);
> err_cur:
> xfs_btree_del_cursor(rmap_cur, error);
> xrep_newbt_cancel(&rr->new_btree);
> @@ -904,7 +883,6 @@ xrep_rtrmapbt_live_update(
> struct xrep_rtrmap *rr;
> struct xfs_mount *mp;
> struct xfs_btree_cur *mcur;
> - struct xfs_buf *mhead_bp;
> struct xfs_trans *tp;
> void *txcookie;
> int error;
> @@ -922,12 +900,7 @@ xrep_rtrmapbt_live_update(
> goto out_abort;
>
> mutex_lock(&rr->lock);
> - error = xfbtree_head_read_buf(rr->rtrmap_btree, tp, &mhead_bp);
> - if (error)
> - goto out_cancel;
> -
> - mcur = xfs_rtrmapbt_mem_cursor(rr->sc->sr.rtg, tp, mhead_bp,
> - rr->rtrmap_btree);
> + mcur = xfs_rtrmapbt_mem_cursor(rr->sc->sr.rtg, tp, rr->rtrmap_btree);
> error = __xfs_rmap_finish_intent(mcur, action, p->startblock,
> p->blockcount, &p->oinfo, p->unwritten);
> xfs_btree_del_cursor(mcur, error);
> diff --git a/fs/xfs/scrub/xfbtree.c b/fs/xfs/scrub/xfbtree.c
> index 7c035ad1f696ac..3620acc008aa59 100644
> --- a/fs/xfs/scrub/xfbtree.c
> +++ b/fs/xfs/scrub/xfbtree.c
> @@ -55,83 +55,6 @@ static inline int xfboff_bitmap_take_first_set(struct xfboff_bitmap *bitmap,
> return 0;
> }
>
> -/* btree ops functions for in-memory btrees. */
> -
> -static xfs_failaddr_t
> -xfs_btree_mem_head_verify(
> - struct xfs_buf *bp)
> -{
> - struct xfs_btree_mem_head *mhead = bp->b_addr;
> - struct xfs_mount *mp = bp->b_mount;
> -
> - if (!xfs_verify_magic(bp, mhead->mh_magic))
> - return __this_address;
> - if (be32_to_cpu(mhead->mh_nlevels) == 0)
> - return __this_address;
> - if (!uuid_equal(&mhead->mh_uuid, &mp->m_sb.sb_meta_uuid))
> - return __this_address;
> -
> - return NULL;
> -}
> -
> -static void
> -xfs_btree_mem_head_read_verify(
> - struct xfs_buf *bp)
> -{
> - xfs_failaddr_t fa = xfs_btree_mem_head_verify(bp);
> -
> - if (fa)
> - xfs_verifier_error(bp, -EFSCORRUPTED, fa);
> -}
> -
> -static void
> -xfs_btree_mem_head_write_verify(
> - struct xfs_buf *bp)
> -{
> - xfs_failaddr_t fa = xfs_btree_mem_head_verify(bp);
> -
> - if (fa)
> - xfs_verifier_error(bp, -EFSCORRUPTED, fa);
> -}
> -
> -static const struct xfs_buf_ops xfs_btree_mem_head_buf_ops = {
> - .name = "xfs_btree_mem_head",
> - .magic = { cpu_to_be32(XFS_BTREE_MEM_HEAD_MAGIC),
> - cpu_to_be32(XFS_BTREE_MEM_HEAD_MAGIC) },
> - .verify_read = xfs_btree_mem_head_read_verify,
> - .verify_write = xfs_btree_mem_head_write_verify,
> - .verify_struct = xfs_btree_mem_head_verify,
> -};
> -
> -/* Initialize the header block for an in-memory btree. */
> -static inline void
> -xfs_btree_mem_head_init(
> - struct xfs_buf *head_bp,
> - unsigned long long owner,
> - xfileoff_t leaf_xfoff)
> -{
> - struct xfs_btree_mem_head *mhead = head_bp->b_addr;
> - struct xfs_mount *mp = head_bp->b_mount;
> -
> - mhead->mh_magic = cpu_to_be32(XFS_BTREE_MEM_HEAD_MAGIC);
> - mhead->mh_nlevels = cpu_to_be32(1);
> - mhead->mh_owner = cpu_to_be64(owner);
> - mhead->mh_root = cpu_to_be64(leaf_xfoff);
> - uuid_copy(&mhead->mh_uuid, &mp->m_sb.sb_meta_uuid);
> -
> - head_bp->b_ops = &xfs_btree_mem_head_buf_ops;
> -}
> -
> -/* Return tree height from the in-memory btree head. */
> -unsigned int
> -xfs_btree_mem_head_nlevels(
> - struct xfs_buf *head_bp)
> -{
> - struct xfs_btree_mem_head *mhead = head_bp->b_addr;
> -
> - return be32_to_cpu(mhead->mh_nlevels);
> -}
> -
> /* Extract the buftarg target for this xfile btree. */
> struct xfs_buftarg *
> xfbtree_target(struct xfbtree *xfbtree)
> @@ -170,7 +93,6 @@ xfbtree_check_ptr(
> int level)
> {
> xfileoff_t bt_xfoff;
> - xfs_failaddr_t fa = NULL;
>
> ASSERT(cur->bc_flags & XFS_BTREE_IN_XFILE);
>
> @@ -180,22 +102,10 @@ xfbtree_check_ptr(
> bt_xfoff = be32_to_cpu(ptr->s);
>
> if (!xfbtree_verify_xfileoff(cur, bt_xfoff)) {
> - fa = __this_address;
> - goto done;
> - }
> -
> - /* Can't point to the head or anything before it */
> - if (bt_xfoff < XFBTREE_INIT_LEAF_BLOCK) {
> - fa = __this_address;
> - goto done;
> - }
> -
> -done:
> - if (fa) {
> xfs_err(cur->bc_mp,
> "In-memory: Corrupt btree %d flags 0x%x pointer at level %d index %d fa %pS.",
> cur->bc_btnum, cur->bc_flags, level, index,
> - fa);
> + __this_address);
> return -EFSCORRUPTED;
> }
> return 0;
> @@ -244,20 +154,10 @@ xfbtree_set_root(
> const union xfs_btree_ptr *ptr,
> int inc)
> {
> - struct xfs_buf *head_bp = cur->bc_mem.head_bp;
> - struct xfs_btree_mem_head *mhead = head_bp->b_addr;
> -
> ASSERT(cur->bc_flags & XFS_BTREE_IN_XFILE);
>
> - if (cur->bc_flags & XFS_BTREE_LONG_PTRS) {
> - mhead->mh_root = ptr->l;
> - } else {
> - uint32_t root = be32_to_cpu(ptr->s);
> -
> - mhead->mh_root = cpu_to_be64(root);
> - }
> - be32_add_cpu(&mhead->mh_nlevels, inc);
> - xfs_trans_log_buf(cur->bc_tp, head_bp, 0, sizeof(*mhead) - 1);
> + cur->bc_mem.xfbtree->root = *ptr;
> + cur->bc_mem.xfbtree->nlevels += inc;
> }
>
> /* Initialize a pointer from the in-memory btree header. */
> @@ -266,18 +166,9 @@ xfbtree_init_ptr_from_cur(
> struct xfs_btree_cur *cur,
> union xfs_btree_ptr *ptr)
> {
> - struct xfs_buf *head_bp = cur->bc_mem.head_bp;
> - struct xfs_btree_mem_head *mhead = head_bp->b_addr;
> -
> ASSERT(cur->bc_flags & XFS_BTREE_IN_XFILE);
>
> - if (cur->bc_flags & XFS_BTREE_LONG_PTRS) {
> - ptr->l = mhead->mh_root;
> - } else {
> - uint64_t root = be64_to_cpu(mhead->mh_root);
> -
> - ptr->s = cpu_to_be32(root);
> - }
> + *ptr = cur->bc_mem.xfbtree->root;
> }
>
> /* Duplicate an in-memory btree cursor. */
> @@ -438,11 +329,11 @@ xfbtree_init_leaf_block(
> const struct xfbtree_config *cfg)
> {
> struct xfs_buf *bp;
> - xfs_daddr_t daddr;
> + xfileoff_t xfoff = xfbt->highest_offset++;
> int error;
>
> - daddr = xfo_to_daddr(XFBTREE_INIT_LEAF_BLOCK);
> - error = xfs_buf_get(xfbt->target, daddr, xfbtree_bbsize(), &bp);
> + error = xfs_buf_get(xfbt->target, xfo_to_daddr(xfoff), xfbtree_bbsize(),
> + &bp);
> if (error)
> return error;
>
> @@ -454,31 +345,10 @@ xfbtree_init_leaf_block(
> if (error)
> return error;
>
> - xfbt->highest_offset++;
> - return 0;
> -}
> -
> -/* Initialize the in-memory btree header block. */
> -STATIC int
> -xfbtree_init_head(
> - struct xfbtree *xfbt)
> -{
> - struct xfs_buf *bp;
> - xfs_daddr_t daddr;
> - int error;
> -
> - daddr = xfo_to_daddr(XFBTREE_HEAD_BLOCK);
> - error = xfs_buf_get(xfbt->target, daddr, xfbtree_bbsize(), &bp);
> - if (error)
> - return error;
> -
> - xfs_btree_mem_head_init(bp, xfbt->owner, XFBTREE_INIT_LEAF_BLOCK);
> - error = xfs_bwrite(bp);
> - xfs_buf_relse(bp);
> - if (error)
> - return error;
> -
> - xfbt->highest_offset++;
> + if (cfg->btree_ops->geom_flags & XFS_BTREE_LONG_PTRS)
> + xfbt->root.l = xfoff;
> + else
> + xfbt->root.s = xfoff;
> return 0;
> }
>
> @@ -519,16 +389,13 @@ xfbtree_create(
> xfbt->minrecs[0] = xfbt->maxrecs[0] / 2;
> xfbt->minrecs[1] = xfbt->maxrecs[1] / 2;
> xfbt->owner = cfg->owner;
> + xfbt->nlevels = 1;
>
> /* Initialize the empty btree. */
> error = xfbtree_init_leaf_block(mp, xfbt, cfg);
> if (error)
> goto err_freesp;
>
> - error = xfbtree_init_head(xfbt);
> - if (error)
> - goto err_freesp;
> -
> trace_xfbtree_create(mp, cfg, xfbt);
>
> *xfbtreep = xfbt;
> @@ -541,37 +408,6 @@ xfbtree_create(
> return error;
> }
>
> -/* Read the in-memory btree head. */
> -int
> -xfbtree_head_read_buf(
> - struct xfbtree *xfbt,
> - struct xfs_trans *tp,
> - struct xfs_buf **bpp)
> -{
> - struct xfs_buftarg *btp = xfbt->target;
> - struct xfs_mount *mp = btp->bt_mount;
> - struct xfs_btree_mem_head *mhead;
> - struct xfs_buf *bp;
> - xfs_daddr_t daddr;
> - int error;
> -
> - daddr = xfo_to_daddr(XFBTREE_HEAD_BLOCK);
> - error = xfs_trans_read_buf(mp, tp, btp, daddr, xfbtree_bbsize(), 0,
> - &bp, &xfs_btree_mem_head_buf_ops);
> - if (error)
> - return error;
> -
> - mhead = bp->b_addr;
> - if (be64_to_cpu(mhead->mh_owner) != xfbt->owner) {
> - xfs_verifier_error(bp, -EFSCORRUPTED, __this_address);
> - xfs_trans_brelse(tp, bp);
> - return -EFSCORRUPTED;
> - }
> -
> - *bpp = bp;
> - return 0;
> -}
> -
> static inline struct xfile *xfbtree_xfile(struct xfbtree *xfbt)
> {
> return xfbt->target->bt_xfile;
> diff --git a/fs/xfs/scrub/xfbtree.h b/fs/xfs/scrub/xfbtree.h
> index ed48981e6c347e..e98f9261464a06 100644
> --- a/fs/xfs/scrub/xfbtree.h
> +++ b/fs/xfs/scrub/xfbtree.h
> @@ -10,17 +10,6 @@
>
> #include "scrub/bitmap.h"
>
> -/* Root block for an in-memory btree. */
> -struct xfs_btree_mem_head {
> - __be32 mh_magic;
> - __be32 mh_nlevels;
> - __be64 mh_owner;
> - __be64 mh_root;
> - uuid_t mh_uuid;
> -};
> -
> -#define XFS_BTREE_MEM_HEAD_MAGIC 0x4341544D /* "CATM" */
> -
> /* xfile-backed in-memory btrees */
>
> struct xfboff_bitmap {
> @@ -34,6 +23,10 @@ struct xfbtree {
> /* Bitmap of free space from pos to used */
> struct xfboff_bitmap freespace;
>
> + /* Fake header block information */
> + union xfs_btree_ptr root;
> + uint32_t nlevels;
> +
> /* Highest xfile offset that has been written to. */
> xfileoff_t highest_offset;
>
> @@ -45,15 +38,6 @@ struct xfbtree {
> unsigned int minrecs[2];
> };
>
> -/* The head of the in-memory btree is always at block 0 */
> -#define XFBTREE_HEAD_BLOCK 0
> -
> -/* in-memory btrees are always created with an empty leaf block at block 1 */
> -#define XFBTREE_INIT_LEAF_BLOCK 1
> -
> -int xfbtree_head_read_buf(struct xfbtree *xfbt, struct xfs_trans *tp,
> - struct xfs_buf **bpp);
> -
> void xfbtree_destroy(struct xfbtree *xfbt);
> int xfbtree_trans_commit(struct xfbtree *xfbt, struct xfs_trans *tp);
> void xfbtree_trans_cancel(struct xfbtree *xfbt, struct xfs_trans *tp);
> --
> 2.39.2
>
>
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [PATCH 1/5] xfs: remove the in-memory btree header block
2024-01-04 1:24 ` Darrick J. Wong
@ 2024-01-04 6:27 ` Christoph Hellwig
2024-01-04 17:25 ` Darrick J. Wong
0 siblings, 1 reply; 19+ messages in thread
From: Christoph Hellwig @ 2024-01-04 6:27 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: Christoph Hellwig, Chandan Babu R, linux-xfs
On Wed, Jan 03, 2024 at 05:24:05PM -0800, Darrick J. Wong wrote:
> Originally it was kinda nice to be able to dump the xfbtree contents
> including the root pointer. That said, it really /does/ complicate
> things.
Doesn't any kind of dump need to talk the btree anyway?
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 1/5] xfs: remove the in-memory btree header block
2024-01-04 6:27 ` Christoph Hellwig
@ 2024-01-04 17:25 ` Darrick J. Wong
0 siblings, 0 replies; 19+ messages in thread
From: Darrick J. Wong @ 2024-01-04 17:25 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: Chandan Babu R, linux-xfs
On Thu, Jan 04, 2024 at 07:27:25AM +0100, Christoph Hellwig wrote:
> On Wed, Jan 03, 2024 at 05:24:05PM -0800, Darrick J. Wong wrote:
> > Originally it was kinda nice to be able to dump the xfbtree contents
> > including the root pointer. That said, it really /does/ complicate
> > things.
>
> Doesn't any kind of dump need to talk the btree anyway?
Not if you're willing to stare at the output from xfile_dump() ;)
--D
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH 2/5] xfs: remove struct xfboff_bitmap
2024-01-03 20:38 RFC: in-memory btree simplifications Christoph Hellwig
2024-01-03 20:38 ` [PATCH 1/5] xfs: remove the in-memory btree header block Christoph Hellwig
@ 2024-01-03 20:38 ` Christoph Hellwig
2024-01-03 20:38 ` [PATCH 3/5] xfs: remove bc_ino.flags Christoph Hellwig
` (2 subsequent siblings)
4 siblings, 0 replies; 19+ messages in thread
From: Christoph Hellwig @ 2024-01-03 20:38 UTC (permalink / raw)
To: Chandan Babu R, Darrick J. Wong; +Cc: linux-xfs
Just use struct xbitmap64 directly.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
fs/xfs/scrub/xfbtree.c | 41 +++++------------------------------------
fs/xfs/scrub/xfbtree.h | 14 +++++---------
2 files changed, 10 insertions(+), 45 deletions(-)
diff --git a/fs/xfs/scrub/xfbtree.c b/fs/xfs/scrub/xfbtree.c
index 3620acc008aa59..63b69aeadc623d 100644
--- a/fs/xfs/scrub/xfbtree.c
+++ b/fs/xfs/scrub/xfbtree.c
@@ -24,37 +24,6 @@
#include "scrub/bitmap.h"
#include "scrub/trace.h"
-/* Bitmaps, but for type-checked for xfileoff_t */
-
-static inline void xfboff_bitmap_init(struct xfboff_bitmap *bitmap)
-{
- xbitmap64_init(&bitmap->xfoffbitmap);
-}
-
-static inline void xfboff_bitmap_destroy(struct xfboff_bitmap *bitmap)
-{
- xbitmap64_destroy(&bitmap->xfoffbitmap);
-}
-
-static inline int xfboff_bitmap_set(struct xfboff_bitmap *bitmap,
- xfs_fileoff_t start, xfs_filblks_t len)
-{
- return xbitmap64_set(&bitmap->xfoffbitmap, start, len);
-}
-
-static inline int xfboff_bitmap_take_first_set(struct xfboff_bitmap *bitmap,
- xfileoff_t *valp)
-{
- uint64_t val;
- int error;
-
- error = xbitmap64_take_first_set(&bitmap->xfoffbitmap, 0, -1ULL, &val);
- if (error)
- return error;
- *valp = val;
- return 0;
-}
-
/* Extract the buftarg target for this xfile btree. */
struct xfs_buftarg *
xfbtree_target(struct xfbtree *xfbtree)
@@ -295,7 +264,7 @@ void
xfbtree_destroy(
struct xfbtree *xfbt)
{
- xfboff_bitmap_destroy(&xfbt->freespace);
+ xbitmap64_destroy(&xfbt->freespace);
xfs_buftarg_drain(xfbt->target);
kfree(xfbt);
}
@@ -377,7 +346,7 @@ xfbtree_create(
if (cfg->flags & XFBTREE_DIRECT_MAP)
xfbt->target->bt_flags |= XFS_BUFTARG_DIRECT_MAP;
- xfboff_bitmap_init(&xfbt->freespace);
+ xbitmap64_init(&xfbt->freespace);
/* Set up min/maxrecs for this btree. */
if (cfg->btree_ops->geom_flags & XFS_BTREE_LONG_PTRS)
@@ -402,7 +371,7 @@ xfbtree_create(
return 0;
err_freesp:
- xfboff_bitmap_destroy(&xfbt->freespace);
+ xbitmap64_destroy(&xfbt->freespace);
xfs_buftarg_drain(xfbt->target);
kfree(xfbt);
return error;
@@ -432,7 +401,7 @@ xfbtree_alloc_block(
* Find the first free block in the free space bitmap and take it. If
* none are found, seek to end of the file.
*/
- error = xfboff_bitmap_take_first_set(&xfbt->freespace, &bt_xfoff);
+ error = xbitmap64_take_first_set(&xfbt->freespace, 0, -1ULL, &bt_xfoff);
if (error == -ENODATA) {
bt_xfoff = xfbt->highest_offset++;
error = 0;
@@ -479,7 +448,7 @@ xfbtree_free_block(
trace_xfbtree_free_block(xfbt, cur, bt_xfoff);
- return xfboff_bitmap_set(&xfbt->freespace, bt_xfoff, bt_xflen);
+ return xbitmap64_set(&xfbt->freespace, bt_xfoff, bt_xflen);
}
/* Return the minimum number of records for a btree block. */
diff --git a/fs/xfs/scrub/xfbtree.h b/fs/xfs/scrub/xfbtree.h
index e98f9261464a06..d17be23aca7dbb 100644
--- a/fs/xfs/scrub/xfbtree.h
+++ b/fs/xfs/scrub/xfbtree.h
@@ -12,24 +12,20 @@
/* xfile-backed in-memory btrees */
-struct xfboff_bitmap {
- struct xbitmap64 xfoffbitmap;
-};
-
struct xfbtree {
/* buffer cache target for the xfile backing this in-memory btree */
struct xfs_buftarg *target;
- /* Bitmap of free space from pos to used */
- struct xfboff_bitmap freespace;
+ /* Highest xfile offset that has been written to. */
+ xfileoff_t highest_offset;
+
+ /* Bitmap of free space from pos to highest_offset */
+ struct xbitmap64 freespace;
/* Fake header block information */
union xfs_btree_ptr root;
uint32_t nlevels;
- /* Highest xfile offset that has been written to. */
- xfileoff_t highest_offset;
-
/* Owner of this btree. */
unsigned long long owner;
--
2.39.2
^ permalink raw reply related [flat|nested] 19+ messages in thread* [PATCH 3/5] xfs: remove bc_ino.flags
2024-01-03 20:38 RFC: in-memory btree simplifications Christoph Hellwig
2024-01-03 20:38 ` [PATCH 1/5] xfs: remove the in-memory btree header block Christoph Hellwig
2024-01-03 20:38 ` [PATCH 2/5] xfs: remove struct xfboff_bitmap Christoph Hellwig
@ 2024-01-03 20:38 ` Christoph Hellwig
2024-01-04 1:10 ` Darrick J. Wong
2024-01-03 20:38 ` [PATCH 4/5] xfs: factor out a xfs_btree_owner helper Christoph Hellwig
2024-01-03 20:38 ` [PATCH 5/5] xfs: embedd struct xfbtree into the owning structure Christoph Hellwig
4 siblings, 1 reply; 19+ messages in thread
From: Christoph Hellwig @ 2024-01-03 20:38 UTC (permalink / raw)
To: Chandan Babu R, Darrick J. Wong; +Cc: linux-xfs
Just move the two flags into bc_flags where there is plenty of space.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
fs/xfs/libxfs/xfs_bmap.c | 27 +++++++++------------------
fs/xfs/libxfs/xfs_bmap_btree.c | 14 ++++----------
fs/xfs/libxfs/xfs_btree.c | 2 +-
fs/xfs/libxfs/xfs_btree.h | 12 ++++++------
fs/xfs/libxfs/xfs_rtrefcount_btree.c | 10 +---------
fs/xfs/libxfs/xfs_rtrmap_btree.c | 10 +---------
6 files changed, 22 insertions(+), 53 deletions(-)
diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
index b14761ec96b87a..d6b62884401c0f 100644
--- a/fs/xfs/libxfs/xfs_bmap.c
+++ b/fs/xfs/libxfs/xfs_bmap.c
@@ -643,7 +643,8 @@ xfs_bmap_extents_to_btree(
*/
xfs_bmbt_iroot_alloc(ip, whichfork);
cur = xfs_bmbt_init_cursor(mp, tp, ip, whichfork);
- cur->bc_ino.flags = wasdel ? XFS_BTCUR_BMBT_WASDEL : 0;
+ if (wasdel)
+ cur->bc_flags |= XFS_BTREE_BMBT_WASDEL;
/*
* Convert to a btree with two levels, one record in root.
*/
@@ -1426,8 +1427,7 @@ xfs_bmap_add_extent_delay_real(
ASSERT(whichfork != XFS_ATTR_FORK);
ASSERT(!isnullstartblock(new->br_startblock));
- ASSERT(!bma->cur ||
- (bma->cur->bc_ino.flags & XFS_BTCUR_BMBT_WASDEL));
+ ASSERT(!bma->cur || (bma->cur->bc_flags & XFS_BTREE_BMBT_WASDEL));
XFS_STATS_INC(mp, xs_add_exlist);
@@ -2686,7 +2686,7 @@ xfs_bmap_add_extent_hole_real(
struct xfs_bmbt_irec old;
ASSERT(!isnullstartblock(new->br_startblock));
- ASSERT(!cur || !(cur->bc_ino.flags & XFS_BTCUR_BMBT_WASDEL));
+ ASSERT(!cur || !(cur->bc_flags & XFS_BTREE_BMBT_WASDEL));
XFS_STATS_INC(mp, xs_add_exlist);
@@ -4223,9 +4223,8 @@ xfs_bmapi_allocate(
*/
bma->nallocs++;
- if (bma->cur)
- bma->cur->bc_ino.flags =
- bma->wasdel ? XFS_BTCUR_BMBT_WASDEL : 0;
+ if (bma->cur && bma->wasdel)
+ bma->cur->bc_flags |= XFS_BTREE_BMBT_WASDEL;
bma->got.br_startoff = bma->offset;
bma->got.br_startblock = bma->blkno;
@@ -4762,10 +4761,8 @@ xfs_bmapi_remap(
ip->i_nblocks += len;
xfs_trans_log_inode(tp, ip, XFS_ILOG_CORE);
- if (ifp->if_format == XFS_DINODE_FMT_BTREE) {
+ if (ifp->if_format == XFS_DINODE_FMT_BTREE)
cur = xfs_bmbt_init_cursor(mp, tp, ip, whichfork);
- cur->bc_ino.flags = 0;
- }
got.br_startoff = bno;
got.br_startblock = startblock;
@@ -5413,7 +5410,6 @@ __xfs_bunmapi(
if (ifp->if_format == XFS_DINODE_FMT_BTREE) {
ASSERT(ifp->if_format == XFS_DINODE_FMT_BTREE);
cur = xfs_bmbt_init_cursor(mp, tp, ip, whichfork);
- cur->bc_ino.flags = 0;
} else
cur = NULL;
@@ -5872,10 +5868,8 @@ xfs_bmap_collapse_extents(
if (error)
return error;
- if (ifp->if_format == XFS_DINODE_FMT_BTREE) {
+ if (ifp->if_format == XFS_DINODE_FMT_BTREE)
cur = xfs_bmbt_init_cursor(mp, tp, ip, whichfork);
- cur->bc_ino.flags = 0;
- }
if (!xfs_iext_lookup_extent(ip, ifp, *next_fsb, &icur, &got)) {
*done = true;
@@ -5989,10 +5983,8 @@ xfs_bmap_insert_extents(
if (error)
return error;
- if (ifp->if_format == XFS_DINODE_FMT_BTREE) {
+ if (ifp->if_format == XFS_DINODE_FMT_BTREE)
cur = xfs_bmbt_init_cursor(mp, tp, ip, whichfork);
- cur->bc_ino.flags = 0;
- }
if (*next_fsb == NULLFSBLOCK) {
xfs_iext_last(ifp, &icur);
@@ -6109,7 +6101,6 @@ xfs_bmap_split_extent(
if (ifp->if_format == XFS_DINODE_FMT_BTREE) {
cur = xfs_bmbt_init_cursor(mp, tp, ip, whichfork);
- cur->bc_ino.flags = 0;
error = xfs_bmbt_lookup_eq(cur, &got, &i);
if (error)
goto del_cursor;
diff --git a/fs/xfs/libxfs/xfs_bmap_btree.c b/fs/xfs/libxfs/xfs_bmap_btree.c
index a6de8b7528aa1c..99b86bbf23c957 100644
--- a/fs/xfs/libxfs/xfs_bmap_btree.c
+++ b/fs/xfs/libxfs/xfs_bmap_btree.c
@@ -169,13 +169,8 @@ xfs_bmbt_dup_cursor(
new = xfs_bmbt_init_cursor(cur->bc_mp, cur->bc_tp,
cur->bc_ino.ip, cur->bc_ino.whichfork);
-
- /*
- * Copy the firstblock, dfops, and flags values,
- * since init cursor doesn't get them.
- */
- new->bc_ino.flags = cur->bc_ino.flags;
-
+ new->bc_flags |= (cur->bc_flags &
+ (XFS_BTREE_BMBT_INVALID_OWNER | XFS_BTREE_BMBT_WASDEL));
return new;
}
@@ -209,7 +204,7 @@ xfs_bmbt_alloc_block(
xfs_rmap_ino_bmbt_owner(&args.oinfo, cur->bc_ino.ip->i_ino,
cur->bc_ino.whichfork);
args.minlen = args.maxlen = args.prod = 1;
- args.wasdel = cur->bc_ino.flags & XFS_BTCUR_BMBT_WASDEL;
+ args.wasdel = cur->bc_flags & XFS_BTREE_BMBT_WASDEL;
if (!args.wasdel && args.tp->t_blk_res == 0)
return -ENOSPC;
@@ -608,7 +603,6 @@ xfs_bmbt_init_common(
cur->bc_ino.ip = ip;
cur->bc_ino.allocated = 0;
- cur->bc_ino.flags = 0;
return cur;
}
@@ -799,7 +793,7 @@ xfs_bmbt_change_owner(
ASSERT(xfs_ifork_ptr(ip, whichfork)->if_format == XFS_DINODE_FMT_BTREE);
cur = xfs_bmbt_init_cursor(ip->i_mount, tp, ip, whichfork);
- cur->bc_ino.flags |= XFS_BTCUR_BMBT_INVALID_OWNER;
+ cur->bc_flags |= XFS_BTREE_BMBT_INVALID_OWNER;
error = xfs_btree_change_owner(cur, new_owner, buffer_list);
xfs_btree_del_cursor(cur, error);
diff --git a/fs/xfs/libxfs/xfs_btree.c b/fs/xfs/libxfs/xfs_btree.c
index be484f86da9859..3bc8aa6049b9a7 100644
--- a/fs/xfs/libxfs/xfs_btree.c
+++ b/fs/xfs/libxfs/xfs_btree.c
@@ -1888,7 +1888,7 @@ xfs_btree_check_block_owner(
return NULL;
}
- if (cur->bc_ino.flags & XFS_BTCUR_BMBT_INVALID_OWNER)
+ if (cur->bc_flags & XFS_BTREE_BMBT_INVALID_OWNER)
return NULL;
if (be64_to_cpu(block->bb_u.l.bb_owner) != cur->bc_ino.ip->i_ino)
diff --git a/fs/xfs/libxfs/xfs_btree.h b/fs/xfs/libxfs/xfs_btree.h
index 503f51ef22f81e..05a4572ce44dd2 100644
--- a/fs/xfs/libxfs/xfs_btree.h
+++ b/fs/xfs/libxfs/xfs_btree.h
@@ -257,12 +257,6 @@ struct xfs_btree_cur_ino {
int allocated;
short forksize;
char whichfork;
- char flags;
-/* We are converting a delalloc reservation */
-#define XFS_BTCUR_BMBT_WASDEL (1 << 0)
-
-/* For extent swap, ignore owner check in verifier */
-#define XFS_BTCUR_BMBT_INVALID_OWNER (1 << 1)
struct xbtree_refc refc;
};
@@ -353,6 +347,12 @@ xfs_btree_cur_sizeof(unsigned int nlevels)
# define XFS_BTREE_IN_XFILE (0)
#endif
+/* We are converting a delalloc reservation (only for bmbt btrees) */
+#define XFS_BTREE_BMBT_WASDEL (1 << 8)
+
+/* For extent swap, ignore owner check in verifier (only for bmbt btrees) */
+#define XFS_BTREE_BMBT_INVALID_OWNER (1 << 9)
+
#define XFS_BTREE_NOERROR 0
#define XFS_BTREE_ERROR 1
diff --git a/fs/xfs/libxfs/xfs_rtrefcount_btree.c b/fs/xfs/libxfs/xfs_rtrefcount_btree.c
index 47ce0acd92a19d..89fa7dcf1225c2 100644
--- a/fs/xfs/libxfs/xfs_rtrefcount_btree.c
+++ b/fs/xfs/libxfs/xfs_rtrefcount_btree.c
@@ -45,15 +45,8 @@ static struct xfs_btree_cur *
xfs_rtrefcountbt_dup_cursor(
struct xfs_btree_cur *cur)
{
- struct xfs_btree_cur *new;
-
- new = xfs_rtrefcountbt_init_cursor(cur->bc_mp, cur->bc_tp,
+ return xfs_rtrefcountbt_init_cursor(cur->bc_mp, cur->bc_tp,
cur->bc_ino.rtg, cur->bc_ino.ip);
-
- /* Copy the flags values since init cursor doesn't get them. */
- new->bc_ino.flags = cur->bc_ino.flags;
-
- return new;
}
STATIC int
@@ -398,7 +391,6 @@ xfs_rtrefcountbt_init_common(
cur->bc_ino.ip = ip;
cur->bc_ino.allocated = 0;
- cur->bc_ino.flags = 0;
cur->bc_ino.refc.nr_ops = 0;
cur->bc_ino.refc.shape_changes = 0;
diff --git a/fs/xfs/libxfs/xfs_rtrmap_btree.c b/fs/xfs/libxfs/xfs_rtrmap_btree.c
index 3b105e2da8468d..95983dc081fa21 100644
--- a/fs/xfs/libxfs/xfs_rtrmap_btree.c
+++ b/fs/xfs/libxfs/xfs_rtrmap_btree.c
@@ -48,15 +48,8 @@ static struct xfs_btree_cur *
xfs_rtrmapbt_dup_cursor(
struct xfs_btree_cur *cur)
{
- struct xfs_btree_cur *new;
-
- new = xfs_rtrmapbt_init_cursor(cur->bc_mp, cur->bc_tp, cur->bc_ino.rtg,
+ return xfs_rtrmapbt_init_cursor(cur->bc_mp, cur->bc_tp, cur->bc_ino.rtg,
cur->bc_ino.ip);
-
- /* Copy the flags values since init cursor doesn't get them. */
- new->bc_ino.flags = cur->bc_ino.flags;
-
- return new;
}
STATIC int
@@ -518,7 +511,6 @@ xfs_rtrmapbt_init_common(
cur->bc_ino.ip = ip;
cur->bc_ino.allocated = 0;
- cur->bc_ino.flags = 0;
cur->bc_ino.rtg = xfs_rtgroup_hold(rtg);
return cur;
--
2.39.2
^ permalink raw reply related [flat|nested] 19+ messages in thread* Re: [PATCH 3/5] xfs: remove bc_ino.flags
2024-01-03 20:38 ` [PATCH 3/5] xfs: remove bc_ino.flags Christoph Hellwig
@ 2024-01-04 1:10 ` Darrick J. Wong
0 siblings, 0 replies; 19+ messages in thread
From: Darrick J. Wong @ 2024-01-04 1:10 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: Chandan Babu R, linux-xfs
On Wed, Jan 03, 2024 at 09:38:34PM +0100, Christoph Hellwig wrote:
> Just move the two flags into bc_flags where there is plenty of space.
Heh, and I just got rid of all the XFS_BTREE_ flags except for one.
Reviewed-by: Darrick J. Wong <djwong@kernel.org>
--D
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
> fs/xfs/libxfs/xfs_bmap.c | 27 +++++++++------------------
> fs/xfs/libxfs/xfs_bmap_btree.c | 14 ++++----------
> fs/xfs/libxfs/xfs_btree.c | 2 +-
> fs/xfs/libxfs/xfs_btree.h | 12 ++++++------
> fs/xfs/libxfs/xfs_rtrefcount_btree.c | 10 +---------
> fs/xfs/libxfs/xfs_rtrmap_btree.c | 10 +---------
> 6 files changed, 22 insertions(+), 53 deletions(-)
>
> diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
> index b14761ec96b87a..d6b62884401c0f 100644
> --- a/fs/xfs/libxfs/xfs_bmap.c
> +++ b/fs/xfs/libxfs/xfs_bmap.c
> @@ -643,7 +643,8 @@ xfs_bmap_extents_to_btree(
> */
> xfs_bmbt_iroot_alloc(ip, whichfork);
> cur = xfs_bmbt_init_cursor(mp, tp, ip, whichfork);
> - cur->bc_ino.flags = wasdel ? XFS_BTCUR_BMBT_WASDEL : 0;
> + if (wasdel)
> + cur->bc_flags |= XFS_BTREE_BMBT_WASDEL;
> /*
> * Convert to a btree with two levels, one record in root.
> */
> @@ -1426,8 +1427,7 @@ xfs_bmap_add_extent_delay_real(
>
> ASSERT(whichfork != XFS_ATTR_FORK);
> ASSERT(!isnullstartblock(new->br_startblock));
> - ASSERT(!bma->cur ||
> - (bma->cur->bc_ino.flags & XFS_BTCUR_BMBT_WASDEL));
> + ASSERT(!bma->cur || (bma->cur->bc_flags & XFS_BTREE_BMBT_WASDEL));
>
> XFS_STATS_INC(mp, xs_add_exlist);
>
> @@ -2686,7 +2686,7 @@ xfs_bmap_add_extent_hole_real(
> struct xfs_bmbt_irec old;
>
> ASSERT(!isnullstartblock(new->br_startblock));
> - ASSERT(!cur || !(cur->bc_ino.flags & XFS_BTCUR_BMBT_WASDEL));
> + ASSERT(!cur || !(cur->bc_flags & XFS_BTREE_BMBT_WASDEL));
>
> XFS_STATS_INC(mp, xs_add_exlist);
>
> @@ -4223,9 +4223,8 @@ xfs_bmapi_allocate(
> */
> bma->nallocs++;
>
> - if (bma->cur)
> - bma->cur->bc_ino.flags =
> - bma->wasdel ? XFS_BTCUR_BMBT_WASDEL : 0;
> + if (bma->cur && bma->wasdel)
> + bma->cur->bc_flags |= XFS_BTREE_BMBT_WASDEL;
>
> bma->got.br_startoff = bma->offset;
> bma->got.br_startblock = bma->blkno;
> @@ -4762,10 +4761,8 @@ xfs_bmapi_remap(
> ip->i_nblocks += len;
> xfs_trans_log_inode(tp, ip, XFS_ILOG_CORE);
>
> - if (ifp->if_format == XFS_DINODE_FMT_BTREE) {
> + if (ifp->if_format == XFS_DINODE_FMT_BTREE)
> cur = xfs_bmbt_init_cursor(mp, tp, ip, whichfork);
> - cur->bc_ino.flags = 0;
> - }
>
> got.br_startoff = bno;
> got.br_startblock = startblock;
> @@ -5413,7 +5410,6 @@ __xfs_bunmapi(
> if (ifp->if_format == XFS_DINODE_FMT_BTREE) {
> ASSERT(ifp->if_format == XFS_DINODE_FMT_BTREE);
> cur = xfs_bmbt_init_cursor(mp, tp, ip, whichfork);
> - cur->bc_ino.flags = 0;
> } else
> cur = NULL;
>
> @@ -5872,10 +5868,8 @@ xfs_bmap_collapse_extents(
> if (error)
> return error;
>
> - if (ifp->if_format == XFS_DINODE_FMT_BTREE) {
> + if (ifp->if_format == XFS_DINODE_FMT_BTREE)
> cur = xfs_bmbt_init_cursor(mp, tp, ip, whichfork);
> - cur->bc_ino.flags = 0;
> - }
>
> if (!xfs_iext_lookup_extent(ip, ifp, *next_fsb, &icur, &got)) {
> *done = true;
> @@ -5989,10 +5983,8 @@ xfs_bmap_insert_extents(
> if (error)
> return error;
>
> - if (ifp->if_format == XFS_DINODE_FMT_BTREE) {
> + if (ifp->if_format == XFS_DINODE_FMT_BTREE)
> cur = xfs_bmbt_init_cursor(mp, tp, ip, whichfork);
> - cur->bc_ino.flags = 0;
> - }
>
> if (*next_fsb == NULLFSBLOCK) {
> xfs_iext_last(ifp, &icur);
> @@ -6109,7 +6101,6 @@ xfs_bmap_split_extent(
>
> if (ifp->if_format == XFS_DINODE_FMT_BTREE) {
> cur = xfs_bmbt_init_cursor(mp, tp, ip, whichfork);
> - cur->bc_ino.flags = 0;
> error = xfs_bmbt_lookup_eq(cur, &got, &i);
> if (error)
> goto del_cursor;
> diff --git a/fs/xfs/libxfs/xfs_bmap_btree.c b/fs/xfs/libxfs/xfs_bmap_btree.c
> index a6de8b7528aa1c..99b86bbf23c957 100644
> --- a/fs/xfs/libxfs/xfs_bmap_btree.c
> +++ b/fs/xfs/libxfs/xfs_bmap_btree.c
> @@ -169,13 +169,8 @@ xfs_bmbt_dup_cursor(
>
> new = xfs_bmbt_init_cursor(cur->bc_mp, cur->bc_tp,
> cur->bc_ino.ip, cur->bc_ino.whichfork);
> -
> - /*
> - * Copy the firstblock, dfops, and flags values,
> - * since init cursor doesn't get them.
> - */
> - new->bc_ino.flags = cur->bc_ino.flags;
> -
> + new->bc_flags |= (cur->bc_flags &
> + (XFS_BTREE_BMBT_INVALID_OWNER | XFS_BTREE_BMBT_WASDEL));
> return new;
> }
>
> @@ -209,7 +204,7 @@ xfs_bmbt_alloc_block(
> xfs_rmap_ino_bmbt_owner(&args.oinfo, cur->bc_ino.ip->i_ino,
> cur->bc_ino.whichfork);
> args.minlen = args.maxlen = args.prod = 1;
> - args.wasdel = cur->bc_ino.flags & XFS_BTCUR_BMBT_WASDEL;
> + args.wasdel = cur->bc_flags & XFS_BTREE_BMBT_WASDEL;
> if (!args.wasdel && args.tp->t_blk_res == 0)
> return -ENOSPC;
>
> @@ -608,7 +603,6 @@ xfs_bmbt_init_common(
>
> cur->bc_ino.ip = ip;
> cur->bc_ino.allocated = 0;
> - cur->bc_ino.flags = 0;
>
> return cur;
> }
> @@ -799,7 +793,7 @@ xfs_bmbt_change_owner(
> ASSERT(xfs_ifork_ptr(ip, whichfork)->if_format == XFS_DINODE_FMT_BTREE);
>
> cur = xfs_bmbt_init_cursor(ip->i_mount, tp, ip, whichfork);
> - cur->bc_ino.flags |= XFS_BTCUR_BMBT_INVALID_OWNER;
> + cur->bc_flags |= XFS_BTREE_BMBT_INVALID_OWNER;
>
> error = xfs_btree_change_owner(cur, new_owner, buffer_list);
> xfs_btree_del_cursor(cur, error);
> diff --git a/fs/xfs/libxfs/xfs_btree.c b/fs/xfs/libxfs/xfs_btree.c
> index be484f86da9859..3bc8aa6049b9a7 100644
> --- a/fs/xfs/libxfs/xfs_btree.c
> +++ b/fs/xfs/libxfs/xfs_btree.c
> @@ -1888,7 +1888,7 @@ xfs_btree_check_block_owner(
> return NULL;
> }
>
> - if (cur->bc_ino.flags & XFS_BTCUR_BMBT_INVALID_OWNER)
> + if (cur->bc_flags & XFS_BTREE_BMBT_INVALID_OWNER)
> return NULL;
>
> if (be64_to_cpu(block->bb_u.l.bb_owner) != cur->bc_ino.ip->i_ino)
> diff --git a/fs/xfs/libxfs/xfs_btree.h b/fs/xfs/libxfs/xfs_btree.h
> index 503f51ef22f81e..05a4572ce44dd2 100644
> --- a/fs/xfs/libxfs/xfs_btree.h
> +++ b/fs/xfs/libxfs/xfs_btree.h
> @@ -257,12 +257,6 @@ struct xfs_btree_cur_ino {
> int allocated;
> short forksize;
> char whichfork;
> - char flags;
> -/* We are converting a delalloc reservation */
> -#define XFS_BTCUR_BMBT_WASDEL (1 << 0)
> -
> -/* For extent swap, ignore owner check in verifier */
> -#define XFS_BTCUR_BMBT_INVALID_OWNER (1 << 1)
> struct xbtree_refc refc;
> };
>
> @@ -353,6 +347,12 @@ xfs_btree_cur_sizeof(unsigned int nlevels)
> # define XFS_BTREE_IN_XFILE (0)
> #endif
>
> +/* We are converting a delalloc reservation (only for bmbt btrees) */
> +#define XFS_BTREE_BMBT_WASDEL (1 << 8)
> +
> +/* For extent swap, ignore owner check in verifier (only for bmbt btrees) */
> +#define XFS_BTREE_BMBT_INVALID_OWNER (1 << 9)
> +
> #define XFS_BTREE_NOERROR 0
> #define XFS_BTREE_ERROR 1
>
> diff --git a/fs/xfs/libxfs/xfs_rtrefcount_btree.c b/fs/xfs/libxfs/xfs_rtrefcount_btree.c
> index 47ce0acd92a19d..89fa7dcf1225c2 100644
> --- a/fs/xfs/libxfs/xfs_rtrefcount_btree.c
> +++ b/fs/xfs/libxfs/xfs_rtrefcount_btree.c
> @@ -45,15 +45,8 @@ static struct xfs_btree_cur *
> xfs_rtrefcountbt_dup_cursor(
> struct xfs_btree_cur *cur)
> {
> - struct xfs_btree_cur *new;
> -
> - new = xfs_rtrefcountbt_init_cursor(cur->bc_mp, cur->bc_tp,
> + return xfs_rtrefcountbt_init_cursor(cur->bc_mp, cur->bc_tp,
> cur->bc_ino.rtg, cur->bc_ino.ip);
> -
> - /* Copy the flags values since init cursor doesn't get them. */
> - new->bc_ino.flags = cur->bc_ino.flags;
> -
> - return new;
> }
>
> STATIC int
> @@ -398,7 +391,6 @@ xfs_rtrefcountbt_init_common(
>
> cur->bc_ino.ip = ip;
> cur->bc_ino.allocated = 0;
> - cur->bc_ino.flags = 0;
> cur->bc_ino.refc.nr_ops = 0;
> cur->bc_ino.refc.shape_changes = 0;
>
> diff --git a/fs/xfs/libxfs/xfs_rtrmap_btree.c b/fs/xfs/libxfs/xfs_rtrmap_btree.c
> index 3b105e2da8468d..95983dc081fa21 100644
> --- a/fs/xfs/libxfs/xfs_rtrmap_btree.c
> +++ b/fs/xfs/libxfs/xfs_rtrmap_btree.c
> @@ -48,15 +48,8 @@ static struct xfs_btree_cur *
> xfs_rtrmapbt_dup_cursor(
> struct xfs_btree_cur *cur)
> {
> - struct xfs_btree_cur *new;
> -
> - new = xfs_rtrmapbt_init_cursor(cur->bc_mp, cur->bc_tp, cur->bc_ino.rtg,
> + return xfs_rtrmapbt_init_cursor(cur->bc_mp, cur->bc_tp, cur->bc_ino.rtg,
> cur->bc_ino.ip);
> -
> - /* Copy the flags values since init cursor doesn't get them. */
> - new->bc_ino.flags = cur->bc_ino.flags;
> -
> - return new;
> }
>
> STATIC int
> @@ -518,7 +511,6 @@ xfs_rtrmapbt_init_common(
>
> cur->bc_ino.ip = ip;
> cur->bc_ino.allocated = 0;
> - cur->bc_ino.flags = 0;
>
> cur->bc_ino.rtg = xfs_rtgroup_hold(rtg);
> return cur;
> --
> 2.39.2
>
>
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH 4/5] xfs: factor out a xfs_btree_owner helper
2024-01-03 20:38 RFC: in-memory btree simplifications Christoph Hellwig
` (2 preceding siblings ...)
2024-01-03 20:38 ` [PATCH 3/5] xfs: remove bc_ino.flags Christoph Hellwig
@ 2024-01-03 20:38 ` Christoph Hellwig
2024-01-04 1:14 ` Darrick J. Wong
2024-01-03 20:38 ` [PATCH 5/5] xfs: embedd struct xfbtree into the owning structure Christoph Hellwig
4 siblings, 1 reply; 19+ messages in thread
From: Christoph Hellwig @ 2024-01-03 20:38 UTC (permalink / raw)
To: Chandan Babu R, Darrick J. Wong; +Cc: linux-xfs
Split out a helper to calculate the owner for a given btree instead
of dulicating the logic in two places.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
fs/xfs/libxfs/xfs_btree.c | 52 +++++++++++++++--------------------
fs/xfs/libxfs/xfs_btree_mem.h | 5 ----
fs/xfs/scrub/xfbtree.c | 29 -------------------
3 files changed, 22 insertions(+), 64 deletions(-)
diff --git a/fs/xfs/libxfs/xfs_btree.c b/fs/xfs/libxfs/xfs_btree.c
index 3bc8aa6049b9a7..bd51c428f66780 100644
--- a/fs/xfs/libxfs/xfs_btree.c
+++ b/fs/xfs/libxfs/xfs_btree.c
@@ -1298,6 +1298,19 @@ xfs_btree_init_buf(
bp->b_ops = ops->buf_ops;
}
+static uint64_t
+xfs_btree_owner(
+ struct xfs_btree_cur *cur)
+{
+#ifdef CONFIG_XFS_BTREE_IN_XFILE
+ if (cur->bc_flags & XFS_BTREE_IN_XFILE)
+ return cur->bc_mem.xfbtree->owner;
+#endif
+ if (cur->bc_flags & XFS_BTREE_LONG_PTRS)
+ return cur->bc_ino.ip->i_ino;
+ return cur->bc_ag.pag->pag_agno;
+}
+
void
xfs_btree_init_block_cur(
struct xfs_btree_cur *cur,
@@ -1305,22 +1318,8 @@ xfs_btree_init_block_cur(
int level,
int numrecs)
{
- __u64 owner;
-
- /*
- * we can pull the owner from the cursor right now as the different
- * owners align directly with the pointer size of the btree. This may
- * change in future, but is safe for current users of the generic btree
- * code.
- */
- if (cur->bc_flags & XFS_BTREE_IN_XFILE)
- owner = xfbtree_owner(cur);
- else if (cur->bc_flags & XFS_BTREE_LONG_PTRS)
- owner = cur->bc_ino.ip->i_ino;
- else
- owner = cur->bc_ag.pag->pag_agno;
-
- xfs_btree_init_buf(cur->bc_mp, bp, cur->bc_ops, level, numrecs, owner);
+ xfs_btree_init_buf(cur->bc_mp, bp, cur->bc_ops, level, numrecs,
+ xfs_btree_owner(cur));
}
/*
@@ -1875,25 +1874,18 @@ xfs_btree_check_block_owner(
struct xfs_btree_cur *cur,
struct xfs_btree_block *block)
{
- if (!xfs_has_crc(cur->bc_mp))
+ if (!xfs_has_crc(cur->bc_mp) ||
+ (cur->bc_flags & XFS_BTREE_BMBT_INVALID_OWNER))
return NULL;
- if (cur->bc_flags & XFS_BTREE_IN_XFILE)
- return xfbtree_check_block_owner(cur, block);
-
- if (!(cur->bc_flags & XFS_BTREE_LONG_PTRS)) {
- if (be32_to_cpu(block->bb_u.s.bb_owner) !=
- cur->bc_ag.pag->pag_agno)
+ if (cur->bc_flags & XFS_BTREE_LONG_PTRS) {
+ if (be64_to_cpu(block->bb_u.l.bb_owner) != xfs_btree_owner(cur))
+ return __this_address;
+ } else {
+ if (be32_to_cpu(block->bb_u.s.bb_owner) != xfs_btree_owner(cur))
return __this_address;
- return NULL;
}
- if (cur->bc_flags & XFS_BTREE_BMBT_INVALID_OWNER)
- return NULL;
-
- if (be64_to_cpu(block->bb_u.l.bb_owner) != cur->bc_ino.ip->i_ino)
- return __this_address;
-
return NULL;
}
diff --git a/fs/xfs/libxfs/xfs_btree_mem.h b/fs/xfs/libxfs/xfs_btree_mem.h
index eeb3340a22d201..3a5492c2cc26b6 100644
--- a/fs/xfs/libxfs/xfs_btree_mem.h
+++ b/fs/xfs/libxfs/xfs_btree_mem.h
@@ -43,9 +43,6 @@ void xfbtree_init_ptr_from_cur(struct xfs_btree_cur *cur,
struct xfs_btree_cur *xfbtree_dup_cursor(struct xfs_btree_cur *cur);
bool xfbtree_verify_xfileoff(struct xfs_btree_cur *cur,
unsigned long long xfoff);
-xfs_failaddr_t xfbtree_check_block_owner(struct xfs_btree_cur *cur,
- struct xfs_btree_block *block);
-unsigned long long xfbtree_owner(struct xfs_btree_cur *cur);
xfs_failaddr_t xfbtree_lblock_verify(struct xfs_buf *bp, unsigned int max_recs);
xfs_failaddr_t xfbtree_sblock_verify(struct xfs_buf *bp, unsigned int max_recs);
unsigned long long xfbtree_buf_to_xfoff(struct xfs_btree_cur *cur,
@@ -102,8 +99,6 @@ static inline unsigned int xfbtree_bbsize(void)
#define xfbtree_alloc_block NULL
#define xfbtree_free_block NULL
#define xfbtree_verify_xfileoff(cur, xfoff) (false)
-#define xfbtree_check_block_owner(cur, block) NULL
-#define xfbtree_owner(cur) (0ULL)
#define xfbtree_buf_to_xfoff(cur, bp) (-1)
static inline int
diff --git a/fs/xfs/scrub/xfbtree.c b/fs/xfs/scrub/xfbtree.c
index 63b69aeadc623d..11dad651508067 100644
--- a/fs/xfs/scrub/xfbtree.c
+++ b/fs/xfs/scrub/xfbtree.c
@@ -165,35 +165,6 @@ xfbtree_dup_cursor(
return ncur;
}
-/* Check the owner of an in-memory btree block. */
-xfs_failaddr_t
-xfbtree_check_block_owner(
- struct xfs_btree_cur *cur,
- struct xfs_btree_block *block)
-{
- struct xfbtree *xfbt = cur->bc_mem.xfbtree;
-
- if (cur->bc_flags & XFS_BTREE_LONG_PTRS) {
- if (be64_to_cpu(block->bb_u.l.bb_owner) != xfbt->owner)
- return __this_address;
-
- return NULL;
- }
-
- if (be32_to_cpu(block->bb_u.s.bb_owner) != xfbt->owner)
- return __this_address;
-
- return NULL;
-}
-
-/* Return the owner of this in-memory btree. */
-unsigned long long
-xfbtree_owner(
- struct xfs_btree_cur *cur)
-{
- return cur->bc_mem.xfbtree->owner;
-}
-
/* Return the xfile offset (in blocks) of a btree buffer. */
unsigned long long
xfbtree_buf_to_xfoff(
--
2.39.2
^ permalink raw reply related [flat|nested] 19+ messages in thread* Re: [PATCH 4/5] xfs: factor out a xfs_btree_owner helper
2024-01-03 20:38 ` [PATCH 4/5] xfs: factor out a xfs_btree_owner helper Christoph Hellwig
@ 2024-01-04 1:14 ` Darrick J. Wong
2024-01-04 6:28 ` Christoph Hellwig
0 siblings, 1 reply; 19+ messages in thread
From: Darrick J. Wong @ 2024-01-04 1:14 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: Chandan Babu R, linux-xfs
On Wed, Jan 03, 2024 at 09:38:35PM +0100, Christoph Hellwig wrote:
> Split out a helper to calculate the owner for a given btree instead
> of dulicating the logic in two places.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
> fs/xfs/libxfs/xfs_btree.c | 52 +++++++++++++++--------------------
> fs/xfs/libxfs/xfs_btree_mem.h | 5 ----
> fs/xfs/scrub/xfbtree.c | 29 -------------------
> 3 files changed, 22 insertions(+), 64 deletions(-)
>
> diff --git a/fs/xfs/libxfs/xfs_btree.c b/fs/xfs/libxfs/xfs_btree.c
> index 3bc8aa6049b9a7..bd51c428f66780 100644
> --- a/fs/xfs/libxfs/xfs_btree.c
> +++ b/fs/xfs/libxfs/xfs_btree.c
> @@ -1298,6 +1298,19 @@ xfs_btree_init_buf(
> bp->b_ops = ops->buf_ops;
> }
>
> +static uint64_t
> +xfs_btree_owner(
> + struct xfs_btree_cur *cur)
> +{
> +#ifdef CONFIG_XFS_BTREE_IN_XFILE
> + if (cur->bc_flags & XFS_BTREE_IN_XFILE)
> + return cur->bc_mem.xfbtree->owner;
> +#endif
Hrm. I guess I never /did/ use xfbtree_owner except for this one file.
> + if (cur->bc_flags & XFS_BTREE_LONG_PTRS)
> + return cur->bc_ino.ip->i_ino;
> + return cur->bc_ag.pag->pag_agno;
> +}
> +
> void
> xfs_btree_init_block_cur(
> struct xfs_btree_cur *cur,
> @@ -1305,22 +1318,8 @@ xfs_btree_init_block_cur(
> int level,
> int numrecs)
> {
> - __u64 owner;
> -
> - /*
> - * we can pull the owner from the cursor right now as the different
> - * owners align directly with the pointer size of the btree. This may
> - * change in future, but is safe for current users of the generic btree
> - * code.
> - */
> - if (cur->bc_flags & XFS_BTREE_IN_XFILE)
> - owner = xfbtree_owner(cur);
> - else if (cur->bc_flags & XFS_BTREE_LONG_PTRS)
> - owner = cur->bc_ino.ip->i_ino;
> - else
> - owner = cur->bc_ag.pag->pag_agno;
> -
> - xfs_btree_init_buf(cur->bc_mp, bp, cur->bc_ops, level, numrecs, owner);
> + xfs_btree_init_buf(cur->bc_mp, bp, cur->bc_ops, level, numrecs,
> + xfs_btree_owner(cur));
> }
>
> /*
> @@ -1875,25 +1874,18 @@ xfs_btree_check_block_owner(
> struct xfs_btree_cur *cur,
> struct xfs_btree_block *block)
> {
> - if (!xfs_has_crc(cur->bc_mp))
> + if (!xfs_has_crc(cur->bc_mp) ||
I wonder, shouldn't this be (bc_flags & XFS_BTREE_CRC_BLOCKS) and not
xfs_has_crc? They're one and the same, but as the geometry flags are
all getting moved to xfs_btree_ops, we ought to be consistent about what
we check.
--D
> + (cur->bc_flags & XFS_BTREE_BMBT_INVALID_OWNER))
> return NULL;
>
> - if (cur->bc_flags & XFS_BTREE_IN_XFILE)
> - return xfbtree_check_block_owner(cur, block);
> -
> - if (!(cur->bc_flags & XFS_BTREE_LONG_PTRS)) {
> - if (be32_to_cpu(block->bb_u.s.bb_owner) !=
> - cur->bc_ag.pag->pag_agno)
> + if (cur->bc_flags & XFS_BTREE_LONG_PTRS) {
> + if (be64_to_cpu(block->bb_u.l.bb_owner) != xfs_btree_owner(cur))
> + return __this_address;
> + } else {
> + if (be32_to_cpu(block->bb_u.s.bb_owner) != xfs_btree_owner(cur))
> return __this_address;
> - return NULL;
> }
>
> - if (cur->bc_flags & XFS_BTREE_BMBT_INVALID_OWNER)
> - return NULL;
> -
> - if (be64_to_cpu(block->bb_u.l.bb_owner) != cur->bc_ino.ip->i_ino)
> - return __this_address;
> -
> return NULL;
> }
>
> diff --git a/fs/xfs/libxfs/xfs_btree_mem.h b/fs/xfs/libxfs/xfs_btree_mem.h
> index eeb3340a22d201..3a5492c2cc26b6 100644
> --- a/fs/xfs/libxfs/xfs_btree_mem.h
> +++ b/fs/xfs/libxfs/xfs_btree_mem.h
> @@ -43,9 +43,6 @@ void xfbtree_init_ptr_from_cur(struct xfs_btree_cur *cur,
> struct xfs_btree_cur *xfbtree_dup_cursor(struct xfs_btree_cur *cur);
> bool xfbtree_verify_xfileoff(struct xfs_btree_cur *cur,
> unsigned long long xfoff);
> -xfs_failaddr_t xfbtree_check_block_owner(struct xfs_btree_cur *cur,
> - struct xfs_btree_block *block);
> -unsigned long long xfbtree_owner(struct xfs_btree_cur *cur);
> xfs_failaddr_t xfbtree_lblock_verify(struct xfs_buf *bp, unsigned int max_recs);
> xfs_failaddr_t xfbtree_sblock_verify(struct xfs_buf *bp, unsigned int max_recs);
> unsigned long long xfbtree_buf_to_xfoff(struct xfs_btree_cur *cur,
> @@ -102,8 +99,6 @@ static inline unsigned int xfbtree_bbsize(void)
> #define xfbtree_alloc_block NULL
> #define xfbtree_free_block NULL
> #define xfbtree_verify_xfileoff(cur, xfoff) (false)
> -#define xfbtree_check_block_owner(cur, block) NULL
> -#define xfbtree_owner(cur) (0ULL)
> #define xfbtree_buf_to_xfoff(cur, bp) (-1)
>
> static inline int
> diff --git a/fs/xfs/scrub/xfbtree.c b/fs/xfs/scrub/xfbtree.c
> index 63b69aeadc623d..11dad651508067 100644
> --- a/fs/xfs/scrub/xfbtree.c
> +++ b/fs/xfs/scrub/xfbtree.c
> @@ -165,35 +165,6 @@ xfbtree_dup_cursor(
> return ncur;
> }
>
> -/* Check the owner of an in-memory btree block. */
> -xfs_failaddr_t
> -xfbtree_check_block_owner(
> - struct xfs_btree_cur *cur,
> - struct xfs_btree_block *block)
> -{
> - struct xfbtree *xfbt = cur->bc_mem.xfbtree;
> -
> - if (cur->bc_flags & XFS_BTREE_LONG_PTRS) {
> - if (be64_to_cpu(block->bb_u.l.bb_owner) != xfbt->owner)
> - return __this_address;
> -
> - return NULL;
> - }
> -
> - if (be32_to_cpu(block->bb_u.s.bb_owner) != xfbt->owner)
> - return __this_address;
> -
> - return NULL;
> -}
> -
> -/* Return the owner of this in-memory btree. */
> -unsigned long long
> -xfbtree_owner(
> - struct xfs_btree_cur *cur)
> -{
> - return cur->bc_mem.xfbtree->owner;
> -}
> -
> /* Return the xfile offset (in blocks) of a btree buffer. */
> unsigned long long
> xfbtree_buf_to_xfoff(
> --
> 2.39.2
>
>
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [PATCH 4/5] xfs: factor out a xfs_btree_owner helper
2024-01-04 1:14 ` Darrick J. Wong
@ 2024-01-04 6:28 ` Christoph Hellwig
0 siblings, 0 replies; 19+ messages in thread
From: Christoph Hellwig @ 2024-01-04 6:28 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: Christoph Hellwig, Chandan Babu R, linux-xfs
On Wed, Jan 03, 2024 at 05:14:00PM -0800, Darrick J. Wong wrote:
> > @@ -1875,25 +1874,18 @@ xfs_btree_check_block_owner(
> > struct xfs_btree_cur *cur,
> > struct xfs_btree_block *block)
> > {
> > - if (!xfs_has_crc(cur->bc_mp))
> > + if (!xfs_has_crc(cur->bc_mp) ||
>
> I wonder, shouldn't this be (bc_flags & XFS_BTREE_CRC_BLOCKS) and not
> xfs_has_crc? They're one and the same, but as the geometry flags are
> all getting moved to xfs_btree_ops, we ought to be consistent about what
> we check.
Sure.
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH 5/5] xfs: embedd struct xfbtree into the owning structure
2024-01-03 20:38 RFC: in-memory btree simplifications Christoph Hellwig
` (3 preceding siblings ...)
2024-01-03 20:38 ` [PATCH 4/5] xfs: factor out a xfs_btree_owner helper Christoph Hellwig
@ 2024-01-03 20:38 ` Christoph Hellwig
2024-01-04 1:21 ` Darrick J. Wong
4 siblings, 1 reply; 19+ messages in thread
From: Christoph Hellwig @ 2024-01-03 20:38 UTC (permalink / raw)
To: Chandan Babu R, Darrick J. Wong; +Cc: linux-xfs
This will allow to use container_of to get at the containing structure,
which will be useful soon.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
.../filesystems/xfs-online-fsck-design.rst | 5 +-
fs/xfs/libxfs/xfs_btree_mem.h | 26 ++--------
fs/xfs/libxfs/xfs_rmap_btree.c | 13 ++---
fs/xfs/libxfs/xfs_rmap_btree.h | 2 +-
fs/xfs/libxfs/xfs_rtrmap_btree.c | 13 ++---
fs/xfs/libxfs/xfs_rtrmap_btree.h | 2 +-
fs/xfs/scrub/rcbag.c | 20 ++++----
fs/xfs/scrub/rcbag_btree.c | 11 ++--
fs/xfs/scrub/rcbag_btree.h | 2 +-
fs/xfs/scrub/rmap_repair.c | 24 ++++-----
fs/xfs/scrub/rtrmap_repair.c | 22 ++++----
fs/xfs/scrub/trace.h | 13 +++--
fs/xfs/scrub/xfbtree.c | 50 ++++++++-----------
13 files changed, 82 insertions(+), 121 deletions(-)
diff --git a/Documentation/filesystems/xfs-online-fsck-design.rst b/Documentation/filesystems/xfs-online-fsck-design.rst
index 29e123189d3039..5563ce9663711c 100644
--- a/Documentation/filesystems/xfs-online-fsck-design.rst
+++ b/Documentation/filesystems/xfs-online-fsck-design.rst
@@ -2277,13 +2277,12 @@ follows:
pointing to the xfile.
3. Pass the buffer cache target, buffer ops, and other information to
- ``xfbtree_create`` to write an initial tree header and root block to the
- xfile.
+ ``xfbtree_init`` to initialize the passed in ``struct xfbtree`` and write an
+ initial root block to the xfile.
Each btree type should define a wrapper that passes necessary arguments to
the creation function.
For example, rmap btrees define ``xfs_rmapbt_mem_create`` to take care of
all the necessary details for callers.
- A ``struct xfbtree`` object will be returned.
4. Pass the xfbtree object to the btree cursor creation function for the
btree type.
diff --git a/fs/xfs/libxfs/xfs_btree_mem.h b/fs/xfs/libxfs/xfs_btree_mem.h
index 3a5492c2cc26b6..0740537a06c6b0 100644
--- a/fs/xfs/libxfs/xfs_btree_mem.h
+++ b/fs/xfs/libxfs/xfs_btree_mem.h
@@ -8,23 +8,6 @@
struct xfbtree;
-struct xfbtree_config {
- /* Buffer ops for the btree root block */
- const struct xfs_btree_ops *btree_ops;
-
- /* Buffer target for the xfile backing this btree. */
- struct xfs_buftarg *target;
-
- /* Owner of this btree. */
- unsigned long long owner;
-
- /* XFBTREE_* flags */
- unsigned int flags;
-};
-
-/* buffers should be directly mapped from memory */
-#define XFBTREE_DIRECT_MAP (1U << 0)
-
#ifdef CONFIG_XFS_BTREE_IN_XFILE
struct xfs_buftarg *xfbtree_target(struct xfbtree *xfbtree);
int xfbtree_check_ptr(struct xfs_btree_cur *cur,
@@ -51,8 +34,9 @@ unsigned long long xfbtree_buf_to_xfoff(struct xfs_btree_cur *cur,
int xfbtree_get_minrecs(struct xfs_btree_cur *cur, int level);
int xfbtree_get_maxrecs(struct xfs_btree_cur *cur, int level);
-int xfbtree_create(struct xfs_mount *mp, const struct xfbtree_config *cfg,
- struct xfbtree **xfbtreep);
+int xfbtree_init(struct xfs_mount *mp, struct xfbtree *xfbt,
+ const struct xfs_btree_ops *btree_ops);
+
int xfbtree_alloc_block(struct xfs_btree_cur *cur,
const union xfs_btree_ptr *start, union xfs_btree_ptr *ptr,
int *stat);
@@ -102,8 +86,8 @@ static inline unsigned int xfbtree_bbsize(void)
#define xfbtree_buf_to_xfoff(cur, bp) (-1)
static inline int
-xfbtree_create(struct xfs_mount *mp, const struct xfbtree_config *cfg,
- struct xfbtree **xfbtreep)
+xfbtree_init(struct xfs_mount *mp, struct xfbtree *xfbt,
+ const struct xfs_btree_ops *btree_ops)
{
return -EOPNOTSUPP;
}
diff --git a/fs/xfs/libxfs/xfs_rmap_btree.c b/fs/xfs/libxfs/xfs_rmap_btree.c
index 41f1b5fa863302..332fdcd07160e4 100644
--- a/fs/xfs/libxfs/xfs_rmap_btree.c
+++ b/fs/xfs/libxfs/xfs_rmap_btree.c
@@ -664,16 +664,11 @@ xfs_rmapbt_mem_create(
struct xfs_mount *mp,
xfs_agnumber_t agno,
struct xfs_buftarg *target,
- struct xfbtree **xfbtreep)
+ struct xfbtree *xfbt)
{
- struct xfbtree_config cfg = {
- .btree_ops = &xfs_rmapbt_mem_ops,
- .target = target,
- .owner = agno,
- .flags = XFBTREE_DIRECT_MAP,
- };
-
- return xfbtree_create(mp, &cfg, xfbtreep);
+ xfbt->target = target;
+ xfbt->owner = agno;
+ return xfbtree_init(mp, xfbt, &xfs_rmapbt_mem_ops);
}
#endif /* CONFIG_XFS_BTREE_IN_XFILE */
diff --git a/fs/xfs/libxfs/xfs_rmap_btree.h b/fs/xfs/libxfs/xfs_rmap_btree.h
index dfe13b8cbb732d..1c114efbc090d5 100644
--- a/fs/xfs/libxfs/xfs_rmap_btree.h
+++ b/fs/xfs/libxfs/xfs_rmap_btree.h
@@ -70,7 +70,7 @@ struct xfbtree;
struct xfs_btree_cur *xfs_rmapbt_mem_cursor(struct xfs_perag *pag,
struct xfs_trans *tp, struct xfbtree *xfbtree);
int xfs_rmapbt_mem_create(struct xfs_mount *mp, xfs_agnumber_t agno,
- struct xfs_buftarg *target, struct xfbtree **xfbtreep);
+ struct xfs_buftarg *target, struct xfbtree *xfbt);
#endif /* CONFIG_XFS_BTREE_IN_XFILE */
#endif /* __XFS_RMAP_BTREE_H__ */
diff --git a/fs/xfs/libxfs/xfs_rtrmap_btree.c b/fs/xfs/libxfs/xfs_rtrmap_btree.c
index 95983dc081fa21..557f829c0826c1 100644
--- a/fs/xfs/libxfs/xfs_rtrmap_btree.c
+++ b/fs/xfs/libxfs/xfs_rtrmap_btree.c
@@ -657,16 +657,11 @@ xfs_rtrmapbt_mem_create(
struct xfs_mount *mp,
xfs_rgnumber_t rgno,
struct xfs_buftarg *target,
- struct xfbtree **xfbtreep)
+ struct xfbtree *xfbt)
{
- struct xfbtree_config cfg = {
- .btree_ops = &xfs_rtrmapbt_mem_ops,
- .target = target,
- .flags = XFBTREE_DIRECT_MAP,
- .owner = rgno,
- };
-
- return xfbtree_create(mp, &cfg, xfbtreep);
+ xfbt->target = target;
+ xfbt->owner = rgno;
+ return xfbtree_init(mp, xfbt, &xfs_rtrmapbt_mem_ops);
}
#endif /* CONFIG_XFS_BTREE_IN_XFILE */
diff --git a/fs/xfs/libxfs/xfs_rtrmap_btree.h b/fs/xfs/libxfs/xfs_rtrmap_btree.h
index 3347205846eb2e..b499fc7fc2e529 100644
--- a/fs/xfs/libxfs/xfs_rtrmap_btree.h
+++ b/fs/xfs/libxfs/xfs_rtrmap_btree.h
@@ -210,7 +210,7 @@ struct xfbtree;
struct xfs_btree_cur *xfs_rtrmapbt_mem_cursor(struct xfs_rtgroup *rtg,
struct xfs_trans *tp, struct xfbtree *xfbtree);
int xfs_rtrmapbt_mem_create(struct xfs_mount *mp, xfs_rgnumber_t rgno,
- struct xfs_buftarg *target, struct xfbtree **xfbtreep);
+ struct xfs_buftarg *target, struct xfbtree *xfbt);
#endif /* CONFIG_XFS_BTREE_IN_XFILE */
#endif /* __XFS_RTRMAP_BTREE_H__ */
diff --git a/fs/xfs/scrub/rcbag.c b/fs/xfs/scrub/rcbag.c
index f28ce02f961c7c..1f3c4555e78ebc 100644
--- a/fs/xfs/scrub/rcbag.c
+++ b/fs/xfs/scrub/rcbag.c
@@ -24,7 +24,7 @@
struct rcbag {
struct xfs_mount *mp;
- struct xfbtree *xfbtree;
+ struct xfbtree xfbtree;
uint64_t nr_items;
};
@@ -62,7 +62,7 @@ rcbag_free(
{
struct rcbag *bag = *bagp;
- xfbtree_destroy(bag->xfbtree);
+ xfbtree_destroy(&bag->xfbtree);
kfree(bag);
*bagp = NULL;
}
@@ -80,7 +80,7 @@ rcbag_add(
int has;
int error;
- cur = rcbagbt_mem_cursor(mp, tp, bag->xfbtree);
+ cur = rcbagbt_mem_cursor(mp, tp, &bag->xfbtree);
error = rcbagbt_lookup_eq(cur, rmap, &has);
if (error)
goto out_cur;
@@ -114,7 +114,7 @@ rcbag_add(
xfs_btree_del_cursor(cur, 0);
- error = xfbtree_trans_commit(bag->xfbtree, tp);
+ error = xfbtree_trans_commit(&bag->xfbtree, tp);
if (error)
return error;
@@ -123,7 +123,7 @@ rcbag_add(
out_cur:
xfs_btree_del_cursor(cur, error);
- xfbtree_trans_cancel(bag->xfbtree, tp);
+ xfbtree_trans_cancel(&bag->xfbtree, tp);
return error;
}
@@ -158,7 +158,7 @@ rcbag_next_edge(
if (next_valid)
next_bno = next_rmap->rm_startblock;
- cur = rcbagbt_mem_cursor(mp, tp, bag->xfbtree);
+ cur = rcbagbt_mem_cursor(mp, tp, &bag->xfbtree);
error = xfs_btree_goto_left_edge(cur);
if (error)
goto out_cur;
@@ -216,7 +216,7 @@ rcbag_remove_ending_at(
int error;
/* go to the right edge of the tree */
- cur = rcbagbt_mem_cursor(mp, tp, bag->xfbtree);
+ cur = rcbagbt_mem_cursor(mp, tp, &bag->xfbtree);
memset(&cur->bc_rec, 0xFF, sizeof(cur->bc_rec));
error = xfs_btree_lookup(cur, XFS_LOOKUP_GE, &has);
if (error)
@@ -252,10 +252,10 @@ rcbag_remove_ending_at(
}
xfs_btree_del_cursor(cur, 0);
- return xfbtree_trans_commit(bag->xfbtree, tp);
+ return xfbtree_trans_commit(&bag->xfbtree, tp);
out_cur:
xfs_btree_del_cursor(cur, error);
- xfbtree_trans_cancel(bag->xfbtree, tp);
+ xfbtree_trans_cancel(&bag->xfbtree, tp);
return error;
}
@@ -272,7 +272,7 @@ rcbag_dump(
int has;
int error;
- cur = rcbagbt_mem_cursor(mp, tp, bag->xfbtree);
+ cur = rcbagbt_mem_cursor(mp, tp, &bag->xfbtree);
error = xfs_btree_goto_left_edge(cur);
if (error)
goto out_cur;
diff --git a/fs/xfs/scrub/rcbag_btree.c b/fs/xfs/scrub/rcbag_btree.c
index 6f0b48b5c37bbd..bbb61d09d97927 100644
--- a/fs/xfs/scrub/rcbag_btree.c
+++ b/fs/xfs/scrub/rcbag_btree.c
@@ -226,15 +226,10 @@ int
rcbagbt_mem_create(
struct xfs_mount *mp,
struct xfs_buftarg *target,
- struct xfbtree **xfbtreep)
+ struct xfbtree *xfbt)
{
- struct xfbtree_config cfg = {
- .btree_ops = &rcbagbt_mem_ops,
- .target = target,
- .flags = XFBTREE_DIRECT_MAP,
- };
-
- return xfbtree_create(mp, &cfg, xfbtreep);
+ xfbt->target = target;
+ return xfbtree_init(mp, xfbt, &rcbagbt_mem_ops);
}
/* Calculate number of records in a refcount bag btree block. */
diff --git a/fs/xfs/scrub/rcbag_btree.h b/fs/xfs/scrub/rcbag_btree.h
index 59d81d707d32a5..4d3d9d1e49e2fe 100644
--- a/fs/xfs/scrub/rcbag_btree.h
+++ b/fs/xfs/scrub/rcbag_btree.h
@@ -65,7 +65,7 @@ struct xfbtree;
struct xfs_btree_cur *rcbagbt_mem_cursor(struct xfs_mount *mp,
struct xfs_trans *tp, struct xfbtree *xfbtree);
int rcbagbt_mem_create(struct xfs_mount *mp, struct xfs_buftarg *target,
- struct xfbtree **xfbtreep);
+ struct xfbtree *xfbt);
int rcbagbt_lookup_eq(struct xfs_btree_cur *cur,
const struct xfs_rmap_irec *rmap, int *success);
diff --git a/fs/xfs/scrub/rmap_repair.c b/fs/xfs/scrub/rmap_repair.c
index ab61f31868f841..abeeee88a6ebb3 100644
--- a/fs/xfs/scrub/rmap_repair.c
+++ b/fs/xfs/scrub/rmap_repair.c
@@ -135,7 +135,7 @@ struct xrep_rmap {
struct mutex lock;
/* rmap records generated from primary metadata */
- struct xfbtree *rmap_btree;
+ struct xfbtree rmap_btree;
struct xfs_scrub *sc;
@@ -237,13 +237,13 @@ xrep_rmap_stash(
trace_xrep_rmap_found(sc->mp, sc->sa.pag->pag_agno, &rmap);
mutex_lock(&rr->lock);
- mcur = xfs_rmapbt_mem_cursor(sc->sa.pag, sc->tp, rr->rmap_btree);
+ mcur = xfs_rmapbt_mem_cursor(sc->sa.pag, sc->tp, &rr->rmap_btree);
error = xfs_rmap_map_raw(mcur, &rmap);
xfs_btree_del_cursor(mcur, error);
if (error)
goto out_cancel;
- error = xfbtree_trans_commit(rr->rmap_btree, sc->tp);
+ error = xfbtree_trans_commit(&rr->rmap_btree, sc->tp);
if (error)
goto out_abort;
@@ -251,7 +251,7 @@ xrep_rmap_stash(
return 0;
out_cancel:
- xfbtree_trans_cancel(rr->rmap_btree, sc->tp);
+ xfbtree_trans_cancel(&rr->rmap_btree, sc->tp);
out_abort:
xchk_iscan_abort(&rr->iscan);
mutex_unlock(&rr->lock);
@@ -1004,7 +1004,7 @@ xrep_rmap_find_rmaps(
* all our records before we start building a new btree, which requires
* a bnobt cursor.
*/
- mcur = xfs_rmapbt_mem_cursor(rr->sc->sa.pag, NULL, rr->rmap_btree);
+ mcur = xfs_rmapbt_mem_cursor(rr->sc->sa.pag, NULL, &rr->rmap_btree);
sc->sa.bno_cur = xfs_allocbt_init_cursor(sc->mp, sc->tp, sc->sa.agf_bp,
sc->sa.pag, XFS_BTNUM_BNO);
@@ -1389,7 +1389,7 @@ xrep_rmap_build_new_tree(
* Count the rmapbt records again, because the space reservation
* for the rmapbt itself probably added more records to the btree.
*/
- rr->mcur = xfs_rmapbt_mem_cursor(rr->sc->sa.pag, NULL, rr->rmap_btree);
+ rr->mcur = xfs_rmapbt_mem_cursor(rr->sc->sa.pag, NULL, &rr->rmap_btree);
error = xrep_rmap_count_records(rr->mcur, &rr->nr_records);
if (error)
@@ -1528,7 +1528,7 @@ xrep_rmap_remove_old_tree(
xagb_bitmap_init(&rfg.rmap_gaps);
/* Compute free space from the new rmapbt. */
- mcur = xfs_rmapbt_mem_cursor(rr->sc->sa.pag, NULL, rr->rmap_btree);
+ mcur = xfs_rmapbt_mem_cursor(rr->sc->sa.pag, NULL, &rr->rmap_btree);
error = xfs_rmap_query_all(mcur, xrep_rmap_find_gaps, &rfg);
xfs_btree_del_cursor(mcur, error);
@@ -1638,14 +1638,14 @@ xrep_rmapbt_live_update(
goto out_abort;
mutex_lock(&rr->lock);
- mcur = xfs_rmapbt_mem_cursor(rr->sc->sa.pag, tp, rr->rmap_btree);
+ mcur = xfs_rmapbt_mem_cursor(rr->sc->sa.pag, tp, &rr->rmap_btree);
error = __xfs_rmap_finish_intent(mcur, action, p->startblock,
p->blockcount, &p->oinfo, p->unwritten);
xfs_btree_del_cursor(mcur, error);
if (error)
goto out_cancel;
- error = xfbtree_trans_commit(rr->rmap_btree, tp);
+ error = xfbtree_trans_commit(&rr->rmap_btree, tp);
if (error)
goto out_cancel;
@@ -1654,7 +1654,7 @@ xrep_rmapbt_live_update(
return NOTIFY_DONE;
out_cancel:
- xfbtree_trans_cancel(rr->rmap_btree, tp);
+ xfbtree_trans_cancel(&rr->rmap_btree, tp);
xrep_trans_cancel_hook_dummy(&txcookie, tp);
out_abort:
mutex_unlock(&rr->lock);
@@ -1697,7 +1697,7 @@ xrep_rmap_setup_scan(
out_iscan:
xchk_iscan_teardown(&rr->iscan);
- xfbtree_destroy(rr->rmap_btree);
+ xfbtree_destroy(&rr->rmap_btree);
out_mutex:
mutex_destroy(&rr->lock);
return error;
@@ -1713,7 +1713,7 @@ xrep_rmap_teardown(
xchk_iscan_abort(&rr->iscan);
xfs_rmap_hook_del(sc->sa.pag, &rr->hooks);
xchk_iscan_teardown(&rr->iscan);
- xfbtree_destroy(rr->rmap_btree);
+ xfbtree_destroy(&rr->rmap_btree);
mutex_destroy(&rr->lock);
}
diff --git a/fs/xfs/scrub/rtrmap_repair.c b/fs/xfs/scrub/rtrmap_repair.c
index 885752c7436b45..5c3b26ca3affd7 100644
--- a/fs/xfs/scrub/rtrmap_repair.c
+++ b/fs/xfs/scrub/rtrmap_repair.c
@@ -77,7 +77,7 @@ struct xrep_rtrmap {
struct mutex lock;
/* rmap records generated from primary metadata */
- struct xfbtree *rtrmap_btree;
+ struct xfbtree rtrmap_btree;
struct xfs_scrub *sc;
@@ -171,13 +171,13 @@ xrep_rtrmap_stash(
/* Add entry to in-memory btree. */
mutex_lock(&rr->lock);
- mcur = xfs_rtrmapbt_mem_cursor(sc->sr.rtg, sc->tp, rr->rtrmap_btree);
+ mcur = xfs_rtrmapbt_mem_cursor(sc->sr.rtg, sc->tp, &rr->rtrmap_btree);
error = xfs_rmap_map_raw(mcur, &rmap);
xfs_btree_del_cursor(mcur, error);
if (error)
goto out_cancel;
- error = xfbtree_trans_commit(rr->rtrmap_btree, sc->tp);
+ error = xfbtree_trans_commit(&rr->rtrmap_btree, sc->tp);
if (error)
goto out_abort;
@@ -185,7 +185,7 @@ xrep_rtrmap_stash(
return 0;
out_cancel:
- xfbtree_trans_cancel(rr->rtrmap_btree, sc->tp);
+ xfbtree_trans_cancel(&rr->rtrmap_btree, sc->tp);
out_abort:
xchk_iscan_abort(&rr->iscan);
mutex_unlock(&rr->lock);
@@ -648,7 +648,7 @@ xrep_rtrmap_find_rmaps(
* check all our records before we start building a new btree, which
* requires the rtbitmap lock.
*/
- mcur = xfs_rtrmapbt_mem_cursor(rr->sc->sr.rtg, NULL, rr->rtrmap_btree);
+ mcur = xfs_rtrmapbt_mem_cursor(rr->sc->sr.rtg, NULL, &rr->rtrmap_btree);
rr->nr_records = 0;
error = xfs_rmap_query_all(mcur, xrep_rtrmap_check_record, rr);
xfs_btree_del_cursor(mcur, error);
@@ -781,7 +781,7 @@ xrep_rtrmap_build_new_tree(
* Create a cursor to the in-memory btree so that we can bulk load the
* new btree.
*/
- rr->mcur = xfs_rtrmapbt_mem_cursor(sc->sr.rtg, NULL, rr->rtrmap_btree);
+ rr->mcur = xfs_rtrmapbt_mem_cursor(sc->sr.rtg, NULL, &rr->rtrmap_btree);
error = xfs_btree_goto_left_edge(rr->mcur);
if (error)
goto err_mcur;
@@ -900,14 +900,14 @@ xrep_rtrmapbt_live_update(
goto out_abort;
mutex_lock(&rr->lock);
- mcur = xfs_rtrmapbt_mem_cursor(rr->sc->sr.rtg, tp, rr->rtrmap_btree);
+ mcur = xfs_rtrmapbt_mem_cursor(rr->sc->sr.rtg, tp, &rr->rtrmap_btree);
error = __xfs_rmap_finish_intent(mcur, action, p->startblock,
p->blockcount, &p->oinfo, p->unwritten);
xfs_btree_del_cursor(mcur, error);
if (error)
goto out_cancel;
- error = xfbtree_trans_commit(rr->rtrmap_btree, tp);
+ error = xfbtree_trans_commit(&rr->rtrmap_btree, tp);
if (error)
goto out_cancel;
@@ -916,7 +916,7 @@ xrep_rtrmapbt_live_update(
return NOTIFY_DONE;
out_cancel:
- xfbtree_trans_cancel(rr->rtrmap_btree, tp);
+ xfbtree_trans_cancel(&rr->rtrmap_btree, tp);
xrep_trans_cancel_hook_dummy(&txcookie, tp);
out_abort:
xchk_iscan_abort(&rr->iscan);
@@ -960,7 +960,7 @@ xrep_rtrmap_setup_scan(
out_iscan:
xchk_iscan_teardown(&rr->iscan);
- xfbtree_destroy(rr->rtrmap_btree);
+ xfbtree_destroy(&rr->rtrmap_btree);
out_bitmap:
xfsb_bitmap_destroy(&rr->old_rtrmapbt_blocks);
mutex_destroy(&rr->lock);
@@ -977,7 +977,7 @@ xrep_rtrmap_teardown(
xchk_iscan_abort(&rr->iscan);
xfs_rtrmap_hook_del(sc->sr.rtg, &rr->hooks);
xchk_iscan_teardown(&rr->iscan);
- xfbtree_destroy(rr->rtrmap_btree);
+ xfbtree_destroy(&rr->rtrmap_btree);
xfsb_bitmap_destroy(&rr->old_rtrmapbt_blocks);
mutex_destroy(&rr->lock);
}
diff --git a/fs/xfs/scrub/trace.h b/fs/xfs/scrub/trace.h
index 6e15de56be2b75..ddd75799ccecd8 100644
--- a/fs/xfs/scrub/trace.h
+++ b/fs/xfs/scrub/trace.h
@@ -26,7 +26,6 @@ struct xchk_iscan;
struct xchk_nlink;
struct xchk_fscounters;
struct xfbtree;
-struct xfbtree_config;
struct xfs_rmap_update_params;
struct xfs_parent_name_irec;
enum xchk_dirpath_outcome;
@@ -2817,10 +2816,10 @@ DEFINE_XREP_DQUOT_EVENT(xrep_quotacheck_dquot);
DEFINE_SCRUB_NLINKS_DIFF_EVENT(xrep_nlinks_update_inode);
DEFINE_SCRUB_NLINKS_DIFF_EVENT(xrep_nlinks_unfixable_inode);
-TRACE_EVENT(xfbtree_create,
- TP_PROTO(struct xfs_mount *mp, const struct xfbtree_config *cfg,
- struct xfbtree *xfbt),
- TP_ARGS(mp, cfg, xfbt),
+TRACE_EVENT(xfbtree_init,
+ TP_PROTO(struct xfs_mount *mp, struct xfbtree *xfbt,
+ const struct xfs_btree_ops *btree_ops),
+ TP_ARGS(mp, xfbt, btree_ops),
TP_STRUCT__entry(
__field(const void *, btree_ops)
__field(unsigned long, xfino)
@@ -2831,13 +2830,13 @@ TRACE_EVENT(xfbtree_create,
__field(unsigned long long, owner)
),
TP_fast_assign(
- __entry->btree_ops = cfg->btree_ops;
+ __entry->btree_ops = btree_ops;
__entry->xfino = xfbtree_ino(xfbt);
__entry->leaf_mxr = xfbt->maxrecs[0];
__entry->node_mxr = xfbt->maxrecs[1];
__entry->leaf_mnr = xfbt->minrecs[0];
__entry->node_mnr = xfbt->minrecs[1];
- __entry->owner = cfg->owner;
+ __entry->owner = xfbt->owner;
),
TP_printk("xfino 0x%lx btree_ops %pS owner 0x%llx leaf_mxr %u leaf_mnr %u node_mxr %u node_mnr %u",
__entry->xfino,
diff --git a/fs/xfs/scrub/xfbtree.c b/fs/xfs/scrub/xfbtree.c
index 11dad651508067..9f59c7f88be9ff 100644
--- a/fs/xfs/scrub/xfbtree.c
+++ b/fs/xfs/scrub/xfbtree.c
@@ -237,18 +237,17 @@ xfbtree_destroy(
{
xbitmap64_destroy(&xfbt->freespace);
xfs_buftarg_drain(xfbt->target);
- kfree(xfbt);
}
/* Compute the number of bytes available for records. */
static inline unsigned int
xfbtree_rec_bytes(
struct xfs_mount *mp,
- const struct xfbtree_config *cfg)
+ const struct xfs_btree_ops *btree_ops)
{
unsigned int blocklen = xfo_to_b(1);
- if (cfg->btree_ops->geom_flags & XFS_BTREE_LONG_PTRS) {
+ if (btree_ops->geom_flags & XFS_BTREE_LONG_PTRS) {
if (xfs_has_crc(mp))
return blocklen - XFS_BTREE_LBLOCK_CRC_LEN;
@@ -266,7 +265,7 @@ STATIC int
xfbtree_init_leaf_block(
struct xfs_mount *mp,
struct xfbtree *xfbt,
- const struct xfbtree_config *cfg)
+ const struct xfs_btree_ops *btree_ops)
{
struct xfs_buf *bp;
xfileoff_t xfoff = xfbt->highest_offset++;
@@ -279,13 +278,13 @@ xfbtree_init_leaf_block(
trace_xfbtree_create_root_buf(xfbt, bp);
- xfs_btree_init_buf(mp, bp, cfg->btree_ops, 0, 0, cfg->owner);
+ xfs_btree_init_buf(mp, bp, btree_ops, 0, 0, xfbt->owner);
error = xfs_bwrite(bp);
xfs_buf_relse(bp);
if (error)
return error;
- if (cfg->btree_ops->geom_flags & XFS_BTREE_LONG_PTRS)
+ if (btree_ops->geom_flags & XFS_BTREE_LONG_PTRS)
xfbt->root.l = xfoff;
else
xfbt->root.s = xfoff;
@@ -294,57 +293,52 @@ xfbtree_init_leaf_block(
/* Create an xfile btree backing thing that can be used for in-memory btrees. */
int
-xfbtree_create(
+xfbtree_init(
struct xfs_mount *mp,
- const struct xfbtree_config *cfg,
- struct xfbtree **xfbtreep)
+ struct xfbtree *xfbt,
+ const struct xfs_btree_ops *btree_ops)
{
- struct xfbtree *xfbt;
- unsigned int blocklen = xfbtree_rec_bytes(mp, cfg);
- unsigned int keyptr_len = cfg->btree_ops->key_len;
+ unsigned int blocklen = xfbtree_rec_bytes(mp, btree_ops);
+ unsigned int keyptr_len = btree_ops->key_len;
int error;
/* Requires an xfile-backed buftarg. */
- if (!(cfg->target->bt_flags & XFS_BUFTARG_XFILE)) {
- ASSERT(cfg->target->bt_flags & XFS_BUFTARG_XFILE);
+ if (!xfbt->target) {
+ ASSERT(xfbt->target);
return -EINVAL;
}
+ if (!(xfbt->target->bt_flags & XFS_BUFTARG_XFILE)) {
+ ASSERT(xfbt->target->bt_flags & XFS_BUFTARG_XFILE);
+ return -EINVAL;
+ }
+ xfbt->target->bt_flags |= XFS_BUFTARG_DIRECT_MAP;
- xfbt = kzalloc(sizeof(struct xfbtree), XCHK_GFP_FLAGS);
- if (!xfbt)
- return -ENOMEM;
- xfbt->target = cfg->target;
- if (cfg->flags & XFBTREE_DIRECT_MAP)
- xfbt->target->bt_flags |= XFS_BUFTARG_DIRECT_MAP;
-
+ xfbt->highest_offset = 0;
xbitmap64_init(&xfbt->freespace);
/* Set up min/maxrecs for this btree. */
- if (cfg->btree_ops->geom_flags & XFS_BTREE_LONG_PTRS)
+ if (btree_ops->geom_flags & XFS_BTREE_LONG_PTRS)
keyptr_len += sizeof(__be64);
else
keyptr_len += sizeof(__be32);
- xfbt->maxrecs[0] = blocklen / cfg->btree_ops->rec_len;
+ xfbt->maxrecs[0] = blocklen / btree_ops->rec_len;
xfbt->maxrecs[1] = blocklen / keyptr_len;
xfbt->minrecs[0] = xfbt->maxrecs[0] / 2;
xfbt->minrecs[1] = xfbt->maxrecs[1] / 2;
- xfbt->owner = cfg->owner;
xfbt->nlevels = 1;
/* Initialize the empty btree. */
- error = xfbtree_init_leaf_block(mp, xfbt, cfg);
+ error = xfbtree_init_leaf_block(mp, xfbt, btree_ops);
if (error)
goto err_freesp;
- trace_xfbtree_create(mp, cfg, xfbt);
+ trace_xfbtree_init(mp, xfbt, btree_ops);
- *xfbtreep = xfbt;
return 0;
err_freesp:
xbitmap64_destroy(&xfbt->freespace);
xfs_buftarg_drain(xfbt->target);
- kfree(xfbt);
return error;
}
--
2.39.2
^ permalink raw reply related [flat|nested] 19+ messages in thread* Re: [PATCH 5/5] xfs: embedd struct xfbtree into the owning structure
2024-01-03 20:38 ` [PATCH 5/5] xfs: embedd struct xfbtree into the owning structure Christoph Hellwig
@ 2024-01-04 1:21 ` Darrick J. Wong
2024-01-04 6:32 ` Christoph Hellwig
0 siblings, 1 reply; 19+ messages in thread
From: Darrick J. Wong @ 2024-01-04 1:21 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: Chandan Babu R, linux-xfs
On Wed, Jan 03, 2024 at 09:38:36PM +0100, Christoph Hellwig wrote:
> This will allow to use container_of to get at the containing structure,
> which will be useful soon.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
> .../filesystems/xfs-online-fsck-design.rst | 5 +-
> fs/xfs/libxfs/xfs_btree_mem.h | 26 ++--------
> fs/xfs/libxfs/xfs_rmap_btree.c | 13 ++---
> fs/xfs/libxfs/xfs_rmap_btree.h | 2 +-
> fs/xfs/libxfs/xfs_rtrmap_btree.c | 13 ++---
> fs/xfs/libxfs/xfs_rtrmap_btree.h | 2 +-
> fs/xfs/scrub/rcbag.c | 20 ++++----
> fs/xfs/scrub/rcbag_btree.c | 11 ++--
> fs/xfs/scrub/rcbag_btree.h | 2 +-
> fs/xfs/scrub/rmap_repair.c | 24 ++++-----
> fs/xfs/scrub/rtrmap_repair.c | 22 ++++----
> fs/xfs/scrub/trace.h | 13 +++--
> fs/xfs/scrub/xfbtree.c | 50 ++++++++-----------
> 13 files changed, 82 insertions(+), 121 deletions(-)
>
> diff --git a/Documentation/filesystems/xfs-online-fsck-design.rst b/Documentation/filesystems/xfs-online-fsck-design.rst
> index 29e123189d3039..5563ce9663711c 100644
> --- a/Documentation/filesystems/xfs-online-fsck-design.rst
> +++ b/Documentation/filesystems/xfs-online-fsck-design.rst
> @@ -2277,13 +2277,12 @@ follows:
> pointing to the xfile.
>
> 3. Pass the buffer cache target, buffer ops, and other information to
> - ``xfbtree_create`` to write an initial tree header and root block to the
> - xfile.
> + ``xfbtree_init`` to initialize the passed in ``struct xfbtree`` and write an
> + initial root block to the xfile.
> Each btree type should define a wrapper that passes necessary arguments to
> the creation function.
> For example, rmap btrees define ``xfs_rmapbt_mem_create`` to take care of
> all the necessary details for callers.
> - A ``struct xfbtree`` object will be returned.
>
> 4. Pass the xfbtree object to the btree cursor creation function for the
> btree type.
> diff --git a/fs/xfs/libxfs/xfs_btree_mem.h b/fs/xfs/libxfs/xfs_btree_mem.h
> index 3a5492c2cc26b6..0740537a06c6b0 100644
> --- a/fs/xfs/libxfs/xfs_btree_mem.h
> +++ b/fs/xfs/libxfs/xfs_btree_mem.h
> @@ -8,23 +8,6 @@
>
> struct xfbtree;
>
> -struct xfbtree_config {
> - /* Buffer ops for the btree root block */
> - const struct xfs_btree_ops *btree_ops;
> -
> - /* Buffer target for the xfile backing this btree. */
> - struct xfs_buftarg *target;
> -
> - /* Owner of this btree. */
> - unsigned long long owner;
> -
> - /* XFBTREE_* flags */
> - unsigned int flags;
> -};
> -
> -/* buffers should be directly mapped from memory */
> -#define XFBTREE_DIRECT_MAP (1U << 0)
> -
> #ifdef CONFIG_XFS_BTREE_IN_XFILE
> struct xfs_buftarg *xfbtree_target(struct xfbtree *xfbtree);
> int xfbtree_check_ptr(struct xfs_btree_cur *cur,
> @@ -51,8 +34,9 @@ unsigned long long xfbtree_buf_to_xfoff(struct xfs_btree_cur *cur,
> int xfbtree_get_minrecs(struct xfs_btree_cur *cur, int level);
> int xfbtree_get_maxrecs(struct xfs_btree_cur *cur, int level);
>
> -int xfbtree_create(struct xfs_mount *mp, const struct xfbtree_config *cfg,
> - struct xfbtree **xfbtreep);
> +int xfbtree_init(struct xfs_mount *mp, struct xfbtree *xfbt,
> + const struct xfs_btree_ops *btree_ops);
Why not pass the xfs_buftarg and the owner into the init function? It
feels a little funny that the callsites are:
xfbt->target = buftarg;
xfbt->owner = agno;
return xfbtree_init(mp, &xfbt, btree_ops);
vs:
return xfbtree_init(mp, &xfbt, buftarg, agno, btree_ops);
It's wonderful to get rid of XFBTREE_DIRECT_MAP though. And I do like
embedding the xfbtree in the callers instead of dynamically allocating
that as well.
Everything else in this patch looks ok to me.
--D
> +
> int xfbtree_alloc_block(struct xfs_btree_cur *cur,
> const union xfs_btree_ptr *start, union xfs_btree_ptr *ptr,
> int *stat);
> @@ -102,8 +86,8 @@ static inline unsigned int xfbtree_bbsize(void)
> #define xfbtree_buf_to_xfoff(cur, bp) (-1)
>
> static inline int
> -xfbtree_create(struct xfs_mount *mp, const struct xfbtree_config *cfg,
> - struct xfbtree **xfbtreep)
> +xfbtree_init(struct xfs_mount *mp, struct xfbtree *xfbt,
> + const struct xfs_btree_ops *btree_ops)
> {
> return -EOPNOTSUPP;
> }
> diff --git a/fs/xfs/libxfs/xfs_rmap_btree.c b/fs/xfs/libxfs/xfs_rmap_btree.c
> index 41f1b5fa863302..332fdcd07160e4 100644
> --- a/fs/xfs/libxfs/xfs_rmap_btree.c
> +++ b/fs/xfs/libxfs/xfs_rmap_btree.c
> @@ -664,16 +664,11 @@ xfs_rmapbt_mem_create(
> struct xfs_mount *mp,
> xfs_agnumber_t agno,
> struct xfs_buftarg *target,
> - struct xfbtree **xfbtreep)
> + struct xfbtree *xfbt)
> {
> - struct xfbtree_config cfg = {
> - .btree_ops = &xfs_rmapbt_mem_ops,
> - .target = target,
> - .owner = agno,
> - .flags = XFBTREE_DIRECT_MAP,
> - };
> -
> - return xfbtree_create(mp, &cfg, xfbtreep);
> + xfbt->target = target;
> + xfbt->owner = agno;
> + return xfbtree_init(mp, xfbt, &xfs_rmapbt_mem_ops);
> }
> #endif /* CONFIG_XFS_BTREE_IN_XFILE */
>
> diff --git a/fs/xfs/libxfs/xfs_rmap_btree.h b/fs/xfs/libxfs/xfs_rmap_btree.h
> index dfe13b8cbb732d..1c114efbc090d5 100644
> --- a/fs/xfs/libxfs/xfs_rmap_btree.h
> +++ b/fs/xfs/libxfs/xfs_rmap_btree.h
> @@ -70,7 +70,7 @@ struct xfbtree;
> struct xfs_btree_cur *xfs_rmapbt_mem_cursor(struct xfs_perag *pag,
> struct xfs_trans *tp, struct xfbtree *xfbtree);
> int xfs_rmapbt_mem_create(struct xfs_mount *mp, xfs_agnumber_t agno,
> - struct xfs_buftarg *target, struct xfbtree **xfbtreep);
> + struct xfs_buftarg *target, struct xfbtree *xfbt);
> #endif /* CONFIG_XFS_BTREE_IN_XFILE */
>
> #endif /* __XFS_RMAP_BTREE_H__ */
> diff --git a/fs/xfs/libxfs/xfs_rtrmap_btree.c b/fs/xfs/libxfs/xfs_rtrmap_btree.c
> index 95983dc081fa21..557f829c0826c1 100644
> --- a/fs/xfs/libxfs/xfs_rtrmap_btree.c
> +++ b/fs/xfs/libxfs/xfs_rtrmap_btree.c
> @@ -657,16 +657,11 @@ xfs_rtrmapbt_mem_create(
> struct xfs_mount *mp,
> xfs_rgnumber_t rgno,
> struct xfs_buftarg *target,
> - struct xfbtree **xfbtreep)
> + struct xfbtree *xfbt)
> {
> - struct xfbtree_config cfg = {
> - .btree_ops = &xfs_rtrmapbt_mem_ops,
> - .target = target,
> - .flags = XFBTREE_DIRECT_MAP,
> - .owner = rgno,
> - };
> -
> - return xfbtree_create(mp, &cfg, xfbtreep);
> + xfbt->target = target;
> + xfbt->owner = rgno;
> + return xfbtree_init(mp, xfbt, &xfs_rtrmapbt_mem_ops);
> }
> #endif /* CONFIG_XFS_BTREE_IN_XFILE */
>
> diff --git a/fs/xfs/libxfs/xfs_rtrmap_btree.h b/fs/xfs/libxfs/xfs_rtrmap_btree.h
> index 3347205846eb2e..b499fc7fc2e529 100644
> --- a/fs/xfs/libxfs/xfs_rtrmap_btree.h
> +++ b/fs/xfs/libxfs/xfs_rtrmap_btree.h
> @@ -210,7 +210,7 @@ struct xfbtree;
> struct xfs_btree_cur *xfs_rtrmapbt_mem_cursor(struct xfs_rtgroup *rtg,
> struct xfs_trans *tp, struct xfbtree *xfbtree);
> int xfs_rtrmapbt_mem_create(struct xfs_mount *mp, xfs_rgnumber_t rgno,
> - struct xfs_buftarg *target, struct xfbtree **xfbtreep);
> + struct xfs_buftarg *target, struct xfbtree *xfbt);
> #endif /* CONFIG_XFS_BTREE_IN_XFILE */
>
> #endif /* __XFS_RTRMAP_BTREE_H__ */
> diff --git a/fs/xfs/scrub/rcbag.c b/fs/xfs/scrub/rcbag.c
> index f28ce02f961c7c..1f3c4555e78ebc 100644
> --- a/fs/xfs/scrub/rcbag.c
> +++ b/fs/xfs/scrub/rcbag.c
> @@ -24,7 +24,7 @@
>
> struct rcbag {
> struct xfs_mount *mp;
> - struct xfbtree *xfbtree;
> + struct xfbtree xfbtree;
> uint64_t nr_items;
> };
>
> @@ -62,7 +62,7 @@ rcbag_free(
> {
> struct rcbag *bag = *bagp;
>
> - xfbtree_destroy(bag->xfbtree);
> + xfbtree_destroy(&bag->xfbtree);
> kfree(bag);
> *bagp = NULL;
> }
> @@ -80,7 +80,7 @@ rcbag_add(
> int has;
> int error;
>
> - cur = rcbagbt_mem_cursor(mp, tp, bag->xfbtree);
> + cur = rcbagbt_mem_cursor(mp, tp, &bag->xfbtree);
> error = rcbagbt_lookup_eq(cur, rmap, &has);
> if (error)
> goto out_cur;
> @@ -114,7 +114,7 @@ rcbag_add(
>
> xfs_btree_del_cursor(cur, 0);
>
> - error = xfbtree_trans_commit(bag->xfbtree, tp);
> + error = xfbtree_trans_commit(&bag->xfbtree, tp);
> if (error)
> return error;
>
> @@ -123,7 +123,7 @@ rcbag_add(
>
> out_cur:
> xfs_btree_del_cursor(cur, error);
> - xfbtree_trans_cancel(bag->xfbtree, tp);
> + xfbtree_trans_cancel(&bag->xfbtree, tp);
> return error;
> }
>
> @@ -158,7 +158,7 @@ rcbag_next_edge(
> if (next_valid)
> next_bno = next_rmap->rm_startblock;
>
> - cur = rcbagbt_mem_cursor(mp, tp, bag->xfbtree);
> + cur = rcbagbt_mem_cursor(mp, tp, &bag->xfbtree);
> error = xfs_btree_goto_left_edge(cur);
> if (error)
> goto out_cur;
> @@ -216,7 +216,7 @@ rcbag_remove_ending_at(
> int error;
>
> /* go to the right edge of the tree */
> - cur = rcbagbt_mem_cursor(mp, tp, bag->xfbtree);
> + cur = rcbagbt_mem_cursor(mp, tp, &bag->xfbtree);
> memset(&cur->bc_rec, 0xFF, sizeof(cur->bc_rec));
> error = xfs_btree_lookup(cur, XFS_LOOKUP_GE, &has);
> if (error)
> @@ -252,10 +252,10 @@ rcbag_remove_ending_at(
> }
>
> xfs_btree_del_cursor(cur, 0);
> - return xfbtree_trans_commit(bag->xfbtree, tp);
> + return xfbtree_trans_commit(&bag->xfbtree, tp);
> out_cur:
> xfs_btree_del_cursor(cur, error);
> - xfbtree_trans_cancel(bag->xfbtree, tp);
> + xfbtree_trans_cancel(&bag->xfbtree, tp);
> return error;
> }
>
> @@ -272,7 +272,7 @@ rcbag_dump(
> int has;
> int error;
>
> - cur = rcbagbt_mem_cursor(mp, tp, bag->xfbtree);
> + cur = rcbagbt_mem_cursor(mp, tp, &bag->xfbtree);
> error = xfs_btree_goto_left_edge(cur);
> if (error)
> goto out_cur;
> diff --git a/fs/xfs/scrub/rcbag_btree.c b/fs/xfs/scrub/rcbag_btree.c
> index 6f0b48b5c37bbd..bbb61d09d97927 100644
> --- a/fs/xfs/scrub/rcbag_btree.c
> +++ b/fs/xfs/scrub/rcbag_btree.c
> @@ -226,15 +226,10 @@ int
> rcbagbt_mem_create(
> struct xfs_mount *mp,
> struct xfs_buftarg *target,
> - struct xfbtree **xfbtreep)
> + struct xfbtree *xfbt)
> {
> - struct xfbtree_config cfg = {
> - .btree_ops = &rcbagbt_mem_ops,
> - .target = target,
> - .flags = XFBTREE_DIRECT_MAP,
> - };
> -
> - return xfbtree_create(mp, &cfg, xfbtreep);
> + xfbt->target = target;
> + return xfbtree_init(mp, xfbt, &rcbagbt_mem_ops);
> }
>
> /* Calculate number of records in a refcount bag btree block. */
> diff --git a/fs/xfs/scrub/rcbag_btree.h b/fs/xfs/scrub/rcbag_btree.h
> index 59d81d707d32a5..4d3d9d1e49e2fe 100644
> --- a/fs/xfs/scrub/rcbag_btree.h
> +++ b/fs/xfs/scrub/rcbag_btree.h
> @@ -65,7 +65,7 @@ struct xfbtree;
> struct xfs_btree_cur *rcbagbt_mem_cursor(struct xfs_mount *mp,
> struct xfs_trans *tp, struct xfbtree *xfbtree);
> int rcbagbt_mem_create(struct xfs_mount *mp, struct xfs_buftarg *target,
> - struct xfbtree **xfbtreep);
> + struct xfbtree *xfbt);
>
> int rcbagbt_lookup_eq(struct xfs_btree_cur *cur,
> const struct xfs_rmap_irec *rmap, int *success);
> diff --git a/fs/xfs/scrub/rmap_repair.c b/fs/xfs/scrub/rmap_repair.c
> index ab61f31868f841..abeeee88a6ebb3 100644
> --- a/fs/xfs/scrub/rmap_repair.c
> +++ b/fs/xfs/scrub/rmap_repair.c
> @@ -135,7 +135,7 @@ struct xrep_rmap {
> struct mutex lock;
>
> /* rmap records generated from primary metadata */
> - struct xfbtree *rmap_btree;
> + struct xfbtree rmap_btree;
>
> struct xfs_scrub *sc;
>
> @@ -237,13 +237,13 @@ xrep_rmap_stash(
> trace_xrep_rmap_found(sc->mp, sc->sa.pag->pag_agno, &rmap);
>
> mutex_lock(&rr->lock);
> - mcur = xfs_rmapbt_mem_cursor(sc->sa.pag, sc->tp, rr->rmap_btree);
> + mcur = xfs_rmapbt_mem_cursor(sc->sa.pag, sc->tp, &rr->rmap_btree);
> error = xfs_rmap_map_raw(mcur, &rmap);
> xfs_btree_del_cursor(mcur, error);
> if (error)
> goto out_cancel;
>
> - error = xfbtree_trans_commit(rr->rmap_btree, sc->tp);
> + error = xfbtree_trans_commit(&rr->rmap_btree, sc->tp);
> if (error)
> goto out_abort;
>
> @@ -251,7 +251,7 @@ xrep_rmap_stash(
> return 0;
>
> out_cancel:
> - xfbtree_trans_cancel(rr->rmap_btree, sc->tp);
> + xfbtree_trans_cancel(&rr->rmap_btree, sc->tp);
> out_abort:
> xchk_iscan_abort(&rr->iscan);
> mutex_unlock(&rr->lock);
> @@ -1004,7 +1004,7 @@ xrep_rmap_find_rmaps(
> * all our records before we start building a new btree, which requires
> * a bnobt cursor.
> */
> - mcur = xfs_rmapbt_mem_cursor(rr->sc->sa.pag, NULL, rr->rmap_btree);
> + mcur = xfs_rmapbt_mem_cursor(rr->sc->sa.pag, NULL, &rr->rmap_btree);
> sc->sa.bno_cur = xfs_allocbt_init_cursor(sc->mp, sc->tp, sc->sa.agf_bp,
> sc->sa.pag, XFS_BTNUM_BNO);
>
> @@ -1389,7 +1389,7 @@ xrep_rmap_build_new_tree(
> * Count the rmapbt records again, because the space reservation
> * for the rmapbt itself probably added more records to the btree.
> */
> - rr->mcur = xfs_rmapbt_mem_cursor(rr->sc->sa.pag, NULL, rr->rmap_btree);
> + rr->mcur = xfs_rmapbt_mem_cursor(rr->sc->sa.pag, NULL, &rr->rmap_btree);
>
> error = xrep_rmap_count_records(rr->mcur, &rr->nr_records);
> if (error)
> @@ -1528,7 +1528,7 @@ xrep_rmap_remove_old_tree(
> xagb_bitmap_init(&rfg.rmap_gaps);
>
> /* Compute free space from the new rmapbt. */
> - mcur = xfs_rmapbt_mem_cursor(rr->sc->sa.pag, NULL, rr->rmap_btree);
> + mcur = xfs_rmapbt_mem_cursor(rr->sc->sa.pag, NULL, &rr->rmap_btree);
>
> error = xfs_rmap_query_all(mcur, xrep_rmap_find_gaps, &rfg);
> xfs_btree_del_cursor(mcur, error);
> @@ -1638,14 +1638,14 @@ xrep_rmapbt_live_update(
> goto out_abort;
>
> mutex_lock(&rr->lock);
> - mcur = xfs_rmapbt_mem_cursor(rr->sc->sa.pag, tp, rr->rmap_btree);
> + mcur = xfs_rmapbt_mem_cursor(rr->sc->sa.pag, tp, &rr->rmap_btree);
> error = __xfs_rmap_finish_intent(mcur, action, p->startblock,
> p->blockcount, &p->oinfo, p->unwritten);
> xfs_btree_del_cursor(mcur, error);
> if (error)
> goto out_cancel;
>
> - error = xfbtree_trans_commit(rr->rmap_btree, tp);
> + error = xfbtree_trans_commit(&rr->rmap_btree, tp);
> if (error)
> goto out_cancel;
>
> @@ -1654,7 +1654,7 @@ xrep_rmapbt_live_update(
> return NOTIFY_DONE;
>
> out_cancel:
> - xfbtree_trans_cancel(rr->rmap_btree, tp);
> + xfbtree_trans_cancel(&rr->rmap_btree, tp);
> xrep_trans_cancel_hook_dummy(&txcookie, tp);
> out_abort:
> mutex_unlock(&rr->lock);
> @@ -1697,7 +1697,7 @@ xrep_rmap_setup_scan(
>
> out_iscan:
> xchk_iscan_teardown(&rr->iscan);
> - xfbtree_destroy(rr->rmap_btree);
> + xfbtree_destroy(&rr->rmap_btree);
> out_mutex:
> mutex_destroy(&rr->lock);
> return error;
> @@ -1713,7 +1713,7 @@ xrep_rmap_teardown(
> xchk_iscan_abort(&rr->iscan);
> xfs_rmap_hook_del(sc->sa.pag, &rr->hooks);
> xchk_iscan_teardown(&rr->iscan);
> - xfbtree_destroy(rr->rmap_btree);
> + xfbtree_destroy(&rr->rmap_btree);
> mutex_destroy(&rr->lock);
> }
>
> diff --git a/fs/xfs/scrub/rtrmap_repair.c b/fs/xfs/scrub/rtrmap_repair.c
> index 885752c7436b45..5c3b26ca3affd7 100644
> --- a/fs/xfs/scrub/rtrmap_repair.c
> +++ b/fs/xfs/scrub/rtrmap_repair.c
> @@ -77,7 +77,7 @@ struct xrep_rtrmap {
> struct mutex lock;
>
> /* rmap records generated from primary metadata */
> - struct xfbtree *rtrmap_btree;
> + struct xfbtree rtrmap_btree;
>
> struct xfs_scrub *sc;
>
> @@ -171,13 +171,13 @@ xrep_rtrmap_stash(
>
> /* Add entry to in-memory btree. */
> mutex_lock(&rr->lock);
> - mcur = xfs_rtrmapbt_mem_cursor(sc->sr.rtg, sc->tp, rr->rtrmap_btree);
> + mcur = xfs_rtrmapbt_mem_cursor(sc->sr.rtg, sc->tp, &rr->rtrmap_btree);
> error = xfs_rmap_map_raw(mcur, &rmap);
> xfs_btree_del_cursor(mcur, error);
> if (error)
> goto out_cancel;
>
> - error = xfbtree_trans_commit(rr->rtrmap_btree, sc->tp);
> + error = xfbtree_trans_commit(&rr->rtrmap_btree, sc->tp);
> if (error)
> goto out_abort;
>
> @@ -185,7 +185,7 @@ xrep_rtrmap_stash(
> return 0;
>
> out_cancel:
> - xfbtree_trans_cancel(rr->rtrmap_btree, sc->tp);
> + xfbtree_trans_cancel(&rr->rtrmap_btree, sc->tp);
> out_abort:
> xchk_iscan_abort(&rr->iscan);
> mutex_unlock(&rr->lock);
> @@ -648,7 +648,7 @@ xrep_rtrmap_find_rmaps(
> * check all our records before we start building a new btree, which
> * requires the rtbitmap lock.
> */
> - mcur = xfs_rtrmapbt_mem_cursor(rr->sc->sr.rtg, NULL, rr->rtrmap_btree);
> + mcur = xfs_rtrmapbt_mem_cursor(rr->sc->sr.rtg, NULL, &rr->rtrmap_btree);
> rr->nr_records = 0;
> error = xfs_rmap_query_all(mcur, xrep_rtrmap_check_record, rr);
> xfs_btree_del_cursor(mcur, error);
> @@ -781,7 +781,7 @@ xrep_rtrmap_build_new_tree(
> * Create a cursor to the in-memory btree so that we can bulk load the
> * new btree.
> */
> - rr->mcur = xfs_rtrmapbt_mem_cursor(sc->sr.rtg, NULL, rr->rtrmap_btree);
> + rr->mcur = xfs_rtrmapbt_mem_cursor(sc->sr.rtg, NULL, &rr->rtrmap_btree);
> error = xfs_btree_goto_left_edge(rr->mcur);
> if (error)
> goto err_mcur;
> @@ -900,14 +900,14 @@ xrep_rtrmapbt_live_update(
> goto out_abort;
>
> mutex_lock(&rr->lock);
> - mcur = xfs_rtrmapbt_mem_cursor(rr->sc->sr.rtg, tp, rr->rtrmap_btree);
> + mcur = xfs_rtrmapbt_mem_cursor(rr->sc->sr.rtg, tp, &rr->rtrmap_btree);
> error = __xfs_rmap_finish_intent(mcur, action, p->startblock,
> p->blockcount, &p->oinfo, p->unwritten);
> xfs_btree_del_cursor(mcur, error);
> if (error)
> goto out_cancel;
>
> - error = xfbtree_trans_commit(rr->rtrmap_btree, tp);
> + error = xfbtree_trans_commit(&rr->rtrmap_btree, tp);
> if (error)
> goto out_cancel;
>
> @@ -916,7 +916,7 @@ xrep_rtrmapbt_live_update(
> return NOTIFY_DONE;
>
> out_cancel:
> - xfbtree_trans_cancel(rr->rtrmap_btree, tp);
> + xfbtree_trans_cancel(&rr->rtrmap_btree, tp);
> xrep_trans_cancel_hook_dummy(&txcookie, tp);
> out_abort:
> xchk_iscan_abort(&rr->iscan);
> @@ -960,7 +960,7 @@ xrep_rtrmap_setup_scan(
>
> out_iscan:
> xchk_iscan_teardown(&rr->iscan);
> - xfbtree_destroy(rr->rtrmap_btree);
> + xfbtree_destroy(&rr->rtrmap_btree);
> out_bitmap:
> xfsb_bitmap_destroy(&rr->old_rtrmapbt_blocks);
> mutex_destroy(&rr->lock);
> @@ -977,7 +977,7 @@ xrep_rtrmap_teardown(
> xchk_iscan_abort(&rr->iscan);
> xfs_rtrmap_hook_del(sc->sr.rtg, &rr->hooks);
> xchk_iscan_teardown(&rr->iscan);
> - xfbtree_destroy(rr->rtrmap_btree);
> + xfbtree_destroy(&rr->rtrmap_btree);
> xfsb_bitmap_destroy(&rr->old_rtrmapbt_blocks);
> mutex_destroy(&rr->lock);
> }
> diff --git a/fs/xfs/scrub/trace.h b/fs/xfs/scrub/trace.h
> index 6e15de56be2b75..ddd75799ccecd8 100644
> --- a/fs/xfs/scrub/trace.h
> +++ b/fs/xfs/scrub/trace.h
> @@ -26,7 +26,6 @@ struct xchk_iscan;
> struct xchk_nlink;
> struct xchk_fscounters;
> struct xfbtree;
> -struct xfbtree_config;
> struct xfs_rmap_update_params;
> struct xfs_parent_name_irec;
> enum xchk_dirpath_outcome;
> @@ -2817,10 +2816,10 @@ DEFINE_XREP_DQUOT_EVENT(xrep_quotacheck_dquot);
> DEFINE_SCRUB_NLINKS_DIFF_EVENT(xrep_nlinks_update_inode);
> DEFINE_SCRUB_NLINKS_DIFF_EVENT(xrep_nlinks_unfixable_inode);
>
> -TRACE_EVENT(xfbtree_create,
> - TP_PROTO(struct xfs_mount *mp, const struct xfbtree_config *cfg,
> - struct xfbtree *xfbt),
> - TP_ARGS(mp, cfg, xfbt),
> +TRACE_EVENT(xfbtree_init,
> + TP_PROTO(struct xfs_mount *mp, struct xfbtree *xfbt,
> + const struct xfs_btree_ops *btree_ops),
> + TP_ARGS(mp, xfbt, btree_ops),
> TP_STRUCT__entry(
> __field(const void *, btree_ops)
> __field(unsigned long, xfino)
> @@ -2831,13 +2830,13 @@ TRACE_EVENT(xfbtree_create,
> __field(unsigned long long, owner)
> ),
> TP_fast_assign(
> - __entry->btree_ops = cfg->btree_ops;
> + __entry->btree_ops = btree_ops;
> __entry->xfino = xfbtree_ino(xfbt);
> __entry->leaf_mxr = xfbt->maxrecs[0];
> __entry->node_mxr = xfbt->maxrecs[1];
> __entry->leaf_mnr = xfbt->minrecs[0];
> __entry->node_mnr = xfbt->minrecs[1];
> - __entry->owner = cfg->owner;
> + __entry->owner = xfbt->owner;
> ),
> TP_printk("xfino 0x%lx btree_ops %pS owner 0x%llx leaf_mxr %u leaf_mnr %u node_mxr %u node_mnr %u",
> __entry->xfino,
> diff --git a/fs/xfs/scrub/xfbtree.c b/fs/xfs/scrub/xfbtree.c
> index 11dad651508067..9f59c7f88be9ff 100644
> --- a/fs/xfs/scrub/xfbtree.c
> +++ b/fs/xfs/scrub/xfbtree.c
> @@ -237,18 +237,17 @@ xfbtree_destroy(
> {
> xbitmap64_destroy(&xfbt->freespace);
> xfs_buftarg_drain(xfbt->target);
> - kfree(xfbt);
> }
>
> /* Compute the number of bytes available for records. */
> static inline unsigned int
> xfbtree_rec_bytes(
> struct xfs_mount *mp,
> - const struct xfbtree_config *cfg)
> + const struct xfs_btree_ops *btree_ops)
> {
> unsigned int blocklen = xfo_to_b(1);
>
> - if (cfg->btree_ops->geom_flags & XFS_BTREE_LONG_PTRS) {
> + if (btree_ops->geom_flags & XFS_BTREE_LONG_PTRS) {
> if (xfs_has_crc(mp))
> return blocklen - XFS_BTREE_LBLOCK_CRC_LEN;
>
> @@ -266,7 +265,7 @@ STATIC int
> xfbtree_init_leaf_block(
> struct xfs_mount *mp,
> struct xfbtree *xfbt,
> - const struct xfbtree_config *cfg)
> + const struct xfs_btree_ops *btree_ops)
> {
> struct xfs_buf *bp;
> xfileoff_t xfoff = xfbt->highest_offset++;
> @@ -279,13 +278,13 @@ xfbtree_init_leaf_block(
>
> trace_xfbtree_create_root_buf(xfbt, bp);
>
> - xfs_btree_init_buf(mp, bp, cfg->btree_ops, 0, 0, cfg->owner);
> + xfs_btree_init_buf(mp, bp, btree_ops, 0, 0, xfbt->owner);
> error = xfs_bwrite(bp);
> xfs_buf_relse(bp);
> if (error)
> return error;
>
> - if (cfg->btree_ops->geom_flags & XFS_BTREE_LONG_PTRS)
> + if (btree_ops->geom_flags & XFS_BTREE_LONG_PTRS)
> xfbt->root.l = xfoff;
> else
> xfbt->root.s = xfoff;
> @@ -294,57 +293,52 @@ xfbtree_init_leaf_block(
>
> /* Create an xfile btree backing thing that can be used for in-memory btrees. */
> int
> -xfbtree_create(
> +xfbtree_init(
> struct xfs_mount *mp,
> - const struct xfbtree_config *cfg,
> - struct xfbtree **xfbtreep)
> + struct xfbtree *xfbt,
> + const struct xfs_btree_ops *btree_ops)
> {
> - struct xfbtree *xfbt;
> - unsigned int blocklen = xfbtree_rec_bytes(mp, cfg);
> - unsigned int keyptr_len = cfg->btree_ops->key_len;
> + unsigned int blocklen = xfbtree_rec_bytes(mp, btree_ops);
> + unsigned int keyptr_len = btree_ops->key_len;
> int error;
>
> /* Requires an xfile-backed buftarg. */
> - if (!(cfg->target->bt_flags & XFS_BUFTARG_XFILE)) {
> - ASSERT(cfg->target->bt_flags & XFS_BUFTARG_XFILE);
> + if (!xfbt->target) {
> + ASSERT(xfbt->target);
> return -EINVAL;
> }
> + if (!(xfbt->target->bt_flags & XFS_BUFTARG_XFILE)) {
> + ASSERT(xfbt->target->bt_flags & XFS_BUFTARG_XFILE);
> + return -EINVAL;
> + }
> + xfbt->target->bt_flags |= XFS_BUFTARG_DIRECT_MAP;
>
> - xfbt = kzalloc(sizeof(struct xfbtree), XCHK_GFP_FLAGS);
> - if (!xfbt)
> - return -ENOMEM;
> - xfbt->target = cfg->target;
> - if (cfg->flags & XFBTREE_DIRECT_MAP)
> - xfbt->target->bt_flags |= XFS_BUFTARG_DIRECT_MAP;
> -
> + xfbt->highest_offset = 0;
> xbitmap64_init(&xfbt->freespace);
>
> /* Set up min/maxrecs for this btree. */
> - if (cfg->btree_ops->geom_flags & XFS_BTREE_LONG_PTRS)
> + if (btree_ops->geom_flags & XFS_BTREE_LONG_PTRS)
> keyptr_len += sizeof(__be64);
> else
> keyptr_len += sizeof(__be32);
> - xfbt->maxrecs[0] = blocklen / cfg->btree_ops->rec_len;
> + xfbt->maxrecs[0] = blocklen / btree_ops->rec_len;
> xfbt->maxrecs[1] = blocklen / keyptr_len;
> xfbt->minrecs[0] = xfbt->maxrecs[0] / 2;
> xfbt->minrecs[1] = xfbt->maxrecs[1] / 2;
> - xfbt->owner = cfg->owner;
> xfbt->nlevels = 1;
>
> /* Initialize the empty btree. */
> - error = xfbtree_init_leaf_block(mp, xfbt, cfg);
> + error = xfbtree_init_leaf_block(mp, xfbt, btree_ops);
> if (error)
> goto err_freesp;
>
> - trace_xfbtree_create(mp, cfg, xfbt);
> + trace_xfbtree_init(mp, xfbt, btree_ops);
>
> - *xfbtreep = xfbt;
> return 0;
>
> err_freesp:
> xbitmap64_destroy(&xfbt->freespace);
> xfs_buftarg_drain(xfbt->target);
> - kfree(xfbt);
> return error;
> }
>
> --
> 2.39.2
>
>
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [PATCH 5/5] xfs: embedd struct xfbtree into the owning structure
2024-01-04 1:21 ` Darrick J. Wong
@ 2024-01-04 6:32 ` Christoph Hellwig
2024-01-04 7:14 ` Darrick J. Wong
0 siblings, 1 reply; 19+ messages in thread
From: Christoph Hellwig @ 2024-01-04 6:32 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: Christoph Hellwig, Chandan Babu R, linux-xfs
On Wed, Jan 03, 2024 at 05:21:33PM -0800, Darrick J. Wong wrote:
> > -int xfbtree_create(struct xfs_mount *mp, const struct xfbtree_config *cfg,
> > - struct xfbtree **xfbtreep);
> > +int xfbtree_init(struct xfs_mount *mp, struct xfbtree *xfbt,
> > + const struct xfs_btree_ops *btree_ops);
>
> Why not pass the xfs_buftarg and the owner into the init function? It
> feels a little funny that the callsites are:
>
> xfbt->target = buftarg;
> xfbt->owner = agno;
> return xfbtree_init(mp, &xfbt, btree_ops);
>
> vs:
>
> return xfbtree_init(mp, &xfbt, buftarg, agno, btree_ops);
Yes, but..
The owner assignment should really just move into the caller of the
helpers, which would clean things up.
And the target one I need to fully understand, but maybe let's bring
this up here and ask the question I was going to ask elsewhere after
doing a bit more research.
The way the in-memory buftargs work right now look weird to me.
Why do we keep the target as a separate concept from the xfbtree?
My logical assumption would be that the xfbtree creates the target
internally and the caller shouldn't have to bother with it.
This also goes further and makes me wonder why the
xfs_buf_cache is embdded in the xfile and not just allocated when
allocating a file-backed buftarg?
Btw, once you start touching the xfbtree can we think a bit about
the naming? Right now we have xfbtree but also a xfs_btree_mem.h,
which seems very confusing. I think just doing a xfs_btree_mem
naming and moving it out of scrub/ would make sense as the concept
isn't really scrub/repair specific. But if we want to stick with
it I'd prefer to not also have _mem-based naming.
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 5/5] xfs: embedd struct xfbtree into the owning structure
2024-01-04 6:32 ` Christoph Hellwig
@ 2024-01-04 7:14 ` Darrick J. Wong
2024-01-04 7:17 ` Christoph Hellwig
0 siblings, 1 reply; 19+ messages in thread
From: Darrick J. Wong @ 2024-01-04 7:14 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: Chandan Babu R, linux-xfs
On Thu, Jan 04, 2024 at 07:32:18AM +0100, Christoph Hellwig wrote:
> On Wed, Jan 03, 2024 at 05:21:33PM -0800, Darrick J. Wong wrote:
> > > -int xfbtree_create(struct xfs_mount *mp, const struct xfbtree_config *cfg,
> > > - struct xfbtree **xfbtreep);
> > > +int xfbtree_init(struct xfs_mount *mp, struct xfbtree *xfbt,
> > > + const struct xfs_btree_ops *btree_ops);
> >
> > Why not pass the xfs_buftarg and the owner into the init function? It
> > feels a little funny that the callsites are:
> >
> > xfbt->target = buftarg;
> > xfbt->owner = agno;
> > return xfbtree_init(mp, &xfbt, btree_ops);
> >
> > vs:
> >
> > return xfbtree_init(mp, &xfbt, buftarg, agno, btree_ops);
>
> Yes, but..
>
> The owner assignment should really just move into the caller of the
> helpers, which would clean things up.
>
> And the target one I need to fully understand, but maybe let's bring
> this up here and ask the question I was going to ask elsewhere after
> doing a bit more research.
>
> The way the in-memory buftargs work right now look weird to me.
>
> Why do we keep the target as a separate concept from the xfbtree?
> My logical assumption would be that the xfbtree creates the target
> internally and the caller shouldn't have to bother with it.
IIRC setting up the shrinker in xfs_alloc_buftarg_common takes some
shrinker lock somewhere, and lockdep complained about a potential
deadlock between the locks that scrub takes if I don't create the xfile
buftarg in the scrub _setup routines. That's why it's not created
internally to the xfbtree.
I agree that it makes much more sense only to create those things when
they're actually needed, but ... hm. Maybe we don't need the xfile
buftarg to be hooked up to the shrinkers, seeing as it's ephemeral
anyway? That would save a lot of fuss and ...
> This also goes further and makes me wonder why the
> xfs_buf_cache is embdded in the xfile and not just allocated when
> allocating a file-backed buftarg?
...maybe we could actually do it this way. I'll look into it tomorrow.
> Btw, once you start touching the xfbtree can we think a bit about
> the naming? Right now we have xfbtree but also a xfs_btree_mem.h,
> which seems very confusing. I think just doing a xfs_btree_mem
> naming and moving it out of scrub/ would make sense as the concept
> isn't really scrub/repair specific. But if we want to stick with
> it I'd prefer to not also have _mem-based naming.
Yes, lets move it to libxfs/xfbtree.[ch].
--D
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 5/5] xfs: embedd struct xfbtree into the owning structure
2024-01-04 7:14 ` Darrick J. Wong
@ 2024-01-04 7:17 ` Christoph Hellwig
2024-01-04 7:22 ` Darrick J. Wong
0 siblings, 1 reply; 19+ messages in thread
From: Christoph Hellwig @ 2024-01-04 7:17 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: Christoph Hellwig, Chandan Babu R, linux-xfs
On Wed, Jan 03, 2024 at 11:14:54PM -0800, Darrick J. Wong wrote:
> IIRC setting up the shrinker in xfs_alloc_buftarg_common takes some
> shrinker lock somewhere, and lockdep complained about a potential
> deadlock between the locks that scrub takes if I don't create the xfile
> buftarg in the scrub _setup routines. That's why it's not created
> internally to the xfbtree.
>
> I agree that it makes much more sense only to create those things when
> they're actually needed, but ... hm. Maybe we don't need the xfile
> buftarg to be hooked up to the shrinkers, seeing as it's ephemeral
> anyway? That would save a lot of fuss and ...
Yes, once we move to a model where the buffer always points to
the shmem page, and we remove the buffer lru for them as we already
have the page LRU there is no point in having a shrinker at all.
> > naming and moving it out of scrub/ would make sense as the concept
> > isn't really scrub/repair specific. But if we want to stick with
> > it I'd prefer to not also have _mem-based naming.
>
> Yes, lets move it to libxfs/xfbtree.[ch].
What does the xf in the various scrubx/xf* thinks stand for, btw?
Why not libxfs/xfs_btree_mem.[ch]?
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 5/5] xfs: embedd struct xfbtree into the owning structure
2024-01-04 7:17 ` Christoph Hellwig
@ 2024-01-04 7:22 ` Darrick J. Wong
2024-01-04 19:28 ` Darrick J. Wong
0 siblings, 1 reply; 19+ messages in thread
From: Darrick J. Wong @ 2024-01-04 7:22 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: Chandan Babu R, linux-xfs
On Thu, Jan 04, 2024 at 08:17:35AM +0100, Christoph Hellwig wrote:
> On Wed, Jan 03, 2024 at 11:14:54PM -0800, Darrick J. Wong wrote:
> > IIRC setting up the shrinker in xfs_alloc_buftarg_common takes some
> > shrinker lock somewhere, and lockdep complained about a potential
> > deadlock between the locks that scrub takes if I don't create the xfile
> > buftarg in the scrub _setup routines. That's why it's not created
> > internally to the xfbtree.
> >
> > I agree that it makes much more sense only to create those things when
> > they're actually needed, but ... hm. Maybe we don't need the xfile
> > buftarg to be hooked up to the shrinkers, seeing as it's ephemeral
> > anyway? That would save a lot of fuss and ...
>
> Yes, once we move to a model where the buffer always points to
> the shmem page, and we remove the buffer lru for them as we already
> have the page LRU there is no point in having a shrinker at all.
Ok. I'll move the shrinker stuff into the real buftarg creation
function. This seems to have become a lot simpler now that the shrinker
is a pointer instead of embedded in the buftarg.
> > > naming and moving it out of scrub/ would make sense as the concept
> > > isn't really scrub/repair specific. But if we want to stick with
> > > it I'd prefer to not also have _mem-based naming.
> >
> > Yes, lets move it to libxfs/xfbtree.[ch].
>
> What does the xf in the various scrubx/xf* thinks stand for, btw?
> Why not libxfs/xfs_btree_mem.[ch]?
"xfile btree".
--D
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 5/5] xfs: embedd struct xfbtree into the owning structure
2024-01-04 7:22 ` Darrick J. Wong
@ 2024-01-04 19:28 ` Darrick J. Wong
2024-01-05 4:27 ` Christoph Hellwig
0 siblings, 1 reply; 19+ messages in thread
From: Darrick J. Wong @ 2024-01-04 19:28 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: Chandan Babu R, linux-xfs
On Wed, Jan 03, 2024 at 11:22:00PM -0800, Darrick J. Wong wrote:
> On Thu, Jan 04, 2024 at 08:17:35AM +0100, Christoph Hellwig wrote:
> > On Wed, Jan 03, 2024 at 11:14:54PM -0800, Darrick J. Wong wrote:
> > > IIRC setting up the shrinker in xfs_alloc_buftarg_common takes some
> > > shrinker lock somewhere, and lockdep complained about a potential
> > > deadlock between the locks that scrub takes if I don't create the xfile
> > > buftarg in the scrub _setup routines. That's why it's not created
> > > internally to the xfbtree.
> > >
> > > I agree that it makes much more sense only to create those things when
> > > they're actually needed, but ... hm. Maybe we don't need the xfile
> > > buftarg to be hooked up to the shrinkers, seeing as it's ephemeral
> > > anyway? That would save a lot of fuss and ...
> >
> > Yes, once we move to a model where the buffer always points to
> > the shmem page, and we remove the buffer lru for them as we already
> > have the page LRU there is no point in having a shrinker at all.
>
> Ok. I'll move the shrinker stuff into the real buftarg creation
> function. This seems to have become a lot simpler now that the shrinker
> is a pointer instead of embedded in the buftarg.
Though looking at buftarg allocation and my old notes from a couple of
years ago -- a second reason for allocating the buftarg during scrub
setup was that the list_lru_init call allocates an array that's
O(nodes_nr) and percpu_counter_init allocates an array that's
O(maxcpus). At the time I decided that it was better to put those large
contiguous memory allocations in the ->setup routine where we don't have
any vfs/xfs locks held, can run direct reclaim, and haven't done any xfs
work yet.
So I'd like to leave the buftarg attached to struct xfs_scrub, though
I'll still get rid of the shrinker for xfile buftargs.
--D
> > > > naming and moving it out of scrub/ would make sense as the concept
> > > > isn't really scrub/repair specific. But if we want to stick with
> > > > it I'd prefer to not also have _mem-based naming.
> > >
> > > Yes, lets move it to libxfs/xfbtree.[ch].
> >
> > What does the xf in the various scrubx/xf* thinks stand for, btw?
> > Why not libxfs/xfs_btree_mem.[ch]?
>
> "xfile btree".
>
> --D
>
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 5/5] xfs: embedd struct xfbtree into the owning structure
2024-01-04 19:28 ` Darrick J. Wong
@ 2024-01-05 4:27 ` Christoph Hellwig
0 siblings, 0 replies; 19+ messages in thread
From: Christoph Hellwig @ 2024-01-05 4:27 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: Christoph Hellwig, Chandan Babu R, linux-xfs
On Thu, Jan 04, 2024 at 11:28:22AM -0800, Darrick J. Wong wrote:
> Though looking at buftarg allocation and my old notes from a couple of
> years ago -- a second reason for allocating the buftarg during scrub
> setup was that the list_lru_init call allocates an array that's
> O(nodes_nr) and percpu_counter_init allocates an array that's
> O(maxcpus). At the time I decided that it was better to put those large
> contiguous memory allocations in the ->setup routine where we don't have
> any vfs/xfs locks held, can run direct reclaim, and haven't done any xfs
> work yet.
Given that we use the page LRU for the shemfs pages, I don't think we
need the buftarg LRU list at all - aging just the buffer container
doesn't make much sense.
^ permalink raw reply [flat|nested] 19+ messages in thread