[PATCH 1/5] scsi_transport_fc: Fix deadlock during fc_remove_host

[PATCH 1/5] scsi_transport_fc: Fix deadlock during fc_remove_host

[Bhanu Prakash Gollapudi]

From: Nithin Nayak Sujir <nsujir@broadcom.com>

Creating and destroying fcoe interface in a tight loop leads to a system
deadlock with the following call traces:

Call Trace:
[<ffffffff814f4b3d>] schedule_timeout+0x1fd/0x2c0
[<ffffffff814f469f>] ? wait_for_common+0x4f/0x190
[<ffffffff814f469f>] ? wait_for_common+0x4f/0x190
[<ffffffff814f4737>] wait_for_common+0xe7/0x190
[<ffffffff81042fa0>] ? default_wake_function+0x0/0x20
[<ffffffff81082c2d>] ? trace_hardirqs_on+0xd/0x10
[<ffffffff814f48bd>] wait_for_completion+0x1d/0x20
[<ffffffff81066d90>] flush_workqueue+0x290/0x5f0
[<ffffffff81066b00>] ? flush_workqueue+0x0/0x5f0
[<ffffffff81067148>] destroy_workqueue+0x38/0x340
[<ffffffffa0260289>] fc_remove_host+0x1b9/0x1f0 [scsi_transport_fc]
[<ffffffffa02ed195>] bnx2fc_if_destroy+0xc5/0x1f0 [bnx2fc]
[<ffffffffa02ed33a>] bnx2fc_destroy+0x7a/0x100 [bnx2fc]
[<ffffffffa02c789b>] fcoe_transport_destroy+0x9b/0x1b0 [libfcoe]
[<ffffffff81069ec2>] param_attr_store+0x52/0x80
[<ffffffff81069976>] module_attr_store+0x26/0x30
[<ffffffff8119e726>] sysfs_write_file+0xe6/0x170
[<ffffffff81134710>] vfs_write+0xd0/0x1a0
[<ffffffff811348e4>] sys_write+0x54/0xa0
[<ffffffff81002e02>] system_call_fastpath+0x16/0x1b
Call Trace:
[<ffffffff81074865>] async_synchronize_cookie_domain+0x75/0x120
[<ffffffff8106caa0>] ? autoremove_wake_function+0x0/0x40
[<ffffffff81074925>] async_synchronize_cookie+0x15/0x20
[<ffffffff8107494c>] async_synchronize_full+0x1c/0x40
[<ffffffffa0057466>] sd_remove+0x36/0xc0 [sd_mod]
[<ffffffff81358a75>] __device_release_driver+0x75/0xe0
[<ffffffff81358bef>] device_release_driver+0x2f/0x50
[<ffffffff81357aee>] bus_remove_device+0xbe/0x120
[<ffffffff813553ef>] device_del+0x12f/0x1e0
[<ffffffff8137454d>] __scsi_remove_device+0xbd/0xc0
[<ffffffff81374585>] scsi_remove_device+0x35/0x50
[<ffffffff813746a7>] __scsi_remove_target+0xe7/0x110
[<ffffffff81374730>] ? __remove_child+0x0/0x30
[<ffffffff81374753>] __remove_child+0x23/0x30
[<ffffffff81354a2c>] device_for_each_child+0x4c/0x80
[<ffffffff81374703>] scsi_remove_target+0x33/0x60
[<ffffffffa02622c6>] fc_starget_delete+0x26/0x30 [scsi_transport_fc]
[<ffffffffa026271a>] fc_rport_final_delete+0xaa/0x200 [scsi_transport_fc]
[<ffffffff8106585a>] process_one_work+0x1aa/0x540
[<ffffffff810657eb>] ? process_one_work+0x13b/0x540
[<ffffffffa0262670>] ? fc_rport_final_delete+0x0/0x200 [scsi_transport_fc]
[<ffffffff81067ac9>] worker_thread+0x179/0x410
[<ffffffff81067950>] ? worker_thread+0x0/0x410
[<ffffffff8106c546>] kthread+0xb6/0xc0
[<ffffffff8103879b>] ? finish_task_switch+0x4b/0xe0
[<ffffffff81003ca4>] kernel_thread_helper+0x4/0x10
[<ffffffff814f7994>] ? restore_args+0x0/0x30
[<ffffffff8106c490>] ? kthread+0x0/0xc0
[<ffffffff81003ca0>] ? kernel_thread_helper+0x0/0x10

fc_remove_host() waits for flushing the workqueue, but it is stuck at flushing
the first work. The first work doesnt complete, because it is waiting for async
layer to complete the IOs. The async layer cannot complete the IO as the
terminate_rport_io for the second work was not called, which will be called
only when the first work completes. Hence the deadlock.  To resolve this
deadlock, the workqueue allocation has been modified from
create_singlethread_workqueue() to alloc_workqueue().

In addition, fc_terminate_rport_io() should be called before the
scsi_flush_work() to avoid the similar deadlock as above.

scsi fc alloc queue. move terminate rport io before flush

Signed-off-by: Nithin Nayak Sujir <nsujir@broadcom.com>
Signed-off-by: Bhanu Prakash Gollapudi <bprakash@broadcom.com>
---
 drivers/scsi/scsi_transport_fc.c |   11 +++++------
 1 files changed, 5 insertions(+), 6 deletions(-)

diff --git a/drivers/scsi/scsi_transport_fc.c b/drivers/scsi/scsi_transport_fc.c
index 998c01b..a8b0621 100644
--- a/drivers/scsi/scsi_transport_fc.c
+++ b/drivers/scsi/scsi_transport_fc.c
@@ -422,8 +422,7 @@ static int fc_host_setup(struct transport_container *tc, struct device *dev,
 
 	snprintf(fc_host->work_q_name, sizeof(fc_host->work_q_name),
 		 "fc_wq_%d", shost->host_no);
-	fc_host->work_q = create_singlethread_workqueue(
-					fc_host->work_q_name);
+	fc_host->work_q = alloc_workqueue(fc_host->work_q_name, 0, 0);
 	if (!fc_host->work_q)
 		return -ENOMEM;
 
@@ -431,8 +430,8 @@ static int fc_host_setup(struct transport_container *tc, struct device *dev,
 	snprintf(fc_host->devloss_work_q_name,
 		 sizeof(fc_host->devloss_work_q_name),
 		 "fc_dl_%d", shost->host_no);
-	fc_host->devloss_work_q = create_singlethread_workqueue(
-					fc_host->devloss_work_q_name);
+	fc_host->devloss_work_q =
+			alloc_workqueue(fc_host->devloss_work_q_name, 0, 0);
 	if (!fc_host->devloss_work_q) {
 		destroy_workqueue(fc_host->work_q);
 		fc_host->work_q = NULL;
@@ -2489,6 +2488,8 @@ fc_rport_final_delete(struct work_struct *work)
 	unsigned long flags;
 	int do_callback = 0;
 
+	fc_terminate_rport_io(rport);
+
 	/*
 	 * if a scan is pending, flush the SCSI Host work_q so that
 	 * that we can reclaim the rport scan work element.
@@ -2496,8 +2497,6 @@ fc_rport_final_delete(struct work_struct *work)
 	if (rport->flags & FC_RPORT_SCAN_PENDING)
 		scsi_flush_work(shost);
 
-	fc_terminate_rport_io(rport);
-
 	/*
 	 * Cancel any outstanding timers. These should really exist
 	 * only when rmmod'ing the LLDD and we're asking for
-- 
1.7.0.6

[PATCH 2/5] bnx2fc: Release the reference to hba only after the interface is destroyed

[Bhanu Prakash Gollapudi]

From: Nithin Nayak Sujir <nsujir@broadcom.com>

Prematurely decrementing the reference may lead to cmd_mgr becoming NULL with
the cmds are still active.

Signed-off-by: Nithin Nayak Sujir <nsujir@broadcom.com>
Signed-off-by: Bhanu Prakash Gollapudi <bprakash@broadcom.com>
---
 drivers/scsi/bnx2fc/bnx2fc_fcoe.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/bnx2fc/bnx2fc_fcoe.c b/drivers/scsi/bnx2fc/bnx2fc_fcoe.c
index 8fb9b6e..734d8c1 100644
--- a/drivers/scsi/bnx2fc/bnx2fc_fcoe.c
+++ b/drivers/scsi/bnx2fc/bnx2fc_fcoe.c
@@ -1370,8 +1370,6 @@ static void bnx2fc_if_destroy(struct fc_lport *lport)
 	/* Free existing transmit skbs */
 	fcoe_clean_pending_queue(lport);
 
-	bnx2fc_interface_put(hba);
-
 	/* Free queued packets for the receive thread */
 	bnx2fc_clean_rx_queue(lport);
 
@@ -1399,6 +1397,8 @@ static void bnx2fc_if_destroy(struct fc_lport *lport)
 
 	/* Release Scsi_Host */
 	scsi_host_put(lport->host);
+
+	bnx2fc_interface_put(hba);
 }
 
 /**
-- 
1.7.0.6

[PATCH 3/5] bnx2fc: call scsi_done if session goes to not ready from ready

[Bhanu Prakash Gollapudi]

From: Nithin Nayak Sujir <nsujir@broadcom.com>

If the session is not ready yet, we ask the SCSI-ml to retry. However, if the
session is just uploaded, we should not retry, but instead call scsi_done to
fail the IO.

Signed-off-by: Nithin Nayak Sujir <nsujir@broadcom.com>
Signed-off-by: Bhanu Prakash Gollapudi <bprakash@broadcom.com>
---
 drivers/scsi/bnx2fc/bnx2fc_io.c |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/drivers/scsi/bnx2fc/bnx2fc_io.c b/drivers/scsi/bnx2fc/bnx2fc_io.c
index 96013a9..f9cb535 100644
--- a/drivers/scsi/bnx2fc/bnx2fc_io.c
+++ b/drivers/scsi/bnx2fc/bnx2fc_io.c
@@ -1663,6 +1663,12 @@ int bnx2fc_queuecommand(struct Scsi_Host *host,
 	tgt = (struct bnx2fc_rport *)&rp[1];
 
 	if (!test_bit(BNX2FC_FLAG_SESSION_READY, &tgt->flags)) {
+		if (test_bit(BNX2FC_FLAG_UPLD_REQ_COMPL, &tgt->flags))  {
+			sc_cmd->result = DID_NO_CONNECT << 16;
+			sc_cmd->scsi_done(sc_cmd);
+			return 0;
+
+		}
 		/*
 		 * Session is not offloaded yet. Let SCSI-ml retry
 		 * the command.
-- 
1.7.0.6

[PATCH 4/5] bnx2fc: Do not use HBA_DBG macro when lport is not available

[Bhanu Prakash Gollapudi]

From: Nithin Nayak Sujir <nsujir@broadcom.com>

Use MISC_DBG instead.

Signed-off-by: Nithin Nayak Sujir <nsujir@broadcom.com>
Signed-off-by: Bhanu Prakash Gollapudi <bprakash@broadcom.com>
---
 drivers/scsi/bnx2fc/bnx2fc_fcoe.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/scsi/bnx2fc/bnx2fc_fcoe.c b/drivers/scsi/bnx2fc/bnx2fc_fcoe.c
index 734d8c1..8075d65 100644
--- a/drivers/scsi/bnx2fc/bnx2fc_fcoe.c
+++ b/drivers/scsi/bnx2fc/bnx2fc_fcoe.c
@@ -1131,7 +1131,7 @@ static void bnx2fc_interface_release(struct kref *kref)
 	struct net_device *phys_dev;
 
 	hba = container_of(kref, struct bnx2fc_hba, kref);
-	BNX2FC_HBA_DBG(hba->ctlr.lp, "Interface is being released\n");
+	BNX2FC_MISC_DBG("Interface is being released\n");
 
 	netdev = hba->netdev;
 	phys_dev = hba->phys_dev;
-- 
1.7.0.6

[PATCH 5/5] bnx2fc: increase cleanup wait time

[Bhanu Prakash Gollapudi]

From: Nithin Nayak Sujir <nsujir@broadcom.com>

FW may take more time cleaning up IOs issued to multiple targets.

Signed-off-by: Nithin Nayak Sujir <nsujir@broadcom.com>
Signed-off-by: Bhanu Prakash Gollapudi <bprakash@broadcom.com>
---
 drivers/scsi/bnx2fc/bnx2fc.h |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/scsi/bnx2fc/bnx2fc.h b/drivers/scsi/bnx2fc/bnx2fc.h
index 24102e8..e3caa50 100644
--- a/drivers/scsi/bnx2fc/bnx2fc.h
+++ b/drivers/scsi/bnx2fc/bnx2fc.h
@@ -130,7 +130,7 @@
 #define BNX2FC_TM_TIMEOUT		60	/* secs */
 #define BNX2FC_IO_TIMEOUT		20000UL	/* msecs */
 
-#define BNX2FC_WAIT_CNT			120
+#define BNX2FC_WAIT_CNT			1200
 #define BNX2FC_FW_TIMEOUT		(3 * HZ)
 #define PORT_MAX			2
 
-- 
1.7.0.6

RE: [PATCH 2/5] bnx2fc: Release the reference to hba only after the interface is destroyed

[Shyam_Iyer]

> -----Original Message-----
> From: linux-scsi-owner@vger.kernel.org [mailto:linux-scsi-
> owner@vger.kernel.org] On Behalf Of Bhanu Prakash Gollapudi
> Sent: Monday, April 25, 2011 3:30 PM
> To: linux-scsi@vger.kernel.org; devel@open-fcoe.org
> Cc: Nithin Nayak Sujir; Bhanu Prakash Gollapudi
> Subject: [PATCH 2/5] bnx2fc: Release the reference to hba only after
> the interface is destroyed
> 
> From: Nithin Nayak Sujir <nsujir@broadcom.com>
> 
> Prematurely decrementing the reference may lead to cmd_mgr becoming
> NULL with
> the cmds are still active.
> 
> Signed-off-by: Nithin Nayak Sujir <nsujir@broadcom.com>
> Signed-off-by: Bhanu Prakash Gollapudi <bprakash@broadcom.com>
> ---
>  drivers/scsi/bnx2fc/bnx2fc_fcoe.c |    4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/scsi/bnx2fc/bnx2fc_fcoe.c
> b/drivers/scsi/bnx2fc/bnx2fc_fcoe.c
> index 8fb9b6e..734d8c1 100644
> --- a/drivers/scsi/bnx2fc/bnx2fc_fcoe.c
> +++ b/drivers/scsi/bnx2fc/bnx2fc_fcoe.c
> @@ -1370,8 +1370,6 @@ static void bnx2fc_if_destroy(struct fc_lport
> *lport)
>  	/* Free existing transmit skbs */
>  	fcoe_clean_pending_queue(lport);
> 
> -	bnx2fc_interface_put(hba);
> -
>  	/* Free queued packets for the receive thread */
>  	bnx2fc_clean_rx_queue(lport);
> 
> @@ -1399,6 +1397,8 @@ static void bnx2fc_if_destroy(struct fc_lport
> *lport)
> 
>  	/* Release Scsi_Host */
>  	scsi_host_put(lport->host);
> +
> +	bnx2fc_interface_put(hba);
>  }
> 


Shouldn't we decrease refcount for interface before the scsi host refcount is decreased..? 
It doesn't probably make a difference here since we are having a 1:1 between an interface and a host but it saves bugs later when things change..

Also it probably makes sense to call scsi_host_put in bnx2fc_destroy rather than in bnx2fc_if_destroy.. 

Maybe we could have more interfaces for a scsi host..

Shout if this doesn't make any sense.

-Shyam Iyer

Re: [PATCH 2/5] bnx2fc: Release the reference to hba only after the interface is destroyed

[Bhanu Prakash Gollapudi]

On 4/25/2011 3:49 PM, Shyam_Iyer@Dell.com wrote:
>
>
>> -----Original Message-----
>> From: linux-scsi-owner@vger.kernel.org [mailto:linux-scsi-
>> owner@vger.kernel.org] On Behalf Of Bhanu Prakash Gollapudi
>> Sent: Monday, April 25, 2011 3:30 PM
>> To: linux-scsi@vger.kernel.org; devel@open-fcoe.org
>> Cc: Nithin Nayak Sujir; Bhanu Prakash Gollapudi
>> Subject: [PATCH 2/5] bnx2fc: Release the reference to hba only after
>> the interface is destroyed
>>
>> From: Nithin Nayak Sujir<nsujir@broadcom.com>
>>
>> Prematurely decrementing the reference may lead to cmd_mgr becoming
>> NULL with
>> the cmds are still active.
>>
>> Signed-off-by: Nithin Nayak Sujir<nsujir@broadcom.com>
>> Signed-off-by: Bhanu Prakash Gollapudi<bprakash@broadcom.com>
>> ---
>>   drivers/scsi/bnx2fc/bnx2fc_fcoe.c |    4 ++--
>>   1 files changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/scsi/bnx2fc/bnx2fc_fcoe.c
>> b/drivers/scsi/bnx2fc/bnx2fc_fcoe.c
>> index 8fb9b6e..734d8c1 100644
>> --- a/drivers/scsi/bnx2fc/bnx2fc_fcoe.c
>> +++ b/drivers/scsi/bnx2fc/bnx2fc_fcoe.c
>> @@ -1370,8 +1370,6 @@ static void bnx2fc_if_destroy(struct fc_lport
>> *lport)
>>   	/* Free existing transmit skbs */
>>   	fcoe_clean_pending_queue(lport);
>>
>> -	bnx2fc_interface_put(hba);
>> -
>>   	/* Free queued packets for the receive thread */
>>   	bnx2fc_clean_rx_queue(lport);
>>
>> @@ -1399,6 +1397,8 @@ static void bnx2fc_if_destroy(struct fc_lport
>> *lport)
>>
>>   	/* Release Scsi_Host */
>>   	scsi_host_put(lport->host);
>> +
>> +	bnx2fc_interface_put(hba);
>>   }
>>
>
>
> Shouldn't we decrease refcount for interface before the scsi host refcount is decreased..?
> It doesn't probably make a difference here since we are having a 1:1 between an interface and a host but it saves bugs later when things change..

Shyam, Thanks for the review.

Actually, it doesn't matter either way. When the hba refcount goes to 0, 
FIP controller and driver's command manager are destroyed which have no 
dependency on scsi_host. There is a 1-1 mapping between lport and the 
scsi_host, where as hba can have multiple scsi hosts, one per NPIV lport.

>
> Also it probably makes sense to call scsi_host_put in bnx2fc_destroy rather than in bnx2fc_if_destroy..
scsi_host is not only for the physical ports, but also for NPIV ports. 
scsi_host_put should also be called for NPIV ports, and hence 
bnx2fc_if_destroy is the right place to call scsi_host_put.

>
> Maybe we could have more interfaces for a scsi host..
>
> Shout if this doesn't make any sense.
>
> -Shyam Iyer
>
>

RE: [PATCH 2/5] bnx2fc: Release the reference to hba only after the interface is destroyed

[Shyam_Iyer]

> -----Original Message-----
> From: Bhanu Prakash Gollapudi [mailto:bprakash@broadcom.com]
> Sent: Monday, April 25, 2011 7:16 PM
> To: Iyer, Shyam
> Cc: linux-scsi@vger.kernel.org; devel@open-fcoe.org; Nithin Sujir
> Subject: Re: [PATCH 2/5] bnx2fc: Release the reference to hba only
> after the interface is destroyed
> 
> On 4/25/2011 3:49 PM, Shyam_Iyer@Dell.com wrote:
> >
> >
> >> -----Original Message-----
> >> From: linux-scsi-owner@vger.kernel.org [mailto:linux-scsi-
> >> owner@vger.kernel.org] On Behalf Of Bhanu Prakash Gollapudi
> >> Sent: Monday, April 25, 2011 3:30 PM
> >> To: linux-scsi@vger.kernel.org; devel@open-fcoe.org
> >> Cc: Nithin Nayak Sujir; Bhanu Prakash Gollapudi
> >> Subject: [PATCH 2/5] bnx2fc: Release the reference to hba only after
> >> the interface is destroyed
> >>
> >> From: Nithin Nayak Sujir<nsujir@broadcom.com>
> >>
> >> Prematurely decrementing the reference may lead to cmd_mgr becoming
> >> NULL with
> >> the cmds are still active.
> >>
> >> Signed-off-by: Nithin Nayak Sujir<nsujir@broadcom.com>
> >> Signed-off-by: Bhanu Prakash Gollapudi<bprakash@broadcom.com>
> >> ---
> >>   drivers/scsi/bnx2fc/bnx2fc_fcoe.c |    4 ++--
> >>   1 files changed, 2 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/drivers/scsi/bnx2fc/bnx2fc_fcoe.c
> >> b/drivers/scsi/bnx2fc/bnx2fc_fcoe.c
> >> index 8fb9b6e..734d8c1 100644
> >> --- a/drivers/scsi/bnx2fc/bnx2fc_fcoe.c
> >> +++ b/drivers/scsi/bnx2fc/bnx2fc_fcoe.c
> >> @@ -1370,8 +1370,6 @@ static void bnx2fc_if_destroy(struct fc_lport
> >> *lport)
> >>   	/* Free existing transmit skbs */
> >>   	fcoe_clean_pending_queue(lport);
> >>
> >> -	bnx2fc_interface_put(hba);
> >> -
> >>   	/* Free queued packets for the receive thread */
> >>   	bnx2fc_clean_rx_queue(lport);
> >>
> >> @@ -1399,6 +1397,8 @@ static void bnx2fc_if_destroy(struct fc_lport
> >> *lport)
> >>
> >>   	/* Release Scsi_Host */
> >>   	scsi_host_put(lport->host);
> >> +
> >> +	bnx2fc_interface_put(hba);
> >>   }
> >>
> >
> >
> > Shouldn't we decrease refcount for interface before the scsi host
> refcount is decreased..?
> > It doesn't probably make a difference here since we are having a 1:1
> between an interface and a host but it saves bugs later when things
> change..
> 
> Shyam, Thanks for the review.
> 
> Actually, it doesn't matter either way. When the hba refcount goes to
> 0,
> FIP controller and driver's command manager are destroyed which have no
> dependency on scsi_host. There is a 1-1 mapping between lport and the
> scsi_host, where as hba can have multiple scsi hosts, one per NPIV
> lport.

Yep... But I was thinking of a bonding scenario when ethX's are bonded together.

I guess the design will not take care of an FIP controller being common to more than one netdev interface.

Sorry.. for picking on this thread.. Agree with the fix.  I realize what I am suggesting requires that the API provided by libfc incorporate changes.
 
> 
> >
> > Also it probably makes sense to call scsi_host_put in bnx2fc_destroy
> rather than in bnx2fc_if_destroy..
> scsi_host is not only for the physical ports, but also for NPIV ports.
> scsi_host_put should also be called for NPIV ports, and hence
> bnx2fc_if_destroy is the right place to call scsi_host_put.

True.. and I see why you say that..

I think I am viewing a scsi_host as a controller at a higher layer than the netdev interface and hence imagining FCoE with features that netdev devices can offer and not really just be legacy FC running on another transport i.e. Ethernet.

It is possible to achieve NPIV and not be tied to one netdev interface as I guess NPIV is just an abstraction on what you see as a single physical port. 
For instance NPIV over a bond0 would be interesting.. as conceptually you are running your FC instance on a single physical port.

If that could happen it would be grand :-)

Copying Robert love(Intel) as I think folks that implement fcoe on netdev interfaces can take advantage of linux's software stack to achive this easily.

> 
> >
> > Maybe we could have more interfaces for a scsi host..
> >
> > Shout if this doesn't make any sense.
> >
> > -Shyam Iyer
> >
> >
> 

RE: [PATCH 2/5] bnx2fc: Release the reference to hba only after the interface is destroyed

[Shyam_Iyer]

> -----Original Message-----
> From: linux-scsi-owner@vger.kernel.org [mailto:linux-scsi-
> owner@vger.kernel.org] On Behalf Of Bhanu Prakash Gollapudi
> Sent: Monday, April 25, 2011 3:30 PM
> To: linux-scsi@vger.kernel.org; devel@open-fcoe.org
> Cc: Nithin Nayak Sujir; Bhanu Prakash Gollapudi
> Subject: [PATCH 2/5] bnx2fc: Release the reference to hba only after
> the interface is destroyed
> 
> From: Nithin Nayak Sujir <nsujir@broadcom.com>
> 
> Prematurely decrementing the reference may lead to cmd_mgr becoming
> NULL with
> the cmds are still active.
> 
> Signed-off-by: Nithin Nayak Sujir <nsujir@broadcom.com>
> Signed-off-by: Bhanu Prakash Gollapudi <bprakash@broadcom.com>
> ---
>  drivers/scsi/bnx2fc/bnx2fc_fcoe.c |    4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/scsi/bnx2fc/bnx2fc_fcoe.c
> b/drivers/scsi/bnx2fc/bnx2fc_fcoe.c
> index 8fb9b6e..734d8c1 100644
> --- a/drivers/scsi/bnx2fc/bnx2fc_fcoe.c
> +++ b/drivers/scsi/bnx2fc/bnx2fc_fcoe.c
> @@ -1370,8 +1370,6 @@ static void bnx2fc_if_destroy(struct fc_lport
> *lport)
>  	/* Free existing transmit skbs */
>  	fcoe_clean_pending_queue(lport);
> 
> -	bnx2fc_interface_put(hba);
> -
>  	/* Free queued packets for the receive thread */
>  	bnx2fc_clean_rx_queue(lport);
> 
> @@ -1399,6 +1397,8 @@ static void bnx2fc_if_destroy(struct fc_lport
> *lport)
> 
>  	/* Release Scsi_Host */
>  	scsi_host_put(lport->host);
> +
> +	bnx2fc_interface_put(hba);
>  }
> 
>  /**
> --
> 1.7.0.6
> 

Reviewed-by: Shyam Iyer <shyam_iyer@dell.com>