Received-SPF: pass (google.com: domain of linux-nfs+bounces-3188-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249;
Date: Mon, 6 May 2024 20:14:07 -0400
From: Chuck Lever <chuck.lever@oracle.com>
To: Rick Macklem <rick.macklem@gmail.com>
Cc: cel@kernel.org, linux-nfs@vger.kernel.org
Subject: Re: [RFC PATCH] NFSD: Force all NFSv4.2 COPY requests to be
 synchronous
Message-ID: <ZjlyT95XGccXXRzL@tissot.1015granger.net>
References: <20240506210408.4760-1-cel@kernel.org>
 <CAM5tNy4nceCa7k+E1sKEAi5GYJvMwYDuApspFJvSdVdXBuHjmw@mail.gmail.com>
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <CAM5tNy4nceCa7k+E1sKEAi5GYJvMwYDuApspFJvSdVdXBuHjmw@mail.gmail.com>
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: 
	=?utf-8?B?bVNYZ0tkVDNMWTNncFhGL2pXNGFxZjBaajVBS2thZ0pIcEw0RnBtQTBmRE1q?=
 =?utf-8?B?ckZWcTVqR0h4Ukhsai8zVHlvUWYwSG1lYm5ra1RSRkRjWnJiNWFtWSsyTVdC?=
 =?utf-8?B?NVdpVEZUOGgwZkZXemg0M0hCNnB1OXBvbzFJREFLZi9GL2RmYjRWeXFUVExv?=
 =?utf-8?B?Nk1WQzFGT1dPMUlaMUlyTmY4TnFjR1I2R3Y0R0p0cXlsT25VeGhqajhLbTlR?=
 =?utf-8?B?M001Rjhmd1ZuMC9QRjdFRmlSY0JmNitVM3E5TnhUYitUSVFZcHRIcXZpd1hL?=
 =?utf-8?B?bnE5aE9yZVNjMms4bEtEYjBSYlBSRFdzZTVwWDd0L2VhcW1ZcVRSOVBlbndv?=
 =?utf-8?B?Nm4rYTRISXdQOVhYa3ExSXFGL2hLTlB0ZktSbHZuZE0rM1cvZUdULzJGbHND?=
 =?utf-8?B?MGpodkhkVllKK2FySFkwTXVlUm91Q3h5VS9qN2l2OWVhbVA5a2FtWkxVNERq?=
 =?utf-8?B?dWplTmIzNE03dVdLTFpGSHFCNE1iRWJrVFdPY05hd3dCNDlvdENJMW9pTDlu?=
 =?utf-8?B?dkk5U2pteDlhU2pJanNON2ptVXMxNWpXcDVQb3hNRER0aE42TlB5Umh5YTBL?=
 =?utf-8?B?RDN1MWMrM1plb21WMnBDWFczR2lCYmVqZGp4Tk9zNm5WSGNHTVVtT2lkSEhy?=
 =?utf-8?B?UGJIbGhlQ3IzTTV6d3d4a1RBaXlUakhFSDBDM2k0VktiZGptUVQwcXZBdGVw?=
 =?utf-8?B?R01zSXp0dDNId09FZDB3Wk5EdDBNU3o5WXpIUCszSms1QVQ2citJY2VTZHBO?=
 =?utf-8?B?Q092U1JyMCtqUUlMaGFCc2ZRSDR5OEwzcVJRSUFCMTdXc1lJYzVMMCtlRytu?=
 =?utf-8?B?U2VQRzhKbGpqTmVrdXpxYVk5Y0dyRktBNFlaTldOVDJaQ0lTZ0l5dG5sZ1Zw?=
 =?utf-8?B?TVN4SUllbHpjMUNUOTM1a25EdjB0RGxCVUZLWXJMeW1mUHFpZnNSaHVDUytq?=
 =?utf-8?B?eEVNWit5N2RkV0MxTjBGWE1sWnlpdCtrYlpjdzFoYjBoRDJVU29uOG9rY29k?=
 =?utf-8?B?a3djL2pYS0xXZ1oxbGxqN0NDTGlLOTdJRmpPU042cTN0RzJzSTVRS1U5Smht?=
 =?utf-8?B?Mk9mUHlNMXdNczg3QkdiT3lyUVVPN1VuZkRKQi8xczR1TzVaeUJaS0FtT0Rp?=
 =?utf-8?B?ZU02MXhFWE04bTJldnM0dTdZNFhjMnZ6SUEzY2c1TEZPKzc3anZwTkNybEVr?=
 =?utf-8?B?SnJQVGxreG1sZW83czNuRDhZSFptM3MycDBkd1B3OTlLTGwzV2J4dE5jNkdj?=
 =?utf-8?B?TWUzaXkzbjFoRzNNY1c1aVVmMWl3SGFwTXFuRU5aU3JrWnJrUHJENkIzS3JH?=
 =?utf-8?B?aEROZmJXM3J3T1YzcUNrWXh1Y0VSR284VHlQbDhSTVF6bHVseVpsTko4Qy9x?=
 =?utf-8?B?ZlV2amJleGg5U1U4anpLSmNNUlVpSDJvVXB2c2RwSzFyU3p0RTVPbWxBZHJO?=
 =?utf-8?B?bUJhYkdkdEJkcGNETndXeFgxZ2xDSDA4T0w1aThQR0tFTGsyUTFoU3F4VHp5?=
 =?utf-8?B?ekpLR1BVUHdXUkFSQm5RR3FhNkpHNlR0cTNabDJtT0RXbjlnNWRiZENIZHJX?=
 =?utf-8?B?UUM4TTNqMGFvQlp5MFVUWVRSb3JwTHhLSUxQRGY2S0RiTzN4TnJBMVp5STNz?=
 =?utf-8?B?dU1tVTM4K2JMNEt0Z1JGTkpvcDlLNDB1allBcXpkdWRBQVIydVM1MWFKbThv?=
 =?utf-8?B?MkoxSkxpaWxPK2QxTlorVjRsTENNbjVoVmpsUFZ4clE4aWs1RHR4SXBPQnh4?=
 =?utf-8?B?NG1oSlJwbE1rTUxJVnlmMm92K3dqLzcxYmRkNHVyV3IxVkREdVExc3NydEta?=
 =?utf-8?B?Z21jeTA3bVZxanV2cnZpaTV0M2tudGVzTS9SbFlsWFg3c3V2YWhiVGdra3Rn?=
 =?utf-8?B?YVd2N1JHa3RPbDAwb2NnZVJpUnpLSjE5bXMzUGM0R25EZ09Yd0dMRnVnSmRK?=
 =?utf-8?B?WWE2N3BPTUFsZG1wUW1Ka0FTWnRWRHc3bGhuaXRUcEdSd2lXcjRxeURXUFlE?=
 =?utf-8?B?YURSdHRlOHd3WGdIdVBiSVJZNUpucDZNUU1wampYRnkzNFByRTRuZURGaUg0?=
 =?utf-8?B?dk1EeGxrRGRSZkxBRXZXek8xTEVUcS9VdXdxRHZPUG1JU0tFOFY1Vk44eHVF?=
 =?utf-8?B?dGNpaTR6akZmSHpKNjRlVGVBWk52SDlUeE5Bak5BckN1SjJRYkEwMzJXM1RJ?=
 =?utf-8?B?S1E9PQ==?=
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-0: 
	Ne0mSVlmdCnhtZt4/xOMKgDwBSj2+e307hDpQ+TqzatRNYtjIMSm5Tt7gR+7h+tzky4cl7N5C+j0AzwnfupuRGi2hyb4HuPHXwVf0h40S41zjeZ2/oWdH7LSF7oXDoN7yj4n9HaNHC5vxFmV4477Y4ymo24m9JNB9d2fH4IOj8MFy3DJNGyLCFC8eUGOjtBYrDePCtZndiY07hcx6csfeVqZzifengm7yeYnc6IAr+MUIPkpduTVL316hOGKNtgwPBEFR8o++xKTeSi1gt0E3P/sISJEa9RmVz7s/UuVGaET6C7OKGDYLmhtn7ihXuzxVtkz0QC2mjL75U8D5MSXMJK2sF8a5F9QF+rLHK1/9s8pU/fCOxCb2Sy9Recjm5lDSPj3M9N+9/jtS9nWHbPthaq3WY9g9fjD+vGJOA3knuOyPkJHc7BlQ4T+1t5aTHenwfuqSdmumAVvqdmIUmIYtIFQbjvMbeB0hLkM/FiJTbUUyg/mTffAa6u4ugVaP5A067RZPQLf/HC/KUGlOtHJEAn8Qfwul7oqjtV6HfOXW6axWWrZWE356cT6/Fgr2tnTsGv5T4tECMaRs37Lbq1ugu7XZGJPPXCoz33xk3WaOd4=
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 435ff802-f6cf-48da-209c-08dc6e2a9e15
X-MS-Exchange-CrossTenant-AuthSource: BN0PR10MB5128.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 07 May 2024 00:14:10.2084
 (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: wwFYT1fIGkJPdoyd/tAtEEz0RX5CNmevAhZL+CIpl4mYZwtMiSrDTOZywMY5DcwtVKcrBN2tYhC9o01l1H03JQ==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: PH8PR10MB6574
X-Proofpoint-Virus-Version: vendor=baseguard
 engine=ICAP:2.0.293,Aquarius:18.0.1039,Hydra:6.0.650,FMLib:17.11.176.26
 definitions=2024-05-06_17,2024-05-06_02,2023-05-22_02
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 phishscore=0 mlxlogscore=999
 bulkscore=0 suspectscore=0 spamscore=0 malwarescore=0 mlxscore=0
 adultscore=0 classifier=spam adjust=0 reason=mlx scancount=1
 engine=8.12.0-2404010000 definitions=main-2405070000
X-Proofpoint-ORIG-GUID: kgsEOwdVWDcKydVbpJBJYyE-CMVhn2Xc
X-Proofpoint-GUID: kgsEOwdVWDcKydVbpJBJYyE-CMVhn2Xc

On Mon, May 06, 2024 at 03:30:18PM -0700, Rick Macklem wrote:
> On Mon, May 6, 2024 at 2:04 PM <cel@kernel.org> wrote:
> >
> > From: Chuck Lever <chuck.lever@oracle.com>
> >
> > We've discovered that delivering a CB_OFFLOAD operation can be
> > unreliable in some pretty unremarkable situations, and the Linux
> > NFS client does not yet support sending an OFFLOAD_STATUS
> > operation to probe whether an asynchronous COPY operation has
> > finished. On Linux NFS clients, COPY can hang until manually
> > interrupted.
> >
> > I've tried a couple of remedies, but so far the side-effects are
> > worse than the disease. For now, force COPY operations to be
> > synchronous so that the use of CB_OFFLOAD is avoided entirely.
> Just a yellow warning light from my experience with the FreeBSD server
> (which always does synchronous Copy's).
> A large synchronous Copy can take a long time, resulting in a slow RPC
> response time. A user reported 25sec.
> The 25sec case turned out to be a ZFS specific issue that could be avoided
> via a ZFS tunable.
> 
> I do not have a good generic solution, but I do have a tunable that can
> be used to clip the Copy len, which is a rather blunt and ugly workaround.
> (The generic copy routine internal to FreeBSD can accept a flag that indicates
> "return after 1sec with a partial copy done", but that is not implemented for
> file systems like ZFS.)
> 
> Although there is no hard limit in the RFCs for RPC response times,
> I've typically
> assumed a max of 1-2sec is desirable.

This is not meant to be a permanent change, but rather one that can
be backported to LTS kernels if we see the need. A long-term fix
will re-enable async COPY but deal properly with the loss of a
CB_OFFLOAD.

The server should return NFS4ERR_DELAY if it expects to take a long
time, no?

> rick
> 
> >
> > I have some patches that add an OFFLOAD_STATUS implementation to the
> > Linux NFS client, but that is not likely to fix older clients.
> >
> > Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
> > ---
> >  fs/nfsd/nfs4proc.c | 7 +++++++
> >  1 file changed, 7 insertions(+)
> >
> > diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
> > index ea3cc3e870a7..12722c709cc6 100644
> > --- a/fs/nfsd/nfs4proc.c
> > +++ b/fs/nfsd/nfs4proc.c
> > @@ -1807,6 +1807,13 @@ nfsd4_copy(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
> >         __be32 status;
> >         struct nfsd4_copy *async_copy = NULL;
> >
> > +       /*
> > +        * Currently, async COPY is not reliable. Force all COPY
> > +        * requests to be synchronous to avoid client application
> > +        * hangs waiting for completion.
> > +        */
> > +       nfsd4_copy_set_sync(copy, true);
> > +
> >         copy->cp_clp = cstate->clp;
> >         if (nfsd4_ssc_is_inter(copy)) {
> >                 trace_nfsd_copy_inter(copy);
> >
> > base-commit: 939cb14d51a150e3c12ef7a8ce0ba04ce6131bd2
> > --
> > 2.44.0
> >
> >

-- 
Chuck Lever