Re: [PATCH 6/6] xfs: create an xattr iteration function for scrub

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

On Wed, Feb 28, 2024 at 09:11:10AM -0800, Christoph Hellwig wrote:
> On Mon, Feb 26, 2024 at 06:30:14PM -0800, Darrick J. Wong wrote:
> > From: Darrick J. Wong <djwong@kernel.org>
> > 
> > Create a streamlined function to walk a file's xattrs, without all the
> > cursor management stuff in the regular listxattr.
> 
> So given that the Linux xattr interface doesn't have cursors

Which IMO is a deficiency that really ought to be rectified.  It's also
totally stupid that it returns E2BIG if the fs returned a list longer
than 64k even if the caller passed in a sufficiently large buffer.

>                                                              and they
> are only around for the XFS listattr by handle interface, why can't
> the normal listxattr syscall also use this cursors-less variant,
> which probably more efficient?  (assuming it is, maybe a little more
> explanation on why you've added this variant would be useful).

It's slightly more efficient since we don't have to keep the
xfs_attr_list_context updated or pay the initial "resynch" cost.  The
context itself is zero-initialized, so it'll pointlessly walk the leaf
entry array after loading the buffer.

The scrub listxattr implementation has the extra overhead of creating a
dabno bitmap while it walks multi-block attr structures so that it can't
get locked in a cycle.  xdabitmap_set can allocate memory for bitmap
records, which we might not want for code that userspace calls.

Another difficulty in porting xfs_vn_listxattr to use this is that we'd
have to hoist both this and xdabitmap out of scrub.

> No need to hold this series for optimizing regular listxattr, just
> thinking out loud here.

<nod> I guess we could share, but if I were to spend time on fixing
listxattr I'd rather upgrade the interface to use a cursor and gain the
ability to return more than 64k of names.

--D