Received-SPF: pass (google.com: domain of linux-ext4-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
From:   Chuck Lever III <chuck.lever@oracle.com>
To:     Jeff Layton <jlayton@kernel.org>
CC:     Theodore Ts'o <tytso@mit.edu>,
        "adilger.kernel@dilger.ca" <adilger.kernel@dilger.ca>,
        "Darrick J. Wong" <djwong@kernel.org>,
        Dave Chinner <david@fromorbit.com>,
        Trond Myklebust <trondmy@hammerspace.com>,
        Neil Brown <neilb@suse.de>, Al Viro <viro@zeniv.linux.org.uk>,
        "zohar@linux.ibm.com" <zohar@linux.ibm.com>,
        "xiubli@redhat.com" <xiubli@redhat.com>,
        Lukas Czerner <lczerner@redhat.com>, Jan Kara <jack@suse.cz>,
        Bruce Fields <bfields@fieldses.org>,
        Christian Brauner <brauner@kernel.org>,
        "fweimer@redhat.com" <fweimer@redhat.com>,
        "linux-btrfs@vger.kernel.org" <linux-btrfs@vger.kernel.org>,
        linux-fsdevel <linux-fsdevel@vger.kernel.org>,
        LKML <linux-kernel@vger.kernel.org>,
        "ceph-devel@vger.kernel.org" <ceph-devel@vger.kernel.org>,
        "linux-ext4@vger.kernel.org" <linux-ext4@vger.kernel.org>,
        Linux NFS Mailing List <linux-nfs@vger.kernel.org>,
        "linux-xfs@vger.kernel.org" <linux-xfs@vger.kernel.org>
Subject: Re: [PATCH v7 6/9] nfsd: move nfsd4_change_attribute to nfsfh.c
Thread-Topic: [PATCH v7 6/9] nfsd: move nfsd4_change_attribute to nfsfh.c
Thread-Index: AQHY4hdBdnRn2o/trUW2gl8SzYdhC64tcYOA
Date:   Thu, 3 Nov 2022 15:42:15 +0000
Message-ID: <732CF94B-CB0C-44E4-8292-F5E248B6560F@oracle.com>
References: <20221017105709.10830-1-jlayton@kernel.org>
 <20221017105709.10830-7-jlayton@kernel.org>
In-Reply-To: <20221017105709.10830-7-jlayton@kernel.org>
Accept-Language: en-US
Content-Language: en-US
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?abGmgR/UEV90rqSmOspPe8qmNAkahamH+nH6Z9RlT6xmfyi3q8VOqIyk41da?=
 =?us-ascii?Q?+5LceHyToz9k+stVZpkpbwDihwcYPiqGNImSJlaOrxzfo6xBxFY8L7kvPWjj?=
 =?us-ascii?Q?0wzqLMJPRCnrF9xAaWjeJpXJt34XS/cGrFQatoIiGhQLBu+b8yn8HHY5+0c+?=
 =?us-ascii?Q?RAuYWo8+DIPTskyOpPh/cYE6pcOM17uDCUE5z8ay19+PXMs5uGMwpH4NpMAv?=
 =?us-ascii?Q?ybxB6y9P8QeB5IjDCPY3ZOXbjIEGCgNlQZSXyiRYPApTZSfU8lWKv0SwvOHI?=
 =?us-ascii?Q?8kLwPHNTYBFhSfWetFNveIMYCuYRPUVSaxIdJG87rwqx5aEbOAB4HPVif/4T?=
 =?us-ascii?Q?UO91IWtYoK/Yei/feMBRWz0QsxCByR33HllxtSOCB/MZg0K9vS9tNNTq1+6U?=
 =?us-ascii?Q?NU559ssC5iIy9iNTpzjQujO0FXYE+ovKXsQZT52K651BUdNRxm7v9Q9yM97K?=
 =?us-ascii?Q?JX7dS88aICVEk9Eba0zPBNa6pAHT/Lur0Tmj7k3l7sY+PWLzh8BDMqE8x8io?=
 =?us-ascii?Q?REXh+UrIgxEE7FMskFQyeU1a84PrZe+631D/KYPdefBSgaI2HVpqVIMB24It?=
 =?us-ascii?Q?JqR5I1hzM3TVito5m6cNohib44G5IRCIKJCw7U5WZ6jkexDCe+c+6pTKvDZJ?=
 =?us-ascii?Q?olGCoPVJ3ozKLFfxYSdrm+YmSX4ihTZEJ7yL0ICEHuO4KLjOPYAgDYdM8Ytq?=
 =?us-ascii?Q?IKgG7CTgPdijPnl9vNiMFiZ0txU5+rBbfigYI05vL1fYGAq3l2q5cHPBlv8a?=
 =?us-ascii?Q?IYn83fleloVKxgTI7sjwoy0NFYTM18ZMTQR925N1/BBKg2vaHD1GAJV7DAMR?=
 =?us-ascii?Q?BzxEzlahMZTmVlz+axQvfQjyJ24We4ck91zeh7hHsmckqzWo5Qyy3Tv+Zmy6?=
 =?us-ascii?Q?o6gR6nXOieKFzkThFmk45mtBCo+VC4TeT92P50xbnyp8FpqAzmm3NCNuSc+s?=
 =?us-ascii?Q?Q8XnMvuei7bvlNPfkQ1mOW2LxpORjCq5AsL5Ur1Kn9KkaU0+31v9pFsAE2x2?=
 =?us-ascii?Q?YvmjNAdpI5ojWVnd0gY0oeppYhd6HhCP8vJj1xeZRv9ACLJXvrvl/169GC5R?=
 =?us-ascii?Q?hLnVDXL9A4pTULvdYhL4b7CnqtjVBjTACsuihZYk2nmXkdWd3UazRXtyqbtm?=
 =?us-ascii?Q?c5eyd/OlZaLVuHaQvOju5SuOhwFlW1HwIMXglXZD31VgaSCJ7AnmZ3YldnFW?=
 =?us-ascii?Q?102ZHuX0cmJrPkHTJGPcdOoCpefsbF1YedjoHCq3sdp4PZ6+rL1XPJSwB/3h?=
 =?us-ascii?Q?WUXKqMMjevVmcKdZZ2RsLtjAQXKw8aQFmbjgb4y8fYVlv0GI4sE90m3fpoGo?=
 =?us-ascii?Q?RY6C5ww/FxAQIEUERQ1vKbqswzzoQbHkgsTfEQV7ZmGpRDJOULMqAyTxcvv9?=
 =?us-ascii?Q?U5nQ9+8kYjvYXul8R+v8slUcf0ZJH9TUnGAXnOF0me8T1nTFrdLQmasw34S2?=
 =?us-ascii?Q?IgrFJK1zrbJZRpOO4tXEyqHFmzAhfrrR8q7spgbC6mbZk1FPaKmmYhAMmAGO?=
 =?us-ascii?Q?GtLi+79fHrGk/gsK2CCUC98P/CJifIxE0bhdGGqscCMfHYZQuQZErArsRtvh?=
 =?us-ascii?Q?SFEQ1R7CCUwrsPCyjWAPFMptmh/Fcjx9lWiHdKLo8P4DxMDAdOUQ+MJQ8673?=
 =?us-ascii?Q?9Q=3D=3D?=
Content-Type: text/plain; charset="us-ascii"
Content-ID: <F01F2D9B0B0D4344BFEED4B60804205A@namprd10.prod.outlook.com>
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-AuthSource: BN0PR10MB5128.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 54de6bc9-6d90-4e5f-3f0e-08dabdb1fbf8
X-MS-Exchange-CrossTenant-originalarrivaltime: 03 Nov 2022 15:42:15.9932
 (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: w/hhV4nPXgNDBLv8lfAqgq3RvxeAFNYwn3qDAU7EhFjPcK5EyoCwK0fNpn0aaaS12iDh1jzvh8bRYDn2q0eAIQ==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CY5PR10MB6144
X-Proofpoint-Virus-Version: vendor=baseguard
 engine=ICAP:2.0.205,Aquarius:18.0.895,Hydra:6.0.545,FMLib:17.11.122.1
 definitions=2022-11-03_04,2022-11-03_01,2022-06-22_01
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 adultscore=0 spamscore=0 bulkscore=0
 suspectscore=0 phishscore=0 malwarescore=0 mlxscore=0 mlxlogscore=999
 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2210170000
 definitions=main-2211030104
X-Proofpoint-ORIG-GUID: 9b5jv_YYu7nUuywBYPAT5lf6mS8X43Dk
X-Proofpoint-GUID: 9b5jv_YYu7nUuywBYPAT5lf6mS8X43Dk
X-Spam-Status: No, score=-2.8 required=5.0 tests=BAYES_00,DKIM_SIGNED,
        DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_LOW,
        RCVD_IN_MSPIKE_H2,SPF_HELO_NONE,SPF_NONE autolearn=ham
        autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-ext4.vger.kernel.org>
X-Mailing-List: linux-ext4@vger.kernel.org


> On Oct 17, 2022, at 6:57 AM, Jeff Layton <jlayton@kernel.org> wrote:
>=20
> This is a pretty big function for inlining. Move it to being
> non-inlined.
>=20
> Reviewed-by: NeilBrown <neilb@suse.de>
> Signed-off-by: Jeff Layton <jlayton@kernel.org>

Acked-by: Chuck Lever <chuck.lever@oracle.com>


> ---
> fs/nfsd/nfsfh.c | 27 +++++++++++++++++++++++++++
> fs/nfsd/nfsfh.h | 29 +----------------------------
> 2 files changed, 28 insertions(+), 28 deletions(-)
>=20
> diff --git a/fs/nfsd/nfsfh.c b/fs/nfsd/nfsfh.c
> index d73434200df9..7030d9209903 100644
> --- a/fs/nfsd/nfsfh.c
> +++ b/fs/nfsd/nfsfh.c
> @@ -748,3 +748,30 @@ enum fsid_source fsid_source(const struct svc_fh *fh=
p)
> 		return FSIDSOURCE_UUID;
> 	return FSIDSOURCE_DEV;
> }
> +
> +/*
> + * We could use i_version alone as the change attribute.  However,
> + * i_version can go backwards after a reboot.  On its own that doesn't
> + * necessarily cause a problem, but if i_version goes backwards and then
> + * is incremented again it could reuse a value that was previously used
> + * before boot, and a client who queried the two values might
> + * incorrectly assume nothing changed.
> + *
> + * By using both ctime and the i_version counter we guarantee that as
> + * long as time doesn't go backwards we never reuse an old value.
> + */
> +u64 nfsd4_change_attribute(struct kstat *stat, struct inode *inode)
> +{
> +	if (inode->i_sb->s_export_op->fetch_iversion)
> +		return inode->i_sb->s_export_op->fetch_iversion(inode);
> +	else if (IS_I_VERSION(inode)) {
> +		u64 chattr;
> +
> +		chattr =3D  stat->ctime.tv_sec;
> +		chattr <<=3D 30;
> +		chattr +=3D stat->ctime.tv_nsec;
> +		chattr +=3D inode_query_iversion(inode);
> +		return chattr;
> +	} else
> +		return time_to_chattr(&stat->ctime);
> +}
> diff --git a/fs/nfsd/nfsfh.h b/fs/nfsd/nfsfh.h
> index c3ae6414fc5c..4c223a7a91d4 100644
> --- a/fs/nfsd/nfsfh.h
> +++ b/fs/nfsd/nfsfh.h
> @@ -291,34 +291,7 @@ static inline void fh_clear_pre_post_attrs(struct sv=
c_fh *fhp)
> 	fhp->fh_pre_saved =3D false;
> }
>=20
> -/*
> - * We could use i_version alone as the change attribute.  However,
> - * i_version can go backwards after a reboot.  On its own that doesn't
> - * necessarily cause a problem, but if i_version goes backwards and then
> - * is incremented again it could reuse a value that was previously used
> - * before boot, and a client who queried the two values might
> - * incorrectly assume nothing changed.
> - *
> - * By using both ctime and the i_version counter we guarantee that as
> - * long as time doesn't go backwards we never reuse an old value.
> - */
> -static inline u64 nfsd4_change_attribute(struct kstat *stat,
> -					 struct inode *inode)
> -{
> -	if (inode->i_sb->s_export_op->fetch_iversion)
> -		return inode->i_sb->s_export_op->fetch_iversion(inode);
> -	else if (IS_I_VERSION(inode)) {
> -		u64 chattr;
> -
> -		chattr =3D  stat->ctime.tv_sec;
> -		chattr <<=3D 30;
> -		chattr +=3D stat->ctime.tv_nsec;
> -		chattr +=3D inode_query_iversion(inode);
> -		return chattr;
> -	} else
> -		return time_to_chattr(&stat->ctime);
> -}
> -
> +u64 nfsd4_change_attribute(struct kstat *stat, struct inode *inode);
> extern void fh_fill_pre_attrs(struct svc_fh *fhp);
> extern void fh_fill_post_attrs(struct svc_fh *fhp);
> extern void fh_fill_both_attrs(struct svc_fh *fhp);
> --=20
> 2.37.3
>=20

--
Chuck Lever