diff mbox

IB/iser: fix connection teardown race condition

Message ID 1495113381-15564-2-git-send-email-vladimirn@mellanox.com (mailing list archive)
State Superseded
Headers show

Commit Message

Vladimir Neyelov May 18, 2017, 1:16 p.m. UTC 
From: root <vladimirn@mellanox.com>

Under heavy iser target(scst) start/stop stress on iser intitiator
side found race condition. When called function iscsi_iser_slave_alloc
iser_conn pointer could be NULL. Because function iscsi_iser_conn_stop
can be called before and free iser connection. Addition global mutex
help to avoid this race.

Signed-off-by: Vladimir Neyelov <vladimirn@mellanox.com>
---
 drivers/infiniband/ulp/iser/iscsi_iser.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

Comments

Dennis Dalessandro May 18, 2017, 8:12 p.m. UTC | #1 
On 5/18/2017 9:16 AM, Vladimir Neyelov wrote:
> From: root <vladimirn@mellanox.com>

root? Might want to check your git config.

-Denny

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Leon Romanovsky May 19, 2017, 3:48 a.m. UTC | #2 
On Thu, May 18, 2017 at 04:12:33PM -0400, Dennis Dalessandro wrote:
> On 5/18/2017 9:16 AM, Vladimir Neyelov wrote:
> > From: root <vladimirn@mellanox.com>
>
> root? Might want to check your git config.

It is not the only one issue, I counted 6 violations of our internal
policy/guidelines.

Thanks

>
> -Denny
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
Doug Ledford June 1, 2017, 8:22 p.m. UTC | #3 
On Fri, 2017-05-19 at 06:48 +0300, Leon Romanovsky wrote:
> On Thu, May 18, 2017 at 04:12:33PM -0400, Dennis Dalessandro wrote:
> > 
> > On 5/18/2017 9:16 AM, Vladimir Neyelov wrote:
> > > 
> > > From: root <vladimirn@mellanox.com>
> > 
> > root? Might want to check your git config.
> 
> It is not the only one issue, I counted 6 violations of our internal
> policy/guidelines.

Is Mellanox planning to address the internal policy issues and
resubmit, or do you want me to munge out the root entry and take this
patch as it is?
Leon Romanovsky June 1, 2017, 8:57 p.m. UTC | #4 
On Thu, Jun 01, 2017 at 04:22:22PM -0400, Doug Ledford wrote:
> On Fri, 2017-05-19 at 06:48 +0300, Leon Romanovsky wrote:
> > On Thu, May 18, 2017 at 04:12:33PM -0400, Dennis Dalessandro wrote:
> > >
> > > On 5/18/2017 9:16 AM, Vladimir Neyelov wrote:
> > > >
> > > > From: root <vladimirn@mellanox.com>
> > >
> > > root? Might want to check your git config.
> >
> > It is not the only one issue, I counted 6 violations of our internal
> > policy/guidelines.
>
> Is Mellanox planning to address the internal policy issues and
> resubmit, or do you want me to munge out the root entry and take this
> patch as it is?

Doug,

I resubmitted it a long time ago. It was already reviewed by Sagi and Max.
https://patchwork.kernel.org/patch/9739179/

Thanks

>
> --
> Doug Ledford <dledford@redhat.com>
>     GPG KeyID: B826A3330E572FDD
>    
> Key fingerprint = AE6B 1BDA 122B 23B4 265B  1274 B826 A333 0E57 2FDD
>
Doug Ledford June 1, 2017, 11 p.m. UTC | #5 
On Thu, 2017-06-01 at 23:57 +0300, Leon Romanovsky wrote:
> On Thu, Jun 01, 2017 at 04:22:22PM -0400, Doug Ledford wrote:
> > 
> > On Fri, 2017-05-19 at 06:48 +0300, Leon Romanovsky wrote:
> > > 
> > > On Thu, May 18, 2017 at 04:12:33PM -0400, Dennis Dalessandro
> > > wrote:
> > > > 
> > > > 
> > > > On 5/18/2017 9:16 AM, Vladimir Neyelov wrote:
> > > > > 
> > > > > 
> > > > > From: root <vladimirn@mellanox.com>
> > > > 
> > > > root? Might want to check your git config.
> > > 
> > > It is not the only one issue, I counted 6 violations of our
> > > internal
> > > policy/guidelines.
> > 
> > Is Mellanox planning to address the internal policy issues and
> > resubmit, or do you want me to munge out the root entry and take
> > this
> > patch as it is?
> 
> Doug,
> 
> I resubmitted it a long time ago. It was already reviewed by Sagi and
> Max.
> https://patchwork.kernel.org/patch/9739179/

Ah, I see.  You submitted it as rdma-next while the original was being
reviewed under -rc fixes.  That kept me from matching up the resubmit
with the original.  I'll drop the original.  Thanks.
Leon Romanovsky June 2, 2017, 3:48 p.m. UTC | #6 
On Thu, Jun 01, 2017 at 07:00:38PM -0400, Doug Ledford wrote:
> On Thu, 2017-06-01 at 23:57 +0300, Leon Romanovsky wrote:
> > On Thu, Jun 01, 2017 at 04:22:22PM -0400, Doug Ledford wrote:
> > >
> > > On Fri, 2017-05-19 at 06:48 +0300, Leon Romanovsky wrote:
> > > >
> > > > On Thu, May 18, 2017 at 04:12:33PM -0400, Dennis Dalessandro
> > > > wrote:
> > > > >
> > > > >
> > > > > On 5/18/2017 9:16 AM, Vladimir Neyelov wrote:
> > > > > >
> > > > > >
> > > > > > From: root <vladimirn@mellanox.com>
> > > > >
> > > > > root? Might want to check your git config.
> > > >
> > > > It is not the only one issue, I counted 6 violations of our
> > > > internal
> > > > policy/guidelines.
> > >
> > > Is Mellanox planning to address the internal policy issues and
> > > resubmit, or do you want me to munge out the root entry and take
> > > this
> > > patch as it is?
> >
> > Doug,
> >
> > I resubmitted it a long time ago. It was already reviewed by Sagi and
> > Max.
> > https://patchwork.kernel.org/patch/9739179/
>
> Ah, I see.  You submitted it as rdma-next while the original was being
> reviewed under -rc fixes.  That kept me from matching up the resubmit
> with the original.  I'll drop the original.  Thanks.

I submitted it to -next, because it fixes the "old" issue and anyway it
will go to the stable.

Thanks

>
> --
> Doug Ledford <dledford@redhat.com>
>     GPG KeyID: B826A3330E572FDD
>    
> Key fingerprint = AE6B 1BDA 122B 23B4 265B  1274 B826 A333 0E57 2FDD
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/infiniband/ulp/iser/iscsi_iser.c b/drivers/infiniband/ulp/iser/iscsi_iser.c
index 5a887ef..62fff2a 100644
--- a/drivers/infiniband/ulp/iser/iscsi_iser.c
+++ b/drivers/infiniband/ulp/iser/iscsi_iser.c
@@ -83,6 +83,7 @@ 
 static struct iscsi_transport iscsi_iser_transport;
 static struct scsi_transport_template *iscsi_iser_scsi_transport;
 static struct workqueue_struct *release_wq;
+static DEFINE_MUTEX(unbind_iser_conn_mutex);
 struct iser_global ig;
 
 int iser_debug_level = 0;
@@ -550,12 +551,14 @@  static void iscsi_iser_cleanup_task(struct iscsi_task *task)
 	 */
 	if (iser_conn) {
 		mutex_lock(&iser_conn->state_mutex);
+		mutex_lock(&unbind_iser_conn_mutex);
 		iser_conn_terminate(iser_conn);
 		iscsi_conn_stop(cls_conn, flag);
 
 		/* unbind */
 		iser_conn->iscsi_conn = NULL;
 		conn->dd_data = NULL;
+		mutex_unlock(&unbind_iser_conn_mutex);
 
 		complete(&iser_conn->stop_completion);
 		mutex_unlock(&iser_conn->state_mutex);
@@ -977,13 +980,20 @@  static int iscsi_iser_slave_alloc(struct scsi_device *sdev)
 	struct iser_conn *iser_conn;
 	struct ib_device *ib_dev;
 
+	mutex_lock(&unbind_iser_conn_mutex);
+
 	session = starget_to_session(scsi_target(sdev))->dd_data;
 	iser_conn = session->leadconn->dd_data;
+	if(!iser_conn) {
+		mutex_unlock(&unbind_iser_conn_mutex);
+		return -EFAULT;
+	}
 	ib_dev = iser_conn->ib_conn.device->ib_device;
 
 	if (!(ib_dev->attrs.device_cap_flags & IB_DEVICE_SG_GAPS_REG))
 		blk_queue_virt_boundary(sdev->request_queue, ~MASK_4K);
 
+	mutex_unlock(&unbind_iser_conn_mutex);
 	return 0;
 }