RE: [PATCH rdma-next] Revert "IB/isert: Fix incorrect release of isert connection"

[Saravanan Vajravel <saravanan.vajravel@broadcom.com>]

[-- Attachment #1: Type: text/plain, Size: 1605 bytes --]

Agree with you Mike. I came to a wrong conclusion based on assumption that
kref_init() is initializing the refcount to zero.

Thanks & Regards,
Saravanan Vajravel
+91-80-46116256


-----Original Message-----
From: Mike Christie <michael.christie@oracle.com>
Sent: Monday, August 21, 2023 9:56 PM
To: Leon Romanovsky <leon@kernel.org>; Jason Gunthorpe <jgg@nvidia.com>
Cc: Leon Romanovsky <leonro@nvidia.com>; Dennis Dalessandro
<dennis.dalessandro@cornelisnetworks.com>; linux-rdma@vger.kernel.org; Sagi
Grimberg <sagi@grimberg.me>; Saravanan Vajravel
<saravanan.vajravel@broadcom.com>; Selvin Xavier
<selvin.xavier@broadcom.com>; target-devel@vger.kernel.org
Subject: Re: [PATCH rdma-next] Revert "IB/isert: Fix incorrect release of
isert connection"

On 8/21/23 2:57 AM, Leon Romanovsky wrote:
> From: Leon Romanovsky <leonro@nvidia.com> Commit: 699826f4e30a
> ("IB/isert: Fix incorrect release of isert connection") is

Was the issue described in that commit just not correct? Or was it supposed
to be for some sort of race'y error path?

It looks like that analysis was wrong for the normal login/logout path where
we call:

1. isert_init_conn sets kref to 1.
2. If we are connected we set kref to 2 via isert_connected_handler ->
kref_get 3. When we logout from there then isert_wait_conn -> queue_work
release_work and release_work does isert_put_conn so kref = 1.
4. Then we do isert_free_conn which does isert_put_conn to set the kref to 0
and then free the conn.

So the patch in this mail looks ok.

I checked most of the error paths, but I might have missed some. It looks ok
for them as well.

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4227 bytes --]