From:   Chuck Lever III <chuck.lever@oracle.com>
To:     Trond Myklebust <trond.myklebust@hammerspace.com>,
        Anna Schumaker <anna@kernel.org>
CC:     Dai Ngo <dai.ngo@oracle.com>, Helen Chao <helen.chao@oracle.com>,
        Linux NFS Mailing List <linux-nfs@vger.kernel.org>
Subject: Fwd: [PATCH] SUNRPC: remove the maximum number of retries in
 call_bind_status
Thread-Topic: [PATCH] SUNRPC: remove the maximum number of retries in
 call_bind_status
Thread-Index: AQHZUe5IYt0u1Oe6Pkap8e148U2puQ==
Date:   Thu, 16 Mar 2023 13:38:55 +0000
Message-ID: <ACE2DBA2-0F3D-47EF-9167-9D45F5BE57E5@oracle.com>
References: <ed870a33-0809-3cfd-2d5a-b97409568b97@oracle.com>
Accept-Language: en-US
Content-Language: en-US
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?ONwLLf7jc5X5f3RThaVA1I3vbtfG42aiM8Fiq+Zhv4zn2W4nzp9XlWR6ZgcI?=
 =?us-ascii?Q?ut3WN0r+4SIr18yM7VfWIrggWeL9MET3cxLoO/epc+AQFZXOcT/Dw44ZYRAo?=
 =?us-ascii?Q?5smuwPina7WmiLjocHl4m9eUC2MKZBsnTTuD9eI6K55ux+ESKo95CAe0zMCF?=
 =?us-ascii?Q?jnmemD3rGMMORKVEGWjPQLnhpAwyCir/Mn7+srCeOuuGt6w4SIwERJoCmvFb?=
 =?us-ascii?Q?90up9FCuCQAPXgxpz4Tw/0csGfjOGmZkFtjswh32T3xyWjaHEJkNltnVsBnM?=
 =?us-ascii?Q?ir+fQmcNutp4d4kRTWwG2nUUKX9VLxAR9qhoRAZT8Wd8HATDufc/tsdzkOlC?=
 =?us-ascii?Q?k2A0klBZZIousj1gIdEcpvxxZpihTlZhv/I5atsHWEpNreocV8+RFj+vRkA8?=
 =?us-ascii?Q?UlA0B6CtpNqZDVakXGryU+iN7Rk9vRFEmTWovbe99KUzt0QKuLU7UCOxOFSJ?=
 =?us-ascii?Q?KT4woPQyURG/CuktcQxiaiDWC+3NIvArb3J+9rA1c67CAQj7JTtM0/gJ2oSo?=
 =?us-ascii?Q?1/kU895aAcFQGvhOLRVQoDQaVdQWXzdksEYqU7BvRrNteyAKmsT6AZ2G2+aj?=
 =?us-ascii?Q?99yWcmqpgWLDGYyCfFhv4T5Abp0Fcj2WoO76prySYDetUZMarLTsiQtZB72+?=
 =?us-ascii?Q?yO5iGFf7pMfWywQc/6UgdmNn4zmc7zvQtkZMfUxk5nbk58UlN7AiqcIMiQBU?=
 =?us-ascii?Q?KSPdsYxwPCxWdvOYqspFB7gh6RTiVnLsoYoW1G7A7+9Av7Hgb/muV+XhW1iI?=
 =?us-ascii?Q?jQki8gZ6edYdcA68cvGlZL/ljL7Kb8felo/vsOh5Gt5wCSYAQxtG72gHw6tJ?=
 =?us-ascii?Q?o7iUCkgDwEgwuucewymVaQb8947eUQAQMvViPElzAtfDYsMun4SXCMz8OscW?=
 =?us-ascii?Q?bK4kGK950y2yYEw9tD1SA/1OAbBpPmpRgvMSgOGSGZAYTb/x3XPIGm/QJ76C?=
 =?us-ascii?Q?JncKl9JMoVWl4yizUDCxb05ok8LOfbTOIoOoBS1sUYSFKcpA6axlHkZq3e/Y?=
 =?us-ascii?Q?nUJP1+3/EsH4V2Sa+Aa6tsYRsum8Kx5GQMtyTFLPJMf3pTTgDGETHIxAwqqT?=
 =?us-ascii?Q?YpNvpD2Y3YhAs0xDT0pbxjp3ZIRdFRseBX2hexkmUmYJUzcaW0ikNWDjtZ6J?=
 =?us-ascii?Q?vDe8GI6v2sABhIzYtuEATIokTzwpIvWw0z/A4cAdK109KBt2e1V/MnKaCbjg?=
 =?us-ascii?Q?dcVjlZ0P4YakchpAvV/hTdmrpKBp9o80yvUltAx/ppXzesauFOxlbQLa1gxX?=
 =?us-ascii?Q?EHm83gVfCht1dV/xio3i2YXgWSsDs0sOhZ4AqrYcu1umSaH99oi4ujS1F0TS?=
 =?us-ascii?Q?nEBhRAJUfKPnkbbNagX6cgQaWg0ZilzZRVyCvrOZl0AIgh/XmX60VF6nqoMH?=
 =?us-ascii?Q?zHO0Ahao6fDqwd+d5rx07uqF067gUVJRvAnZMLaQCi/L9fDtiB3SnIo7VCCS?=
 =?us-ascii?Q?TLsROwWdf+VHi+uvCAsGO0p5VMd1BRRlvi6D2OUN7uUQLY7MEi+hgDAR2ods?=
 =?us-ascii?Q?mxc+ZqFHnYo6vGuEHbeYGfaXbDojpE8Z/lZwdPnP35YjVBYImSmBfe8uQLjz?=
 =?us-ascii?Q?YU1AEwpBlFt27eKBHUGyHQxW8eQLlFJbIJzvH/3Oi2EKiRzaoA9dYdoGl3H7?=
 =?us-ascii?Q?6w=3D=3D?=
Content-Type: text/plain; charset="us-ascii"
Content-ID: <0088E1329710674CA71D507C3BCD6CFB@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: adakSal85xsQftxomVd060TNIFqGHn26ZjXS4y49gOFvX/YAax/IFBT1aqn/YbB7Ed6lkqM9AQkWgTZzuKsRXWHiOjjLhOwDownM/K7byyarTZCugvOBTE+tlFlblc5UnHiOrGFjuNqWhH6yVf0viOgDyqZ0J4VraIMTFr5AYcXPeC1Y+PkkNsxGDz8hmV7f2wUXdNf4WgtII3TW8dC/CnhWZ/shy1D/c3jg43Zj7fJTqrSRwZ3xVdCwDXaFOssnqKcqL6qpAoRz3zLLgZI4JqRGu3+q2ot+/rdC3N00FNNxAmhzkf3xiLfrbWQeRzPvh3MxFn+v8T9SHLrP/EuOu+GsApbFzKw2OCiorh/KfC6yl+dR0VRBSZWCMvfcqtl2CPscnRZ20kx+IGFptfBlkhLIFMNBiJiCdMPxCYFcQqYMxQS83NkPZsPdff7KQw0Xte76NxPhtiBuA7E1wm7HTDosmhplri2EOCri0X6IR+MTe0NcGe7OQ9hEuSou5PUCXE6wz9cboJJ+EeNZHAAQ7ye3/vxP+515OJKA8UGJgI47mqXBr5F/MA/0+O842y2zVR2Tocv+L+TC7VPwQPyjVII6ZbsK4mRB5S9+mOGRkUM5J1jvc15iXGZCxsvcE/Qd6AaPS+/q7kXsif7duQmfyYL/jWcIFvE7mYf31+9M4zHDuHcz0orZuyD1AXs4KScN99i8z7HJhRQgbwym9gcoaykdZcKG0ogAjDi4WUpsT3kEDduGMU6JPSUmdd+hWjAdwGJUTU/YLwKrpXXbDgF88fjt/RBdJnUpwMBXP22ctI4qenlOq5zGLuKmJ+ftgeTVs4P6fuAXzw0/3DvbBb4liA==
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: b28a5162-f140-4c84-eb45-08db2623ca23
X-MS-Exchange-CrossTenant-originalarrivaltime: 16 Mar 2023 13:38:55.9966
 (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: Jjd15nNyrN8ZCqpgKcJTGf4j0nZbYDKqkZ244waN01B9ulEaNNRxiObI0iQfRZGNliMXRLxepZwzH5KutjLVZw==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: IA1PR10MB6100
X-Proofpoint-Virus-Version: vendor=baseguard
 engine=ICAP:2.0.254,Aquarius:18.0.942,Hydra:6.0.573,FMLib:17.11.170.22
 definitions=2023-03-16_09,2023-03-16_01,2023-02-09_01
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 malwarescore=0 mlxlogscore=999
 bulkscore=0 mlxscore=0 adultscore=0 suspectscore=0 phishscore=0
 spamscore=0 classifier=spam adjust=0 reason=mlx scancount=1
 engine=8.12.0-2303150002 definitions=main-2303160111
X-Proofpoint-ORIG-GUID: 8DMweeuNP0AxlINVrvZlvbUWHJXkdSAx
X-Proofpoint-GUID: 8DMweeuNP0AxlINVrvZlvbUWHJXkdSAx
Precedence: bulk
List-ID: <linux-nfs.vger.kernel.org>
X-Mailing-List: linux-nfs@vger.kernel.org

We need to hear from the NFS client maintainers about
the below question and the patch.


> Begin forwarded message:
>=20
> From: dai.ngo@oracle.com
> Subject: Re: [PATCH] SUNRPC: remove the maximum number of retries in call=
_bind_status
> Date: March 14, 2023 at 12:19:30 PM EDT
> To: Chuck Lever III <chuck.lever@oracle.com>
> Cc: Linux NFS Mailing List <linux-nfs@vger.kernel.org>, Helen Chao <helen=
.chao@oracle.com>
>=20
>=20
> On 3/8/23 11:03 AM, dai.ngo@oracle.com wrote:
>> On 3/8/23 10:50 AM, Chuck Lever III wrote:
>>>=20
>>>> On Mar 8, 2023, at 1:45 PM, Dai Ngo <dai.ngo@oracle.com> wrote:
>>>>=20
>>>> Currently call_bind_status places a hard limit of 3 to the number of
>>>> retries on EACCES error. This limit was done to accommodate the behavi=
or
>>>> of a buggy server that keeps returning garbage when the NLM daemon is
>>>> killed on the NFS server. However this change causes problem for other
>>>> servers that take a little longer than 9 seconds for the port mapper t=
o
>>>> become ready when the NFS server is restarted.
>>>>=20
>>>> This patch removes this hard coded limit and let the RPC handles
>>>> the retry according to whether the export is soft or hard mounted.
>>>>=20
>>>> To avoid the hang with buggy server, the client can use soft mount for
>>>> the export.
>>>>=20
>>>> Fixes: 0b760113a3a1 ("NLM: Don't hang forever on NLM unlock requests")
>>>> Reported-by: Helen Chao <helen.chao@oracle.com>
>>>> Tested-by: Helen Chao <helen.chao@oracle.com>
>>>> Signed-off-by: Dai Ngo <dai.ngo@oracle.com>
>>> Helen is the royal queen of ^C  ;-)
>>>=20
>>> Did you try ^C on a mount while it waits for a rebind?
>>=20
>> She uses a test script that restarts the NFS server while NLM lock test
>> is running. The failure is random, sometimes it fails and sometimes it
>> passes depending on when the LOCK/UNLOCK requests come in so I think
>> it's hard to time it to do the ^C, but I will ask.
>=20
> We did the test with ^C and here is what we found.
>=20
> For synchronous RPC task the signal was delivered to the RPC task and
> the task exit with -ERESTARTSYS from __rpc_execute as expected.
>=20
> For asynchronous RPC task the process that invokes the RPC task to send
> the request detected the signal in rpc_wait_for_completion_task and exits
> with -ERESTARTSYS. However the async RPC was allowed to continue to run
> to completion. So if the async RPC task was retrying an operation and
> the NFS server was down, it will retry forever if this is a hard mount
> or until the NFS server comes back up.
>=20
> The question for the list is should we propagate the signal to the async
> task via rpc_signal_task to stop its execution or just leave it alone as =
is.
>=20
> -Dai
>=20
>>=20
>> Thanks,
>> -Dai
>>=20
>>>=20
>>>=20
>>>> ---
>>>> include/linux/sunrpc/sched.h | 3 +--
>>>> net/sunrpc/clnt.c            | 3 ---
>>>> net/sunrpc/sched.c           | 1 -
>>>> 3 files changed, 1 insertion(+), 6 deletions(-)
>>>>=20
>>>> diff --git a/include/linux/sunrpc/sched.h b/include/linux/sunrpc/sched=
.h
>>>> index b8ca3ecaf8d7..8ada7dc802d3 100644
>>>> --- a/include/linux/sunrpc/sched.h
>>>> +++ b/include/linux/sunrpc/sched.h
>>>> @@ -90,8 +90,7 @@ struct rpc_task {
>>>> #endif
>>>>     unsigned char        tk_priority : 2,/* Task priority */
>>>>                 tk_garb_retry : 2,
>>>> -                tk_cred_retry : 2,
>>>> -                tk_rebind_retry : 2;
>>>> +                tk_cred_retry : 2;
>>>> };
>>>>=20
>>>> typedef void            (*rpc_action)(struct rpc_task *);
>>>> diff --git a/net/sunrpc/clnt.c b/net/sunrpc/clnt.c
>>>> index 0b0b9f1eed46..63b438d8564b 100644
>>>> --- a/net/sunrpc/clnt.c
>>>> +++ b/net/sunrpc/clnt.c
>>>> @@ -2050,9 +2050,6 @@ call_bind_status(struct rpc_task *task)
>>>>             status =3D -EOPNOTSUPP;
>>>>             break;
>>>>         }
>>>> -        if (task->tk_rebind_retry =3D=3D 0)
>>>> -            break;
>>>> -        task->tk_rebind_retry--;
>>>>         rpc_delay(task, 3*HZ);
>>>>         goto retry_timeout;
>>>>     case -ENOBUFS:
>>>> diff --git a/net/sunrpc/sched.c b/net/sunrpc/sched.c
>>>> index be587a308e05..c8321de341ee 100644
>>>> --- a/net/sunrpc/sched.c
>>>> +++ b/net/sunrpc/sched.c
>>>> @@ -817,7 +817,6 @@ rpc_init_task_statistics(struct rpc_task *task)
>>>>     /* Initialize retry counters */
>>>>     task->tk_garb_retry =3D 2;
>>>>     task->tk_cred_retry =3D 2;
>>>> -    task->tk_rebind_retry =3D 2;
>>>>=20
>>>>     /* starting timestamp */
>>>>     task->tk_start =3D ktime_get();
>>>> --=20
>>>> 2.9.5
>>>>=20
>>> --=20
>>> Chuck Lever

--
Chuck Lever