Re: [PATCH] bnx2fc: fix incorrect DMA memory mapping in bnx2fc_map_sg()
On Mon, 2014-08-25 at 15:15 -0400, Chad Dupuis wrote:
On Fri, 22 Aug 2014, Eddie Wai wrote:
On Fri, 2014-08-22 at 20:12 +0200, Maurizio Lombardi wrote:
Hi Chad,
On 08/22/2014 02:08 PM, Chad Dupuis wrote:
Eddie, Maurizio,
Since it looks like there can be a difference in the pdev would it make
sense instead to convert the unmap function to use the bare DMA API so
we're consistent in our use of hba-pcidev-dev? Maybe something like
this:
I looked at what the bnx2i driver code does, it has a hba-pcidev pointer
too but makes use of scsi_map_sg()/scsi_unmap_sg().
So, from my point of view, it is the bnx2fc's map operation (that bypasses
scsi_map_sg() and uses dma_map_sg()) to look like a suspicious exception
and I decided to change it.
So far, I didn't get any error or strange behaviour after this change.
Eddie, what do you think about it?
Regards,
Maurizio Lombardi
diff --git a/drivers/scsi/bnx2fc/bnx2fc_io.c
b/drivers/scsi/bnx2fc/bnx2fc_io.c
index 32a5e0a..8b4adcf 100644
--- a/drivers/scsi/bnx2fc/bnx2fc_io.c
+++ b/drivers/scsi/bnx2fc/bnx2fc_io.c
@@ -1700,9 +1700,12 @@ static int bnx2fc_build_bd_list_from_sg(struct
bnx2fc_cmd *io_req)
static void bnx2fc_unmap_sg_list(struct bnx2fc_cmd *io_req)
{
struct scsi_cmnd *sc = io_req-sc_cmd;
+ struct bnx2fc_interface *interface = io_req-port-priv;
+ struct bnx2fc_hba *hba = interface-hba;
- if (io_req-bd_tbl-bd_valid sc) {
- scsi_dma_unmap(sc);
+ if (io_req-bd_tbl-bd_valid sc scsi_sg_count(sc)) {
+ dma_unmap_sg(hba-pcidev-dev, scsi_sglist(sc),
+ scsi_sg_count(sc), sc-sc_data_direction);
io_req-bd_tbl-bd_valid = 0;
}
}
On Fri, 22 Aug 2014, Maurizio Lombardi wrote:
Hi Eddie,
On 08/20/2014 07:35 PM, Eddie Wai wrote:
On Mon, 2014-08-04 at 10:20 +0200, Maurizio Lombardi wrote:
In the bnx2fc_map_sg() function, the original behaviour is to
allocate the DMA memory by directly calling dma_map_sg()
instead of using scsi_dma_map().
In contrast, bnx2fc_unmap_sg_list() calls scsi_dma_unmap().
The problem is that bnx2fc_map_sg() passes to the dma_map_sg() function
the pointer to the device structure taken from pci_dev
and not from Scsi_Host (as scsi_dma_map/unmap() do).
Great find, Maurizio, Thanks again.
Thanks :)
In some circumstances, the two devices may be different and the
DMA library will be unable to find the correct entry
when freeing the memory.
I'm curious at how the hba-pcidev-dev in the dma_map_sg call could end
up being different from the scsi_cmnd's device-host-dma_dev... Can
you share the test scenario?
It happens every time you set up an fcoe interface, so you just have to
compile the
kernel with the DMA API debug option.
It is 100% reproducible with RHEL6, RHEL7 and the latest mainline kernel
also.
This got me thinking that the scsi_host's dma_dev must have been changed
recently and I think I found the culprit:
http://www.spinics.net/lists/linux-scsi/msg65225.html
So back when, we changed the scsi_host's parent from hba-pcidev-dev to
the fcoe_ctlr_device's dev to basically align with the soft fcoe code to
fix a fcoemon expectation. Although this was changed correctly, the sg
DMA mapping routine did not adapt to it; hence the mismatch.
Prior to this change, both sg dma map and unmap were done to the
hba-pcidev-dev along with all other DMA mapping routines. I also see
that even though bnx2i uses the scsi_map_sg/scsi_unmap_sg pairs,
ultimately, they were operating on the hba-pcidev-dev device.
My opinion is that it would probably be in our best interest to have all
DMA mapping/unmapping routines continue to operate on the
hba-pcidev-dev directly to prevent any unforeseen DMA problems.
Thanks Eddie. Would my original patch work then (I added comments per
Christoph's request):
diff --git a/drivers/scsi/bnx2fc/bnx2fc_io.c
b/drivers/scsi/bnx2fc/bnx2fc_io.c
index 32a5e0a..6a61a0d 100644
--- a/drivers/scsi/bnx2fc/bnx2fc_io.c
+++ b/drivers/scsi/bnx2fc/bnx2fc_io.c
@@ -1651,6 +1651,10 @@ static int bnx2fc_map_sg(struct bnx2fc_cmd *io_req)
u64 addr;
int i;
+ /*
+* Use dma_map_sg directly to ensure we're using the correct
+* dev struct off of pcidev.
+*/
sg_count = dma_map_sg(hba-pcidev-dev, scsi_sglist(sc),
scsi_sg_count(sc), sc-sc_data_direction);
scsi_for_each_sg(sc, sg, sg_count, i) {
@@ -1700,9 +1704,16 @@ static int bnx2fc_build_bd_list_from_sg(struct
bnx2fc_cmd *io_req)
static void bnx2fc_unmap_sg_list(struct bnx2fc_cmd *io_req)
{
struct scsi_cmnd *sc = io_req-sc_cmd;
+ struct bnx2fc_interface *interface = io_req-port-priv;
+ struct bnx2fc_hba *hba = interface-hba;
- if (io_req-bd_tbl-bd_valid
Re: [PATCH] bnx2fc: fix incorrect DMA memory mapping in bnx2fc_map_sg()
On Fri, 2014-08-22 at 20:12 +0200, Maurizio Lombardi wrote:
Hi Chad,
On 08/22/2014 02:08 PM, Chad Dupuis wrote:
Eddie, Maurizio,
Since it looks like there can be a difference in the pdev would it make
sense instead to convert the unmap function to use the bare DMA API so
we're consistent in our use of hba-pcidev-dev? Maybe something like this:
I looked at what the bnx2i driver code does, it has a hba-pcidev pointer too
but makes use of scsi_map_sg()/scsi_unmap_sg().
So, from my point of view, it is the bnx2fc's map operation (that bypasses
scsi_map_sg() and uses dma_map_sg()) to look like a suspicious exception and
I decided to change it.
So far, I didn't get any error or strange behaviour after this change.
Eddie, what do you think about it?
Regards,
Maurizio Lombardi
diff --git a/drivers/scsi/bnx2fc/bnx2fc_io.c
b/drivers/scsi/bnx2fc/bnx2fc_io.c
index 32a5e0a..8b4adcf 100644
--- a/drivers/scsi/bnx2fc/bnx2fc_io.c
+++ b/drivers/scsi/bnx2fc/bnx2fc_io.c
@@ -1700,9 +1700,12 @@ static int bnx2fc_build_bd_list_from_sg(struct
bnx2fc_cmd *io_req)
static void bnx2fc_unmap_sg_list(struct bnx2fc_cmd *io_req)
{
struct scsi_cmnd *sc = io_req-sc_cmd;
+ struct bnx2fc_interface *interface = io_req-port-priv;
+ struct bnx2fc_hba *hba = interface-hba;
- if (io_req-bd_tbl-bd_valid sc) {
- scsi_dma_unmap(sc);
+ if (io_req-bd_tbl-bd_valid sc scsi_sg_count(sc)) {
+ dma_unmap_sg(hba-pcidev-dev, scsi_sglist(sc),
+ scsi_sg_count(sc), sc-sc_data_direction);
io_req-bd_tbl-bd_valid = 0;
}
}
On Fri, 22 Aug 2014, Maurizio Lombardi wrote:
Hi Eddie,
On 08/20/2014 07:35 PM, Eddie Wai wrote:
On Mon, 2014-08-04 at 10:20 +0200, Maurizio Lombardi wrote:
In the bnx2fc_map_sg() function, the original behaviour is to
allocate the DMA memory by directly calling dma_map_sg()
instead of using scsi_dma_map().
In contrast, bnx2fc_unmap_sg_list() calls scsi_dma_unmap().
The problem is that bnx2fc_map_sg() passes to the dma_map_sg() function
the pointer to the device structure taken from pci_dev
and not from Scsi_Host (as scsi_dma_map/unmap() do).
Great find, Maurizio, Thanks again.
Thanks :)
In some circumstances, the two devices may be different and the
DMA library will be unable to find the correct entry
when freeing the memory.
I'm curious at how the hba-pcidev-dev in the dma_map_sg call could end
up being different from the scsi_cmnd's device-host-dma_dev... Can
you share the test scenario?
It happens every time you set up an fcoe interface, so you just have to
compile the
kernel with the DMA API debug option.
It is 100% reproducible with RHEL6, RHEL7 and the latest mainline kernel
also.
This got me thinking that the scsi_host's dma_dev must have been changed
recently and I think I found the culprit:
http://www.spinics.net/lists/linux-scsi/msg65225.html
So back when, we changed the scsi_host's parent from hba-pcidev-dev to
the fcoe_ctlr_device's dev to basically align with the soft fcoe code to
fix a fcoemon expectation. Although this was changed correctly, the sg
DMA mapping routine did not adapt to it; hence the mismatch.
Prior to this change, both sg dma map and unmap were done to the
hba-pcidev-dev along with all other DMA mapping routines. I also see
that even though bnx2i uses the scsi_map_sg/scsi_unmap_sg pairs,
ultimately, they were operating on the hba-pcidev-dev device.
My opinion is that it would probably be in our best interest to have all
DMA mapping/unmapping routines continue to operate on the
hba-pcidev-dev directly to prevent any unforeseen DMA problems.
I see that scsi_dma_map could possibly return a -ENOMEM. It would be
best if we also add the check for sg_count being 0 and return the
bd_count or 0.
scsi_for_each_sg(sc, sg, sg_count, i) {
sg_len = sg_dma_len(sg);
addr = sg_dma_address(sg);
True, but sg_count is only used for the scsi_for_each(),
if it is 0 or -ENOMEM it will simply skip the loop and will execute
return bd_count.
Regards,
Maurizio Lombardi
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] bnx2fc: do not add shared skbs to the fcoe_rx_list
On Fri, 2014-07-25 at 10:12 +0200, Maurizio Lombardi wrote:
Hi Eddie,
just want to add you to the CC list.
Some days ago the bnx2fc's maintainer email address has been updated,
this should be the new one: qlogic-storage-upstr...@qlogic.com
I tried to send this patch to the new address but I received the following
delivery failure notification:
Delivery has failed to these recipients or groups:
dept_linux...@qlogic.commailto:dept_linux...@qlogic.com
Your message can't be delivered because delivery to this address is
restricted.
is this still a problem? The mail reflector seems to work for me...
On 07/25/2014 10:02 AM, Maurizio Lombardi wrote:
In some cases, the fcoe_rx_list may contains multiple instances
of the same skb (the so called shared skbs).
the bnx2fc_l2_rcv thread is a loop that extracts a skb from the list,
modifies (and destroys) its content and the proceed to the next one.
The problem is that if the skb is shared, the remaining instances will
be corrupted.
The solution is to use skb_share_check() before adding the skb to the
fcoe_rx_list.
[ 6286.808725] [ cut here ]
[ 6286.808729] WARNING: at include/scsi/fc_frame.h:173
bnx2fc_l2_rcv_thread+0x425/0x450 [bnx2fc]()
[ 6286.808748] Modules linked in: bnx2x(-) mdio dm_service_time bnx2fc cnic
uio fcoe libfcoe 8021q garp stp mrp libfc llc scsi_transport_fc scsi_tgt sg
iTCO_wdt iTCO_vendor_support coretemp kvm_intel kvm crct10dif_pclmul
crc32_pclmul crc32c_intel e1000e ghash_clmulni_intel aesni_intel lrw
gf128mul glue_helper ablk_helper ptp cryptd hpilo serio_raw hpwdt lpc_ich
pps_core ipmi_si pcspkr mfd_core ipmi_msghandler shpchp pcc_cpufreq mperf
nfsd auth_rpcgss nfs_acl lockd sunrpc dm_multipath xfs libcrc32c
ata_generic pata_acpi sd_mod crc_t10dif crct10dif_common mgag200
syscopyarea sysfillrect sysimgblt i2c_algo_bit ata_piix drm_kms_helper ttm
drm libata i2c_core hpsa dm_mirror dm_region_hash dm_log dm_mod [last
unloaded: mdio]
[ 6286.808750] CPU: 3 PID: 1304 Comm: bnx2fc_l2_threa Not tainted
3.10.0-121.el7.x86_64 #1
[ 6286.808750] Hardware name: HP ProLiant DL120 G7, BIOS J01 07/01/2013
[ 6286.808752] 0b36e715 8800deba1e00
815ec0ba
[ 6286.808753] 8800deba1e38 8105dee1 a05618c0
8801e4c81888
[ 6286.808754] e8663868 8801f402b180 8801f56bc000
8800deba1e48
[ 6286.808754] Call Trace:
[ 6286.808759] [815ec0ba] dump_stack+0x19/0x1b
[ 6286.808762] [8105dee1] warn_slowpath_common+0x61/0x80
[ 6286.808763] [8105e00a] warn_slowpath_null+0x1a/0x20
[ 6286.808765] [a054f415] bnx2fc_l2_rcv_thread+0x425/0x450
[bnx2fc]
[ 6286.808767] [a054eff0] ? bnx2fc_disable+0x90/0x90 [bnx2fc]
[ 6286.808769] [81085aef] kthread+0xcf/0xe0
[ 6286.808770] [81085a20] ? kthread_create_on_node+0x140/0x140
[ 6286.808772] [815fc76c] ret_from_fork+0x7c/0xb0
[ 6286.808773] [81085a20] ? kthread_create_on_node+0x140/0x140
[ 6286.808774] ---[ end trace c6cdb939184ccb4e ]---
Signed-off-by: Maurizio Lombardi mlomb...@redhat.com
---
drivers/scsi/bnx2fc/bnx2fc_fcoe.c | 7 +++
1 file changed, 7 insertions(+)
diff --git a/drivers/scsi/bnx2fc/bnx2fc_fcoe.c
b/drivers/scsi/bnx2fc/bnx2fc_fcoe.c
index 785d0d7..a190ab6 100644
--- a/drivers/scsi/bnx2fc/bnx2fc_fcoe.c
+++ b/drivers/scsi/bnx2fc/bnx2fc_fcoe.c
@@ -411,6 +411,7 @@ static int bnx2fc_rcv(struct sk_buff *skb, struct
net_device *dev,
struct fc_frame_header *fh;
struct fcoe_rcv_info *fr;
struct fcoe_percpu_s *bg;
+ struct sk_buff *tmp_skb;
unsigned short oxid;
interface = container_of(ptype, struct bnx2fc_interface,
@@ -423,6 +424,12 @@ static int bnx2fc_rcv(struct sk_buff *skb, struct
net_device *dev,
goto err;
}
+ tmp_skb = skb_share_check(skb, GFP_ATOMIC);
+ if (!tmp_skb)
+ goto err;
+
+ skb = tmp_skb;
+
if (unlikely(eth_hdr(skb)-h_proto != htons(ETH_P_FCOE))) {
printk(KERN_ERR PFX bnx2fc_rcv: Wrong FC type frame\n);
goto err;
Seems logical, but this patch requires some testing which ought to be
verified by the Qlogic team. Thanks.
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] bnx2fc: fix incorrect DMA memory mapping in bnx2fc_map_sg()
On Mon, 2014-08-04 at 10:20 +0200, Maurizio Lombardi wrote:
In the bnx2fc_map_sg() function, the original behaviour is to
allocate the DMA memory by directly calling dma_map_sg()
instead of using scsi_dma_map().
In contrast, bnx2fc_unmap_sg_list() calls scsi_dma_unmap().
The problem is that bnx2fc_map_sg() passes to the dma_map_sg() function
the pointer to the device structure taken from pci_dev
and not from Scsi_Host (as scsi_dma_map/unmap() do).
Great find, Maurizio, Thanks again.
In some circumstances, the two devices may be different and the
DMA library will be unable to find the correct entry
when freeing the memory.
I'm curious at how the hba-pcidev-dev in the dma_map_sg call could end
up being different from the scsi_cmnd's device-host-dma_dev... Can
you share the test scenario?
This patch fixes the bug by replacing dma_map_sg() with scsi_dma_map().
[56068.747547] WARNING: CPU: 1 PID: 12048 at lib/dma-debug.c:1080
check_unmap+0x90a/0xa00()
[56068.785706] fcoe ctlr_0: DMA-API: device driver tries to free DMA memory
it has not allocated [device address=0x000202a1aa00] [size=36 bytes]
[56068.846700] Modules linked in: 8021q garp stp mrp llc fcoe bnx2fc cnic uio
libfcoe libfc scsi_transport_fc scsi_tgt cfg80211 rfkill x86_pkg_temp_thermal
coretemp kvm_intel kvm crct10dif_pclmul iTCO_wdt crc32_pclmul gpio_ich
crc32c_intel iTCO_vendor_support ghash_clmulni_intel joydev lpc_ich hpwdt
nfsd mfd_core microcode hpilo serio_raw pcspkr ipmi_si auth_rpcgss nfs_acl
lockd shpchp ipmi_msghandler sunrpc xfs e1000e mgag200 i2c_algo_bit
drm_kms_helper ttm ptp drm ata_generic pata_acpi bnx2x i2c_core pps_core hpsa
mdio libcrc32c
[56069.077673] CPU: 1 PID: 12048 Comm: bnx2fc_thread/1 Not tainted
3.16.0-rc7+ #1
[56069.115286] Hardware name: HP ProLiant DL120 G7, BIOS J01 07/01/2013
[56069.145091] 0009 8801f36dbbd8 816c323a
8801f36dbc20
[56069.185083] 8801f36dbc10 8108350d 8800e9880960
8801f36dbd00
[56069.222738] 0024 000202a1aa00 0001
8801f36dbc70
[56069.263389] Call Trace:
[56069.275945] [816c323a] dump_stack+0x45/0x56
[56069.302133] [8108350d] warn_slowpath_common+0x7d/0xa0
[56069.334050] [8108357c] warn_slowpath_fmt+0x4c/0x50
[56069.364537] [8135214c] ? debug_dma_mapping_error+0x7c/0x90
[56069.398268] [8135423a] check_unmap+0x90a/0xa00
[56069.423761] [81354485] debug_dma_unmap_sg+0x65/0x110
[56069.474134] [8145ee93] scsi_dma_unmap+0x73/0xb0
[56069.499313] [a04cd9bc] bnx2fc_process_scsi_cmd_compl+0xcc/0x2a0
[bnx2fc]
[56069.535588] [a04c759b] bnx2fc_process_cq_compl+0x21b/0x280
[bnx2fc]
[56069.570311] [a04c2d76] bnx2fc_percpu_io_thread+0x106/0x180
[bnx2fc]
[56069.604560] [a04c2c70] ? bnx2fc_get_src_mac+0x20/0x20 [bnx2fc]
[56069.635801] [810a4392] kthread+0xd2/0xf0
[56069.659421] [810a42c0] ? kthread_create_on_node+0x170/0x170
[56069.689416] [816ca3bc] ret_from_fork+0x7c/0xb0
[56069.715510] [810a42c0] ? kthread_create_on_node+0x170/0x170
Signed-off-by: Maurizio Lombardi mlomb...@redhat.com
---
drivers/scsi/bnx2fc/bnx2fc_io.c | 6 ++
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/scsi/bnx2fc/bnx2fc_io.c b/drivers/scsi/bnx2fc/bnx2fc_io.c
index 7bc47fc..fe2cd9b 100644
--- a/drivers/scsi/bnx2fc/bnx2fc_io.c
+++ b/drivers/scsi/bnx2fc/bnx2fc_io.c
@@ -1640,8 +1640,6 @@ static int bnx2fc_split_bd(struct bnx2fc_cmd *io_req,
u64 addr, int sg_len,
static int bnx2fc_map_sg(struct bnx2fc_cmd *io_req)
{
- struct bnx2fc_interface *interface = io_req-port-priv;
- struct bnx2fc_hba *hba = interface-hba;
struct scsi_cmnd *sc = io_req-sc_cmd;
struct fcoe_bd_ctx *bd = io_req-bd_tbl-bd_tbl;
struct scatterlist *sg;
@@ -1653,8 +1651,8 @@ static int bnx2fc_map_sg(struct bnx2fc_cmd *io_req)
u64 addr;
int i;
- sg_count = dma_map_sg(hba-pcidev-dev, scsi_sglist(sc),
- scsi_sg_count(sc), sc-sc_data_direction);
+ sg_count = scsi_dma_map(sc);
+
I see that scsi_dma_map could possibly return a -ENOMEM. It would be
best if we also add the check for sg_count being 0 and return the
bd_count or 0.
scsi_for_each_sg(sc, sg, sg_count, i) {
sg_len = sg_dma_len(sg);
addr = sg_dma_address(sg);
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] scsi: bnx2i: bnx2i_iscsi.c: Cleaning up variable is set more than once
On Wed, 2014-06-25 at 16:26 +0200, Maurizio Lombardi wrote: Hi, On 06/25/2014 04:04 PM, Rickard Strandqvist wrote: A struct member variable is set to different values without having used in between. This was found using a static code analysis program called cppcheck Signed-off-by: Rickard Strandqvist rickard_strandqv...@spectrumdigital.se --- drivers/scsi/bnx2i/bnx2i_iscsi.c |1 - 1 file changed, 1 deletion(-) diff --git a/drivers/scsi/bnx2i/bnx2i_iscsi.c b/drivers/scsi/bnx2i/bnx2i_iscsi.c index 166543f..fdf7bc3 100644 --- a/drivers/scsi/bnx2i/bnx2i_iscsi.c +++ b/drivers/scsi/bnx2i/bnx2i_iscsi.c @@ -1643,7 +1643,6 @@ static void bnx2i_conn_get_stats(struct iscsi_cls_conn *cls_conn, stats-r2t_pdus = conn-r2t_pdus_cnt; stats-tmfcmd_pdus = conn-tmfcmd_pdus_cnt; stats-tmfrsp_pdus = conn-tmfrsp_pdus_cnt; - stats-custom_length = 3; strcpy(stats-custom[2].desc, eh_abort_cnt); stats-custom[2].value = conn-eh_abort_cnt; stats-digest_err = 0; Eddie, The code modifies the content of stats-custom[2], so shouldn't custom_length be set to 3? Why is it set to zero at the end of this function? Nice find. This is literally a day1 bug. Yes, I agree that the custom_length should be left at 3. Otherwise, the nlmsg replied back to the application would not have the custom message. Thanks. Regards, Maurizio Lombardi -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] bnx2fc: do not scan uninitialized lists in case of error.
On Thu, 2014-06-19 at 15:05 +0200, Maurizio Lombardi wrote:
In case of of error, the bnx2fc_cmd_mgr_alloc() function will call
the bnx2fc_cmd_mgr_free() to perform the cleanup.
The problem is that in one case the latter may try to scan
some not-yet initialized lists, resulting in a kernel panic.
This patch prevents this from happening by freeing the lists
before calling bnx2fc_cmd_mgr_free().
Signed-off-by: Maurizio Lombardi mlomb...@redhat.com
---
drivers/scsi/bnx2fc/bnx2fc_io.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/scsi/bnx2fc/bnx2fc_io.c b/drivers/scsi/bnx2fc/bnx2fc_io.c
index 32a5e0a..7bc47fc 100644
--- a/drivers/scsi/bnx2fc/bnx2fc_io.c
+++ b/drivers/scsi/bnx2fc/bnx2fc_io.c
@@ -282,6 +282,8 @@ struct bnx2fc_cmd_mgr *bnx2fc_cmd_mgr_alloc(struct
bnx2fc_hba *hba)
arr_sz, GFP_KERNEL);
if (!cmgr-free_list_lock) {
printk(KERN_ERR PFX failed to alloc free_list_lock\n);
+ kfree(cmgr-free_list);
+ cmgr-free_list = NULL;
goto mem_err;
}
Agreed. Thanks for fixing this, Maurizio.
Acked-by: Eddie Wai eddie@broadcom.com
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] bnx2fc: Do not log error for netevents that need no action
On Fri, 2014-06-06 at 13:05 -0500, shirishpargaon...@gmail.com wrote: From: Shirish Pargaonkar shirishpargaon...@gmail.com Do not log error for netevents that need no action such as NETDEV_REGISTER 0x0005, NETDEV_CHANGEADDR, and NETDEV_CHANGENAME. It results in logging error messages such as these [ 35.315872] bnx2fc: Unknown netevent 5 [ 35.315935] bnx2fc: Unknown netevent 8 [ 35.353866] bnx2fc: Unknown netevent 10 and generating bug reports. Remove logging this message as an ERROR instead of turning them into either DEBUG or INFO level messages. Signed-by: Shirish Pargaonkar spargaon...@suse.com --- drivers/scsi/bnx2fc/bnx2fc_fcoe.c | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/scsi/bnx2fc/bnx2fc_fcoe.c b/drivers/scsi/bnx2fc/bnx2fc_fcoe.c index 1d41f4b..c7388a8 100644 --- a/drivers/scsi/bnx2fc/bnx2fc_fcoe.c +++ b/drivers/scsi/bnx2fc/bnx2fc_fcoe.c @@ -856,7 +856,6 @@ static void bnx2fc_indicate_netevent(void *context, unsigned long event, return; default: - printk(KERN_ERR PFX Unknown netevent %ld, event); return; } Thanks for fixing this, Shirish. It makes sense to suppress the unwanted netevent messages. The patch looks good to me, but we should really get the current bnx2fc maintainer from Qlogic to ACK this. Acked-by: Eddie Wai eddie@broadcom.com -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Open-FCoE] [PATCH] bnx2fc: Improve stats update mechanism
On Tue, 2014-06-03 at 10:54 -0400, Neil Horman wrote: On Sat, May 31, 2014 at 08:37:57AM -0400, Neil Horman wrote: On Sat, May 31, 2014 at 05:13:11AM +, Eddie Wai wrote: On May 30, 2014, at 7:48 PM, Neil Horman nhor...@tuxdriver.com wrote: On Fri, May 30, 2014 at 02:59:43PM -0700, Eddie Wai wrote: Thanks for fixing this. The patch generally looks good, but I do have a few comments. On Fri, 2014-05-30 at 11:01 -0400, Neil Horman wrote: Recently had this warning reported: [ 290.489047] Call Trace: [ 290.489053] [8169efec] dump_stack+0x19/0x1b [ 290.489055] [810ac7a9] __might_sleep+0x179/0x230 [ 290.489057] [816a4ad5] mutex_lock_nested+0x55/0x520 [ 290.489061] [a01b9905] ? bnx2fc_l2_rcv_thread+0xc5/0x4c0 [bnx2fc] [ 290.489065] [a0174c1a] fc_vport_id_lookup+0x3a/0xa0 [libfc] [ 290.489068] [a01b9a6c] bnx2fc_l2_rcv_thread+0x22c/0x4c0 [bnx2fc] [ 290.489070] [a01b9840] ? bnx2fc_vport_destroy+0x110/0x110 [bnx2fc] [ 290.489073] [8109e0cd] kthread+0xed/0x100 [ 290.489075] [8109dfe0] ? insert_kthread_work+0x80/0x80 [ 290.489077] [816b2fec] ret_from_fork+0x7c/0xb0 [ 290.489078] [8109dfe0] ? insert_kthread_work+0x80/0x80 Its due to the fact that we call a potentially sleeping function from the bnx2fc rcv path with preemption disabled (via the get_cpu call embedded in the per-cpu variable stats lookup in bnx2fc_l2_rcv_thread. Easy enough fix, we can just move the stats collection later in the function where we are sure we won't preempt or sleep. This also allows us to not have to enable pre-emption when doing a per-cpu lookup, since we're certain not to get You mean this allows us to not have to 'disable' pre-emption, right? Can you elaborate on how we can be sure that we won't get preempted immediately after retrieving the CPU variable? I would think it'll be okay to call get_cpu at this stage as there won't be any sleepable mutex lock calls before the put_cpu. We can't be sure, but I would assert it doesn't really matter at this point. The area in which we update stats is so small that, even if we do hit the unlikely chance that we get pre-empted, and then rescheduled on a different cpu, it won't matter. We could add the get_cpu/put_cpu back if you're really intent on it, but I'm not sure its worthwhile given that this is a hot path. I agree with your assessment. But code wise just so bnx2fc is consistent to the software FCoE counterpart, I would advice to use the same get_cpu to retrieve that stats CPU variable. Thanks. Actually I woke up this morning meaning to send a follow on note addressing that. The Soft FCoE counterpart to this function acutally works the way bnx2fc reception does after my patch here. Thats because the stats are updated with the cpu held, but the frame is actually received by the fc layer immediately after cpu_put is called. The implication is that the task can get preempted right after we update the stats and re-enable preemption, meaning that we may actually receive the frame on a different cpu than the cpu we updated the stats on. I was planning on sending a patch to switch get_cpu/put_cpu in the soft FCoE code to just use smp_processor_id(), since it doesn't help anything. bnx2fc is actually in a slightly better position than soft FCoE. soft FCoE creates a thread for each cpu, but doesn't explicitly assign single cpu affinity for each task, meaning per-cpu stats are actually relevant. For bnx2fc, you only create a single task at module init, meaning there is no parallel reception of frames. As such the per-cpu tasks are really more of an aggregate measure (since the stat updates are all serialized anyway by the single thread accross cpus). That's correct. bnx2fc uses only a single thread for this L2 recv path because fast path traffic normally goes to the offload path. The bottom line is that you can't hold a cpu while both doing the work of frame reception and updating statistics, unless you want to avoid sleeping functions in the entire receive path, and once you separate the two by only holding the cpu during stats update, you run the risk of changing the cpu after you've processed the frame, but before you dereference the per_cpu pointer. I can still re-add the get_cpu/put_cpu if you want, but I really don't see the purpose of the extra overhead given the above, and my intention is to remove it from the soft FCoE code as well. Nah, just as long as we are consistent, that's good enough for me! Thanks. Regards Neil Can I get further comment or an ACK on this so the fcoe tree can pull it in? Acked
Re: [Open-FCoE] [PATCH] bnx2fc: Improve stats update mechanism
Thanks for fixing this. The patch generally looks good, but I do have a
few comments.
On Fri, 2014-05-30 at 11:01 -0400, Neil Horman wrote:
Recently had this warning reported:
[ 290.489047] Call Trace:
[ 290.489053] [8169efec] dump_stack+0x19/0x1b
[ 290.489055] [810ac7a9] __might_sleep+0x179/0x230
[ 290.489057] [816a4ad5] mutex_lock_nested+0x55/0x520
[ 290.489061] [a01b9905] ? bnx2fc_l2_rcv_thread+0xc5/0x4c0
[bnx2fc]
[ 290.489065] [a0174c1a] fc_vport_id_lookup+0x3a/0xa0 [libfc]
[ 290.489068] [a01b9a6c] bnx2fc_l2_rcv_thread+0x22c/0x4c0 [bnx2fc]
[ 290.489070] [a01b9840] ? bnx2fc_vport_destroy+0x110/0x110
[bnx2fc]
[ 290.489073] [8109e0cd] kthread+0xed/0x100
[ 290.489075] [8109dfe0] ? insert_kthread_work+0x80/0x80
[ 290.489077] [816b2fec] ret_from_fork+0x7c/0xb0
[ 290.489078] [8109dfe0] ? insert_kthread_work+0x80/0x80
Its due to the fact that we call a potentially sleeping function from the
bnx2fc
rcv path with preemption disabled (via the get_cpu call embedded in the
per-cpu
variable stats lookup in bnx2fc_l2_rcv_thread.
Easy enough fix, we can just move the stats collection later in the function
where we are sure we won't preempt or sleep. This also allows us to not have
to
enable pre-emption when doing a per-cpu lookup, since we're certain not to get
You mean this allows us to not have to 'disable' pre-emption, right?
Can you elaborate on how we can be sure that we won't get preempted
immediately after retrieving the CPU variable? I would think it'll be
okay to call get_cpu at this stage as there won't be any sleepable mutex
lock calls before the put_cpu.
rescheduled.
Signed-off-by: Neil Horman nhor...@tuxdriver.com
CC: linux-scsi@vger.kernel.org
CC: Robert Love robert.w.l...@intel.com
CC: Vasu Dev vasu@intel.com
---
drivers/scsi/bnx2fc/bnx2fc_fcoe.c | 16
1 file changed, 4 insertions(+), 12 deletions(-)
diff --git a/drivers/scsi/bnx2fc/bnx2fc_fcoe.c
b/drivers/scsi/bnx2fc/bnx2fc_fcoe.c
index 9b94850..475eee5 100644
--- a/drivers/scsi/bnx2fc/bnx2fc_fcoe.c
+++ b/drivers/scsi/bnx2fc/bnx2fc_fcoe.c
@@ -516,23 +516,17 @@ static void bnx2fc_recv_frame(struct sk_buff *skb)
skb_pull(skb, sizeof(struct fcoe_hdr));
fr_len = skb-len - sizeof(struct fcoe_crc_eof);
- stats = per_cpu_ptr(lport-stats, get_cpu());
- stats-RxFrames++;
- stats-RxWords += fr_len / FCOE_WORD_TO_BYTE;
-
fp = (struct fc_frame *)skb;
fc_frame_init(fp);
fr_dev(fp) = lport;
fr_sof(fp) = hp-fcoe_sof;
if (skb_copy_bits(skb, fr_len, crc_eof, sizeof(crc_eof))) {
- put_cpu();
kfree_skb(skb);
return;
}
fr_eof(fp) = crc_eof.fcoe_eof;
fr_crc(fp) = crc_eof.fcoe_crc32;
if (pskb_trim(skb, fr_len)) {
- put_cpu();
kfree_skb(skb);
return;
}
@@ -544,7 +538,6 @@ static void bnx2fc_recv_frame(struct sk_buff *skb)
port = lport_priv(vn_port);
if (!ether_addr_equal(port-data_src_addr, dest_mac)) {
BNX2FC_HBA_DBG(lport, fpma mismatch\n);
- put_cpu();
kfree_skb(skb);
return;
}
@@ -552,7 +545,6 @@ static void bnx2fc_recv_frame(struct sk_buff *skb)
if (fh-fh_r_ctl == FC_RCTL_DD_SOL_DATA
fh-fh_type == FC_TYPE_FCP) {
/* Drop FCP data. We dont this in L2 path */
- put_cpu();
kfree_skb(skb);
return;
}
@@ -562,7 +554,6 @@ static void bnx2fc_recv_frame(struct sk_buff *skb)
case ELS_LOGO:
if (ntoh24(fh-fh_s_id) == FC_FID_FLOGI) {
/* drop non-FIP LOGO */
- put_cpu();
kfree_skb(skb);
return;
}
@@ -572,22 +563,23 @@ static void bnx2fc_recv_frame(struct sk_buff *skb)
if (fh-fh_r_ctl == FC_RCTL_BA_ABTS) {
/* Drop incoming ABTS */
- put_cpu();
kfree_skb(skb);
return;
}
+ stats = per_cpu_ptr(lport-stats, smp_processor_id());
+ stats-RxFrames++;
+ stats-RxWords += fr_len / FCOE_WORD_TO_BYTE;
+
if (le32_to_cpu(fr_crc(fp)) !=
~crc32(~0, skb-data, fr_len)) {
if (stats-InvalidCRCCount 5)
printk(KERN_WARNING PFX dropping frame with
CRC error\n);
stats-InvalidCRCCount++;
- put_cpu();
kfree_skb(skb);
return;
}
- put_cpu();
fc_exch_recv(lport, fp);
}
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to
Re: [Open-FCoE] [PATCH] bnx2fc: Improve stats update mechanism
On May 30, 2014, at 7:48 PM, Neil Horman nhor...@tuxdriver.com wrote:
On Fri, May 30, 2014 at 02:59:43PM -0700, Eddie Wai wrote:
Thanks for fixing this. The patch generally looks good, but I do have a
few comments.
On Fri, 2014-05-30 at 11:01 -0400, Neil Horman wrote:
Recently had this warning reported:
[ 290.489047] Call Trace:
[ 290.489053] [8169efec] dump_stack+0x19/0x1b
[ 290.489055] [810ac7a9] __might_sleep+0x179/0x230
[ 290.489057] [816a4ad5] mutex_lock_nested+0x55/0x520
[ 290.489061] [a01b9905] ? bnx2fc_l2_rcv_thread+0xc5/0x4c0
[bnx2fc]
[ 290.489065] [a0174c1a] fc_vport_id_lookup+0x3a/0xa0 [libfc]
[ 290.489068] [a01b9a6c] bnx2fc_l2_rcv_thread+0x22c/0x4c0
[bnx2fc]
[ 290.489070] [a01b9840] ? bnx2fc_vport_destroy+0x110/0x110
[bnx2fc]
[ 290.489073] [8109e0cd] kthread+0xed/0x100
[ 290.489075] [8109dfe0] ? insert_kthread_work+0x80/0x80
[ 290.489077] [816b2fec] ret_from_fork+0x7c/0xb0
[ 290.489078] [8109dfe0] ? insert_kthread_work+0x80/0x80
Its due to the fact that we call a potentially sleeping function from the
bnx2fc
rcv path with preemption disabled (via the get_cpu call embedded in the
per-cpu
variable stats lookup in bnx2fc_l2_rcv_thread.
Easy enough fix, we can just move the stats collection later in the function
where we are sure we won't preempt or sleep. This also allows us to not
have to
enable pre-emption when doing a per-cpu lookup, since we're certain not to
get
You mean this allows us to not have to 'disable' pre-emption, right?
Can you elaborate on how we can be sure that we won't get preempted
immediately after retrieving the CPU variable? I would think it'll be
okay to call get_cpu at this stage as there won't be any sleepable mutex
lock calls before the put_cpu.
We can't be sure, but I would assert it doesn't really matter at this point.
The area in which we update stats is so small that, even if we do hit the
unlikely chance that we get pre-empted, and then rescheduled on a different
cpu,
it won't matter. We could add the get_cpu/put_cpu back if you're really
intent
on it, but I'm not sure its worthwhile given that this is a hot path.
I agree with your assessment. But code wise just so bnx2fc is consistent to
the software FCoE counterpart, I would advice to use the same get_cpu to
retrieve that stats CPU variable. Thanks.
Neil
rescheduled.
Signed-off-by: Neil Horman nhor...@tuxdriver.com
CC: linux-scsi@vger.kernel.org
CC: Robert Love robert.w.l...@intel.com
CC: Vasu Dev vasu@intel.com
---
drivers/scsi/bnx2fc/bnx2fc_fcoe.c | 16
1 file changed, 4 insertions(+), 12 deletions(-)
diff --git a/drivers/scsi/bnx2fc/bnx2fc_fcoe.c
b/drivers/scsi/bnx2fc/bnx2fc_fcoe.c
index 9b94850..475eee5 100644
--- a/drivers/scsi/bnx2fc/bnx2fc_fcoe.c
+++ b/drivers/scsi/bnx2fc/bnx2fc_fcoe.c
@@ -516,23 +516,17 @@ static void bnx2fc_recv_frame(struct sk_buff *skb)
skb_pull(skb, sizeof(struct fcoe_hdr));
fr_len = skb-len - sizeof(struct fcoe_crc_eof);
-stats = per_cpu_ptr(lport-stats, get_cpu());
-stats-RxFrames++;
-stats-RxWords += fr_len / FCOE_WORD_TO_BYTE;
-
fp = (struct fc_frame *)skb;
fc_frame_init(fp);
fr_dev(fp) = lport;
fr_sof(fp) = hp-fcoe_sof;
if (skb_copy_bits(skb, fr_len, crc_eof, sizeof(crc_eof))) {
-put_cpu();
kfree_skb(skb);
return;
}
fr_eof(fp) = crc_eof.fcoe_eof;
fr_crc(fp) = crc_eof.fcoe_crc32;
if (pskb_trim(skb, fr_len)) {
-put_cpu();
kfree_skb(skb);
return;
}
@@ -544,7 +538,6 @@ static void bnx2fc_recv_frame(struct sk_buff *skb)
port = lport_priv(vn_port);
if (!ether_addr_equal(port-data_src_addr, dest_mac)) {
BNX2FC_HBA_DBG(lport, fpma mismatch\n);
-put_cpu();
kfree_skb(skb);
return;
}
@@ -552,7 +545,6 @@ static void bnx2fc_recv_frame(struct sk_buff *skb)
if (fh-fh_r_ctl == FC_RCTL_DD_SOL_DATA
fh-fh_type == FC_TYPE_FCP) {
/* Drop FCP data. We dont this in L2 path */
-put_cpu();
kfree_skb(skb);
return;
}
@@ -562,7 +554,6 @@ static void bnx2fc_recv_frame(struct sk_buff *skb)
case ELS_LOGO:
if (ntoh24(fh-fh_s_id) == FC_FID_FLOGI) {
/* drop non-FIP LOGO */
-put_cpu();
kfree_skb(skb);
return;
}
@@ -572,22 +563,23 @@ static void bnx2fc_recv_frame(struct sk_buff *skb)
if (fh-fh_r_ctl == FC_RCTL_BA_ABTS) {
/* Drop incoming ABTS */
-put_cpu();
kfree_skb(skb);
return;
}
+stats = per_cpu_ptr(lport-stats, smp_processor_id());
+stats-RxFrames++;
+stats-RxWords += fr_len / FCOE_WORD_TO_BYTE;
+
if (le32_to_cpu(fr_crc(fp)) !=
~crc32(~0, skb-data, fr_len)) {
if (stats
Re: [PATCH] bnx2i: Make boot_nic entry visible in the sysfs session objects
FYI, this was part of the boot_nic sysfs patch which would allow iscsid to sync_session with info from the iBFT for iSCSI BFS via offload. This looks good to me. Acked-by: Eddie Wai eddie@broadcom.com On Mon, 2014-05-19 at 07:32 -0400, vikas.chaudh...@qlogic.com wrote: From: Tej Parkash tej.park...@qlogic.com Signed-off-by: Tej Parkash tej.park...@qlogic.com Signed-off-by: Vikas Chaudhary vikas.chaudh...@qlogic.com --- drivers/scsi/bnx2i/bnx2i_iscsi.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/scsi/bnx2i/bnx2i_iscsi.c b/drivers/scsi/bnx2i/bnx2i_iscsi.c index 166543f..cabc8c1 100644 --- a/drivers/scsi/bnx2i/bnx2i_iscsi.c +++ b/drivers/scsi/bnx2i/bnx2i_iscsi.c @@ -2234,6 +2234,9 @@ static umode_t bnx2i_attr_is_visible(int param_type, int param) case ISCSI_PARAM_TGT_RESET_TMO: case ISCSI_PARAM_IFACE_NAME: case ISCSI_PARAM_INITIATOR_NAME: + case ISCSI_PARAM_BOOT_ROOT: + case ISCSI_PARAM_BOOT_NIC: + case ISCSI_PARAM_BOOT_TARGET: return S_IRUGO; default: return 0; -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 0/3] bnx2fc: fix memory leaks and NULL pointer dereferences
The patchset looks good. Thanks. Acked-by: Eddie Wai eddie@broadcom.com On Tue, 2014-04-01 at 13:58 +0200, Maurizio Lombardi wrote: PATCH 1/3 removes a unused variable from the bnx2fc_free_hash_table() function, PATCH 2/3 fixes a memory leak and some NULL pointer dereferences in the bnx2fc_free_hash_table() function that may happen if bnx2fc_allocate_hash_table() fails. PATCH 3/3 fixes a memory leak in the bnx2fc_allocate_hash_table() function. Maurizio Lombardi (3): bnx2fc: remove unused variable hash_table_size bnx2fc: fix memory leak and potential NULL pointer dereference. bnx2fc: fix memory leak in bnx2fc_allocate_hash_table() drivers/scsi/bnx2fc/bnx2fc_hwi.c | 64 +++- 1 file changed, 37 insertions(+), 27 deletions(-) -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] bnx2fc: remove unused variable hash_table_size
Looks good. Thanks.
Acked-by: Eddie Wai eddie@broadcom.com
On Fri, 2014-03-07 at 15:37 +0100, Maurizio Lombardi wrote:
hash_table_size is not used by the bnx2fc_free_hash_table() function.
Signed-off-by: Maurizio Lombardi mlomb...@redhat.com
---
drivers/scsi/bnx2fc/bnx2fc_hwi.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/drivers/scsi/bnx2fc/bnx2fc_hwi.c
b/drivers/scsi/bnx2fc/bnx2fc_hwi.c
index 46a3765..261af2a 100644
--- a/drivers/scsi/bnx2fc/bnx2fc_hwi.c
+++ b/drivers/scsi/bnx2fc/bnx2fc_hwi.c
@@ -1966,12 +1966,9 @@ static void bnx2fc_free_hash_table(struct bnx2fc_hba
*hba)
{
int i;
int segment_count;
- int hash_table_size;
u32 *pbl;
segment_count = hba-hash_tbl_segment_count;
- hash_table_size = BNX2FC_NUM_MAX_SESS * BNX2FC_MAX_ROWS_IN_HASH_TBL *
- sizeof(struct fcoe_hash_table_entry);
pbl = hba-hash_tbl_pbl;
for (i = 0; i segment_count; ++i) {
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] bnx2fc: fix memory leak and potential NULL pointer dereference.
Hello Maurizio,
Thanks for looking into this. The patch looks good, but I would like to
extend it with the following allocation failure portion to completely
remove the memory leak. Please incorporate this into your next
submission.
Thanks,
Eddie
@@ -2020,7 +2026,7 @@ static int bnx2fc_allocate_hash_table(struct
bnx2fc_hba *hba)
dma_segment_array = kzalloc(dma_segment_array_size, GFP_KERNEL);
if (!dma_segment_array) {
printk(KERN_ERR PFX hash table pointers (dma) alloc
failed\n);
- return -ENOMEM;
+ goto free_hash_tbl_seg;
}
for (i = 0; i segment_count; ++i) {
@@ -2039,7 +2045,7 @@ static int bnx2fc_allocate_hash_table(struct
bnx2fc_hba *hba)
hba-hash_tbl_segments[i] = NULL;
}
kfree(dma_segment_array);
- return -ENOMEM;
+ goto free_hash_tbl_seg;
}
memset(hba-hash_tbl_segments[i], 0,
BNX2FC_HASH_TBL_CHUNK_SIZE);
@@ -2077,6 +2083,11 @@ static int bnx2fc_allocate_hash_table(struct
bnx2fc_hba *hba)
}
kfree(dma_segment_array);
return 0;
+
+free_hash_tbl_seg:
+ kfree(hba-hash_tbl_segments);
+ hba-hash_tbl_segments = NULL;
+ return -ENOMEM;
}
On Fri, 2014-03-07 at 15:37 +0100, Maurizio Lombardi wrote:
If bnx2fc_allocate_hash_table() for some reasons fails, it is possible that
the
hash_tbl_segments or the hash_tbl_pbl pointers are NULL.
In this case bnx2fc_free_hash_table() will panic the system.
this patch also fixes a memory leak, the hash_tbl_segments pointer was never
freed.
Signed-off-by: Maurizio Lombardi mlomb...@redhat.com
---
drivers/scsi/bnx2fc/bnx2fc_hwi.c | 34 --
1 file changed, 20 insertions(+), 14 deletions(-)
diff --git a/drivers/scsi/bnx2fc/bnx2fc_hwi.c
b/drivers/scsi/bnx2fc/bnx2fc_hwi.c
index 261af2a..f83bae4 100644
--- a/drivers/scsi/bnx2fc/bnx2fc_hwi.c
+++ b/drivers/scsi/bnx2fc/bnx2fc_hwi.c
@@ -1968,21 +1968,27 @@ static void bnx2fc_free_hash_table(struct bnx2fc_hba
*hba)
int segment_count;
u32 *pbl;
- segment_count = hba-hash_tbl_segment_count;
-
- pbl = hba-hash_tbl_pbl;
- for (i = 0; i segment_count; ++i) {
- dma_addr_t dma_address;
-
- dma_address = le32_to_cpu(*pbl);
- ++pbl;
- dma_address += ((u64)le32_to_cpu(*pbl)) 32;
- ++pbl;
- dma_free_coherent(hba-pcidev-dev,
- BNX2FC_HASH_TBL_CHUNK_SIZE,
- hba-hash_tbl_segments[i],
- dma_address);
+ if (hba-hash_tbl_segments) {
+
+ pbl = hba-hash_tbl_pbl;
+ if (pbl) {
+ segment_count = hba-hash_tbl_segment_count;
+ for (i = 0; i segment_count; ++i) {
+ dma_addr_t dma_address;
+
+ dma_address = le32_to_cpu(*pbl);
+ ++pbl;
+ dma_address += ((u64)le32_to_cpu(*pbl)) 32;
+ ++pbl;
+ dma_free_coherent(hba-pcidev-dev,
+ BNX2FC_HASH_TBL_CHUNK_SIZE,
+ hba-hash_tbl_segments[i],
+ dma_address);
+ }
+ }
+ kfree(hba-hash_tbl_segments);
+ hba-hash_tbl_segments = NULL;
}
if (hba-hash_tbl_pbl) {
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] bnx2fc: fix memory leak and potential NULL pointer dereference.
On Sat, 2014-03-08 at 00:21 +0100, Maurizio Lombardi wrote:
Hi Eddie,
On Fri, Mar 07, 2014 at 11:21:45AM -0800, Eddie Wai wrote:
Hello Maurizio,
Thanks for looking into this. The patch looks good, but I would like to
extend it with the following allocation failure portion to completely
remove the memory leak. Please incorporate this into your next
submission.
Thanks for the patch you sent,
maybe it is even better if I modify bnx2fc_allocate_hash_table() in such a way
that, in case of error, it frees all the memory it has allocated.
Note that your patch does not free the table if
hba-hash_tbl_pbl = dma_alloc_coherent(...) fails (line 2058).
It just requires a very little change to your patch, I'll send it tomorrow.
You're right, the hash_tbl_segments[i] entries will also need to be
freed upon the hash_tbl_pbl allocation failure. Thanks again!
Regards,
Maurizio Lombardi
Thanks,
Eddie
@@ -2020,7 +2026,7 @@ static int bnx2fc_allocate_hash_table(struct
bnx2fc_hba *hba)
dma_segment_array = kzalloc(dma_segment_array_size, GFP_KERNEL);
if (!dma_segment_array) {
printk(KERN_ERR PFX hash table pointers (dma) alloc
failed\n);
- return -ENOMEM;
+ goto free_hash_tbl_seg;
}
for (i = 0; i segment_count; ++i) {
@@ -2039,7 +2045,7 @@ static int bnx2fc_allocate_hash_table(struct
bnx2fc_hba *hba)
hba-hash_tbl_segments[i] = NULL;
}
kfree(dma_segment_array);
- return -ENOMEM;
+ goto free_hash_tbl_seg;
}
memset(hba-hash_tbl_segments[i], 0,
BNX2FC_HASH_TBL_CHUNK_SIZE);
@@ -2077,6 +2083,11 @@ static int bnx2fc_allocate_hash_table(struct
bnx2fc_hba *hba)
}
kfree(dma_segment_array);
return 0;
+
+free_hash_tbl_seg:
+ kfree(hba-hash_tbl_segments);
+ hba-hash_tbl_segments = NULL;
+ return -ENOMEM;
}
On Fri, 2014-03-07 at 15:37 +0100, Maurizio Lombardi wrote:
If bnx2fc_allocate_hash_table() for some reasons fails, it is possible
that the
hash_tbl_segments or the hash_tbl_pbl pointers are NULL.
In this case bnx2fc_free_hash_table() will panic the system.
this patch also fixes a memory leak, the hash_tbl_segments pointer was
never
freed.
Signed-off-by: Maurizio Lombardi mlomb...@redhat.com
---
drivers/scsi/bnx2fc/bnx2fc_hwi.c | 34 --
1 file changed, 20 insertions(+), 14 deletions(-)
diff --git a/drivers/scsi/bnx2fc/bnx2fc_hwi.c
b/drivers/scsi/bnx2fc/bnx2fc_hwi.c
index 261af2a..f83bae4 100644
--- a/drivers/scsi/bnx2fc/bnx2fc_hwi.c
+++ b/drivers/scsi/bnx2fc/bnx2fc_hwi.c
@@ -1968,21 +1968,27 @@ static void bnx2fc_free_hash_table(struct
bnx2fc_hba *hba)
int segment_count;
u32 *pbl;
- segment_count = hba-hash_tbl_segment_count;
-
- pbl = hba-hash_tbl_pbl;
- for (i = 0; i segment_count; ++i) {
- dma_addr_t dma_address;
-
- dma_address = le32_to_cpu(*pbl);
- ++pbl;
- dma_address += ((u64)le32_to_cpu(*pbl)) 32;
- ++pbl;
- dma_free_coherent(hba-pcidev-dev,
- BNX2FC_HASH_TBL_CHUNK_SIZE,
- hba-hash_tbl_segments[i],
- dma_address);
+ if (hba-hash_tbl_segments) {
+
+ pbl = hba-hash_tbl_pbl;
+ if (pbl) {
+ segment_count = hba-hash_tbl_segment_count;
+ for (i = 0; i segment_count; ++i) {
+ dma_addr_t dma_address;
+
+ dma_address = le32_to_cpu(*pbl);
+ ++pbl;
+ dma_address += ((u64)le32_to_cpu(*pbl)) 32;
+ ++pbl;
+ dma_free_coherent(hba-pcidev-dev,
+ BNX2FC_HASH_TBL_CHUNK_SIZE,
+ hba-hash_tbl_segments[i],
+ dma_address);
+ }
+ }
+ kfree(hba-hash_tbl_segments);
+ hba-hash_tbl_segments = NULL;
}
if (hba-hash_tbl_pbl) {
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
re: [SCSI] bnx2fc: Broadcom FCoE offload driver
Hello Dan,
Thanks for bringing this to our attention. We'll fix it upon our next
patchset submission.
Thanks,
Eddie
On Thu, 2014-02-13 at 12:52 +0300, Dan Carpenter wrote:
Hello Bhanu Gollapudi,
The patch 853e2bd2103a: [SCSI] bnx2fc: Broadcom FCoE offload driver
from Feb 4, 2011, leads to the following static checker warning:
drivers/scsi/bnx2fc/bnx2fc_hwi.c:1660 bnx2fc_init_mp_task()
warn: we tested 'task_type == 3' before and it was 'false'
drivers/scsi/bnx2fc/bnx2fc_hwi.c
1586 void bnx2fc_init_mp_task(struct bnx2fc_cmd *io_req,
1587 struct fcoe_task_ctx_entry *task)
1588 {
1589 struct bnx2fc_mp_req *mp_req = (io_req-mp_req);
1590 struct bnx2fc_rport *tgt = io_req-tgt;
1591 struct fc_frame_header *fc_hdr;
1592 struct fcoe_ext_mul_sges_ctx *sgl;
1593 u8 task_type = 0;
^
Should this be FCOE_TASK_TYPE_UNSOLICITED (3)?
UNSOLICITED task type is not being used by bnx2fc at the moment. The
more preferable fix would be to un-initialize this and then change the
cmd_type to a switch statement and supply a default exit. We don't want
to see the firmware getting initialized by uninitialized requests.
1594 u64 *hdr;
1595 u64 temp_hdr[3];
1596 u32 context_id;
1597
1598
1599 /* Obtain task_type */
1600 if ((io_req-cmd_type == BNX2FC_TASK_MGMT_CMD) ||
1601 (io_req-cmd_type == BNX2FC_ELS)) {
1602 task_type = FCOE_TASK_TYPE_MIDPATH;
1603 } else if (io_req-cmd_type == BNX2FC_ABTS) {
1604 task_type = FCOE_TASK_TYPE_ABTS;
1605 }
1606
1607 memset(task, 0, sizeof(struct fcoe_task_ctx_entry));
1608
1609 /* Setup the task from io_req for easy reference */
1610 io_req-task = task;
1611
1612 BNX2FC_IO_DBG(io_req, Init MP task for cmd_type = %d
task_type = %d\n,
1613 io_req-cmd_type, task_type);
1614
1615 /* Tx only */
1616 if ((task_type == FCOE_TASK_TYPE_MIDPATH) ||
1617 (task_type == FCOE_TASK_TYPE_UNSOLICITED)) {
^^
Not possible.
1618 task-txwr_only.sgl_ctx.sgl.mul_sgl.cur_sge_addr.lo =
1619 (u32)mp_req-mp_req_bd_dma;
1620 task-txwr_only.sgl_ctx.sgl.mul_sgl.cur_sge_addr.hi =
1621 (u32)((u64)mp_req-mp_req_bd_dma
32);
1622 task-txwr_only.sgl_ctx.sgl.mul_sgl.sgl_size = 1;
1623 }
regards,
dan carpenter
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[LSF/MM TOPIC] [ATTEND] scsi_eh/scsi-mq changes
Hello, I would like to attend LSF/MM 2014 to participate in the discussions about the proposed SCSI error handling and the impending scsi-mq changes from the LLDD perspective. Eddie -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/3] BNX2FC: Fixed scsi_remove_target soft lockup when rmmod bnx2x
The problem has been identified to be a change in the scsi_remove_device path where a call to the pm_runtime_set_memalloc_noio was added when del_gendisk is called in this path. Note that the new pm routine attempts to cycle through all parent devices from the FC target device to set the memalloc_noio flag. Because of this new change, a dependency was created between the FC target device and the parent netdev device in the destroy path. In order to synchronized the destroy paths, bnx2fc has been modified to flush all destroy workqueues in the NETDEV_UNREGISTER return path. [4.123584] BUG: soft lockup - CPU#8 stuck for 22s! [kworker/8:3:8082] [4.123713] Call Trace: [4.123719] [815dfbe0] klist_next+0x20/0xf0 [4.123725] [813e9220] ? pm_save_wakeup_count+0x70/0x70 [4.123731] [813d9e4e] device_for_each_child+0x4e/0x70 [4.123735] [813e9554] pm_runtime_set_memalloc_noio+0x94/0xf0 [4.123740] [812d4d74] del_gendisk+0x264/0x2a0 [4.123747] [a00c6dc9] sd_remove+0x69/0xb0 [sd_mod] [4.123751] [813de24f] __device_release_driver+0x7f/0xf0 [4.123754] [813de2e3] device_release_driver+0x23/0x30 [4.123757] [813ddab4] bus_remove_device+0xf4/0x170 [4.123760] [813da475] device_del+0x135/0x1d0 [4.123765] [81411b75] __scsi_remove_device+0xc5/0xd0 [4.123768] [81411ba6] scsi_remove_device+0x26/0x40 [4.123770] [81411d40] scsi_remove_target+0x160/0x210 [4.123775] [a0420e4c] fc_rport_final_delete+0xac/0x1f0 [scsi_transport_fc] [4.123780] [810774ab] process_one_work+0x17b/0x460 [4.123783] [8107825b] worker_thread+0x11b/0x400 [4.123786] [81078140] ? rescuer_thread+0x3e0/0x3e0 [4.123791] [8107e9c0] kthread+0xc0/0xd0 [4.123794] [8107e900] ? kthread_create_on_node+0x110/0x110 [4.123798] [8160ceec] ret_from_fork+0x7c/0xb0 [4.123801] [8107e900] ? kthread_create_on_node+0x110/0x110 Signed-off-by: Eddie Wai eddie@broadcom.com --- drivers/scsi/bnx2fc/bnx2fc_fcoe.c |6 ++ 1 files changed, 6 insertions(+), 0 deletions(-) diff --git a/drivers/scsi/bnx2fc/bnx2fc_fcoe.c b/drivers/scsi/bnx2fc/bnx2fc_fcoe.c index 9b94850..4cabd3e 100644 --- a/drivers/scsi/bnx2fc/bnx2fc_fcoe.c +++ b/drivers/scsi/bnx2fc/bnx2fc_fcoe.c @@ -850,6 +850,9 @@ static void bnx2fc_indicate_netevent(void *context, unsigned long event, __bnx2fc_destroy(interface); } mutex_unlock(bnx2fc_dev_lock); + + /* Ensure ALL destroy work has been completed before return */ + flush_workqueue(bnx2fc_wq); return; default: @@ -2389,6 +2392,9 @@ static void bnx2fc_ulp_exit(struct cnic_dev *dev) __bnx2fc_destroy(interface); mutex_unlock(bnx2fc_dev_lock); + /* Ensure ALL destroy work has been completed before return */ + flush_workqueue(bnx2fc_wq); + bnx2fc_ulp_stop(hba); /* unregister cnic device */ if (test_and_clear_bit(BNX2FC_CNIC_REGISTERED, hba-reg_with_cnic)) -- 1.7.1 -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 3/3] BNX2FC: Updated version to 2.4.2
Old version: 2.4.1 New version: 2.4.2 Signed-off-by: Eddie Wai eddie@broadcom.com --- drivers/scsi/bnx2fc/bnx2fc.h |2 +- drivers/scsi/bnx2fc/bnx2fc_fcoe.c |2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/scsi/bnx2fc/bnx2fc.h b/drivers/scsi/bnx2fc/bnx2fc.h index 2e984e3..6a97665 100644 --- a/drivers/scsi/bnx2fc/bnx2fc.h +++ b/drivers/scsi/bnx2fc/bnx2fc.h @@ -64,7 +64,7 @@ #include bnx2fc_constants.h #define BNX2FC_NAMEbnx2fc -#define BNX2FC_VERSION 2.4.1 +#define BNX2FC_VERSION 2.4.2 #define PFXbnx2fc: diff --git a/drivers/scsi/bnx2fc/bnx2fc_fcoe.c b/drivers/scsi/bnx2fc/bnx2fc_fcoe.c index 4cabd3e..6287f6a 100644 --- a/drivers/scsi/bnx2fc/bnx2fc_fcoe.c +++ b/drivers/scsi/bnx2fc/bnx2fc_fcoe.c @@ -22,7 +22,7 @@ DEFINE_PER_CPU(struct bnx2fc_percpu_s, bnx2fc_percpu); #define DRV_MODULE_NAMEbnx2fc #define DRV_MODULE_VERSION BNX2FC_VERSION -#define DRV_MODULE_RELDATE Sep 17, 2013 +#define DRV_MODULE_RELDATE Dec 11, 2013 static char version[] = -- 1.7.1 -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/3] BNX2FC: Fixed the handling for the SCSI retry delay
SCSI retry delay upon SAM_STAT_BUSY/_SET_FULL was not being handled
in bnx2fc. This patch adds such handling by returning TARGET_BUSY
to the SCSI ML for the corresponding LUN until the retry timer expires.
Signed-off-by: Eddie Wai eddie@broadcom.com
---
drivers/scsi/bnx2fc/bnx2fc.h |1 +
drivers/scsi/bnx2fc/bnx2fc_io.c | 19 ++-
drivers/scsi/bnx2fc/bnx2fc_tgt.c |1 +
3 files changed, 20 insertions(+), 1 deletions(-)
diff --git a/drivers/scsi/bnx2fc/bnx2fc.h b/drivers/scsi/bnx2fc/bnx2fc.h
index 1ebf3fb..2e984e3 100644
--- a/drivers/scsi/bnx2fc/bnx2fc.h
+++ b/drivers/scsi/bnx2fc/bnx2fc.h
@@ -367,6 +367,7 @@ struct bnx2fc_rport {
atomic_t num_active_ios;
u32 flush_in_prog;
unsigned long timestamp;
+ unsigned long retry_delay_timestamp;
struct list_head free_task_list;
struct bnx2fc_cmd *pending_queue[BNX2FC_SQ_WQES_MAX+1];
struct list_head active_cmd_queue;
diff --git a/drivers/scsi/bnx2fc/bnx2fc_io.c b/drivers/scsi/bnx2fc/bnx2fc_io.c
index ed88089..d2cabc9 100644
--- a/drivers/scsi/bnx2fc/bnx2fc_io.c
+++ b/drivers/scsi/bnx2fc/bnx2fc_io.c
@@ -1871,7 +1871,15 @@ int bnx2fc_queuecommand(struct Scsi_Host *host,
rc = SCSI_MLQUEUE_TARGET_BUSY;
goto exit_qcmd;
}
-
+ if (tgt-retry_delay_timestamp) {
+ if (time_after(jiffies, tgt-retry_delay_timestamp)) {
+ tgt-retry_delay_timestamp = 0;
+ } else {
+ /* If retry_delay timer is active, flow off the ML */
+ rc = SCSI_MLQUEUE_TARGET_BUSY;
+ goto exit_qcmd;
+ }
+ }
io_req = bnx2fc_cmd_alloc(tgt);
if (!io_req) {
rc = SCSI_MLQUEUE_HOST_BUSY;
@@ -1961,6 +1969,15 @@ void bnx2fc_process_scsi_cmd_compl(struct bnx2fc_cmd
*io_req,
fcp_resid = 0x%x\n,
io_req-cdb_status, io_req-fcp_resid);
sc_cmd-result = (DID_OK 16) | io_req-cdb_status;
+
+ if (io_req-cdb_status == SAM_STAT_TASK_SET_FULL ||
+ io_req-cdb_status == SAM_STAT_BUSY) {
+ /* Set the jiffies + retry_delay_timer * 100ms
+ for the rport/tgt */
+ tgt-retry_delay_timestamp = jiffies +
+ fcp_rsp-retry_delay_timer * HZ / 10;
+ }
+
}
if (io_req-fcp_resid)
scsi_set_resid(sc_cmd, io_req-fcp_resid);
diff --git a/drivers/scsi/bnx2fc/bnx2fc_tgt.c b/drivers/scsi/bnx2fc/bnx2fc_tgt.c
index 4d93177..68948b7 100644
--- a/drivers/scsi/bnx2fc/bnx2fc_tgt.c
+++ b/drivers/scsi/bnx2fc/bnx2fc_tgt.c
@@ -386,6 +386,7 @@ static int bnx2fc_init_tgt(struct bnx2fc_rport *tgt,
tgt-rq_prod_idx = 0x8000;
tgt-rq_cons_idx = 0;
atomic_set(tgt-num_active_ios, 0);
+ tgt-retry_delay_timestamp = 0;
if (rdata-flags FC_RP_FLAGS_RETRY
rdata-ids.roles FC_RPORT_ROLE_FCP_TARGET
--
1.7.1
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 2/4] BNX2FC: hung task timeout warning observed when rmmod bnx2x with active FCoE targets
Tomas, I have already took this up to James yesterday and he said he will be fixing it over the weekend. Thanks, Eddie On Oct 18, 2013, at 6:49 AM, Tomas Henzl the...@redhat.com wrote: On 09/26/2013 07:01 AM, Eddie Wai wrote: [v2] - removed the interface-enabled flag setting which prevented the fcoe ctlr link from being brought back up after a MTU change Maybe I miss something but it seems to me that the previous version has been taken into the for-next branch - 881e188ec7078f6850170ca72cc60621fe0672e1 If it is so, and the V2 is the right one, post a V1-V2 diff Thanks, Tomas A rtnl_lock deadlock was observed from the rmmod thread where it tries to unregister the fcoe_ctlr device. This unregistration triggered a flush of the sysfs queue of the associated ctlr and led to a call to the set_fcoe_ctlr_enabled routine. This will eventually propagate down to call the bnx2fc_disable routine and contented for the rtnl_lock in the same context. This patch creates a subset of the bnx2fc_enable/disable routine which removes the unnecesary rtnl_lock and the bnx2fc_dev_lock acquisition from the set_fcoe_ctlr_enabled path. kernel: INFO: task rmmod:7874 blocked for more than 120 seconds. kernel: Tainted: GW --- 2.6.32-415.0.1.el6.x86_64 #1 kernel: echo 0 /proc/sys/kernel/hung_task_timeout_secs disables this message. kernel: rmmod D 000f 0 7874 6518 0x0080 kernel: 88022158f7d8 0086 kernel: 88023fe72600 88043c74d410 88043c74d400 88043c74d000 kernel: 88021ecbe5f8 88022158ffd8 fbc8 88021ecbe5f8 kernel: Call Trace: kernel: [81525985] schedule_timeout+0x215/0x2e0 kernel: [810680c0] ? pick_next_task_fair+0xd0/0x130 kernel: [81524858] ? schedule+0x178/0x3b2 kernel: [81525603] wait_for_common+0x123/0x180 kernel: [81066b40] ? default_wake_function+0x0/0x20 kernel: [811a486e] ? ifind_fast+0x5e/0xb0 kernel: [8152571d] wait_for_completion+0x1d/0x20 kernel: [81203868] sysfs_addrm_finish+0x228/0x270 kernel: [812014ab] sysfs_hash_and_remove+0x5b/0x90 kernel: [812056af] sysfs_remove_group+0x5f/0x100 kernel: [81367e8b] device_remove_groups+0x3b/0x60 kernel: [8136811d] device_remove_attrs+0x3d/0x90 kernel: [81368295] device_del+0x125/0x1e0 kernel: [81368372] device_unregister+0x22/0x60 kernel: [a038ead2] fcoe_ctlr_device_delete+0xe2/0xf4 [libfcoe] kernel: [a03c43cb] bnx2fc_interface_release+0x5b/0x90 [bnx2fc] kernel: [a03c4370] ? bnx2fc_interface_release+0x0/0x90 [bnx2fc] kernel: [812835e7] kref_put+0x37/0x70 kernel: [a03c4192] __bnx2fc_destroy+0x72/0xa0 [bnx2fc] kernel: [a03c5265] bnx2fc_ulp_exit+0xf5/0x160 [bnx2fc]- got bnx2fc_dev_lock mutex_lock kernel: [a03b03c6] cnic_ulp_exit+0xb6/0xc0 [cnic] kernel: [a03b5418] cnic_netdev_event+0x368/0x370 [cnic] kernel: [a038c56c] ? fcoe_del_netdev_mapping+0x8c/0xa0 [libfcoe] kernel: [8152a6e5] notifier_call_chain+0x55/0x80 kernel: [810a0a46] raw_notifier_call_chain+0x16/0x20 kernel: [81459beb] call_netdevice_notifiers+0x1b/0x20 kernel: [8145ab34] rollback_registered_many+0x154/0x280 kernel: [8145ad08] rollback_registered+0x38/0x50 kernel: [8145ad78] unregister_netdevice_queue+0x58/0xa0 kernel: [8145add0] unregister_netdevice+0x10/0x20 kernel: [8145adfe] unregister_netdev+0x1e/0x30 - got rtnl_lock! kernel: [a0122278] __bnx2x_remove+0x48/0x270 [bnx2x] - got rel rtnl_lock kernel: [a0122554] bnx2x_remove_one+0x44/0x80 [bnx2x] kernel: [812a3af7] pci_device_remove+0x37/0x70 kernel: [8136b2ef] __device_release_driver+0x6f/0xe0 kernel: [8136b428] driver_detach+0xc8/0xd0 kernel: [8136a22e] bus_remove_driver+0x8e/0x110 kernel: [8136bc12] driver_unregister+0x62/0xa0 kernel: [812a3e04] pci_unregister_driver+0x44/0xb0 kernel: [a0191954] bnx2x_cleanup+0x18/0x73 [bnx2x] kernel: [810b8be4] sys_delete_module+0x194/0x260 kernel: [810e1347] ? audit_syscall_entry+0x1d7/0x200 kernel: [8100b072] system_call_fastpath+0x16/0x1b Signed-off-by: Eddie Wai eddie@broadcom.com --- drivers/scsi/bnx2fc/bnx2fc_fcoe.c | 59 +++- 1 files changed, 44 insertions(+), 15 deletions(-) diff --git a/drivers/scsi/bnx2fc/bnx2fc_fcoe.c b/drivers/scsi/bnx2fc/bnx2fc_fcoe.c index 69ac554..7a25766 100644 --- a/drivers/scsi/bnx2fc/bnx2fc_fcoe.c +++ b/drivers/scsi/bnx2fc/bnx2fc_fcoe.c @@ -2004,6 +2004,24 @@ static void bnx2fc_ulp_init(struct cnic_dev *dev) set_bit(BNX2FC_CNIC_REGISTERED, hba-reg_with_cnic); } +/* Assumes rtnl_lock and the bnx2fc_dev_lock are already
[PATCH v2 2/4] BNX2FC: hung task timeout warning observed when rmmod bnx2x with active FCoE targets
[v2] - removed the interface-enabled flag setting which prevented the
fcoe ctlr link from being brought back up after a MTU change
A rtnl_lock deadlock was observed from the rmmod thread where it
tries to unregister the fcoe_ctlr device. This unregistration
triggered a flush of the sysfs queue of the associated ctlr and led to
a call to the set_fcoe_ctlr_enabled routine. This will eventually propagate
down to call the bnx2fc_disable routine and contented for the rtnl_lock
in the same context.
This patch creates a subset of the bnx2fc_enable/disable routine which
removes the unnecesary rtnl_lock and the bnx2fc_dev_lock acquisition from
the set_fcoe_ctlr_enabled path.
kernel: INFO: task rmmod:7874 blocked for more than 120 seconds.
kernel: Tainted: GW ---2.6.32-415.0.1.el6.x86_64
#1
kernel: echo 0 /proc/sys/kernel/hung_task_timeout_secs disables this
message.
kernel: rmmod D 000f 0 7874 6518 0x0080
kernel: 88022158f7d8 0086
kernel: 88023fe72600 88043c74d410 88043c74d400 88043c74d000
kernel: 88021ecbe5f8 88022158ffd8 fbc8 88021ecbe5f8
kernel: Call Trace:
kernel: [81525985] schedule_timeout+0x215/0x2e0
kernel: [810680c0] ? pick_next_task_fair+0xd0/0x130
kernel: [81524858] ? schedule+0x178/0x3b2
kernel: [81525603] wait_for_common+0x123/0x180
kernel: [81066b40] ? default_wake_function+0x0/0x20
kernel: [811a486e] ? ifind_fast+0x5e/0xb0
kernel: [8152571d] wait_for_completion+0x1d/0x20
kernel: [81203868] sysfs_addrm_finish+0x228/0x270
kernel: [812014ab] sysfs_hash_and_remove+0x5b/0x90
kernel: [812056af] sysfs_remove_group+0x5f/0x100
kernel: [81367e8b] device_remove_groups+0x3b/0x60
kernel: [8136811d] device_remove_attrs+0x3d/0x90
kernel: [81368295] device_del+0x125/0x1e0
kernel: [81368372] device_unregister+0x22/0x60
kernel: [a038ead2] fcoe_ctlr_device_delete+0xe2/0xf4 [libfcoe]
kernel: [a03c43cb] bnx2fc_interface_release+0x5b/0x90 [bnx2fc]
kernel: [a03c4370] ? bnx2fc_interface_release+0x0/0x90 [bnx2fc]
kernel: [812835e7] kref_put+0x37/0x70
kernel: [a03c4192] __bnx2fc_destroy+0x72/0xa0 [bnx2fc]
kernel: [a03c5265] bnx2fc_ulp_exit+0xf5/0x160 [bnx2fc]- got
bnx2fc_dev_lock mutex_lock
kernel: [a03b03c6] cnic_ulp_exit+0xb6/0xc0 [cnic]
kernel: [a03b5418] cnic_netdev_event+0x368/0x370 [cnic]
kernel: [a038c56c] ? fcoe_del_netdev_mapping+0x8c/0xa0 [libfcoe]
kernel: [8152a6e5] notifier_call_chain+0x55/0x80
kernel: [810a0a46] raw_notifier_call_chain+0x16/0x20
kernel: [81459beb] call_netdevice_notifiers+0x1b/0x20
kernel: [8145ab34] rollback_registered_many+0x154/0x280
kernel: [8145ad08] rollback_registered+0x38/0x50
kernel: [8145ad78] unregister_netdevice_queue+0x58/0xa0
kernel: [8145add0] unregister_netdevice+0x10/0x20
kernel: [8145adfe] unregister_netdev+0x1e/0x30 -
got rtnl_lock!
kernel: [a0122278] __bnx2x_remove+0x48/0x270 [bnx2x] - got rel
rtnl_lock
kernel: [a0122554] bnx2x_remove_one+0x44/0x80 [bnx2x]
kernel: [812a3af7] pci_device_remove+0x37/0x70
kernel: [8136b2ef] __device_release_driver+0x6f/0xe0
kernel: [8136b428] driver_detach+0xc8/0xd0
kernel: [8136a22e] bus_remove_driver+0x8e/0x110
kernel: [8136bc12] driver_unregister+0x62/0xa0
kernel: [812a3e04] pci_unregister_driver+0x44/0xb0
kernel: [a0191954] bnx2x_cleanup+0x18/0x73 [bnx2x]
kernel: [810b8be4] sys_delete_module+0x194/0x260
kernel: [810e1347] ? audit_syscall_entry+0x1d7/0x200
kernel: [8100b072] system_call_fastpath+0x16/0x1b
Signed-off-by: Eddie Wai eddie@broadcom.com
---
drivers/scsi/bnx2fc/bnx2fc_fcoe.c | 59 +++-
1 files changed, 44 insertions(+), 15 deletions(-)
diff --git a/drivers/scsi/bnx2fc/bnx2fc_fcoe.c
b/drivers/scsi/bnx2fc/bnx2fc_fcoe.c
index 69ac554..7a25766 100644
--- a/drivers/scsi/bnx2fc/bnx2fc_fcoe.c
+++ b/drivers/scsi/bnx2fc/bnx2fc_fcoe.c
@@ -2004,6 +2004,24 @@ static void bnx2fc_ulp_init(struct cnic_dev *dev)
set_bit(BNX2FC_CNIC_REGISTERED, hba-reg_with_cnic);
}
+/* Assumes rtnl_lock and the bnx2fc_dev_lock are already taken */
+static int __bnx2fc_disable(struct fcoe_ctlr *ctlr)
+{
+ struct bnx2fc_interface *interface = fcoe_ctlr_priv(ctlr);
+
+ if (interface-enabled == true) {
+ if (!ctlr-lp) {
+ pr_err(PFX __bnx2fc_disable: lport not found\n);
+ return -ENODEV;
+ } else {
+ interface-enabled = false;
+ fcoe_ctlr_link_down(ctlr
[PATCH 0/4] Fixed a race condition and a rtnl_lock deadlock for bnx2fc
Hello Robert and all, My name is Eddie and I'm taking over for Bhanu as the new maintainer for bnx2fc (I'm also the current maintainer for bnx2i). Please include me in all bnx2fc related discussions moving forward. Thanks! The following is a set of patches which fixes a race condition and a rtnl_lock in bnx2fc as described. The first patch fixes the race condition between bnx2fc's SCSI_CMD timeout handling and the SCSI layer's task abort handling. There is a situation where the corresponding scsi_done for the timed out SCSI_CMD would never get called. The second patch fixes a rtnl_lock deadlock in the rmmod bnx2x path where the fcoemon's write to the ctlr-enabled sysfs control param made the call to bnx2fc_disable which contented for the rtnl_lock. In this path of operation, there is no need to lock the netdev nor the interface as the ctlr is directly passed from the store_ctlr_enabled routine and the interface cannot be released until the this sysfs write is finished anyway. The __bnx2fc_enable/disable routine can continue to persist even after the bnx2fc_enable/disable are deprecated. I've also bumped the version to 2.4.1 in an effort to align the upstream and the out-of-tree releases of bnx2fc. Comments are welcome. Thanks. Eddie Eddie Wai (4): BNX2FC: Fixed a SCSI CMD cmpl race condition between ABTS and CLEANUP BNX2FC: hung task timeout warning observed when rmmod bnx2x with active FCoE targets BNX2FC: Bump version from 1.0.14 to 2.4.1 MAINTAINER: Updated maintainer info for bnx2fc MAINTAINERS |2 +- drivers/scsi/bnx2fc/bnx2fc.h |2 +- drivers/scsi/bnx2fc/bnx2fc_fcoe.c | 62 +++- drivers/scsi/bnx2fc/bnx2fc_io.c |6 +++ 4 files changed, 54 insertions(+), 18 deletions(-) -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 4/4] MAINTAINER: Updated maintainer info for bnx2fc
Signed-off-by: Eddie Wai eddie@broadcom.com --- MAINTAINERS |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/MAINTAINERS b/MAINTAINERS index e5e2518..26bc416 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -1825,7 +1825,7 @@ S:Supported F: drivers/net/wireless/brcm80211/ BROADCOM BNX2FC 10 GIGABIT FCOE DRIVER -M: Bhanu Prakash Gollapudi bprak...@broadcom.com +M: Eddie Wai eddie@broadcom.com L: linux-scsi@vger.kernel.org S: Supported F: drivers/scsi/bnx2fc/ -- 1.7.1 -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/4] BNX2FC: hung task timeout warning observed when rmmod bnx2x with active FCoE targets
A rtnl_lock deadlock was observed from the rmmod thread where it
tries to unregister the fcoe_ctlr device. This unregistration
triggered a flush of the sysfs queue of the associated ctlr and led to
a call to the set_fcoe_ctlr_enabled routine. This will eventually propagate
down to call the bnx2fc_disable routine and contented for the rtnl_lock
in the same context.
This patch creates a subset of the bnx2fc_enable/disable routine which
removes the unnecesary rtnl_lock and the bnx2fc_dev_lock acquisition from
the set_fcoe_ctlr_enabled path.
kernel: INFO: task rmmod:7874 blocked for more than 120 seconds.
kernel: Tainted: GW ---2.6.32-415.0.1.el6.x86_64
#1
kernel: echo 0 /proc/sys/kernel/hung_task_timeout_secs disables this
message.
kernel: rmmod D 000f 0 7874 6518 0x0080
kernel: 88022158f7d8 0086
kernel: 88023fe72600 88043c74d410 88043c74d400 88043c74d000
kernel: 88021ecbe5f8 88022158ffd8 fbc8 88021ecbe5f8
kernel: Call Trace:
kernel: [81525985] schedule_timeout+0x215/0x2e0
kernel: [810680c0] ? pick_next_task_fair+0xd0/0x130
kernel: [81524858] ? schedule+0x178/0x3b2
kernel: [81525603] wait_for_common+0x123/0x180
kernel: [81066b40] ? default_wake_function+0x0/0x20
kernel: [811a486e] ? ifind_fast+0x5e/0xb0
kernel: [8152571d] wait_for_completion+0x1d/0x20
kernel: [81203868] sysfs_addrm_finish+0x228/0x270
kernel: [812014ab] sysfs_hash_and_remove+0x5b/0x90
kernel: [812056af] sysfs_remove_group+0x5f/0x100
kernel: [81367e8b] device_remove_groups+0x3b/0x60
kernel: [8136811d] device_remove_attrs+0x3d/0x90
kernel: [81368295] device_del+0x125/0x1e0
kernel: [81368372] device_unregister+0x22/0x60
kernel: [a038ead2] fcoe_ctlr_device_delete+0xe2/0xf4 [libfcoe]
kernel: [a03c43cb] bnx2fc_interface_release+0x5b/0x90 [bnx2fc]
kernel: [a03c4370] ? bnx2fc_interface_release+0x0/0x90 [bnx2fc]
kernel: [812835e7] kref_put+0x37/0x70
kernel: [a03c4192] __bnx2fc_destroy+0x72/0xa0 [bnx2fc]
kernel: [a03c5265] bnx2fc_ulp_exit+0xf5/0x160 [bnx2fc]- got
bnx2fc_dev_lock mutex_lock
kernel: [a03b03c6] cnic_ulp_exit+0xb6/0xc0 [cnic]
kernel: [a03b5418] cnic_netdev_event+0x368/0x370 [cnic]
kernel: [a038c56c] ? fcoe_del_netdev_mapping+0x8c/0xa0 [libfcoe]
kernel: [8152a6e5] notifier_call_chain+0x55/0x80
kernel: [810a0a46] raw_notifier_call_chain+0x16/0x20
kernel: [81459beb] call_netdevice_notifiers+0x1b/0x20
kernel: [8145ab34] rollback_registered_many+0x154/0x280
kernel: [8145ad08] rollback_registered+0x38/0x50
kernel: [8145ad78] unregister_netdevice_queue+0x58/0xa0
kernel: [8145add0] unregister_netdevice+0x10/0x20
kernel: [8145adfe] unregister_netdev+0x1e/0x30 -
got rtnl_lock!
kernel: [a0122278] __bnx2x_remove+0x48/0x270 [bnx2x] - got rel
rtnl_lock
kernel: [a0122554] bnx2x_remove_one+0x44/0x80 [bnx2x]
kernel: [812a3af7] pci_device_remove+0x37/0x70
kernel: [8136b2ef] __device_release_driver+0x6f/0xe0
kernel: [8136b428] driver_detach+0xc8/0xd0
kernel: [8136a22e] bus_remove_driver+0x8e/0x110
kernel: [8136bc12] driver_unregister+0x62/0xa0
kernel: [812a3e04] pci_unregister_driver+0x44/0xb0
kernel: [a0191954] bnx2x_cleanup+0x18/0x73 [bnx2x]
kernel: [810b8be4] sys_delete_module+0x194/0x260
kernel: [810e1347] ? audit_syscall_entry+0x1d7/0x200
kernel: [8100b072] system_call_fastpath+0x16/0x1b
Signed-off-by: Eddie Wai eddie@broadcom.com
---
drivers/scsi/bnx2fc/bnx2fc_fcoe.c | 60 +++-
1 files changed, 45 insertions(+), 15 deletions(-)
diff --git a/drivers/scsi/bnx2fc/bnx2fc_fcoe.c
b/drivers/scsi/bnx2fc/bnx2fc_fcoe.c
index 69ac554..5d059cb 100644
--- a/drivers/scsi/bnx2fc/bnx2fc_fcoe.c
+++ b/drivers/scsi/bnx2fc/bnx2fc_fcoe.c
@@ -1809,6 +1809,7 @@ static void bnx2fc_stop(struct bnx2fc_interface
*interface)
FC_PORTTYPE_UNKNOWN;
mutex_unlock(lport-lp_mutex);
fc_host_port_type(lport-host) = FC_PORTTYPE_UNKNOWN;
+ interface-enabled = false;
fcoe_ctlr_link_down(ctlr);
fcoe_clean_pending_queue(lport);
}
@@ -2004,6 +2005,24 @@ static void bnx2fc_ulp_init(struct cnic_dev *dev)
set_bit(BNX2FC_CNIC_REGISTERED, hba-reg_with_cnic);
}
+/* Assumes rtnl_lock and the bnx2fc_dev_lock are already taken */
+static int __bnx2fc_disable(struct fcoe_ctlr *ctlr)
+{
+ struct bnx2fc_interface *interface = fcoe_ctlr_priv(ctlr);
+
+ if (interface-enabled == true) {
+ if (!ctlr-lp) {
+ pr_err(PFX __bnx2fc_disable: lport
[PATCH 1/4] BNX2FC: Fixed a SCSI CMD cmpl race condition between ABTS and CLEANUP
In the case when a SCSI_CMD times out, bnx2fc will initiate the sending of the ABTS. However, if the SCSI layer's SCSI command timer also times out, it'll instantiate a task abort of the same xid. The race condition this patch tries to fix is as follows: SCSI_CMD timeout (20s) thread 1 thread 2 send ABTS rx ABTS cmpl task abort_eh explicit LOGO since ABTS was engaged CLEANUP cmpl SCSI_CMD cmpl (ABTS cmpl) instantiate RRQ wait 10s attempt to send RRQ (because of LOGO, it wouldn't continue) Note that there is no call to scsi_done for this SCSI_CMD cmpletion in this path. The patch changes the path of execution to call scsi_done immediately instead of instantiating the RRQ. Signed-off-by: Eddie Wai eddie@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 575142e..ed88089 100644 --- a/drivers/scsi/bnx2fc/bnx2fc_io.c +++ b/drivers/scsi/bnx2fc/bnx2fc_io.c @@ -1246,6 +1246,12 @@ int bnx2fc_eh_abort(struct scsi_cmnd *sc_cmd) kref_put(io_req-refcount, bnx2fc_cmd_release); /* drop timer hold */ rc = bnx2fc_expl_logo(lport, io_req); + /* This only occurs when an task abort was requested while ABTS + is in progress. Setting the IO_CLEANUP flag will skip the + RRQ process in the case when the fw generated SCSI_CMD cmpl + was a result from the ABTS request rather than the CLEANUP + request */ + set_bit(BNX2FC_FLAG_IO_CLEANUP, io_req-req_flags); goto out; } -- 1.7.1 -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 3/4] BNX2FC: Bump version from 1.0.14 to 2.4.1
Signed-off-by: Eddie Wai eddie@broadcom.com --- drivers/scsi/bnx2fc/bnx2fc.h |2 +- drivers/scsi/bnx2fc/bnx2fc_fcoe.c |2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/scsi/bnx2fc/bnx2fc.h b/drivers/scsi/bnx2fc/bnx2fc.h index 08b22a9..6991027 100644 --- a/drivers/scsi/bnx2fc/bnx2fc.h +++ b/drivers/scsi/bnx2fc/bnx2fc.h @@ -64,7 +64,7 @@ #include bnx2fc_constants.h #define BNX2FC_NAMEbnx2fc -#define BNX2FC_VERSION 1.0.14 +#define BNX2FC_VERSION 2.4.1 #define PFXbnx2fc: diff --git a/drivers/scsi/bnx2fc/bnx2fc_fcoe.c b/drivers/scsi/bnx2fc/bnx2fc_fcoe.c index 5d059cb..ae5220e 100644 --- a/drivers/scsi/bnx2fc/bnx2fc_fcoe.c +++ b/drivers/scsi/bnx2fc/bnx2fc_fcoe.c @@ -22,7 +22,7 @@ DEFINE_PER_CPU(struct bnx2fc_percpu_s, bnx2fc_percpu); #define DRV_MODULE_NAMEbnx2fc #define DRV_MODULE_VERSION BNX2FC_VERSION -#define DRV_MODULE_RELDATE Mar 08, 2013 +#define DRV_MODULE_RELDATE Sep 17, 2013 static char version[] = -- 1.7.1 -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/2] MAINTAINER: Added maintainer info for bnx2i
Signed-off-by: Eddie Wai eddie@broadcom.com --- MAINTAINERS |6 ++ 1 files changed, 6 insertions(+), 0 deletions(-) diff --git a/MAINTAINERS b/MAINTAINERS index 50105f9..e297cb0 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -1816,6 +1816,12 @@ L: linux-scsi@vger.kernel.org S: Supported F: drivers/scsi/bnx2fc/ +BROADCOM BNX2I 1/10 GIGABIT iSCSI DRIVER +M: Eddie Wai eddie@broadcom.com +L: linux-scsi@vger.kernel.org +S: Supported +F: drivers/scsi/bnx2i/ + BROADCOM SPECIFIC AMBA DRIVER (BCMA) M: Rafał Miłecki zaj...@gmail.com L: linux-wirel...@vger.kernel.org -- 1.7.7.4 -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/2] BNX2I: Update version and copyright year 2013
Old version: 2.7.2.2 New version: 2.7.6.2 Signed-off-by: Eddie Wai eddie@broadcom.com --- drivers/scsi/bnx2i/57xx_iscsi_constants.h |2 +- drivers/scsi/bnx2i/57xx_iscsi_hsi.h |2 +- drivers/scsi/bnx2i/bnx2i.h|2 +- drivers/scsi/bnx2i/bnx2i_hwi.c|2 +- drivers/scsi/bnx2i/bnx2i_init.c |6 +++--- drivers/scsi/bnx2i/bnx2i_iscsi.c |2 +- drivers/scsi/bnx2i/bnx2i_sysfs.c |2 +- 7 files changed, 9 insertions(+), 9 deletions(-) diff --git a/drivers/scsi/bnx2i/57xx_iscsi_constants.h b/drivers/scsi/bnx2i/57xx_iscsi_constants.h index 25093a0..3d33767 100644 --- a/drivers/scsi/bnx2i/57xx_iscsi_constants.h +++ b/drivers/scsi/bnx2i/57xx_iscsi_constants.h @@ -1,6 +1,6 @@ /* 57xx_iscsi_constants.h: Broadcom NetXtreme II iSCSI HSI * - * Copyright (c) 2006 - 2012 Broadcom Corporation + * Copyright (c) 2006 - 2013 Broadcom Corporation * * This program is free software; you can redistribute it and/or modify * it under the terms of the GNU General Public License as published by diff --git a/drivers/scsi/bnx2i/57xx_iscsi_hsi.h b/drivers/scsi/bnx2i/57xx_iscsi_hsi.h index f2db5fe..37049e4 100644 --- a/drivers/scsi/bnx2i/57xx_iscsi_hsi.h +++ b/drivers/scsi/bnx2i/57xx_iscsi_hsi.h @@ -1,6 +1,6 @@ /* 57xx_iscsi_hsi.h: Broadcom NetXtreme II iSCSI HSI. * - * Copyright (c) 2006 - 2012 Broadcom Corporation + * Copyright (c) 2006 - 2013 Broadcom Corporation * * This program is free software; you can redistribute it and/or modify * it under the terms of the GNU General Public License as published by diff --git a/drivers/scsi/bnx2i/bnx2i.h b/drivers/scsi/bnx2i/bnx2i.h index f109e3b..6940f09 100644 --- a/drivers/scsi/bnx2i/bnx2i.h +++ b/drivers/scsi/bnx2i/bnx2i.h @@ -1,6 +1,6 @@ /* bnx2i.h: Broadcom NetXtreme II iSCSI driver. * - * Copyright (c) 2006 - 2012 Broadcom Corporation + * Copyright (c) 2006 - 2013 Broadcom Corporation * Copyright (c) 2007, 2008 Red Hat, Inc. All rights reserved. * Copyright (c) 2007, 2008 Mike Christie * diff --git a/drivers/scsi/bnx2i/bnx2i_hwi.c b/drivers/scsi/bnx2i/bnx2i_hwi.c index a28b03e..af3e675 100644 --- a/drivers/scsi/bnx2i/bnx2i_hwi.c +++ b/drivers/scsi/bnx2i/bnx2i_hwi.c @@ -1,6 +1,6 @@ /* bnx2i_hwi.c: Broadcom NetXtreme II iSCSI driver. * - * Copyright (c) 2006 - 2012 Broadcom Corporation + * Copyright (c) 2006 - 2013 Broadcom Corporation * Copyright (c) 2007, 2008 Red Hat, Inc. All rights reserved. * Copyright (c) 2007, 2008 Mike Christie * diff --git a/drivers/scsi/bnx2i/bnx2i_init.c b/drivers/scsi/bnx2i/bnx2i_init.c index 50fef69..b6f6f43 100644 --- a/drivers/scsi/bnx2i/bnx2i_init.c +++ b/drivers/scsi/bnx2i/bnx2i_init.c @@ -1,6 +1,6 @@ /* bnx2i.c: Broadcom NetXtreme II iSCSI driver. * - * Copyright (c) 2006 - 2012 Broadcom Corporation + * Copyright (c) 2006 - 2013 Broadcom Corporation * Copyright (c) 2007, 2008 Red Hat, Inc. All rights reserved. * Copyright (c) 2007, 2008 Mike Christie * @@ -18,8 +18,8 @@ static struct list_head adapter_list = LIST_HEAD_INIT(adapter_list); static u32 adapter_count; #define DRV_MODULE_NAMEbnx2i -#define DRV_MODULE_VERSION 2.7.2.2 -#define DRV_MODULE_RELDATE Apr 25, 2012 +#define DRV_MODULE_VERSION 2.7.6.2 +#define DRV_MODULE_RELDATE Jun 06, 2013 static char version[] = Broadcom NetXtreme II iSCSI Driver DRV_MODULE_NAME \ diff --git a/drivers/scsi/bnx2i/bnx2i_iscsi.c b/drivers/scsi/bnx2i/bnx2i_iscsi.c index 0056e47..fabeb88 100644 --- a/drivers/scsi/bnx2i/bnx2i_iscsi.c +++ b/drivers/scsi/bnx2i/bnx2i_iscsi.c @@ -1,7 +1,7 @@ /* * bnx2i_iscsi.c: Broadcom NetXtreme II iSCSI driver. * - * Copyright (c) 2006 - 2012 Broadcom Corporation + * Copyright (c) 2006 - 2013 Broadcom Corporation * Copyright (c) 2007, 2008 Red Hat, Inc. All rights reserved. * Copyright (c) 2007, 2008 Mike Christie * diff --git a/drivers/scsi/bnx2i/bnx2i_sysfs.c b/drivers/scsi/bnx2i/bnx2i_sysfs.c index c61cf7a..a0a3d9f 100644 --- a/drivers/scsi/bnx2i/bnx2i_sysfs.c +++ b/drivers/scsi/bnx2i/bnx2i_sysfs.c @@ -1,6 +1,6 @@ /* bnx2i_sysfs.c: Broadcom NetXtreme II iSCSI driver. * - * Copyright (c) 2004 - 2012 Broadcom Corporation + * Copyright (c) 2004 - 2013 Broadcom Corporation * * This program is free software; you can redistribute it and/or modify * it under the terms of the GNU General Public License as published by -- 1.7.7.4 -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2] LIBISCSI: Added new boot entries in the session sysfs
v2: Moved the new BOOT params to the end of the iscsi_param enum
This is the kernel part of the modification to extract the net params
from the ibft sysfs to the iface struct used for the connection
request upon sync_session in the open-iscsi util.
Three new session sysfs params are defined:
boot_root - holds the name of the /sys/firmware/ibft or iscsi_rootN
boot_nic - holds the ethernetN name
boot_target - holds the targetN name
Signed-off-by: Eddie Wai eddie@broadcom.com
---
drivers/scsi/libiscsi.c | 18 ++
drivers/scsi/scsi_transport_iscsi.c | 12
include/scsi/iscsi_if.h |5 +
include/scsi/libiscsi.h |4
4 files changed, 39 insertions(+), 0 deletions(-)
diff --git a/drivers/scsi/libiscsi.c b/drivers/scsi/libiscsi.c
index 5de9469..ae69dfc 100644
--- a/drivers/scsi/libiscsi.c
+++ b/drivers/scsi/libiscsi.c
@@ -2808,6 +2808,9 @@ void iscsi_session_teardown(struct iscsi_cls_session
*cls_session)
kfree(session-targetname);
kfree(session-targetalias);
kfree(session-initiatorname);
+ kfree(session-boot_root);
+ kfree(session-boot_nic);
+ kfree(session-boot_target);
kfree(session-ifacename);
iscsi_destroy_session(cls_session);
@@ -3248,6 +3251,12 @@ int iscsi_set_param(struct iscsi_cls_conn *cls_conn,
return iscsi_switch_str_param(session-ifacename, buf);
case ISCSI_PARAM_INITIATOR_NAME:
return iscsi_switch_str_param(session-initiatorname, buf);
+ case ISCSI_PARAM_BOOT_ROOT:
+ return iscsi_switch_str_param(session-boot_root, buf);
+ case ISCSI_PARAM_BOOT_NIC:
+ return iscsi_switch_str_param(session-boot_nic, buf);
+ case ISCSI_PARAM_BOOT_TARGET:
+ return iscsi_switch_str_param(session-boot_target, buf);
default:
return -ENOSYS;
}
@@ -3326,6 +3335,15 @@ int iscsi_session_get_param(struct iscsi_cls_session
*cls_session,
case ISCSI_PARAM_INITIATOR_NAME:
len = sprintf(buf, %s\n, session-initiatorname);
break;
+ case ISCSI_PARAM_BOOT_ROOT:
+ len = sprintf(buf, %s\n, session-boot_root);
+ break;
+ case ISCSI_PARAM_BOOT_NIC:
+ len = sprintf(buf, %s\n, session-boot_nic);
+ break;
+ case ISCSI_PARAM_BOOT_TARGET:
+ len = sprintf(buf, %s\n, session-boot_target);
+ break;
default:
return -ENOSYS;
}
diff --git a/drivers/scsi/scsi_transport_iscsi.c
b/drivers/scsi/scsi_transport_iscsi.c
index 133926b..abf7c40 100644
--- a/drivers/scsi/scsi_transport_iscsi.c
+++ b/drivers/scsi/scsi_transport_iscsi.c
@@ -3473,6 +3473,9 @@ iscsi_session_attr(tgt_reset_tmo,
ISCSI_PARAM_TGT_RESET_TMO, 0);
iscsi_session_attr(ifacename, ISCSI_PARAM_IFACE_NAME, 0);
iscsi_session_attr(initiatorname, ISCSI_PARAM_INITIATOR_NAME, 0);
iscsi_session_attr(targetalias, ISCSI_PARAM_TARGET_ALIAS, 0);
+iscsi_session_attr(boot_root, ISCSI_PARAM_BOOT_ROOT, 0);
+iscsi_session_attr(boot_nic, ISCSI_PARAM_BOOT_NIC, 0);
+iscsi_session_attr(boot_target, ISCSI_PARAM_BOOT_TARGET, 0);
static ssize_t
show_priv_session_state(struct device *dev, struct device_attribute *attr,
@@ -3568,6 +3571,9 @@ static struct attribute *iscsi_session_attrs[] = {
dev_attr_sess_ifacename.attr,
dev_attr_sess_initiatorname.attr,
dev_attr_sess_targetalias.attr,
+ dev_attr_sess_boot_root.attr,
+ dev_attr_sess_boot_nic.attr,
+ dev_attr_sess_boot_target.attr,
dev_attr_priv_sess_recovery_tmo.attr,
dev_attr_priv_sess_state.attr,
dev_attr_priv_sess_creator.attr,
@@ -3631,6 +3637,12 @@ static umode_t iscsi_session_attr_is_visible(struct
kobject *kobj,
param = ISCSI_PARAM_INITIATOR_NAME;
else if (attr == dev_attr_sess_targetalias.attr)
param = ISCSI_PARAM_TARGET_ALIAS;
+ else if (attr == dev_attr_sess_boot_root.attr)
+ param = ISCSI_PARAM_BOOT_ROOT;
+ else if (attr == dev_attr_sess_boot_nic.attr)
+ param = ISCSI_PARAM_BOOT_NIC;
+ else if (attr == dev_attr_sess_boot_target.attr)
+ param = ISCSI_PARAM_BOOT_TARGET;
else if (attr == dev_attr_priv_sess_recovery_tmo.attr)
return S_IRUGO | S_IWUSR;
else if (attr == dev_attr_priv_sess_state.attr)
diff --git a/include/scsi/iscsi_if.h b/include/scsi/iscsi_if.h
index fe7f06c..9d28ded 100644
--- a/include/scsi/iscsi_if.h
+++ b/include/scsi/iscsi_if.h
@@ -489,6 +489,11 @@ enum iscsi_param {
ISCSI_PARAM_CHAP_IN_IDX,
ISCSI_PARAM_CHAP_OUT_IDX,
+
+ ISCSI_PARAM_BOOT_ROOT,
+ ISCSI_PARAM_BOOT_NIC,
+ ISCSI_PARAM_BOOT_TARGET,
+
/* must always be last */
ISCSI_PARAM_MAX,
};
diff --git a/include/scsi/libiscsi.h b/include/scsi/libiscsi.h
index 09c041e..4265a4b
[PATCH] LIBISCSI: Added new boot entries in the session sysfs
This is the kernel part of the modification to extract the net params
from the ibft sysfs to the iface struct used for the connection
request upon sync_session in the open-iscsi util.
Three new session sysfs params are defined:
boot_root - holds the name of the /sys/firmware/ibft or iscsi_rootN
boot_nic - holds the ethernetN name
boot_target - holds the targetN name
Signed-off-by: Eddie Wai eddie@broadcom.com
---
drivers/scsi/libiscsi.c | 18 ++
drivers/scsi/scsi_transport_iscsi.c | 12
include/scsi/iscsi_if.h |4
include/scsi/libiscsi.h |4
4 files changed, 38 insertions(+), 0 deletions(-)
diff --git a/drivers/scsi/libiscsi.c b/drivers/scsi/libiscsi.c
index 5de9469..ae69dfc 100644
--- a/drivers/scsi/libiscsi.c
+++ b/drivers/scsi/libiscsi.c
@@ -2808,6 +2808,9 @@ void iscsi_session_teardown(struct iscsi_cls_session
*cls_session)
kfree(session-targetname);
kfree(session-targetalias);
kfree(session-initiatorname);
+ kfree(session-boot_root);
+ kfree(session-boot_nic);
+ kfree(session-boot_target);
kfree(session-ifacename);
iscsi_destroy_session(cls_session);
@@ -3248,6 +3251,12 @@ int iscsi_set_param(struct iscsi_cls_conn *cls_conn,
return iscsi_switch_str_param(session-ifacename, buf);
case ISCSI_PARAM_INITIATOR_NAME:
return iscsi_switch_str_param(session-initiatorname, buf);
+ case ISCSI_PARAM_BOOT_ROOT:
+ return iscsi_switch_str_param(session-boot_root, buf);
+ case ISCSI_PARAM_BOOT_NIC:
+ return iscsi_switch_str_param(session-boot_nic, buf);
+ case ISCSI_PARAM_BOOT_TARGET:
+ return iscsi_switch_str_param(session-boot_target, buf);
default:
return -ENOSYS;
}
@@ -3326,6 +3335,15 @@ int iscsi_session_get_param(struct iscsi_cls_session
*cls_session,
case ISCSI_PARAM_INITIATOR_NAME:
len = sprintf(buf, %s\n, session-initiatorname);
break;
+ case ISCSI_PARAM_BOOT_ROOT:
+ len = sprintf(buf, %s\n, session-boot_root);
+ break;
+ case ISCSI_PARAM_BOOT_NIC:
+ len = sprintf(buf, %s\n, session-boot_nic);
+ break;
+ case ISCSI_PARAM_BOOT_TARGET:
+ len = sprintf(buf, %s\n, session-boot_target);
+ break;
default:
return -ENOSYS;
}
diff --git a/drivers/scsi/scsi_transport_iscsi.c
b/drivers/scsi/scsi_transport_iscsi.c
index 133926b..abf7c40 100644
--- a/drivers/scsi/scsi_transport_iscsi.c
+++ b/drivers/scsi/scsi_transport_iscsi.c
@@ -3473,6 +3473,9 @@ iscsi_session_attr(tgt_reset_tmo,
ISCSI_PARAM_TGT_RESET_TMO, 0);
iscsi_session_attr(ifacename, ISCSI_PARAM_IFACE_NAME, 0);
iscsi_session_attr(initiatorname, ISCSI_PARAM_INITIATOR_NAME, 0);
iscsi_session_attr(targetalias, ISCSI_PARAM_TARGET_ALIAS, 0);
+iscsi_session_attr(boot_root, ISCSI_PARAM_BOOT_ROOT, 0);
+iscsi_session_attr(boot_nic, ISCSI_PARAM_BOOT_NIC, 0);
+iscsi_session_attr(boot_target, ISCSI_PARAM_BOOT_TARGET, 0);
static ssize_t
show_priv_session_state(struct device *dev, struct device_attribute *attr,
@@ -3568,6 +3571,9 @@ static struct attribute *iscsi_session_attrs[] = {
dev_attr_sess_ifacename.attr,
dev_attr_sess_initiatorname.attr,
dev_attr_sess_targetalias.attr,
+ dev_attr_sess_boot_root.attr,
+ dev_attr_sess_boot_nic.attr,
+ dev_attr_sess_boot_target.attr,
dev_attr_priv_sess_recovery_tmo.attr,
dev_attr_priv_sess_state.attr,
dev_attr_priv_sess_creator.attr,
@@ -3631,6 +3637,12 @@ static umode_t iscsi_session_attr_is_visible(struct
kobject *kobj,
param = ISCSI_PARAM_INITIATOR_NAME;
else if (attr == dev_attr_sess_targetalias.attr)
param = ISCSI_PARAM_TARGET_ALIAS;
+ else if (attr == dev_attr_sess_boot_root.attr)
+ param = ISCSI_PARAM_BOOT_ROOT;
+ else if (attr == dev_attr_sess_boot_nic.attr)
+ param = ISCSI_PARAM_BOOT_NIC;
+ else if (attr == dev_attr_sess_boot_target.attr)
+ param = ISCSI_PARAM_BOOT_TARGET;
else if (attr == dev_attr_priv_sess_recovery_tmo.attr)
return S_IRUGO | S_IWUSR;
else if (attr == dev_attr_priv_sess_state.attr)
diff --git a/include/scsi/iscsi_if.h b/include/scsi/iscsi_if.h
index fe7f06c..f9cc89b 100644
--- a/include/scsi/iscsi_if.h
+++ b/include/scsi/iscsi_if.h
@@ -487,6 +487,10 @@ enum iscsi_param {
ISCSI_PARAM_TGT_RESET_TMO,
ISCSI_PARAM_TARGET_ALIAS,
+ ISCSI_PARAM_BOOT_ROOT,
+ ISCSI_PARAM_BOOT_NIC,
+ ISCSI_PARAM_BOOT_TARGET,
+
ISCSI_PARAM_CHAP_IN_IDX,
ISCSI_PARAM_CHAP_OUT_IDX,
/* must always be last */
diff --git a/include/scsi/libiscsi.h b/include/scsi/libiscsi.h
index 09c041e..4265a4b 100644
--- a/include/scsi
[PATCH] SCSI: amd_iommu dma_boundary overflow
Hello,
For a 64-bit DMA capable PCIe storage HBA running under the 64-bit
AMD-VI IOMMU environment, the amd_iommu code was observed to hit an
overflow when it tries to page align the dma_parms-segment_boundary_mask.
This overflow would eventually trigger the BUG_ON in the iommu-helper's
iommu_is_span_boundary is_power_of_2 check.
A little back tracking shows that since the device is 64-bit DMA capable,
the pcidev-dma_mask was correctly set to DMA_BIT_MASK(64). This dma_mask
was then transferred to the SCSI host's dma_boundary param (by the iSCSI
driver) and was eventually used to populate the q-limits.seg_boundary_mask
(via blk_queue_segment_boundary) and the dma_parms-segment_boundary_mask
(via dma_set_seg_boundary) during the scsi queue allocation.
The code seems correct as it make sense to impose the same hardware
segment boundary limit on both the blk queue and the DMA code. It would
be an easy alternative to simply prevent the shost-dma_boundary from
being set to DMA_BIT_MASK(64), but it seems more correct to fix the
amd_iommu code itself to detect and handle this max 64-bit mask condition.
Please let me know your comments.
Thanks,
Eddie
Signed-off-by: Eddie Wai eddie@broadcom.com
---
drivers/iommu/amd_iommu.c |7 +--
1 files changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/iommu/amd_iommu.c b/drivers/iommu/amd_iommu.c
index d90a421..63185a1 100644
--- a/drivers/iommu/amd_iommu.c
+++ b/drivers/iommu/amd_iommu.c
@@ -1526,11 +1526,14 @@ static unsigned long dma_ops_area_alloc(struct device
*dev,
unsigned long boundary_size;
unsigned long address = -1;
unsigned long limit;
+ unsigned long mask;
next_bit = PAGE_SHIFT;
- boundary_size = ALIGN(dma_get_seg_boundary(dev) + 1,
- PAGE_SIZE) PAGE_SHIFT;
+ mask = dma_get_seg_boundary(dev);
+ boundary_size = mask + 1 ?
+ ALIGN(mask + 1, PAGE_SIZE) PAGE_SHIFT :
+ 1UL (BITS_PER_LONG - PAGE_SHIFT);
for (;i max_index; ++i) {
unsigned long offset = dom-aperture[i]-offset PAGE_SHIFT;
--
1.7.7.4
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] LIBISCSI: Added the new boot_nic entry in the session sysfs
This is the kernel part of the modification to extract the net params
from the ibft sysfs to the iface struct used for the connection
request upon sync_session in the open-iscsi util.
Signed-off-by: Eddie Wai eddie@broadcom.com
---
drivers/scsi/libiscsi.c |6 ++
drivers/scsi/scsi_transport_iscsi.c |4
include/scsi/iscsi_if.h |2 ++
include/scsi/libiscsi.h |2 ++
4 files changed, 14 insertions(+), 0 deletions(-)
diff --git a/drivers/scsi/libiscsi.c b/drivers/scsi/libiscsi.c
index 82c3fd4..4f4c154 100644
--- a/drivers/scsi/libiscsi.c
+++ b/drivers/scsi/libiscsi.c
@@ -2809,6 +2809,7 @@ void iscsi_session_teardown(struct iscsi_cls_session
*cls_session)
kfree(session-targetname);
kfree(session-targetalias);
kfree(session-initiatorname);
+ kfree(session-boot_nic);
kfree(session-ifacename);
iscsi_destroy_session(cls_session);
@@ -3248,6 +3249,8 @@ int iscsi_set_param(struct iscsi_cls_conn *cls_conn,
return iscsi_switch_str_param(session-ifacename, buf);
case ISCSI_PARAM_INITIATOR_NAME:
return iscsi_switch_str_param(session-initiatorname, buf);
+ case ISCSI_PARAM_BOOT_NIC:
+ return iscsi_switch_str_param(session-boot_nic, buf);
default:
return -ENOSYS;
}
@@ -3326,6 +3329,9 @@ int iscsi_session_get_param(struct iscsi_cls_session
*cls_session,
case ISCSI_PARAM_INITIATOR_NAME:
len = sprintf(buf, %s\n, session-initiatorname);
break;
+ case ISCSI_PARAM_BOOT_NIC:
+ len = sprintf(buf, %s\n, session-boot_nic);
+ break;
default:
return -ENOSYS;
}
diff --git a/drivers/scsi/scsi_transport_iscsi.c
b/drivers/scsi/scsi_transport_iscsi.c
index 31969f2..2cc28fe 100644
--- a/drivers/scsi/scsi_transport_iscsi.c
+++ b/drivers/scsi/scsi_transport_iscsi.c
@@ -2484,6 +2484,7 @@ iscsi_session_attr(tgt_reset_tmo,
ISCSI_PARAM_TGT_RESET_TMO, 0);
iscsi_session_attr(ifacename, ISCSI_PARAM_IFACE_NAME, 0);
iscsi_session_attr(initiatorname, ISCSI_PARAM_INITIATOR_NAME, 0);
iscsi_session_attr(targetalias, ISCSI_PARAM_TARGET_ALIAS, 0);
+iscsi_session_attr(boot_nic, ISCSI_PARAM_BOOT_NIC, 0);
static ssize_t
show_priv_session_state(struct device *dev, struct device_attribute *attr,
@@ -2570,6 +2571,7 @@ static struct attribute *iscsi_session_attrs[] = {
dev_attr_sess_ifacename.attr,
dev_attr_sess_initiatorname.attr,
dev_attr_sess_targetalias.attr,
+ dev_attr_sess_boot_nic.attr,
dev_attr_priv_sess_recovery_tmo.attr,
dev_attr_priv_sess_state.attr,
dev_attr_priv_sess_creator.attr,
@@ -2632,6 +2634,8 @@ static umode_t iscsi_session_attr_is_visible(struct
kobject *kobj,
param = ISCSI_PARAM_INITIATOR_NAME;
else if (attr == dev_attr_sess_targetalias.attr)
param = ISCSI_PARAM_TARGET_ALIAS;
+ else if (attr == dev_attr_sess_boot_nic.attr)
+ param = ISCSI_PARAM_BOOT_NIC;
else if (attr == dev_attr_priv_sess_recovery_tmo.attr)
return S_IRUGO | S_IWUSR;
else if (attr == dev_attr_priv_sess_state.attr)
diff --git a/include/scsi/iscsi_if.h b/include/scsi/iscsi_if.h
index 917741b..874c8f3 100644
--- a/include/scsi/iscsi_if.h
+++ b/include/scsi/iscsi_if.h
@@ -452,6 +452,8 @@ enum iscsi_param {
ISCSI_PARAM_TGT_RESET_TMO,
ISCSI_PARAM_TARGET_ALIAS,
+ ISCSI_PARAM_BOOT_NIC,
+
ISCSI_PARAM_CHAP_IN_IDX,
ISCSI_PARAM_CHAP_OUT_IDX,
/* must always be last */
diff --git a/include/scsi/libiscsi.h b/include/scsi/libiscsi.h
index 6e33386..7ba5cc8 100644
--- a/include/scsi/libiscsi.h
+++ b/include/scsi/libiscsi.h
@@ -287,6 +287,8 @@ struct iscsi_session {
char*targetalias;
char*ifacename;
char*initiatorname;
+ char*boot_nic;
+
/* control data */
struct iscsi_transport *tt;
struct Scsi_Host*host;
--
1.7.7.4
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] BNX2I: Removed the individual PCI DEVICE ID checking
Removed the individual PCI DEVICE ID checking inside bnx2i. The device
type can easily be read from the corresponding cnic-flags. This will
free bnx2i from having to get updated for every new device ID that gets
added.
Signed-off-by: Eddie Wai eddie@broadcom.com
Acked-by: Michael Chan mc...@broadcom.com
---
drivers/scsi/bnx2i/bnx2i.h |2 +-
drivers/scsi/bnx2i/bnx2i_init.c | 43 +++---
drivers/scsi/bnx2i/bnx2i_iscsi.c |2 +-
3 files changed, 19 insertions(+), 28 deletions(-)
diff --git a/drivers/scsi/bnx2i/bnx2i.h b/drivers/scsi/bnx2i/bnx2i.h
index 3f9e706..b44d04e 100644
--- a/drivers/scsi/bnx2i/bnx2i.h
+++ b/drivers/scsi/bnx2i/bnx2i.h
@@ -800,7 +800,7 @@ extern struct device_attribute *bnx2i_dev_attributes[];
/*
* Function Prototypes
*/
-extern void bnx2i_identify_device(struct bnx2i_hba *hba);
+extern void bnx2i_identify_device(struct bnx2i_hba *hba, struct cnic_dev *dev);
extern void bnx2i_ulp_init(struct cnic_dev *dev);
extern void bnx2i_ulp_exit(struct cnic_dev *dev);
diff --git a/drivers/scsi/bnx2i/bnx2i_init.c b/drivers/scsi/bnx2i/bnx2i_init.c
index b17637a..ee009e4ad 100644
--- a/drivers/scsi/bnx2i/bnx2i_init.c
+++ b/drivers/scsi/bnx2i/bnx2i_init.c
@@ -79,42 +79,33 @@ static struct notifier_block bnx2i_cpu_notifier = {
/**
* bnx2i_identify_device - identifies NetXtreme II device type
* @hba: Adapter structure pointer
+ * @cnic: Corresponding cnic device
*
* This function identifies the NX2 device type and sets appropriate
* queue mailbox register access method, 5709 requires driver to
* access MBOX regs using *bin* mode
*/
-void bnx2i_identify_device(struct bnx2i_hba *hba)
+void bnx2i_identify_device(struct bnx2i_hba *hba, struct cnic_dev *dev)
{
hba-cnic_dev_type = 0;
- if ((hba-pci_did == PCI_DEVICE_ID_NX2_5706) ||
- (hba-pci_did == PCI_DEVICE_ID_NX2_5706S))
- set_bit(BNX2I_NX2_DEV_5706, hba-cnic_dev_type);
- else if ((hba-pci_did == PCI_DEVICE_ID_NX2_5708) ||
- (hba-pci_did == PCI_DEVICE_ID_NX2_5708S))
- set_bit(BNX2I_NX2_DEV_5708, hba-cnic_dev_type);
- else if ((hba-pci_did == PCI_DEVICE_ID_NX2_5709) ||
- (hba-pci_did == PCI_DEVICE_ID_NX2_5709S)) {
- set_bit(BNX2I_NX2_DEV_5709, hba-cnic_dev_type);
- hba-mail_queue_access = BNX2I_MQ_BIN_MODE;
- } else if (hba-pci_did == PCI_DEVICE_ID_NX2_57710||
- hba-pci_did == PCI_DEVICE_ID_NX2_57711||
- hba-pci_did == PCI_DEVICE_ID_NX2_57711E ||
- hba-pci_did == PCI_DEVICE_ID_NX2_57712||
- hba-pci_did == PCI_DEVICE_ID_NX2_57712E ||
- hba-pci_did == PCI_DEVICE_ID_NX2_57800||
- hba-pci_did == PCI_DEVICE_ID_NX2_57800_MF ||
- hba-pci_did == PCI_DEVICE_ID_NX2_57800_VF ||
- hba-pci_did == PCI_DEVICE_ID_NX2_57810||
- hba-pci_did == PCI_DEVICE_ID_NX2_57810_MF ||
- hba-pci_did == PCI_DEVICE_ID_NX2_57810_VF ||
- hba-pci_did == PCI_DEVICE_ID_NX2_57840||
- hba-pci_did == PCI_DEVICE_ID_NX2_57840_MF ||
- hba-pci_did == PCI_DEVICE_ID_NX2_57840_VF)
+ if (test_bit(CNIC_F_BNX2_CLASS, dev-flags)) {
+ if (hba-pci_did == PCI_DEVICE_ID_NX2_5706 ||
+ hba-pci_did == PCI_DEVICE_ID_NX2_5706S) {
+ set_bit(BNX2I_NX2_DEV_5706, hba-cnic_dev_type);
+ } else if (hba-pci_did == PCI_DEVICE_ID_NX2_5708 ||
+ hba-pci_did == PCI_DEVICE_ID_NX2_5708S) {
+ set_bit(BNX2I_NX2_DEV_5708, hba-cnic_dev_type);
+ } else if (hba-pci_did == PCI_DEVICE_ID_NX2_5709 ||
+ hba-pci_did == PCI_DEVICE_ID_NX2_5709S) {
+ set_bit(BNX2I_NX2_DEV_5709, hba-cnic_dev_type);
+ hba-mail_queue_access = BNX2I_MQ_BIN_MODE;
+ }
+ } else if (test_bit(CNIC_F_BNX2X_CLASS, dev-flags)) {
set_bit(BNX2I_NX2_DEV_57710, hba-cnic_dev_type);
- else
+ } else {
printk(KERN_ALERT bnx2i: unknown device, 0x%x\n,
hba-pci_did);
+ }
}
diff --git a/drivers/scsi/bnx2i/bnx2i_iscsi.c b/drivers/scsi/bnx2i/bnx2i_iscsi.c
index 3b34c13..0056e47 100644
--- a/drivers/scsi/bnx2i/bnx2i_iscsi.c
+++ b/drivers/scsi/bnx2i/bnx2i_iscsi.c
@@ -808,7 +808,7 @@ struct bnx2i_hba *bnx2i_alloc_hba(struct cnic_dev *cnic)
hba-pci_func = PCI_FUNC(hba-pcidev-devfn);
hba-pci_devno = PCI_SLOT(hba-pcidev-devfn);
- bnx2i_identify_device(hba);
+ bnx2i_identify_device(hba, cnic);
bnx2i_setup_host_queue_size(hba, shost);
hba-reg_base = pci_resource_start(hba-pcidev, 0);
--
1.7.7.4
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body
[PATCH] BNX2I: Fixed NULL ptr deference for 1G bnx2 Linux iSCSI offload
This patch fixes the following kernel panic invoked by uninitialized fields in the chip initialization for the 1G bnx2 iSCSI offload. One of the bits in the chip initialization is being used by the latest firmware to control overflow packets. When this control bit gets enabled erroneously, it would ultimately result in a bad packet placement which would cause the bnx2 driver to dereference a NULL ptr in the placement handler. This can happen under certain stress I/O environment under the Linux iSCSI offload operation. This change only affects Broadcom's 5709 chipset. Unable to handle kernel NULL pointer dereference at 0008 RIP: [881f0e7d] :bnx2:bnx2_poll_work+0xd0d/0x13c5 Pid: 0, comm: swapper Tainted: G 2.6.18-333.el5debug #2 RIP: 0010:[881f0e7d] [881f0e7d] :bnx2:bnx2_poll_work+0xd0d/0x13c5 RSP: 0018:8101b575bd50 EFLAGS: 00010216 RAX: 0005 RBX: 81007c5fb180 RCX: RDX: 0ffc RSI: 817e8000 RDI: 0220 RBP: 81015bbd7ec0 R08: 8100817e9000 R09: R10: 81007c5fb180 R11: 00c8 R12: 7a25a010 R13: R14: 0005 R15: 810159f80558 FS: () GS:8101afebc240() knlGS: CS: 0010 DS: 0018 ES: 0018 CR0: 8005003b CR2: 0008 CR3: 00201000 CR4: 06a0 Process swapper (pid: 0, threadinfo 8101b5754000, task 8101afebd820) Stack: 000b 810159f8 0040 810159f80520 810159f80500 00cf00cf8008e84b c200100939e0 810009035b20 5029 00be0001 8100817e7810 00d08101b575bea8 Call Trace: IRQ [8008e0d0] show_schedstat+0x1c2/0x25b [881f1886] :bnx2:bnx2_poll+0xf6/0x231 [8000c9b9] net_rx_action+0xac/0x1b1 [800125a0] __do_softirq+0x89/0x133 [8005e30c] call_softirq+0x1c/0x28 [8006d5de] do_softirq+0x2c/0x7d [8006d46e] do_IRQ+0xee/0xf7 [8005d625] ret_from_intr+0x0/0xa EOI [801a5780] acpi_processor_idle_simple+0x1c5/0x341 [801a573d] acpi_processor_idle_simple+0x182/0x341 [801a55bb] acpi_processor_idle_simple+0x0/0x341 [80049560] cpu_idle+0x95/0xb8 [80078b1c] start_secondary+0x479/0x488 Signed-off-by: Eddie Wai eddie@broadcom.com Cc: sta...@kernel.org --- drivers/scsi/bnx2i/bnx2i_hwi.c |3 +++ 1 files changed, 3 insertions(+), 0 deletions(-) diff --git a/drivers/scsi/bnx2i/bnx2i_hwi.c b/drivers/scsi/bnx2i/bnx2i_hwi.c index 86a12b4..3878e62 100644 --- a/drivers/scsi/bnx2i/bnx2i_hwi.c +++ b/drivers/scsi/bnx2i/bnx2i_hwi.c @@ -1264,6 +1264,9 @@ int bnx2i_send_fw_iscsi_init_msg(struct bnx2i_hba *hba) int rc = 0; u64 mask64; + memset(iscsi_init, 0x00, sizeof(struct iscsi_kwqe_init1)); + memset(iscsi_init2, 0x00, sizeof(struct iscsi_kwqe_init2)); + bnx2i_adjust_qp_size(hba); iscsi_init.flags = -- 1.7.7.4 -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
