Received-SPF: pass (google.com: domain of linux-nfs+bounces-1990-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249;
Date: Fri, 16 Feb 2024 08:33:55 -0500
From: Chuck Lever <chuck.lever@oracle.com>
To: "trondmy@kernel.org" <trondmy@kernel.org>
Cc: "linux-nfs@vger.kernel.org" <linux-nfs@vger.kernel.org>
Subject: Re: [PATCH v2 1/2] nfsd: Fix a regression in nfsd_setattr()
Message-ID: <Zc9kQ1Autf6xdcii@tissot.1015granger.net>
References: <20240216012451.22725-1-trondmy@kernel.org>
 <20240216012451.22725-2-trondmy@kernel.org>
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20240216012451.22725-2-trondmy@kernel.org>
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: 
	=?us-ascii?Q?4uhUaKtYzLb64T0RWPbo3BtLH2+C6JKkIt0K9gYD1GRTDc6XBaDRw9zuhYsX?=
 =?us-ascii?Q?dxfDGQUBvn7FFH/IGW7CokEJtooRtug2kmYi4XPlSsVgWSW31GGTrxXyrb5R?=
 =?us-ascii?Q?pk5/HvZORksHOxNH/O8EyzA7EsIrnLNB/ewI4MAQ3wlgqQ1IV6RFF+5NHLY5?=
 =?us-ascii?Q?innmdVz2AvI5jDMgh87PoMYUx3i5WOGtGlI7tJnzl/uMFnSFNaJPwId1/adY?=
 =?us-ascii?Q?xEcj3MMB8lLgipBvro78HLBFtbvfBo6OpS9JMCwDe3svzGPNo5DyJTaDK6TM?=
 =?us-ascii?Q?NSjoOmoN/lmUP7W075+ezrTSRXTh1L/4o1RLtoQF6ITqInP6iuPJKjj1kj0O?=
 =?us-ascii?Q?0r23Y0O1V0M0gNKJ76FGNeSlfmsKgDkVOPDfcxd6tvKXFX6MCbEwZUjHbeji?=
 =?us-ascii?Q?2x1b8CYrk32H//xsNMVUDdl90gAkXVMyF0vX+8Vc92zpyxpDj1jSuLHza9Pi?=
 =?us-ascii?Q?RQz8811vgT32oLKEyl2AMd5ufWknhxl/HR/z7+K69yssmuuW4CfSd4p9Uzwp?=
 =?us-ascii?Q?FT5EiyheZfhT8mbHV9kB9rAGCu2iPKDtZx3hOrny6acl+OeURDM0w6NLckZe?=
 =?us-ascii?Q?5iYcdSyuIGbahijK+bliiRxDUYWkb21NkOTV5fBLq/Agoquaqm2bI5c3Xfti?=
 =?us-ascii?Q?7Pzjp4+/ohPP6FSEUxW1i8HNiPnsTPjCkiRmd7ECLEzwhV7+ZkoSrYy5agDD?=
 =?us-ascii?Q?Xc3JX7ud8bPqss1MZZlzWH5AD9bh2If6NCuwsWqIXEP2+rscL2uOvKII+0Of?=
 =?us-ascii?Q?WJ6OSdC8+RdDf/aNx0Qd32+wH1xcWKR2NeFTbzdaV91k4Wj3XgQAdAev7UqD?=
 =?us-ascii?Q?JngUjvVHEH4ZkMq77PcuvoRhjPlkB31x3C/xqpmnAPVXYXUqJWEWBC9xSfyZ?=
 =?us-ascii?Q?Z1+2j6LlevWTSKzihFCzPeb4zRfRAeJIeKrQQonpgPM+3Xzg6iFox4V4KmRS?=
 =?us-ascii?Q?mUM6LRZblrEn/e19kMBcf9QJuYVQWatKDZfLXewJcUkgGeuUenKFzEKAu75s?=
 =?us-ascii?Q?8V9kXrTxl3mvYeP0KaKxUr/bXKvS90TvKVEwwrGXAoMr9NQvkFykyuU1pmme?=
 =?us-ascii?Q?JLvrl/AmIfBb6ocDJ4sEJwTFlXzECxVD15VW4DJyhNFyo8poSTpmsdNotvZe?=
 =?us-ascii?Q?hEhCbYNPUHXHF59fBtDjARFEf4a8donDR7WAN6IlvORcMWduDa4Eiqvb6xc+?=
 =?us-ascii?Q?KKKQt7dGicR+5+t/UMva0ld9nQ0pHzJIM9D6ceEleR7bRNtke0d3EtBdwhLr?=
 =?us-ascii?Q?CNUZFUPFy6bTnb9PgU5OuAsyD7T3j+xE9vcmHc1mqsBLwFo+8ZOBNR08qgS8?=
 =?us-ascii?Q?UF5hTbM1Xyx6cMiuk1psL0y6B0HgThqLvT9sIehfiyWiDYEBeXInbzmrpKzd?=
 =?us-ascii?Q?tfbASs13DBRsVYTDtHXgfQufDPhJVPZUBqotxjalCIWU4Q/vruNvEOf7r9hD?=
 =?us-ascii?Q?1tFHKflQbzpGB2wPBLa2gqIb3b5s0M89n8TpQZBiIqz33ICrofgNxr2bblrt?=
 =?us-ascii?Q?tkYwcvMmv+HbNFBv2+6b7EUYA1RLVleHvSIVvUUz31e6wOPlkk2xuRrIpsjA?=
 =?us-ascii?Q?tCAeitrahUNlGcc2EwFJOZlqm0X9xHLik0TwC2Daf5btlACHXGQHyqPHYsl7?=
 =?us-ascii?Q?Cw=3D=3D?=
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-0: 
	8J/7ibyx84DKVPtwayAdmK7dP/1TUbKB901J8KD+C21szUXa5zcXszmecEW/ayIPffzb6+/mSj2rodJZyCM1NkM7q1/j6CM2z61UNUy/7NePeABnslmTiA5bRqXwqVdNoYpXSdMYL1yxRIiS0m45ReUJFyeeckR/uSY5MmDSjiXFKyYZQHsm8rhhXjapDPfwr+/mqs1Rz5B79Ryjf80HXyd+PdkxZ0w/k3SsEAh01oU1C8BTELO8Qpddjh/B1ka5vQERRbVhv1zZ1NQzoXWCpetsjLVkhYmg8HEPnKTmuLlJsXmfF/9SmF7Bm5mAXEl3yBDNGWxDXQ8itZt0yGY3Rto8zyrijiNI/Pq7nVwWoBMyZETsDagYpC8Fvio6KApjV9koi6yudkuXxKak2c0kmw1cw8HbYrywYhiEpuTGLpk82ImsUGXL41yiG6S4ik5aawqkvXQ2GtFt5zdcAKkHVnlQiA8E0dJhJZ1tDkgcfFnJHWNsruogkS4y3owYkyswhd3xncLZ+u+R+jYL2scSFpNU+tGDp2CJQLt5eegjNvuJUircUFb128SoANwy8Bp2MF2uv9HSGOcS7Mswzuv4FMJ8tWYzdPSyOJcgPKhYUVs=
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 733dc817-bfa5-4035-4a07-08dc2ef3eda9
X-MS-Exchange-CrossTenant-AuthSource: BN0PR10MB5128.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 16 Feb 2024 13:33:58.8284
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 4e2c6054-71cb-48f1-bd6c-3a9705aca71b
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: c0niGBf6B1UILKRPzxKKJC5qWAB55uh6uS8XgJB2F7Hflfcn0w+4whJqu1HMYxqVTQbMRSM6qqKnBI3RVGtAPA==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM6PR10MB4348
X-Proofpoint-Virus-Version: vendor=baseguard
 engine=ICAP:2.0.272,Aquarius:18.0.1011,Hydra:6.0.619,FMLib:17.11.176.26
 definitions=2024-02-16_11,2024-02-16_01,2023-05-22_02
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 malwarescore=0 mlxscore=0 spamscore=0
 adultscore=0 phishscore=0 bulkscore=0 mlxlogscore=930 suspectscore=0
 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2311290000
 definitions=main-2402160109
X-Proofpoint-GUID: 6zStbLM7DW6ABKKUAYWh83DKtSL4HaKL
X-Proofpoint-ORIG-GUID: 6zStbLM7DW6ABKKUAYWh83DKtSL4HaKL

On Thu, Feb 15, 2024 at 08:24:50PM -0500, trondmy@kernel.org wrote:
> From: Trond Myklebust <trond.myklebust@hammerspace.com>
> 
> Commit bb4d53d66e4b broke the NFSv3 pre/post op attributes behaviour
> when doing a SETATTR rpc call by stripping out the calls to
> fh_fill_pre_attrs() and fh_fill_post_attrs().

Can you give more detail about what broke?


> Fixes: bb4d53d66e4b ("NFSD: use (un)lock_inode instead of fh_(un)lock for file operations")
> Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com>
> ---
>  fs/nfsd/nfs4proc.c | 4 ++++
>  fs/nfsd/vfs.c      | 9 +++++++--
>  2 files changed, 11 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
> index 14712fa08f76..e6d8624efc83 100644
> --- a/fs/nfsd/nfs4proc.c
> +++ b/fs/nfsd/nfs4proc.c
> @@ -1143,6 +1143,7 @@ nfsd4_setattr(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
>  	};
>  	struct inode *inode;
>  	__be32 status = nfs_ok;
> +	bool save_no_wcc;
>  	int err;
>  
>  	if (setattr->sa_iattr.ia_valid & ATTR_SIZE) {
> @@ -1168,8 +1169,11 @@ nfsd4_setattr(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
>  
>  	if (status)
>  		goto out;
> +	save_no_wcc = cstate->current_fh.fh_no_wcc;
> +	cstate->current_fh.fh_no_wcc = true;
>  	status = nfsd_setattr(rqstp, &cstate->current_fh, &attrs,
>  				0, (time64_t)0);
> +	cstate->current_fh.fh_no_wcc = save_no_wcc;
>  	if (!status)
>  		status = nfserrno(attrs.na_labelerr);
>  	if (!status)
> diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c
> index 6e7e37192461..58fab461bc00 100644
> --- a/fs/nfsd/vfs.c
> +++ b/fs/nfsd/vfs.c
> @@ -497,7 +497,7 @@ nfsd_setattr(struct svc_rqst *rqstp, struct svc_fh *fhp,
>  	int		accmode = NFSD_MAY_SATTR;
>  	umode_t		ftype = 0;
>  	__be32		err;
> -	int		host_err;
> +	int		host_err = 0;
>  	bool		get_write_count;
>  	bool		size_change = (iap->ia_valid & ATTR_SIZE);
>  	int		retries;
> @@ -555,6 +555,9 @@ nfsd_setattr(struct svc_rqst *rqstp, struct svc_fh *fhp,
>  	}
>  
>  	inode_lock(inode);
> +	err = fh_fill_pre_attrs(fhp);
> +	if (err)
> +		goto out_unlock;
>  	for (retries = 1;;) {
>  		struct iattr attrs;
>  
> @@ -582,13 +585,15 @@ nfsd_setattr(struct svc_rqst *rqstp, struct svc_fh *fhp,
>  		attr->na_aclerr = set_posix_acl(&nop_mnt_idmap,
>  						dentry, ACL_TYPE_DEFAULT,
>  						attr->na_dpacl);
> +	fh_fill_post_attrs(fhp);
> +out_unlock:
>  	inode_unlock(inode);
>  	if (size_change)
>  		put_write_access(inode);
>  out:
>  	if (!host_err)
>  		host_err = commit_metadata(fhp);
> -	return nfserrno(host_err);
> +	return err != 0 ? err : nfserrno(host_err);
>  }
>  
>  #if defined(CONFIG_NFSD_V4)
> -- 
> 2.43.1
> 
> 

-- 
Chuck Lever