Re: [PATCH] common/attr: reduce MAX_ATTRVAL_SIZE for NFS

[David Disseldorp <ddiss@suse.de>]

Thanks for the review, Dave...

On Mon, 11 Apr 2022 15:47:14 +1000, Dave Chinner wrote:

> > This is due to the MAX_ATTRVAL_SIZE=65536 setting for NFS, which exceeds
> > the Btrfs (and XFS) limit of MAX_ATTRVAL_SIZE=64. Change NFS to use this
> > lower bound value.  
> 
> I think that what XFS/UDF/BTRFS set here is bogus.
> 
> There is *one* test - generic/020 - that uses MAX_ATTRS and
> MAX_ATTRVAL_SIZE, and  it uses MAX_ATTRVAL_SIZE as a byte count.
> 
> Which means for those 3 filesytems, the correct value for them is
> 65536, not 64....
> 
> This looks like it was broken back when the test was made generic
> - the dd command before this used a bs=1024, so a maz size of 64
> would have been correct. Except the dd command also go changed to
> use bs=1, which meant 64 bytes....
> 
> So, yeah, the test got "broken" for XFS back in 2012 and so the
> correct fix here is to change (at least) XFS and btrfs to have a
> MAX_ATTRVAL_SIZE=65536....

I'll change XFS over to use a 64K bytes limit. It looks like we'll need
a separate special case for Btrfs, as the size limit also takes the attr
name length into account:

 79 int btrfs_setxattr(struct btrfs_trans_handle *trans, struct inode *inode,
 80                    const char *name, const void *value, size_t size, int flags)
 81 {
 ...
 91         if (name_len + size > BTRFS_MAX_XATTR_SIZE(root->fs_info))
 92                 return -ENOSPC;

BTRFS_MAX_XATTR_SIZE() is also 16K by default on my test system. Given
that this is for generic/020 only, my plan is to turn the existing
MAX_ATTRVAL_SIZE logic into a helper function.

Cheers, David