Re: [PATCH 00/13] client side user xattr (RFC8276) support

Anna Schumaker
2020-03-12T20:10:10Z

Hi Frank,

On Wed, Mar 11, 2020 at 3:57 PM Frank van der Linden
<fllinden@amazon.com> wrote:
>
> This patchset implements the client side for NFS user extended attributes,
> as defined in RFC8726.
>
> This was originally posted as an RFC in:
>
> https://patchwork.kernel.org/cover/11143565/
>
> Patch 1 is shared with the server side patch, posted
> separately.
>
> Most comments in there still apply, except that:
>
> 1. Client side caching is now included in this patch set.
> 2. As per the discussion, user extended attributes are enabled if
> the client and server support them (e.g. they support 4.2 and
> advertise the user extended attribute FATTR). There are no longer
> options to switch them off on either the client or the server.
> 3. The code is no longer conditioned on a config option.
> 4. The number of patches has been reduced somewhat by merging
> smaller, related ones.
>
> The client side caching is implemented through a per-inode hash table,
> which is allocated on demand. See fs/nfs/nfs42xattr.c for details.
>
> This has been tested as follows:
>
> * Linux client and server:
> * Test all corner cases (XATTR_SIZE_*)
> * Test all failure cases (no xattr, setxattr with different or
> invalid flags, etc).
> * Verify the content of xattrs across several operations.
> * Use KASAN and KMEMLEAK for a longer mix of testruns to verify
> that there were no leaks (after unmounting the filesystem).
> * Stress tested caching, trying to run the client out of memory.

I'm curious if you've tried xfstests with your patches? There are a
handful of tests using xattrs that might be good to check with, too:

anna@gouda % grep xattr -l tests/generic/[0-9][0-9][0-9]
tests/generic/037
tests/generic/062
tests/generic/066
tests/generic/093
tests/generic/117
tests/generic/337
tests/generic/377
tests/generic/403
tests/generic/425
tests/generic/454
tests/generic/489
tests/generic/523
tests/generic/529
tests/generic/556

Thanks,
Anna

>
> * Tested against the FreeBSD-current implementation as well, which works
> (after I fixed 2 bugs in that implementation, which I'm sending out to
> them too).
>
> * Not tested: RDMA (I couldn't get a setup going).
>
> Frank van der Linden (13):
> nfs,nfsd: NFSv4.2 extended attribute protocol definitions
> nfs: add client side only definitions for user xattrs
> NFSv4.2: query the server for extended attribute support
> NFSv4.2: define limits and sizes for user xattr handling
> NFSv4.2: add client side XDR handling for extended attributes
> nfs: define nfs_access_get_cached function
> NFSv4.2: query the extended attribute access bits
> nfs: modify update_changeattr to deal with regular files
> nfs: define and use the NFS_INO_INVALID_XATTR flag
> nfs: make the buf_to_pages_noslab function available to the nfs code
> NFSv4.2: add the extended attribute proc functions.
> NFSv4.2: hook in the user extended attribute handlers
> NFSv4.2: add client side xattr caching.
>
> fs/nfs/Makefile | 1 +
> fs/nfs/client.c | 19 +-
> fs/nfs/dir.c | 24 +-
> fs/nfs/inode.c | 16 +-
> fs/nfs/internal.h | 28 ++
> fs/nfs/nfs42.h | 24 +
> fs/nfs/nfs42proc.c | 248 ++++++++++
> fs/nfs/nfs42xattr.c | 1083 +++++++++++++++++++++++++++++++++++++++++++
> fs/nfs/nfs42xdr.c | 442 ++++++++++++++++++
> fs/nfs/nfs4_fs.h | 5 +
> fs/nfs/nfs4client.c | 31 ++
> fs/nfs/nfs4proc.c | 248 ++++++++--
> fs/nfs/nfs4super.c | 10 +
> fs/nfs/nfs4xdr.c | 29 ++
> fs/nfs/nfstrace.h | 3 +-
> include/linux/nfs4.h | 25 +
> include/linux/nfs_fs.h | 12 +
> include/linux/nfs_fs_sb.h | 6 +
> include/linux/nfs_xdr.h | 60 ++-
> include/uapi/linux/nfs4.h | 3 +
> include/uapi/linux/nfs_fs.h | 1 +
> 21 files changed, 2276 insertions(+), 42 deletions(-)
> create mode 100644 fs/nfs/nfs42xattr.c
>
> --
> 2.16.6
>