Received-SPF: pass (google.com: domain of linux-nfs-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
From:   "NeilBrown" <neilb@suse.de>
To:     "J. Bruce Fields" <bfields@fieldses.org>
Cc:     "Chuck Lever III" <chuck.lever@oracle.com>,
        "Linux NFS Mailing List" <linux-nfs@vger.kernel.org>
Subject: Re: nfs/001 failing
In-reply-to: <20220906122714.GA25323@fieldses.org>
References: <BF47B6B7-CB52-4E14-94B0-E28FD5C52234@oracle.com>,
 <20220906122714.GA25323@fieldses.org>
Date:   Wed, 07 Sep 2022 09:11:08 +1000
Message-id: <166250586898.30452.12563131953046174303@noble.neil.brown.name>
Precedence: bulk

On Tue, 06 Sep 2022, J. Bruce Fields wrote:
> On Mon, Sep 05, 2022 at 04:29:16PM +0000, Chuck Lever III wrote:
> > Bruce reminded me I'm not the only one seeing this failure
> > these days:
> >=20
> > > nfs/001 4s ... - output mismatch (see /root/xfstests-dev/results//nfs/0=
01.out.bad)
> > >    --- tests/nfs/001.out	2019-12-20 17:34:10.569343364 -0500
> > >    +++ /root/xfstests-dev/results//nfs/001.out.bad	2022-09-04 20:01:35.=
502462323 -0400
> > >    @@ -1,2 +1,2 @@
> > >     QA output created by 001
> > >    -203
> > >    +3
> > >    ...
> >=20
> > I'm looking at about 5 other priority bugs at the moment. Can
> > someone else do a little triage?
>=20
> For what it's worth, a bisect lands on
> c0cbe70742f4a70893cd6e5f6b10b6e89b6db95b "NFSD: add posix ACLs to struct
> nfsd_attrs".
>=20
> Haven't really looked at nfs/001 except to note it does have something
> to do with ACLs, so that checks out....

I see the problem.
acl setting was moved to nfsd_setattr().
But nfsd_setattr() contains the lines

	if (!iap->ia_valid)
		return 0;

In the setacl case, ia_valid is 0.

Maybe something like:

diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c
index 528afc3be7af..0582a5b16237 100644
--- a/fs/nfsd/vfs.c
+++ b/fs/nfsd/vfs.c
@@ -395,9 +395,6 @@ nfsd_setattr(struct svc_rqst *rqstp, struct svc_fh *fhp,
 	if (S_ISLNK(inode->i_mode))
 		iap->ia_valid &=3D ~ATTR_MODE;
=20
-	if (!iap->ia_valid)
-		return 0;
-
 	nfsd_sanitize_attrs(inode, iap);
=20
 	if (check_guard && guardtime !=3D inode->i_ctime.tv_sec)
@@ -448,8 +445,10 @@ nfsd_setattr(struct svc_rqst *rqstp, struct svc_fh *fhp,
 			goto out_unlock;
 	}
=20
-	iap->ia_valid |=3D ATTR_CTIME;
-	host_err =3D notify_change(&init_user_ns, dentry, iap, NULL);
+	if (iap->ia_valid) {
+		iap->ia_valid |=3D ATTR_CTIME;
+		host_err =3D notify_change(&init_user_ns, dentry, iap, NULL);
+	}
=20
 out_unlock:
 	if (attr->na_seclabel && attr->na_seclabel->len)


Does that seem reasonable?

Thanks,
NeilBrown