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:     Anna Schumaker <anna.schumaker@netapp.com>
CC:     Chuck Lever <cel@kernel.org>, Tom Talpey <tom@talpey.com>,
        Linux NFS Mailing List <linux-nfs@vger.kernel.org>,
        linux-rdma <linux-rdma@vger.kernel.org>,
        Olga Kornievskaia <kolga@netapp.com>
Subject: Re: [PATCH] xprtrdma: Remap Receive buffers after a reconnect
Thread-Topic: [PATCH] xprtrdma: Remap Receive buffers after a reconnect
Thread-Index: AQHZtmXW1KhFMZKe0k63dgGlYrfiG6+7DzkAgAAgMYCAAXTdgA==
Date:   Sun, 16 Jul 2023 17:11:29 +0000
Message-ID: <1678F63C-DBBB-48FE-B2B9-0C2A5CD5217C@oracle.com>
References: <168934757954.2781502.4228890662497418497.stgit@morisot.1015granger.net>
 <a9f18064-45d4-4346-0156-dcd74e001b82@talpey.com>
 <FF1F6B85-A97D-41A0-B28F-AF7A568EEC68@oracle.com>
In-Reply-To: <FF1F6B85-A97D-41A0-B28F-AF7A568EEC68@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?qtKS1FV3OJkTk/tOmT+TzYsRTywEGrAlwpVW6DAqXgs6//B1RUB3BqpV/l5z?=
 =?us-ascii?Q?a4hFBXP55NS/Bcedz+DxPZAciJ6yowGdwW4zu/WnYPsnjelbUfXP5mFj8lf5?=
 =?us-ascii?Q?URb54Q1rKqBS9xeY8Fc/gPelYeZtyXn7SNYvTO7jdaZxf6VcmDw2Xxe6S/GI?=
 =?us-ascii?Q?vHgx3RaCq4iB82anNfI4yJXLCpd3W85NkHceFCuAMvm0qLSIq4+d7CGq1B56?=
 =?us-ascii?Q?qMLjsfeeRDkQqFVGY2BOLEvCulvcsjKmSQc/2hdG2Mv3+IjcWQtWEAF010rx?=
 =?us-ascii?Q?TdEkODnWNgrQwBZk2WUMjJ9kz+cBB9qVw6CrPwuHf6/6CBf3fSr4thMjGWSd?=
 =?us-ascii?Q?RRgLlP9lh8rfeWjCj+FuemJqHM6D2t1WKyF52Y8kcFXQlaYktNwoTurtTJqh?=
 =?us-ascii?Q?XDnc41w0mrEfLlxLdVDxMmlyH68e5rdHmKKczHhWUcWN1kLMklGq2e+Yq0Jw?=
 =?us-ascii?Q?kh29oIxXnIyyVQKmV21yl4bGTRStAv1P1rPeTnFcLDjfHw7gyPFm9CHWBMBO?=
 =?us-ascii?Q?kUNFdZ4V01b45DXWzATN/Yml3maK8iX5j2pO5Jejri+W51NqrtxTrK+tnZXN?=
 =?us-ascii?Q?sf625j5qhMMcxr9yehVfGCepqg5U6wG+FZxWHI7L1em+1xoW9aKdVZTfk9Ie?=
 =?us-ascii?Q?m6E8EyoKQLh2QsVJtq7VKXgq3tWdIjhjr1KmsZsyiZYoWCwcHMNyezDG1yTw?=
 =?us-ascii?Q?pYM0Rmgep7ixATyzLwGu8T7/YEvjLC06lDi1JuuCXOLWINhUzXvV2xVcQj85?=
 =?us-ascii?Q?61084W0V/sA4nr15Xst1N+VxV0LzyfmZRXQ478WnAVpgT41spOGn5L1QfX7v?=
 =?us-ascii?Q?+FqQGg2dPr6Itlz9qP9v+fX6VA++guahxLNm6IRoVlgefOZ/0qRJF13pezNV?=
 =?us-ascii?Q?5OGNkNqYdpwfHIOz7azroga24IJErmh/jSLDeM7z6SiQpeuDG94Kizof9fWr?=
 =?us-ascii?Q?lTBB72Zlh+UTbTs1Q71Q9il6z6XOeNASwEWQR/OBTlZ3GJ7uaXvdWug3QEov?=
 =?us-ascii?Q?yJPSsP5+h4Qt8Ze8UR2AXvz06nD5wkHnKKMXsC12YDYCh3oXyaxJqRDwy6K3?=
 =?us-ascii?Q?5RGfDEpEcCH/qlAkjajcU4EAeE9GcOABkktc2ES87zNzyz11wFtEgXP5EXSm?=
 =?us-ascii?Q?CCpNghPmVv2T6Unw7s0+lizkUa/8tRupvxm/D3UGlY0Y7WiO+Sy2UKIeLdVw?=
 =?us-ascii?Q?/rpezovzewnbLlB067amS9mivt1boIDbyzVJge8AfE18jPkQAGfXCZSg2lXE?=
 =?us-ascii?Q?Aopdcvt4FJh4d+nzGQRr6oNzZajmMgN1T7fqlHdbvhYUDdteF9vJHvdo28Tf?=
 =?us-ascii?Q?BtibUibY5DynIggvyGHHOcLiemBwqyNlBSs4enAuZLmSqMJZPNaDdRSCKeZf?=
 =?us-ascii?Q?r4UIMT81qv8BSdhfIVP+2Dqbuw5aRjj2d5UcFq76veEPVeEuyWgaLbZyrRw7?=
 =?us-ascii?Q?3J8b/NI/sWRaSQ1BmiZk9T2w+SZmzeUirpD3Ruq19khyHGbCPHUn2uKURQmG?=
 =?us-ascii?Q?wzjvhGfxUttC4gugxG4oLKUFEkUxnST7MUZgdlMy98XRucUh2KqeB8Ty+xW+?=
 =?us-ascii?Q?PEWEuIcQJy2+QHA2py48RZsyX93Hhr+Gh/nxq9rInqNlIKg429rb8f76k7v5?=
 =?us-ascii?Q?8Q=3D=3D?=
Content-Type: text/plain; charset="us-ascii"
Content-ID: <7EF2BFC827BB3B4CB295B152C4379700@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: WB7Ca4zuRSeSrcvhs/ZUUy5IJDaUZoSYjvlmxMvVrHbJ3QYbOzhP2YSaGNJRgT+OZ0goVA2Dm1svYZ7ZKOlBOtcg6pgwkHQsjJAYfyQj8x0q/la5aPWvbvlZqgv8oxAVmM2GR/KwkOUdlOX6UflDToxldByjTP0OwRx8lyB6rHQ08yIL/T3JvQVyxbv/hUdn42KdPYCcqkIFXMdBTajRw6L0GTbm4mlhYC5Xsv8WNQ4c0Os7PVnU/PrMWU7/u5blE/Jy5xrCPHryJfla1HdGAD73EHEnGUytFf2BuXMq4P04r5HT6K4VnwAH05EAfHzgGeeKFd5Z4jvHcHWRfmB+xIioHO/2QSvr+OV0R7AdL+vaLJ4L1eQ80vUu0BRFvPKvHSE80+x00MZMH4TAwydpffLYQObpn+cRkhvvIkI8xKSs5PAhpEQED3NAJtVb6/XZ9a3Qa0/k0SwtU6ylzC/RU4TEyF0C4/IxhkB2V3ztyCpkus+Oy98FJPlo3pNYQkDv2ONzgsgSTPgYl1itClWiP/ontjUuRhuQmNpRgGveQLfFbONzoHwMvbXawhTHIM60nXY/E5p0kCDCFndYAT9SOs4O9SsqniHcK5zRFklQm9EAqxcfZNys+JDAkViaI4YPcuFEivUru/2bkYVDYwMjZ0x6S9Ot+DvXTiAjXH3Op751Lx41oL4UgVdqMnmm181AAsvQA3IlOsavnry9+Volg9BAIJe5P1Z25ZbKiM3HvU32O8Fu9rH7Kok0GXukbjCUWz8Ek972wSgEGFO+BHnvR8JNJxgbwf6i4xsCMyMC9otg4FbiJrwnQZZpraoZExpi4U5qZOLG7nZWyjl8faiwNE3Vx8DVNfxbFFgqWv0iJrk=
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: e155690f-cbf1-44ee-ba9a-08db861fb281
X-MS-Exchange-CrossTenant-originalarrivaltime: 16 Jul 2023 17:11:29.9969
 (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: 8R0oqQzrjUntmZinqQVuMX2ErgLgR315/xTuFxIdN0BFAY/KPnddNiqrAF0k6uu1WofN+mkwFpX+3lisQmnRoA==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: PH0PR10MB4726
X-Proofpoint-Virus-Version: vendor=baseguard
 engine=ICAP:2.0.254,Aquarius:18.0.957,Hydra:6.0.591,FMLib:17.11.176.26
 definitions=2023-07-16_04,2023-07-13_01,2023-05-22_02
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 suspectscore=0 mlxscore=0 phishscore=0
 adultscore=0 spamscore=0 bulkscore=0 malwarescore=0 mlxlogscore=999
 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2306200000
 definitions=main-2307160163
X-Proofpoint-ORIG-GUID: ighcbo9fo8NYdfgrzAivIr-s7KqkFpuW
X-Proofpoint-GUID: ighcbo9fo8NYdfgrzAivIr-s7KqkFpuW
X-Spam-Status: No, score=-3.1 required=5.0 tests=BAYES_00,DKIM_SIGNED,
        DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_BLOCKED,
        RCVD_IN_MSPIKE_H5,RCVD_IN_MSPIKE_WL,SPF_HELO_NONE,SPF_NONE,
        T_SCC_BODY_TEXT_LINE 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 Jul 15, 2023, at 2:56 PM, Chuck Lever III <chuck.lever@oracle.com> wro=
te:
>=20
>=20
>=20
>> On Jul 15, 2023, at 1:01 PM, Tom Talpey <tom@talpey.com> wrote:
>>=20
>> On 7/14/2023 11:13 AM, Chuck Lever wrote:
>>> From: Chuck Lever <chuck.lever@oracle.com>
>>> On server-initiated disconnect, rpcrdma_xprt_disconnect() DMA-unmaps
>>> the transport's Receive buffers, but rpcrdma_post_recvs() neglected
>>> to remap them after a new connection had been established. The
>>> result is immediate failure of the new connection with the Receives
>>> flushing with LOCAL_PROT_ERR.
>>=20
>> The fix is correct, my only comment is that the failure occurs when the
>> first message is received, and only the first CQE is a LOCAL_PROT_ERR.
>> The remainder of the posted receives are simply flushed.
>>=20
>> Same result of course! The summary is ok as-is. Important fix.
>>=20
>> Reviewed-by: Tom Talpey <tom@talpey.com>
>=20
> Thanks for the look!
>=20
> Anna, when applying this patch, can you replace:
>=20
> "The result is immediate failure of the new connection with the Receives
> flushing with LOCAL_PROT_ERR."
>=20
> with
>=20
> "The result is immediate failure of the new connection with the first
> unmapped Receive completing with LOCAL_PROT_ERR."

Also, please add:

Reported-by: Olga Kornievskaia <kolga@netapp.com>


>>> Fixes: 671c450b6fe0 ("xprtrdma: Fix oops in Receive handler after devic=
e removal")
>>> Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
>>> ---
>>> net/sunrpc/xprtrdma/verbs.c |    9 ++++-----
>>> 1 file changed, 4 insertions(+), 5 deletions(-)
>>> Hi Anna-
>>> Can you apply this for 6.5-rc ?
>>> diff --git a/net/sunrpc/xprtrdma/verbs.c b/net/sunrpc/xprtrdma/verbs.c
>>> index b098fde373ab..28c0771c4e8c 100644
>>> --- a/net/sunrpc/xprtrdma/verbs.c
>>> +++ b/net/sunrpc/xprtrdma/verbs.c
>>> @@ -935,9 +935,6 @@ struct rpcrdma_rep *rpcrdma_rep_create(struct rpcrd=
ma_xprt *r_xprt,
>>>  if (!rep->rr_rdmabuf)
>>>  goto out_free;
>>> - if (!rpcrdma_regbuf_dma_map(r_xprt, rep->rr_rdmabuf))
>>> - goto out_free_regbuf;
>>> -
>>>  rep->rr_cid.ci_completion_id =3D
>>>  atomic_inc_return(&r_xprt->rx_ep->re_completion_ids);
>>> @@ -956,8 +953,6 @@ struct rpcrdma_rep *rpcrdma_rep_create(struct rpcrd=
ma_xprt *r_xprt,
>>>  spin_unlock(&buf->rb_lock);
>>>  return rep;
>>> -out_free_regbuf:
>>> - rpcrdma_regbuf_free(rep->rr_rdmabuf);
>>> out_free:
>>>  kfree(rep);
>>> out:
>>> @@ -1363,6 +1358,10 @@ void rpcrdma_post_recvs(struct rpcrdma_xprt *r_x=
prt, int needed, bool temp)
>>>  rep =3D rpcrdma_rep_create(r_xprt, temp);
>>>  if (!rep)
>>>  break;
>>> + if (!rpcrdma_regbuf_dma_map(r_xprt, rep->rr_rdmabuf)) {
>>> + rpcrdma_rep_put(buf, rep);
>>> + break;
>>> + }
>>>    rep->rr_cid.ci_queue_id =3D ep->re_attr.recv_cq->res.id;
>>>  trace_xprtrdma_post_recv(rep);
>=20
> --
> Chuck Lever


--
Chuck Lever