Re: [PATCH 2/4] xfs: factor out xfs_da3_node_entry_remove

[Long Li <leo.lilong@huawei.com>]

On Mon, Mar 09, 2026 at 09:42:29AM -0700, Darrick J. Wong wrote:
> On Mon, Mar 09, 2026 at 04:27:50PM +0800, Long Li wrote:
> > Factor out wrapper xfs_da3_node_entry_remove function, which
> > exported for external use.
> > 
> > Signed-off-by: Long Li <leo.lilong@huawei.com>
> > ---
> >  fs/xfs/libxfs/xfs_da_btree.c | 54 ++++++++++++++++++++++++++++--------
> >  fs/xfs/libxfs/xfs_da_btree.h |  2 ++
> >  2 files changed, 45 insertions(+), 11 deletions(-)
> > 
> > diff --git a/fs/xfs/libxfs/xfs_da_btree.c b/fs/xfs/libxfs/xfs_da_btree.c
> > index 766631f0562e..466c43098768 100644
> > --- a/fs/xfs/libxfs/xfs_da_btree.c
> > +++ b/fs/xfs/libxfs/xfs_da_btree.c
> > @@ -1506,21 +1506,20 @@ xfs_da3_fixhashpath(
> >  }
> >  
> >  /*
> > - * Remove an entry from an intermediate node.
> > + * Internal implementation to remove an entry from an intermediate node.
> >   */
> >  STATIC void
> > -xfs_da3_node_remove(
> > -	struct xfs_da_state	*state,
> > -	struct xfs_da_state_blk	*drop_blk)
> > +__xfs_da3_node_remove(
> > +	struct xfs_trans	*tp,
> > +	struct xfs_inode	*dp,
> > +	struct xfs_da_geometry  *geo,
> > +	struct xfs_da_state_blk *drop_blk)
> >  {
> >  	struct xfs_da_intnode	*node;
> >  	struct xfs_da3_icnode_hdr nodehdr;
> >  	struct xfs_da_node_entry *btree;
> >  	int			index;
> >  	int			tmp;
> > -	struct xfs_inode	*dp = state->args->dp;
> > -
> > -	trace_xfs_da_node_remove(state->args);
> >  
> >  	node = drop_blk->bp->b_addr;
> >  	xfs_da3_node_hdr_from_disk(dp->i_mount, &nodehdr, node);
> > @@ -1536,17 +1535,17 @@ xfs_da3_node_remove(
> >  		tmp  = nodehdr.count - index - 1;
> >  		tmp *= (uint)sizeof(xfs_da_node_entry_t);
> >  		memmove(&btree[index], &btree[index + 1], tmp);
> > -		xfs_trans_log_buf(state->args->trans, drop_blk->bp,
> > +		xfs_trans_log_buf(tp, drop_blk->bp,
> >  		    XFS_DA_LOGRANGE(node, &btree[index], tmp));
> >  		index = nodehdr.count - 1;
> >  	}
> >  	memset(&btree[index], 0, sizeof(xfs_da_node_entry_t));
> > -	xfs_trans_log_buf(state->args->trans, drop_blk->bp,
> > +	xfs_trans_log_buf(tp, drop_blk->bp,
> >  	    XFS_DA_LOGRANGE(node, &btree[index], sizeof(btree[index])));
> >  	nodehdr.count -= 1;
> >  	xfs_da3_node_hdr_to_disk(dp->i_mount, node, &nodehdr);
> > -	xfs_trans_log_buf(state->args->trans, drop_blk->bp,
> > -	    XFS_DA_LOGRANGE(node, &node->hdr, state->args->geo->node_hdr_size));
> > +	xfs_trans_log_buf(tp, drop_blk->bp,
> > +	    XFS_DA_LOGRANGE(node, &node->hdr, geo->node_hdr_size));
> >  
> >  	/*
> >  	 * Copy the last hash value from the block to propagate upwards.
> > @@ -1554,6 +1553,39 @@ xfs_da3_node_remove(
> >  	drop_blk->hashval = be32_to_cpu(btree[index - 1].hashval);
> >  }
> >  
> > +/*
> > + * Remove an entry from an intermediate node.
> > + */
> > +STATIC void
> > +xfs_da3_node_remove(
> > +	struct xfs_da_state	*state,
> > +	struct xfs_da_state_blk	*drop_blk)
> > +{
> > +	trace_xfs_da_node_remove(state->args);
> 
> Style nit: blank line after the tracepoint.
> 
> > +	__xfs_da3_node_remove(state->args->trans, state->args->dp,
> > +			state->args->geo, drop_blk);
> > +}
> > +
> > +/*
> > + * Remove an entry from a node at the specified index, this is an exported
> > + * wrapper for removing entries from intermediate nodes.
> > + */
> > +void
> > +xfs_da3_node_entry_remove(
> 
> This only applies to attr(ibute) structures, as evidenced by m_attr_geo
> below.  I think this ought to be named xfs_attr3_node_entry_remove.
> 

Agree with you, I will update it in the next version.

Thanks,
Long Li

> > +	struct xfs_trans	*tp,
> > +	struct xfs_inode	*dp,
> > +	struct xfs_buf		*bp,
> > +	int			index)
> > +{
> > +	struct xfs_da_state_blk blk;
> 
> 	struct xfs_da_state_blk blk = {
> 		.index		= index,
> 		.bp		= bp,
> 	};
> 
> Otherwise this looks ok to me.
> 
> --D
> 
> > +
> > +	memset(&blk, 0, sizeof(blk));
> > +	blk.index = index;
> > +	blk.bp = bp;
> > +
> > +	__xfs_da3_node_remove(tp, dp, dp->i_mount->m_attr_geo, &blk);
> > +}
> > +
> >  /*
> >   * Unbalance the elements between two intermediate nodes,
> >   * move all Btree elements from one node into another.
> > diff --git a/fs/xfs/libxfs/xfs_da_btree.h b/fs/xfs/libxfs/xfs_da_btree.h
> > index 354d5d65043e..6cec4313c83c 100644
> > --- a/fs/xfs/libxfs/xfs_da_btree.h
> > +++ b/fs/xfs/libxfs/xfs_da_btree.h
> > @@ -184,6 +184,8 @@ int	xfs_da3_split(xfs_da_state_t *state);
> >  int	xfs_da3_join(xfs_da_state_t *state);
> >  void	xfs_da3_fixhashpath(struct xfs_da_state *state,
> >  			    struct xfs_da_state_path *path_to_to_fix);
> > +void	xfs_da3_node_entry_remove(struct xfs_trans *tp, struct xfs_inode *dp,
> > +				struct xfs_buf *bp, int index);
> >  
> >  /*
> >   * Routines used for finding things in the Btree.
> > -- 
> > 2.39.2
> > 
> > 
>