Received-SPF: pass (google.com: best guess record for domain of linux-nfs-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
MIME-Version: 1.0
References: <20200311195613.26108-1-fllinden@amazon.com>
In-Reply-To: <20200311195613.26108-1-fllinden@amazon.com>
From:   Anna Schumaker <anna.schumaker@netapp.com>
Date:   Thu, 12 Mar 2020 16:09:51 -0400
Message-ID: <CAFX2Jf=g2Pv62cnciB4VG6HTndJSrtfeSR_oVu9PmiBez8_Upw@mail.gmail.com>
Subject: Re: [PATCH 00/13] client side user xattr (RFC8276) support
To:     Frank van der Linden <fllinden@amazon.com>
Cc:     trond.myklebust@hammerspace.com,
        Linux NFS Mailing List <linux-nfs@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Sender: linux-nfs-owner@vger.kernel.org
Precedence: bulk

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
>