Received-SPF: pass (google.com: domain of linux-nfs+bounces-3839-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1;
Precedence: bulk
MIME-Version: 1.0
References: <20240613041136.506908-1-trond.myklebust@hammerspace.com>
 <20240613041136.506908-2-trond.myklebust@hammerspace.com> <20240613041136.506908-3-trond.myklebust@hammerspace.com>
 <20240613041136.506908-4-trond.myklebust@hammerspace.com> <20240613041136.506908-5-trond.myklebust@hammerspace.com>
 <20240613041136.506908-6-trond.myklebust@hammerspace.com> <20240613041136.506908-7-trond.myklebust@hammerspace.com>
 <20240613041136.506908-8-trond.myklebust@hammerspace.com> <20240613041136.506908-9-trond.myklebust@hammerspace.com>
 <20240613041136.506908-10-trond.myklebust@hammerspace.com>
 <20240613041136.506908-11-trond.myklebust@hammerspace.com>
 <20240613041136.506908-12-trond.myklebust@hammerspace.com> <20240613041136.506908-13-trond.myklebust@hammerspace.com>
In-Reply-To: <20240613041136.506908-13-trond.myklebust@hammerspace.com>
From: Anna Schumaker <schumaker.anna@gmail.com>
Date: Fri, 14 Jun 2024 12:32:37 -0400
Message-ID: <CAFX2Jfk7u37+AOX_o1ZRf-QX_abSDXpKaEpHt=iOvL5Bq6opTQ@mail.gmail.com>
Subject: Re: [PATCH 12/19] NFSv4: Fix up delegated attributes in nfs_setattr
To: trondmy@gmail.com
Cc: linux-nfs@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

Hi Trond,

On Thu, Jun 13, 2024 at 12:18=E2=80=AFAM <trondmy@gmail.com> wrote:
>
> From: Trond Myklebust <trond.myklebust@primarydata.com>
>
> nfs_setattr calls nfs_update_inode() directly, so we have to reset the
> m/ctime there.
>
> Signed-off-by: Trond Myklebust <trond.myklebust@primarydata.com>
> Signed-off-by: Lance Shelton <lance.shelton@hammerspace.com>
> Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com>
> ---
>  fs/nfs/inode.c | 43 +++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 43 insertions(+)

After applying this patch I see a handful of new failures on xfstests:
generic/075, generic/086, generic/112, generic/332, generic/346,
generic/647, and generic/729. I see the first five on NFS v4.2, but
647 and 729 both fail on v4.1 in addition to v4.2.

I hope this helps!
Anna

>
> diff --git a/fs/nfs/inode.c b/fs/nfs/inode.c
> index 91c0aeaf6c1e..e03c512c8535 100644
> --- a/fs/nfs/inode.c
> +++ b/fs/nfs/inode.c
> @@ -605,6 +605,46 @@ nfs_fhget(struct super_block *sb, struct nfs_fh *fh,=
 struct nfs_fattr *fattr)
>  }
>  EXPORT_SYMBOL_GPL(nfs_fhget);
>
> +static void
> +nfs_fattr_fixup_delegated(struct inode *inode, struct nfs_fattr *fattr)
> +{
> +       unsigned long cache_validity =3D NFS_I(inode)->cache_validity;
> +
> +       if (!nfs_have_read_or_write_delegation(inode))
> +               return;
> +
> +       if (!(cache_validity & NFS_INO_REVAL_FORCED))
> +               cache_validity &=3D ~(NFS_INO_INVALID_ATIME
> +                               | NFS_INO_INVALID_CHANGE
> +                               | NFS_INO_INVALID_CTIME
> +                               | NFS_INO_INVALID_MTIME
> +                               | NFS_INO_INVALID_SIZE);
> +
> +       if (!(cache_validity & NFS_INO_INVALID_SIZE))
> +               fattr->valid &=3D ~(NFS_ATTR_FATTR_PRESIZE
> +                               | NFS_ATTR_FATTR_SIZE);
> +
> +       if (!(cache_validity & NFS_INO_INVALID_CHANGE))
> +               fattr->valid &=3D ~(NFS_ATTR_FATTR_PRECHANGE
> +                               | NFS_ATTR_FATTR_CHANGE);
> +
> +       if (nfs_have_delegated_mtime(inode)) {
> +               if (!(cache_validity & NFS_INO_INVALID_CTIME))
> +                       fattr->valid &=3D ~(NFS_ATTR_FATTR_PRECTIME
> +                                       | NFS_ATTR_FATTR_CTIME);
> +
> +               if (!(cache_validity & NFS_INO_INVALID_MTIME))
> +                       fattr->valid &=3D ~(NFS_ATTR_FATTR_PREMTIME
> +                                       | NFS_ATTR_FATTR_MTIME);
> +
> +               if (!(cache_validity & NFS_INO_INVALID_ATIME))
> +                       fattr->valid &=3D ~NFS_ATTR_FATTR_ATIME;
> +       } else if (nfs_have_delegated_atime(inode)) {
> +               if (!(cache_validity & NFS_INO_INVALID_ATIME))
> +                       fattr->valid &=3D ~NFS_ATTR_FATTR_ATIME;
> +       }
> +}
> +
>  void nfs_update_delegated_atime(struct inode *inode)
>  {
>         spin_lock(&inode->i_lock);
> @@ -2163,6 +2203,9 @@ static int nfs_update_inode(struct inode *inode, st=
ruct nfs_fattr *fattr)
>          */
>         nfsi->read_cache_jiffies =3D fattr->time_start;
>
> +       /* Fix up any delegated attributes in the struct nfs_fattr */
> +       nfs_fattr_fixup_delegated(inode, fattr);
> +
>         save_cache_validity =3D nfsi->cache_validity;
>         nfsi->cache_validity &=3D ~(NFS_INO_INVALID_ATTR
>                         | NFS_INO_INVALID_ATIME
> --
> 2.45.2
>
>