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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 4/25/2011 3:49 PM, Shyam_Iyer@xxxxxxxx wrote:


-----Original Message-----
From: linux-scsi-owner@xxxxxxxxxxxxxxx [mailto:linux-scsi-
owner@xxxxxxxxxxxxxxx] On Behalf Of Bhanu Prakash Gollapudi
Sent: Monday, April 25, 2011 3:30 PM
To: linux-scsi@xxxxxxxxxxxxxxx; devel@xxxxxxxxxxxxx
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@xxxxxxxxxxxx>

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

Signed-off-by: Nithin Nayak Sujir<nsujir@xxxxxxxxxxxx>
Signed-off-by: Bhanu Prakash Gollapudi<bprakash@xxxxxxxxxxxx>
---
  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




--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Index of Archives]     [SCSI Target Devel]     [Linux SCSI Target Infrastructure]     [Kernel Newbies]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Linux IIO]     [Samba]     [Device Mapper]
  Powered by Linux