move xfs_bmbt_killroot to common code
PATCH 22/26 move xfs_bmbt_killroot to common code
xfs_bmbt_killroot is a mostly generic implementation of moving from a real block based root to an inode based root. So move it to xfs_btree.c where it can use all the nice infrastructure there and make it pointer size agnostic
The new name for it is xfs_btree_root_to_iroot which is not very nice but at least slightly more descriptive than the old name.
fs/xfs/xfs_btree.c +106 -0
@@ -2838,3 +2838,109 @@ error0:XFS_BTREE_TRACE_CURSOR (cur,XBT_ERROR );return error; } + +int +xfs_btree_root_to_iroot ( +struct xfs_btree_cur *cur) +{ +int whichfork = cur->bc_private.b.whichfork; +struct xfs_inode *ip = cur->bc_private.b.ip; +struct xfs_ifork *ifp =XFS_IFORK_PTR (ip, whichfork); +struct xfs_btree_block *block; +struct xfs_btree_block *cblock; +union xfs_btree_key *kp; +union xfs_btree_key *ckp; +union xfs_btree_ptr *pp; +union xfs_btree_ptr *cpp; +struct xfs_buf *cbp; +int level; +int index; +int numrecs; +#ifdef DEBUG +union xfs_btree_ptr ptr; +int i; +#endif + +XFS_BTREE_TRACE_CURSOR (cur,XBT_ENTRY ); + level = cur->bc_nlevels -1 ; +ASSERT (level >=1 ); + +/* +* Don't deal with the root block needs to be a leaf case. +* We're just going to turn the thing back into extents anyway. +*/ +if (level ==1 ) + goto out0; + + block =xfs_btree_get_block (cur, level, &cbp); + +/* +* Give up if the root has multiple children. +*/ +if (xfs_btree_get_numrecs (block) !=1 ) + goto out0; + +/* +* Only do this if the next level will fit. +* Then the data must be copied up to the inode, +* instead of freeing the root you free the next level. +*/ + cblock =xfs_btree_get_block (cur, level -1 , &cbp); + numrecs =xfs_btree_get_numrecs (cblock); + +if (numrecs > cur->bc_ops->get_dmaxrecs (cur, level)) + goto out0; + +XFS_BTREE_STATS_INC (cur, killroot); + +#ifdef DEBUG +xfs_btree_get_sibling (cur, block, &ptr,XFS_BB_LEFTSIB ); +ASSERT (xfs_btree_ptr_is_null (cur, &ptr)); +xfs_btree_get_sibling (cur, block, &ptr,XFS_BB_RIGHTSIB ); +ASSERT (xfs_btree_ptr_is_null (cur, &ptr)); + +// XXX(hch): this assert is bmap btree specific +ASSERT (cur->bc_ops->get_maxrecs (cur, level) == +XFS_BMAP_BROOT_MAXRECS (ifp->if_broot_bytes)); +#endif + + index = numrecs - cur->bc_ops->get_maxrecs (cur, level); +if (index) { + cur->bc_ops->realloc_root (cur, index); + block = (struct xfs_btree_block *)ifp->if_broot; + } + +be16_add (&block->bb_numrecs, index); +ASSERT (block->bb_numrecs == numrecs); + + kp = cur->bc_ops->key_addr (cur,1 , block); + ckp = cur->bc_ops->key_addr (cur,1 , cblock); + cur->bc_ops->copy_keys (cur, ckp, kp, numrecs); + + pp = cur->bc_ops->ptr_addr (cur,1 , block); + cpp = cur->bc_ops->ptr_addr (cur,1 , cblock); +#ifdef DEBUG +for (i =0 ; i < numrecs; i++) { +int error; + + error =xfs_btree_check_ptr (cur, cpp, i, level -1 ); +if (error) { +XFS_BTREE_TRACE_CURSOR (cur,XBT_ERROR ); +return error; + } + } +#endif +xfs_btree_copy_ptrs (cur, cpp, pp, numrecs); + + cur->bc_ops->free_block (cur, cbp,1 ); +XFS_BTREE_STATS_INC (cur, free); + + cur->bc_bufs[level -1 ] =NULL ; +be16_add (&block->bb_level,-1 ); +xfs_trans_log_inode (cur->bc_tp, ip, +XFS_ILOG_CORE |XFS_ILOG_FBROOT (cur->bc_private.b.whichfork)); + cur->bc_nlevels--; +out0: +XFS_BTREE_TRACE_CURSOR (cur,XBT_EXIT ); +return 0 ; +}
fs/xfs/xfs_alloc_btree.c +34 -0
@@ -781,6 +781,39 @@ xfs_allocbt_alloc_block(return 0 ; } +STATIC int +xfs_allocbt_free_block ( +struct xfs_btree_cur *cur, +struct xfs_buf *bp, +int size) +{ +struct xfs_agf *agf =XFS_BUF_TO_AGF (cur->bc_private.a.agbp); +xfs_agblock_t bno; +int error; + + bno =XFS_DADDR_TO_AGBNO (cur->bc_mp,XFS_BUF_ADDR (bp)); + + error =xfs_alloc_put_freelist (cur->bc_tp, cur->bc_private.a.agbp, +NULL , bno, size); +if (error) +return error; + +/* +* Since blocks move to the free list without the coordination used in +* xfs_bmap_finish, we can't allow block to be available for +* reallocation and non-transaction writing (user data) until we know +* that the transaction that moved it to the free list is permanently +* on disk. We track the blocks by declaring these blocks as "busy"; +* the busy list is maintained on a per-ag basis and each transaction +* records which entries should be removed when the iclog commits to +* disk. If a busy block is allocated, the iclog is pushed up to the +* LSN that freed the block. +*/ +xfs_alloc_mark_busy (cur->bc_tp,be32_to_cpu (agf->agf_seqno), bno,1 ); +xfs_trans_agbtree_delta (cur->bc_tp,-1 ); +return 0 ; +} +/* * Update the longest extent in the AGF */
@@ -1153,6 +1186,7 @@ static const struct xfs_btree_ops xfs_al .dup_cursor = xfs_allocbt_dup_cursor, .set_root = xfs_allocbt_set_root, .alloc_block = xfs_allocbt_alloc_block, + .free_block = xfs_allocbt_free_block, .update_lastrec = xfs_allocbt_update_lastrec, .get_maxrecs = xfs_allocbt_get_maxrecs, .init_key_from_rec = xfs_allocbt_init_key_from_rec,
fs/xfs/xfs_bmap_btree.c +21 -94
151 lines not on this page · raw message
fs/xfs/xfs_btree.h +3 -0
@@ -198,6 +198,8 @@ struct xfs_btree_ops {union xfs_btree_ptr *start_bno, union xfs_btree_ptr *new_bno, int length, int *stat); +int (*free_block)(struct xfs_btree_cur *cur,struct xfs_buf *bp, +int length);/* updated last record information */ void (*update_lastrec)(struct xfs_btree_cur *cur,
@@ -596,6 +598,7 @@ int xfs_btree_split(struct xfs_btree_cur union xfs_btree_key *, struct xfs_btree_cur **, int *);int xfs_btree_new_root (struct xfs_btree_cur *,int *);int xfs_btree_iroot_to_root (struct xfs_btree_cur *,int *,int *); +int xfs_btree_root_to_iroot (struct xfs_btree_cur *);int xfs_btree_insert (struct xfs_btree_cur *,int *);
fs/xfs/xfs_ialloc_btree.c +17 -0
@@ -710,6 +710,22 @@ xfs_inobt_alloc_block(return 0 ; } +STATIC int +xfs_inobt_free_block ( +struct xfs_btree_cur *cur, +struct xfs_buf *bp, +int size) +{ +int error; + + error =xfs_free_extent (cur->bc_tp, +XFS_DADDR_TO_FSB (cur->bc_mp,XFS_BUF_ADDR (bp)),1 ); +if (error) +return error; + +xfs_trans_binval (cur->bc_tp, bp); +return 0 ; +} STATIC int xfs_inobt_get_maxrecs(
@@ -1001,6 +1017,7 @@ static const struct xfs_btree_ops xfs_in .dup_cursor = xfs_inobt_dup_cursor, .set_root = xfs_inobt_set_root, .alloc_block = xfs_inobt_alloc_block, + .free_block = xfs_inobt_free_block, .get_maxrecs = xfs_inobt_get_maxrecs, .init_key_from_rec = xfs_inobt_init_key_from_rec, .init_ptr_from_cur = xfs_inobt_init_ptr_from_cur,
Quoted 7 lines · Christoph Hellwig
xfs_bmbt_killrootis a mostly generic implementation of moving from a real block based root to an inode based root. So move it toxfs_btree.cwhere it can use all the nice infrastructure there and make it pointer size agnosticThe new name for it is
xfs_btree_root_to_irootwhich is not very nice but at least slightly more descriptive than the old name.
.....
fs/xfs/xfs_btree.c error0: · line 2866
Quoted 4 lines
fs/xfs/xfs_btree.c +4
-0+ +XFS_BTREE_TRACE_CURSOR (cur,XBT_ENTRY ); + level = cur->bc_nlevels -1 ; +ASSERT (level >=1 );
probably should assert root in inode is set here.
fs/xfs/xfs_btree.c error0: · line 2892
Quoted 5 lines
fs/xfs/xfs_btree.c +5
-0+ cblock =xfs_btree_get_block (cur, level -1 , &cbp); + numrecs =xfs_btree_get_numrecs (cblock); + +if (numrecs > cur->bc_ops->get_dmaxrecs (cur, level)) + goto out0;
^^^^
Stray whitespace.
fs/xfs/xfs_btree.c error0: · line 2903
Quoted 11 lines
fs/xfs/xfs_btree.c +11
-0+ +XFS_BTREE_STATS_INC (cur, killroot); + +#ifdef DEBUG +xfs_btree_get_sibling (cur, block, &ptr,XFS_BB_LEFTSIB ); +ASSERT (xfs_btree_ptr_is_null (cur, &ptr)); +xfs_btree_get_sibling (cur, block, &ptr,XFS_BB_RIGHTSIB ); +ASSERT (xfs_btree_ptr_is_null (cur, &ptr)); + +// XXX(hch): this assert is bmap btree specific + ASSERT(cur->bc_ops->get_maxrecs(cur, level) ==
^
fs/xfs/xfs_btree.c error0: · line 2904
Quoted 1 line
fs/xfs/xfs_btree.c +1
-0+ XFS_BMAP_BROOT_MAXRECS(ifp->if_broot_bytes));
Stray whitespace. As to the assert - what is it really trying to check? That the btree root space in the inode is large enough to fit the max number of records? If so, does it really need to be checked here (i.e. could the caller do it?)
Otherwise looks ok.
Cheers,
Dave.
Signature · 2 lines
Dave Chinner [email protected]
Quoted 8 lines · Dave Chinner
Quoted 7 lines · Christoph Hellwig
xfs_bmbt_killrootis a mostly generic implementation of moving from a real block based root to an inode based root. So move it toxfs_btree.cwhere it can use all the nice infrastructure there and make it pointer size agnosticThe new name for it is
xfs_btree_root_to_irootwhich is not very nice but at least slightly more descriptive than the old name.
Quoted 7 lines
.....
fs/xfs/xfs_btree.c error0: · line 2866
Quoted 4 lines
fs/xfs/xfs_btree.c +4
-0+ +XFS_BTREE_TRACE_CURSOR (cur,XBT_ENTRY ); + level = cur->bc_nlevels -1 ; +ASSERT (level >=1 );probably should assert root in inode is set here.
We have that check just before the call, so this might be a little overkill..
Quoted 8 lines
fs/xfs/xfs_btree.c error0: · line 2892
Quoted 6 lines
fs/xfs/xfs_btree.c +5
-0+ cblock =xfs_btree_get_block (cur, level -1 , &cbp); + numrecs =xfs_btree_get_numrecs (cblock); + +if (numrecs > cur->bc_ops->get_dmaxrecs (cur, level)) + goto out0;^^^^Stray whitespace.
Already fixed before your mail after I ran checkpatch.pl over all patches - there were a few spread over the various patches.
Quoted 9 lines
fs/xfs/xfs_btree.c error0: · line 2903
Quoted 2 lines
fs/xfs/xfs_btree.c +2
-0+// XXX(hch): this assert is bmap btree specific + ASSERT(cur->bc_ops->get_maxrecs(cur, level) ==^
fs/xfs/xfs_btree.c error0: · line 2904
Quoted 1 line
fs/xfs/xfs_btree.c +1
-0+ XFS_BMAP_BROOT_MAXRECS(ifp->if_broot_bytes));Stray whitespace. As to the assert - what is it really trying to check? That the btree root space in the inode is large enough to fit the max number of records? If so, does it really need to be checked here (i.e. could the caller do it?)
I don't think it makes much sense at all. My preference would be to simply kill it.
Quoted 17 lines · Christoph Hellwig
Quoted 8 lines · Dave Chinner
Quoted 7 lines · Christoph Hellwig
xfs_bmbt_killrootis a mostly generic implementation of moving from a real block based root to an inode based root. So move it toxfs_btree.cwhere it can use all the nice infrastructure there and make it pointer size agnosticThe new name for it is
xfs_btree_root_to_irootwhich is not very nice but at least slightly more descriptive than the old name.Quoted 7 lines
.....
fs/xfs/xfs_btree.c error0: · line 2866
Quoted 4 lines
fs/xfs/xfs_btree.c +4
-0+ +XFS_BTREE_TRACE_CURSOR (cur,XBT_ENTRY ); + level = cur->bc_nlevels -1 ; +ASSERT (level >=1 );probably should assert root in inode is set here.
Quoted 2 lines
We have that check just before the call, so this might be a little overkill..
Fair enough.
Quoted 10 lines
Quoted 9 lines
fs/xfs/xfs_btree.c error0: · line 2903
Quoted 2 lines
fs/xfs/xfs_btree.c +2
-0+// XXX(hch): this assert is bmap btree specific + ASSERT(cur->bc_ops->get_maxrecs(cur, level) ==^
fs/xfs/xfs_btree.c error0: · line 2904
Quoted 1 line
fs/xfs/xfs_btree.c +1
-0+ XFS_BMAP_BROOT_MAXRECS(ifp->if_broot_bytes));Stray whitespace. As to the assert - what is it really trying to check? That the btree root space in the inode is large enough to fit the max number of records? If so, does it really need to be checked here (i.e. could the caller do it?)
Quoted 2 lines
I don't think it makes much sense at all. My preference would be to simply kill it.
Agreed. Kill it - I don't think it adds any value.
Cheers,
Dave.
Signature · 2 lines
Dave Chinner [email protected]