[PATCH] common/attr: remove the MAX_ATTRS and MAX_ATTRVAL_SIZE for nfs

[PATCH] common/attr: remove the MAX_ATTRS and MAX_ATTRVAL_SIZE for nfs

[Hao Xu]

The block size of localfs for nfs may be different with nfs itself.
So it's pointless to test nfs on xattrs size, just remove the special
judge code.

Fixes: commit da3cdb3b91ca ("common/attr: set MAX_ATTR values correctly for NFS")
Signed-off-by: Hao Xu <haoxu@linux.alibaba.com>
---
 common/attr | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/common/attr b/common/attr
index 42ceab92335a..542dff34bf55 100644
--- a/common/attr
+++ b/common/attr
@@ -253,7 +253,7 @@ _getfattr()
 
 # set maximum total attr space based on fs type
 case "$FSTYP" in
-xfs|udf|pvfs2|9p|ceph|nfs)
+xfs|udf|pvfs2|9p|ceph)
 	MAX_ATTRS=1000
 	;;
 *)
@@ -273,7 +273,7 @@ xfs|udf|btrfs)
 pvfs2)
 	MAX_ATTRVAL_SIZE=8192
 	;;
-9p|ceph|nfs)
+9p|ceph)
 	MAX_ATTRVAL_SIZE=65536
 	;;
 bcachefs)
-- 
2.24.4

Re: [PATCH] common/attr: remove the MAX_ATTRS and MAX_ATTRVAL_SIZE for nfs

[Frank van der Linden]

On Wed, Aug 04, 2021 at 03:41:48PM +0800, Hao Xu wrote:
> The block size of localfs for nfs may be different with nfs itself.
> So it's pointless to test nfs on xattrs size, just remove the special
> judge code.
> 
> Fixes: commit da3cdb3b91ca ("common/attr: set MAX_ATTR values correctly for NFS")
> Signed-off-by: Hao Xu <haoxu@linux.alibaba.com>
> ---
>  common/attr | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/common/attr b/common/attr
> index 42ceab92335a..542dff34bf55 100644
> --- a/common/attr
> +++ b/common/attr
> @@ -253,7 +253,7 @@ _getfattr()
> 
>  # set maximum total attr space based on fs type
>  case "$FSTYP" in
> -xfs|udf|pvfs2|9p|ceph|nfs)
> +xfs|udf|pvfs2|9p|ceph)
>         MAX_ATTRS=1000
>         ;;
>  *)
> @@ -273,7 +273,7 @@ xfs|udf|btrfs)
>  pvfs2)
>         MAX_ATTRVAL_SIZE=8192
>         ;;
> -9p|ceph|nfs)
> +9p|ceph)
>         MAX_ATTRVAL_SIZE=65536
>         ;;
>  bcachefs)
> --
> 2.24.4
> 

That doesn't quite work either, right? You have now removed the specific
values set for NFS, but the defaults will still be used (which may also
be wrong).

It seems to me that there are two ways that this can be dealt with:

1) Find a way to determine the maximum attribute size over an NFS mount,
   possibly by trying different sizes. If it doesn't work out, skip the test
2) Always skip the test.

- Frank

Re: [PATCH] common/attr: remove the MAX_ATTRS and MAX_ATTRVAL_SIZE for nfs

[Eryu Guan]

On Wed, Aug 04, 2021 at 03:41:48PM +0800, Hao Xu wrote:
> The block size of localfs for nfs may be different with nfs itself.
> So it's pointless to test nfs on xattrs size, just remove the special
> judge code.
> 
> Fixes: commit da3cdb3b91ca ("common/attr: set MAX_ATTR values correctly for NFS")
> Signed-off-by: Hao Xu <haoxu@linux.alibaba.com>
> ---
>  common/attr | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/common/attr b/common/attr
> index 42ceab92335a..542dff34bf55 100644
> --- a/common/attr
> +++ b/common/attr
> @@ -253,7 +253,7 @@ _getfattr()
>  
>  # set maximum total attr space based on fs type
>  case "$FSTYP" in
> -xfs|udf|pvfs2|9p|ceph|nfs)
> +xfs|udf|pvfs2|9p|ceph)
>  	MAX_ATTRS=1000

This makes generic/020 _notrun on nfs, I don't think that's what we
want.

I think MAX_ATTRS is a best-effort guess based on filesystem block size,
it's not a accurate hard limit. And if there's no good way to tell the
fs blocksize on nfs server side, then we could make a conservative
assumption, e.g. the blocksize is 1k, and colculate MAX_ATTRS and
MAX_ATTRVAL_SIZE based on that assumption.

Thanks,
Eryu

>  	;;
>  *)
> @@ -273,7 +273,7 @@ xfs|udf|btrfs)
>  pvfs2)
>  	MAX_ATTRVAL_SIZE=8192
>  	;;
> -9p|ceph|nfs)
> +9p|ceph)
>  	MAX_ATTRVAL_SIZE=65536
>  	;;
>  bcachefs)
> -- 
> 2.24.4