Re: [PATCH v2 2/4] xfs: factor out xfs_attr3_node_entry_remove

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

On Fri, Mar 13, 2026 at 08:01:12AM -0700, Darrick J. Wong wrote:
> On Thu, Mar 12, 2026 at 04:57:58PM +0800, Long Li wrote:
> > Factor out wrapper xfs_attr3_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..0b9fe3e4370d 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);
> > +
> > +	__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.
> 
> "exported" is confusing since in kernel-land that usually implies
> EXPORT_SYMBOL_GPL(), which this clearly isn't.  You could trim this to
> 
> /*
>  * Remove an entry from an intermediate attr node at the specified
>  * index.
>  */
> 
> With that touched up,
> Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>
> 
> --D

Thanks for your review, it will be update.

Long Li