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;
From:   Chuck Lever III <chuck.lever@oracle.com>
To:     Jeff Layton <jlayton@kernel.org>
CC:     Linux NFS Mailing List <linux-nfs@vger.kernel.org>,
        Neil Brown <neilb@suse.de>
Subject: Re: [PATCH v3 2/4] nfsd: rework refcounting in filecache
Thread-Topic: [PATCH v3 2/4] nfsd: rework refcounting in filecache
Thread-Index: AQHY6v8c+/AX0w4PQ0iPSe2aFwWbQK4kNtsAgAAGgQCABeCZAA==
Date:   Tue, 1 Nov 2022 13:58:11 +0000
Message-ID: <494CEF8A-E9AD-400B-92E6-EEE3B2DB431D@oracle.com>
References: <20221028185712.79863-1-jlayton@kernel.org>
 <20221028185712.79863-3-jlayton@kernel.org>
 <96D34180-0E11-4582-8B45-B3FD9CD8F2DB@oracle.com>
 <432239da4d20337f6f14c91f40fb4432e637a662.camel@kernel.org>
In-Reply-To: <432239da4d20337f6f14c91f40fb4432e637a662.camel@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?Uc5qW64KA3YILAn5Oj3/EfeULi3X/oTSkYLgwDll81lOu98euE5W0mtIKJyF?=
 =?us-ascii?Q?0MTmceho+S+t2RNexMq/cbVMS2ujK+25/Hb/HSZ+covf0VUn1RJUDGL8Y8iN?=
 =?us-ascii?Q?DTxEZSuL+sq6PUHM/28BEwfgoherpL8kSPvcJB4xcnE5XbioszlCUtcqYOYR?=
 =?us-ascii?Q?Tjcjw5sFYsNxVPBuS2Iym5vlgU3c2iWlp5k7eoMaTYIEPKWLA0u2liAsZXKa?=
 =?us-ascii?Q?FxgkLdOAKMjIEVpfFK/9Xk6ob/gNSpiWUSu/RFbbJfPIuQtB+X+K3qZOMS7E?=
 =?us-ascii?Q?6hxhxmZrHRqvAane8IxGfsUahJ2x8Uo40wg9RHtUkw1Tfa0uIDLctxH3fbYa?=
 =?us-ascii?Q?b63gQ1i+BH9GiqtuMs27krzFehBqF0PrtRzRAQJh6u+sJ2vmy6Chf1cnkk7/?=
 =?us-ascii?Q?rMLD8Z3Ew35sKOz4A9PYljHz8BEdUlzmF3g00oXlm44AkQ8CNw3Yyi2tgX+q?=
 =?us-ascii?Q?am/HgPlkMRv/qaHDzPljEcSMv6zebvPrA14FEUVOXRrH9Q3zl/9QSrHtJzs3?=
 =?us-ascii?Q?V/PlWmWstBKpADFQXHZ9pj7KYjBApAx1igvV6jBglQW6F4l3d9vK2ZeyZHCV?=
 =?us-ascii?Q?tXB+At6xIJ0clemTrqH6jUETR8Gvvh0zBWeN/848s6Ee4iR26JNds8gonWiy?=
 =?us-ascii?Q?MGamfB6QvBNDWG97nBH+jSFYStXzAFMK0uCSf3SbYp32J3ytLEm+SllZpoO5?=
 =?us-ascii?Q?SKsEbfKnpxkkmL8IxTs1zGKic8jiEbwVuQzYujM0r6tWUN3WE/j8LuLQmNGd?=
 =?us-ascii?Q?0sMO+PsfUapLe49V4dBPLHzR6N/Ys5GIi1gXnpS9NaHMU3yWc2NQSgRXL92z?=
 =?us-ascii?Q?5ky6SEU0dWOrdndvLhyNuiehSdZWxdx5hO2MTzXjgUcWaXUpRKgIV/K9lV0b?=
 =?us-ascii?Q?vwiSGfg0Oh3iMITNgX5xSw4B/V2DQpbr+aFQSlI3+njFLPM4RM9C3OBk6jsq?=
 =?us-ascii?Q?I8zyV+iB7KeTtRfvvqNuU1w5iZGRTeF50PW25Fc44TOnR/YkAwFnx/FhNFZ+?=
 =?us-ascii?Q?TM/KMAr8/Gn+YSCzQtN86r16tkYB/jO/CDnKYBsdSxIekHjeLsUALaLvX9PZ?=
 =?us-ascii?Q?iLLdGy8PkdQTtzyRxFvnAg2RN+GbW0DEcFVkH0Q0M7JyuehrapsdJzjXv0ZF?=
 =?us-ascii?Q?RWQ86L8J55SZsZ6YNFLHxwUx+zR0zPukIuuEDcuXGIKZJf/Z/VVazLfWTR5v?=
 =?us-ascii?Q?bzmrRjsbPa7xkwWWLvKmwLdtVSsn05r4l+xuZ/Si1z7e/OU5gSoTLzFgodAw?=
 =?us-ascii?Q?wMUUlu/BBGBFefGvCV6sK/jOIKLvqFxCioPe1QEnHy9P7UsahrDEMz2o/+3I?=
 =?us-ascii?Q?jRWdos23IKFwFxgsMbIncjz+TCdd0MD9v2QX28vx9opD+NEU99fsB9C1HWXi?=
 =?us-ascii?Q?KCWZVTDQIJuMZH6vqrZlMm1RoMpqrnaLDFyRIm530PmRWrVpFAHxjp2Pkfz5?=
 =?us-ascii?Q?fKvAOxKbV1DbO0xImYwhO+na/iPWrgGdIGJwEF6C/oaCpHsFj1LmnaUum6t3?=
 =?us-ascii?Q?hh7l9Zlqob/3FMszqHWTZ1QkUoC+mTn2lyWTJOCMDA0fgTJUaI5yqY98PS6c?=
 =?us-ascii?Q?c5r3M+2M4DhfTLrA5e82dl7ap4i7VlvNsEBxcvdd9BktcGWqGufDTcg3MHHq?=
 =?us-ascii?Q?ng=3D=3D?=
Content-Type: text/plain; charset="us-ascii"
Content-ID: <CEB5A1C086CB0D4B889F029EB148BC16@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: dc60016a-f15e-4c20-af41-08dabc111cd9
X-MS-Exchange-CrossTenant-originalarrivaltime: 01 Nov 2022 13:58:11.0215
 (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: WPhcOl4iB8fdL8a6A0HYhp/0+a6ffJiYJYAkIm11TxuRq0B0q6gFWCcTeHoVn5puqxWSDmKEnPLVBDxbwaC2OQ==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BL3PR10MB6257
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-01_07,2022-11-01_02,2022-06-22_01
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 adultscore=0 bulkscore=0 suspectscore=0
 phishscore=0 malwarescore=0 spamscore=0 mlxscore=0 mlxlogscore=892
 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2210170000
 definitions=main-2211010106
X-Proofpoint-GUID: gKinAiQDlE3bkxIHB6sBaku52eTpA-fc
X-Proofpoint-ORIG-GUID: gKinAiQDlE3bkxIHB6sBaku52eTpA-fc
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-nfs.vger.kernel.org>
X-Mailing-List: linux-nfs@vger.kernel.org


> On Oct 28, 2022, at 4:13 PM, Jeff Layton <jlayton@kernel.org> wrote:
>=20
> On Fri, 2022-10-28 at 19:49 +0000, Chuck Lever III wrote:
>>=20
>>> On Oct 28, 2022, at 2:57 PM, Jeff Layton <jlayton@kernel.org> wrote:
>=20
>> I'm still not sold on the idea of a synchronous flush in nfsd_file_free(=
).
>=20
> I think that we need to call this there to ensure that writeback errors
> are handled. I worry that if try to do this piecemeal, we could end up
> missing errors when they fall off the LRU.
>=20
>> That feels like a deadlock waiting to happen and quite difficult to
>> reproduce because I/O there is rarely needed. It could help to put a
>> might_sleep() in nfsd_file_fsync(), at least, but I would prefer not to
>> drive I/O in that path at all.
>=20
> I don't quite grok the potential for a deadlock here. nfsd_file_free
> already has to deal with blocking activities due to it effective doing a
> close(). This is just another one. That's why nfsd_file_put has a
> might_sleep in it (to warn its callers).
>=20
> What's the deadlock scenario you envision?

I never answered this question.

I'll say up front that I believe this problem exists in the current code
base, so what follows is meant to document an existing issue rather than
a problem with this patch series.

The filecache sets up a shrinker callback. This callback uses the same
or similar code paths as the filecache garbage collector.

Dai has found that trying to fsync inside a shrinker callback will
lead to deadlocks on some filesystems (notably I believe he was testing
btrfs at the time).

To address this, the filecache shrinker callback could avoid evicting
nfsd_file items that are open for write.


--
Chuck Lever