From:   Chuck Lever III <chuck.lever@oracle.com>
To:     Jeff Layton <jlayton@kernel.org>
CC:     Linux NFS Mailing List <linux-nfs@vger.kernel.org>
Subject: Re: [PATCH] nfsd: allow reaping files that are still under writeback
Thread-Topic: [PATCH] nfsd: allow reaping files that are still under writeback
Thread-Index: AQHZP+kYZqo+lvM+xk+OVBsJow3AIK7OhuYA
Date:   Tue, 14 Feb 2023 14:48:13 +0000
Message-ID: <E1A055FD-45C3-47A0-A6CB-296C84985D43@oracle.com>
References: <20230213202346.291008-1-jlayton@kernel.org>
In-Reply-To: <20230213202346.291008-1-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?GFrZzx/KsIFYRp/cBl8gc0V/TeWc2V/TPNWNlel+B+7mz3ZOhgq2kVpFR8rN?=
 =?us-ascii?Q?Zo0JBvqjo8mtr45HOTz1A8tJP+WHN2aJJyJS+CQVOQa4AmpNHwAYMO4UHw2Y?=
 =?us-ascii?Q?+dUnDqrzNZcas5sruJBpC4vhtLVOZpwr9oSnM7gxjRw6ZM+e6zf8yNv7ZqTr?=
 =?us-ascii?Q?FnxQYiEMhIzO70jMB2LUHwlALNLtfjCRcsF24Rh+3dVRSIIY5sMCckZec8O0?=
 =?us-ascii?Q?mRfs83yAm3yHilOg7KpGpph/bGipnwbqZ0jYS23x4wJCmnkHhVN0AGl+EmAG?=
 =?us-ascii?Q?s+qWCxX1C2SRnNK/INe74vyvyXSdIoqvWaNT85/qu5o3AwlMCG3q+yhQ2pkL?=
 =?us-ascii?Q?1ZVjnvViG5PO4xfOpwezkCL6rAtnCTeQ/7gzSVtSmcKlT4XkGDmbYoCU0e0Y?=
 =?us-ascii?Q?UADMxm4QszRgKDB1H+91DFBevgNGVUCngiMQbLmg1BeCyOkMe+XfacL25mU7?=
 =?us-ascii?Q?3bd0i2eQSgxP4KIayAhmV4cbDOeYAU72hqvCvGbowQ5j8AZ5DdrJV0bZN04z?=
 =?us-ascii?Q?9+BpH1/BNrw2ZzvIH6pJFeHaWnkXHcm/6C5xTp5IWjBAa6QYqdjdebo6mvu4?=
 =?us-ascii?Q?0cY8uYKCPyQtpCdN9fH2uA054OyjY//MRthOweKjDI3QpJhghG1m0rfqKpvn?=
 =?us-ascii?Q?HYRcbsvopv34txWlsyz5EyXAR92Z9iXgzJVAUeVoMmxhz77+0+FVR3oKysC7?=
 =?us-ascii?Q?28A7pG//ORUm5Fq5FgYYi26uDHKWqdrnmApP0CKcnT6AoY9bjCmRSxwzKy2A?=
 =?us-ascii?Q?rt0G07qhjBANbr303wIRjCnfGjiXVG7BxU+NvRRyUelqRCV96o7xlAFcFO8v?=
 =?us-ascii?Q?XlBCikv++Xg9/d5a+MTN3WO4KQgbL/kK+xWjSC/cWP8Zhw4Ckq2ciqYt9AUW?=
 =?us-ascii?Q?oCaajA5gCyoZQd2VTjXHae5CrF7K/88qEOyH/9R2r2MAw0D6bvoCBh3BJQv8?=
 =?us-ascii?Q?nvoV+GFV8VsHQVAY/XzyREEtgWie6rO6Pmry7VC51MchoN1NTqR5Kl8UeTlQ?=
 =?us-ascii?Q?/r7+khP6sE/tBj8tzGPhdUFm7dbCOPJr5iEsYisJDu5wxj+lOwUsalD7vX8S?=
 =?us-ascii?Q?YoJUIOyeN14wawvM17HIouJeGeN67RMw6XCKZQUgsiF8PXhi5hPo7CYmomCU?=
 =?us-ascii?Q?S3OSV2IQJn6ZsG7zbNah8mniqLEhWRrwgPGXnxlioLdaXeNMgUFHeS6kmhmH?=
 =?us-ascii?Q?m5YozJd0JjZqNsT+wMyll5EJ/4b2PE5OtV3CNnqMOZNWRqn7XyQw8czJ6Drf?=
 =?us-ascii?Q?J05sAx38V54Ybny5Kw74FPth192NuNOFHDGtUNU4/wFZboXWFkFr1M8mamL0?=
 =?us-ascii?Q?MGR3zChtnBynSLGSViCBlvyGfAGEW5iyCSVDnprTyqALtM3WB2AIaPsJMr+G?=
 =?us-ascii?Q?yt0oVjMbDYizpmRycyKxP+11iWG3fk7I0aUFpsxWfV1dfcDQPOizYW8SftTO?=
 =?us-ascii?Q?/ROiswZTb8Z0x9XjseqG0p2ER+ewY65cCZvySJchm+yrTYJpKcxCZ+3rMlbW?=
 =?us-ascii?Q?ECIR6tiFEUDdWiazyRP3ybfhrf7Le+Y2K5AbvERbQPIrsAJa+cho7IULY3dg?=
 =?us-ascii?Q?0zsWtRH6kepFLTEv5mel6oDd/917yvDqqECas7mEEHq2G4xE3Nr4i+Rusfo3?=
 =?us-ascii?Q?DA=3D=3D?=
Content-Type: text/plain; charset="us-ascii"
Content-ID: <867625E909FA0A489682CA2B4490E523@namprd10.prod.outlook.com>
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-0: +Bd/y0/KndLJ9xsVAA8TpXw+H0XZPRJmmuGx8Wi3LRGcFmWKRm9LOdasIb4BAtzcMz5DqyJY4m7e+SRgDjTthMGWIovUQZYE7ncESQct7jLZNaWQ3p4vA1rgdUFp6OlB+8XqHJZkiyTCYxNUx+JSLCgA68SZ6UYM09CvDW1vwoifY6S98IrWSCSwHpk1VQGdKcCWdGsDX6wkEH/gPMNKgb80B9JP7i4YxPUB6MyxvZiZr1Ld40XHfkpPDjUIW3IFfFle/UI75ZEHcZ5uYa4Zr5Yhp0ZiEdfCG8oAVuXzIAd5FCVX39FJ1OgbqGDNefxtDNy5YVW65GQZHeXuc8bH2ApRexxWPqoQDhb5uKKlBQZTi4UG8KOFJDE0G67mSCH6xEsQCrAOf6qP8x56sChqghqDjpo1/LYTmkMTTyaK5HweNCJ+xXHVQsrN88ujln7ZIWpokF7tA0A8sqvFmwXMr8Y0Kn0222l8xVS01S5sIidhwGm/rSleK9QfVrndj7raaLJjhoETpEZfpb4FHUwqs9aycI3jeLdphwn9D1PTjVI8Ka1ZEhjkPGF31eQ+PlfddpxXPbnOgYgvF2UnljERjBAt1NVkkjDjAB3W2xzniNn1xJcimRcJ3R8j/gSskzEs1gXfA3Q7NAXwzOgrDivjhBpv4LeQGgLTPCygl13PZpUTxrh9SqG83tmlKy4UhBB+FtwtZQewEWxhW4Xx++9JaDCvb59tVg7cnblM2XD25ZkONKYbWiCzYTGowZykBL+ND7u15Z64MGK4e0Xvzn4JW+P15JdqHBQ90vkn3MXGlW8=
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: 856a2713-22ee-4d96-1fa8-08db0e9a7fa6
X-MS-Exchange-CrossTenant-originalarrivaltime: 14 Feb 2023 14:48:13.1921
 (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: nZ1PyiiXjwb8TLimuZRS0a3+lhgYwvKNcKB/pyvlOELd9w8+qgQP5RcXgQfp9qXSFwdcvY61rxc3K+AwJgo2Hg==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: MN2PR10MB4239
X-Proofpoint-Virus-Version: vendor=baseguard
 engine=ICAP:2.0.219,Aquarius:18.0.930,Hydra:6.0.562,FMLib:17.11.170.22
 definitions=2023-02-14_10,2023-02-14_01,2023-02-09_01
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 malwarescore=0 phishscore=0
 suspectscore=0 mlxscore=0 spamscore=0 mlxlogscore=999 adultscore=0
 bulkscore=0 classifier=spam adjust=0 reason=mlx scancount=1
 engine=8.12.0-2212070000 definitions=main-2302140127
X-Proofpoint-GUID: UTQK8nxN9dkR__2VW03BGOfV58QpZ7uZ
X-Proofpoint-ORIG-GUID: UTQK8nxN9dkR__2VW03BGOfV58QpZ7uZ
Precedence: bulk
List-ID: <linux-nfs.vger.kernel.org>
X-Mailing-List: linux-nfs@vger.kernel.org


> On Feb 13, 2023, at 3:23 PM, Jeff Layton <jlayton@kernel.org> wrote:
>=20
> There's no reason to delay reaping an nfsd_file just because its
> underlying inode is still under writeback. nfsd just relies on client
> activity or the local flusher threads to do writeback.
>=20
> Holding the file open does nothing to facilitate that, nor does it help
> with tracking errors. Just allow it to close and let the kernel do
> writeback as it normally would.
>=20
> Signed-off-by: Jeff Layton <jlayton@kernel.org>

Thanks! Applied to topic-filecache-cleanups.


> ---
> fs/nfsd/filecache.c | 22 ----------------------
> 1 file changed, 22 deletions(-)
>=20
> diff --git a/fs/nfsd/filecache.c b/fs/nfsd/filecache.c
> index e6617431df7c..3b9a10378c83 100644
> --- a/fs/nfsd/filecache.c
> +++ b/fs/nfsd/filecache.c
> @@ -296,19 +296,6 @@ nfsd_file_free(struct nfsd_file *nf)
> 	call_rcu(&nf->nf_rcu, nfsd_file_slab_free);
> }
>=20
> -static bool
> -nfsd_file_check_writeback(struct nfsd_file *nf)
> -{
> -	struct file *file =3D nf->nf_file;
> -	struct address_space *mapping;
> -
> -	if (!file || !(file->f_mode & FMODE_WRITE))
> -		return false;
> -	mapping =3D file->f_mapping;
> -	return mapping_tagged(mapping, PAGECACHE_TAG_DIRTY) ||
> -		mapping_tagged(mapping, PAGECACHE_TAG_WRITEBACK);
> -}
> -
> static bool nfsd_file_lru_add(struct nfsd_file *nf)
> {
> 	set_bit(NFSD_FILE_REFERENCED, &nf->nf_flags);
> @@ -438,15 +425,6 @@ nfsd_file_lru_cb(struct list_head *item, struct list=
_lru_one *lru,
> 	/* We should only be dealing with GC entries here */
> 	WARN_ON_ONCE(!test_bit(NFSD_FILE_GC, &nf->nf_flags));
>=20
> -	/*
> -	 * Don't throw out files that are still undergoing I/O or
> -	 * that have uncleared errors pending.
> -	 */
> -	if (nfsd_file_check_writeback(nf)) {
> -		trace_nfsd_file_gc_writeback(nf);
> -		return LRU_SKIP;
> -	}
> -
> 	/* If it was recently added to the list, skip it */
> 	if (test_and_clear_bit(NFSD_FILE_REFERENCED, &nf->nf_flags)) {
> 		trace_nfsd_file_gc_referenced(nf);
> --=20
> 2.39.1
>=20

--
Chuck Lever