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

[Dave Chinner <david@fromorbit.com>]

On Mon, Apr 11, 2022 at 10:57:52AM +0200, David Disseldorp wrote:
> 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.

Sounds good! Can you do the same with MAX_ATTRS, too? It's a g/020
only value, too.

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com