Re: [PATCH] xfs: speed up parent pointer operations

["Darrick J. Wong" <djwong@kernel.org>]

On Tue, Jan 06, 2026 at 10:04:46PM -0800, Christoph Hellwig wrote:
> On Tue, Jan 06, 2026 at 04:09:07PM -0800, Darrick J. Wong wrote:
> > In principle, yes.  For a generic xattr version you'd probably want to
> > check for a large valuelen on the set side so that we don't waste time
> > scanning the sf structure when we're just going to end up in remote
> > value territory anyway.
> 
> Yeah.
> 
> > 
> > >         It might be nice to just do this for set and remove in
> > > xfs_attr_defer_add and have it handle all attr operations.
> > 
> > Yeah.  I think it's more logical to put these new shortcut calls in
> > xfs_attr_set because we're be deciding /not/ to invoke the deferred
> > xattr mechanism.
> 
> Or that, yes.

It turns out that for replace, it's more convenient to do it separately
in the xattr and parent pointer code because parent pointer replacements
require switching new_name -> name and new_value -> value after the
remove step; and the rename optimization is different for parent
pointers vs. every other xattr.

> > > And for replace we should be able to optimize this even further
> > > by adding a new xfs_attr_sf_replacename that just checks if the new
> > > version would fit, and then memmove everything behind the changed
> > > attr and update it in place.  This should improve the operation a lot
> > > more.
> > 
> > That would depends on the frequency of non-parent pointer xattr
> > operations where the value doesn't change size modulo the rounding
> > factor.
> 
> Well, the same applies to value changes - in the shortform format name
> and value are basically one blob, split by namelen.  So anything
> replacing an existing attr with a new one, either due to a name change
> for parent pointers, or due to a value change otherwise can just move
> things beyond the attribute and update in place trivially.  For
> replacing values with values of the same size things are even simpler.

Yes it is pretty simple:

int
xfs_attr_shortform_replace(
	struct xfs_da_args		*args)
{
	struct xfs_attr_sf_entry	*sfe;

	ASSERT(args->dp->i_af.if_format == XFS_DINODE_FMT_LOCAL);

	trace_xfs_attr_sf_replace(args);

	sfe = xfs_attr_sf_findname(args);
	if (!sfe)
		return -ENOATTR;

	if (sfe->valuelen != args->valuelen)
		return -ENOSPC;

	memcpy(&sfe->nameval[sfe->namelen], args->value, args->valuelen);
	xfs_trans_log_inode(args->trans, args->dp,
			XFS_ILOG_CORE | XFS_ILOG_ADATA);
	return 0;
}

(parent pointers are a tad more difficult because we have to copy the
new name as well as the new value)

IIRC there's no rounding applied to shortform attr entries, so we have
to have an exact match on the value length.

> > I also wonder how much benefit anyone really gets from doing this to
> > regular xattrs, but once I'm more convinced that it's solid w.r.t.
> > parent pointers it's trivial to move it to xattrs too.
> 
> Not sure what counts as regular, but I'm pretty sure it would help
> quite a bit for inheriting xattrs or other security attributes.

Here, by "regular" I meant "not parent pointers" but yeah.  It'll
probably help everyone to implement the shortcuts.

--D