[PATCH 0/2] qla2xxx: Patches for 4.3.0-rc5+ scsi "misc" branch.

[PATCH 0/2] qla2xxx: Patches for 4.3.0-rc5+ scsi "misc" branch.

[Himanshu Madhani]

Hi James, Christoph,

Please apply the following patches to the scsi tree, misc branch  at your
earliest convenience.

Thanks,
-Himanshu

Himanshu Madhani (2):
  qla2xxx: Remove unavailable firmware files
  qla2xxx: fix rwlock recursion

 drivers/scsi/qla2xxx/qla_nx.c |   58 ++++++++++++++++++++--------------------
 drivers/scsi/qla2xxx/qla_os.c |   29 --------------------
 2 files changed, 29 insertions(+), 58 deletions(-)

-- 
1.7.7

[PATCH 1/2] qla2xxx: Remove unavailable firmware files

[Himanshu Madhani]

Remove firmware binary names for the ISPs, which
are not submitted to linux-firmware

Signed-off-by: Himanshu Madhani <himanshu.madhani@qlogic.com>
Signed-off-by: Giridhar Malavali <giridhar.malavali@qlogic.com>
---
 drivers/scsi/qla2xxx/qla_os.c |   29 -----------------------------
 1 files changed, 0 insertions(+), 29 deletions(-)

diff --git a/drivers/scsi/qla2xxx/qla_os.c b/drivers/scsi/qla2xxx/qla_os.c
index c2dd17b..41aca16 100644
--- a/drivers/scsi/qla2xxx/qla_os.c
+++ b/drivers/scsi/qla2xxx/qla_os.c
@@ -5293,11 +5293,6 @@ qla2x00_timer(scsi_qla_host_t *vha)
 #define FW_ISP2322	3
 #define FW_ISP24XX	4
 #define FW_ISP25XX	5
-#define FW_ISP81XX	6
-#define FW_ISP82XX	7
-#define FW_ISP2031	8
-#define FW_ISP8031	9
-#define FW_ISP27XX	10
 
 #define FW_FILE_ISP21XX	"ql2100_fw.bin"
 #define FW_FILE_ISP22XX	"ql2200_fw.bin"
@@ -5305,12 +5300,6 @@ qla2x00_timer(scsi_qla_host_t *vha)
 #define FW_FILE_ISP2322	"ql2322_fw.bin"
 #define FW_FILE_ISP24XX	"ql2400_fw.bin"
 #define FW_FILE_ISP25XX	"ql2500_fw.bin"
-#define FW_FILE_ISP81XX	"ql8100_fw.bin"
-#define FW_FILE_ISP82XX	"ql8200_fw.bin"
-#define FW_FILE_ISP2031	"ql2600_fw.bin"
-#define FW_FILE_ISP8031	"ql8300_fw.bin"
-#define FW_FILE_ISP27XX	"ql2700_fw.bin"
-
 
 static DEFINE_MUTEX(qla_fw_lock);
 
@@ -5321,11 +5310,6 @@ static struct fw_blob qla_fw_blobs[FW_BLOBS] = {
 	{ .name = FW_FILE_ISP2322, .segs = { 0x800, 0x1c000, 0x1e000, 0 }, },
 	{ .name = FW_FILE_ISP24XX, },
 	{ .name = FW_FILE_ISP25XX, },
-	{ .name = FW_FILE_ISP81XX, },
-	{ .name = FW_FILE_ISP82XX, },
-	{ .name = FW_FILE_ISP2031, },
-	{ .name = FW_FILE_ISP8031, },
-	{ .name = FW_FILE_ISP27XX, },
 };
 
 struct fw_blob *
@@ -5346,16 +5330,6 @@ qla2x00_request_firmware(scsi_qla_host_t *vha)
 		blob = &qla_fw_blobs[FW_ISP24XX];
 	} else if (IS_QLA25XX(ha)) {
 		blob = &qla_fw_blobs[FW_ISP25XX];
-	} else if (IS_QLA81XX(ha)) {
-		blob = &qla_fw_blobs[FW_ISP81XX];
-	} else if (IS_QLA82XX(ha)) {
-		blob = &qla_fw_blobs[FW_ISP82XX];
-	} else if (IS_QLA2031(ha)) {
-		blob = &qla_fw_blobs[FW_ISP2031];
-	} else if (IS_QLA8031(ha)) {
-		blob = &qla_fw_blobs[FW_ISP8031];
-	} else if (IS_QLA27XX(ha)) {
-		blob = &qla_fw_blobs[FW_ISP27XX];
 	} else {
 		return NULL;
 	}
@@ -5844,6 +5818,3 @@ MODULE_FIRMWARE(FW_FILE_ISP2300);
 MODULE_FIRMWARE(FW_FILE_ISP2322);
 MODULE_FIRMWARE(FW_FILE_ISP24XX);
 MODULE_FIRMWARE(FW_FILE_ISP25XX);
-MODULE_FIRMWARE(FW_FILE_ISP2031);
-MODULE_FIRMWARE(FW_FILE_ISP8031);
-MODULE_FIRMWARE(FW_FILE_ISP27XX);
-- 
1.7.7

[PATCH 2/2] qla2xxx: fix rwlock recursion

[Himanshu Madhani]

This patch fixes rwlock recursion introduced for ISP82XX by
commit 8dfa4b5a ("qla2xxx: Fix sparse annotations")

Original patch fixes other sparse warnings which did not cause
any regression. So instead of reverting complete patch, this patch
will revert only part which caused recursion.

stack track has following signature

kernel:BUG: rwlock recursion on CPU#2, insmod/39333, ffff8803e998cb28
kernel: ffffffff8181d7de ffff88040f2af8a8 ffffffff810a8047 ffff8803e998cb28
kernel: ffff8803e998cb40 ffff88040f2af8c8 ffffffff810a833a 0000000000000086
kernel: Call Trace:
kernel: [<ffffffff812bce44>] dump_stack+0x48/0x64
kernel: [<ffffffff810a8047>] rwlock_bug+0x67/0x70
kernel: [<ffffffff810a833a>] do_raw_write_lock+0x8a/0xa0
kernel: [<ffffffff815f3033>] _raw_write_lock_irqsave+0x63/0x80
kernel: [<ffffffffa08087c8>] ? qla82xx_rd_32+0xe8/0x140 [qla2xxx]
kernel: [<ffffffffa08087c8>] qla82xx_rd_32+0xe8/0x140 [qla2xxx]
kernel: [<ffffffffa0808845>] qla82xx_crb_win_lock+0x25/0x60 [qla2xxx]
kernel: [<ffffffffa0808976>] qla82xx_wr_32+0xf6/0x150 [qla2xxx]
kernel: [<ffffffffa0808aa4>] ? qla82xx_disable_intrs+0x34/0x80 [qla2xxx]
kernel: [<ffffffffa0808ac0>] qla82xx_disable_intrs+0x50/0x80 [qla2xxx]
kernel: [<ffffffffa080630a>] qla82xx_reset_chip+0x1a/0x20 [qla2xxx]
kernel: [<ffffffffa07d6ef2>] qla2x00_initialize_adapter+0x132/0x420 [qla2xxx]
kernel: [<ffffffffa08087c8>] ? qla82xx_rd_32+0xe8/0x140 [qla2xxx]
kernel: [<ffffffffa08087c8>] qla82xx_rd_32+0xe8/0x140 [qla2xxx]
kernel: [<ffffffffa0808845>] qla82xx_crb_win_lock+0x25/0x60 [qla2xxx]
kernel: [<ffffffffa0808976>] qla82xx_wr_32+0xf6/0x150 [qla2xxx]
kernel: [<ffffffffa0808aa4>] ? qla82xx_disable_intrs+0x34/0x80 [qla2xxx]
kernel: [<ffffffffa0808ac0>] qla82xx_disable_intrs+0x50/0x80 [qla2xxx]
kernel: [<ffffffffa080630a>] qla82xx_reset_chip+0x1a/0x20 [qla2xxx]
kernel: [<ffffffffa07d6ef2>] qla2x00_initialize_adapter+0x132/0x420 [qla2xxx]
kernel: [<ffffffffa07c964e>] qla2x00_probe_one+0xefe/0x2130 [qla2xxx]
kernel: [<ffffffff813ee31c>] ? __pm_runtime_resume+0x6c/0x90
kernel: [<ffffffff8130052c>] local_pci_probe+0x4c/0xa0
kernel: [<ffffffff81300603>] pci_call_probe+0x83/0xa0
kernel: [<ffffffff813008cf>] pci_device_probe+0x7f/0xb0
kernel: [<ffffffff813e2e83>] really_probe+0x133/0x390
kernel: [<ffffffff813e3139>] driver_probe_device+0x59/0xd0
kernel: [<ffffffff813e3251>] __driver_attach+0xa1/0xb0
kernel: [<ffffffff813e31b0>] ? driver_probe_device+0xd0/0xd0
kernel: [<ffffffff813e0cdd>] bus_for_each_dev+0x8d/0xb0
kernel: [<ffffffff813e28ee>] driver_attach+0x1e/0x20
kernel: [<ffffffff813e2252>] bus_add_driver+0x1d2/0x290
kernel: [<ffffffff813e3970>] driver_register+0x60/0xe0
kernel: [<ffffffff813009e4>] __pci_register_driver+0x64/0x70
kernel: [<ffffffffa04bc1cb>] qla2x00_module_init+0x1cb/0x21b [qla2xxx]
kernel: [<ffffffffa04bc000>] ? 0xffffffffa04bc000
kernel: [<ffffffff8100027d>] do_one_initcall+0xad/0x1c0
kernel: [<ffffffff810e2859>] do_init_module+0x69/0x210
kernel: [<ffffffff810e4e5c>] load_module+0x5cc/0x750
kernel: [<ffffffff810e1980>] ? mod_sysfs_teardown+0x140/0x140
kernel: [<ffffffff812c9565>] ? copy_user_enhanced_fast_string+0x5/0x10
kernel: [<ffffffff810e12f0>] ? module_sect_show+0x30/0x30
kernel: [<ffffffff810e1a8d>] ? copy_module_from_user+0x8d/0xf0
kernel: [<ffffffff810e5162>] SyS_init_module+0x92/0xc0
kernel: [<ffffffff815f37d7>] entry_SYSCALL_64_fastpath+0x12/0x6f

Signed-off-by: Himanshu Madhani <himanshu.madhani@qlogic.com>
Signed-off-by: Giridhar Malavali <giridhar.malavali@qlogic.com>
---
 drivers/scsi/qla2xxx/qla_nx.c |   58 ++++++++++++++++++++--------------------
 1 files changed, 29 insertions(+), 29 deletions(-)

diff --git a/drivers/scsi/qla2xxx/qla_nx.c b/drivers/scsi/qla2xxx/qla_nx.c
index eb0cc54..96f9b39 100644
--- a/drivers/scsi/qla2xxx/qla_nx.c
+++ b/drivers/scsi/qla2xxx/qla_nx.c
@@ -347,31 +347,33 @@ char *qdev_state(uint32_t dev_state)
 }
 
 /*
- * In: 'off_in' is offset from CRB space in 128M pci map
- * Out: 'off_out' is 2M pci map addr
+ * In: 'off' is offset from CRB space in 128M pci map
+ * Out: 'off' is 2M pci map addr
  * side effect: lock crb window
  */
 static void
-qla82xx_pci_set_crbwindow_2M(struct qla_hw_data *ha, ulong off_in,
-			     void __iomem **off_out)
+qla82xx_pci_set_crbwindow_2M(struct qla_hw_data *ha, ulong *off)
 {
 	u32 win_read;
 	scsi_qla_host_t *vha = pci_get_drvdata(ha->pdev);
 
-	ha->crb_win = CRB_HI(off_in);
-	writel(ha->crb_win, CRB_WINDOW_2M + ha->nx_pcibase);
+	ha->crb_win = CRB_HI(*off);
+	writel(ha->crb_win,
+	    (void __iomem *)(CRB_WINDOW_2M + ha->nx_pcibase));
 
 	/* Read back value to make sure write has gone through before trying
 	 * to use it.
 	 */
-	win_read = RD_REG_DWORD(CRB_WINDOW_2M + ha->nx_pcibase);
+	win_read = RD_REG_DWORD((void __iomem *)
+	    (CRB_WINDOW_2M + ha->nx_pcibase));
 	if (win_read != ha->crb_win) {
 		ql_dbg(ql_dbg_p3p, vha, 0xb000,
 		    "%s: Written crbwin (0x%x) "
 		    "!= Read crbwin (0x%x), off=0x%lx.\n",
-		    __func__, ha->crb_win, win_read, off_in);
+		    __func__, ha->crb_win, win_read, *off);
 	}
-	*off_out = (off_in & MASK(16)) + CRB_INDIRECT_2M + ha->nx_pcibase;
+	*off = (*off & MASK(16)) + CRB_INDIRECT_2M +
+	    (unsigned long)ha->nx_pcibase;
 }
 
 static inline unsigned long
@@ -416,30 +418,30 @@ qla82xx_pci_set_crbwindow(struct qla_hw_data *ha, u64 off)
 }
 
 static int
-qla82xx_pci_get_crb_addr_2M(struct qla_hw_data *ha, ulong off_in,
-			    void __iomem **off_out)
+qla82xx_pci_get_crb_addr_2M(struct qla_hw_data *ha, ulong *off)
 {
 	struct crb_128M_2M_sub_block_map *m;
 
-	if (off_in >= QLA82XX_CRB_MAX)
+	if (*off >= QLA82XX_CRB_MAX)
 		return -1;
 
-	if (off_in >= QLA82XX_PCI_CAMQM && off_in < QLA82XX_PCI_CAMQM_2M_END) {
-		*off_out = (off_in - QLA82XX_PCI_CAMQM) +
-		    QLA82XX_PCI_CAMQM_2M_BASE + ha->nx_pcibase;
+	if (*off >= QLA82XX_PCI_CAMQM && (*off < QLA82XX_PCI_CAMQM_2M_END)) {
+		*off = (*off - QLA82XX_PCI_CAMQM) +
+		    QLA82XX_PCI_CAMQM_2M_BASE + (unsigned long)ha->nx_pcibase;
 		return 0;
 	}
 
-	if (off_in < QLA82XX_PCI_CRBSPACE)
+	if (*off < QLA82XX_PCI_CRBSPACE)
 		return -1;
 
-	*off_out = (void __iomem *)(off_in - QLA82XX_PCI_CRBSPACE);
+	*off -= QLA82XX_PCI_CRBSPACE;
 
 	/* Try direct map */
-	m = &crb_128M_2M_map[CRB_BLK(off_in)].sub_block[CRB_SUBBLK(off_in)];
+	m = &crb_128M_2M_map[CRB_BLK(*off)].sub_block[CRB_SUBBLK(*off)];
 
-	if (m->valid && (m->start_128M <= off_in) && (m->end_128M > off_in)) {
-		*off_out = off_in + m->start_2M - m->start_128M + ha->nx_pcibase;
+	if (m->valid && (m->start_128M <= *off) && (m->end_128M > *off)) {
+		*off = *off + m->start_2M - m->start_128M +
+		    (unsigned long) ha->nx_pcibase;
 		return 0;
 	}
 	/* Not in direct map, use crb window */
@@ -465,13 +467,12 @@ static int qla82xx_crb_win_lock(struct qla_hw_data *ha)
 }
 
 int
-qla82xx_wr_32(struct qla_hw_data *ha, ulong off_in, u32 data)
+qla82xx_wr_32(struct qla_hw_data *ha, ulong off, u32 data)
 {
-	void __iomem *off;
 	unsigned long flags = 0;
 	int rv;
 
-	rv = qla82xx_pci_get_crb_addr_2M(ha, off_in, &off);
+	rv = qla82xx_pci_get_crb_addr_2M(ha, &off);
 
 	BUG_ON(rv == -1);
 
@@ -480,7 +481,7 @@ qla82xx_wr_32(struct qla_hw_data *ha, ulong off_in, u32 data)
 		write_lock_irqsave(&ha->hw_lock, flags);
 #endif
 		qla82xx_crb_win_lock(ha);
-		qla82xx_pci_set_crbwindow_2M(ha, off_in, &off);
+		qla82xx_pci_set_crbwindow_2M(ha, &off);
 	}
 
 	writel(data, (void __iomem *)off);
@@ -495,14 +496,13 @@ qla82xx_wr_32(struct qla_hw_data *ha, ulong off_in, u32 data)
 }
 
 int
-qla82xx_rd_32(struct qla_hw_data *ha, ulong off_in)
+qla82xx_rd_32(struct qla_hw_data *ha, ulong off)
 {
-	void __iomem *off;
 	unsigned long flags = 0;
 	int rv;
 	u32 data;
 
-	rv = qla82xx_pci_get_crb_addr_2M(ha, off_in, &off);
+	rv = qla82xx_pci_get_crb_addr_2M(ha, &off);
 
 	BUG_ON(rv == -1);
 
@@ -511,9 +511,9 @@ qla82xx_rd_32(struct qla_hw_data *ha, ulong off_in)
 		write_lock_irqsave(&ha->hw_lock, flags);
 #endif
 		qla82xx_crb_win_lock(ha);
-		qla82xx_pci_set_crbwindow_2M(ha, off_in, &off);
+		qla82xx_pci_set_crbwindow_2M(ha, &off);
 	}
-	data = RD_REG_DWORD(off);
+	data = RD_REG_DWORD((void __iomem *)off);
 
 	if (rv == 1) {
 		qla82xx_rd_32(ha, QLA82XX_PCIE_REG(PCIE_SEM7_UNLOCK));
-- 
1.7.7

Re: [PATCH 2/2] qla2xxx: fix rwlock recursion

[Bart Van Assche]

On 10/14/2015 08:57 AM, Himanshu Madhani wrote:
> -qla82xx_pci_set_crbwindow_2M(struct qla_hw_data *ha, ulong off_in,
> -			     void __iomem **off_out)
> +qla82xx_pci_set_crbwindow_2M(struct qla_hw_data *ha, ulong *off)
>   {

Hello Himanshu,

Do you think it would be possible to keep the input and output offset as 
separate arguments ? Something that is very unfortunate about this patch 
is that it converts the qla82xx_pci_set_crbwindow_2M() function again 
into a function that expects an unsigned long in *off and writes an I/O 
memory address into *off. It is impossible to avoid that source code 
does not trigger any sparse warning if such functions occur.

Thanks,

Bart.

Re: [PATCH 2/2] qla2xxx: fix rwlock recursion

[Bart Van Assche]

On 10/14/2015 08:57 AM, Himanshu Madhani wrote:
> This patch fixes rwlock recursion introduced for ISP82XX by
> commit 8dfa4b5a ("qla2xxx: Fix sparse annotations")

Hello Himanshu,

Would it be possible to test whether the (untested) patch below also 
fixes the observed rwlock recursion ?

Thanks,

Bart.



[PATCH] qla2xxx: Fix rwlock recursion

Fixes: 8dfa4b5a9b44 ("qla2xxx: Fix sparse annotation")
Signed-off-by: Bart Van Assche <bart.vanassche@sandisk.com>
Reported-by: Himanshu Madhani <himanshu.madhani@qlogic.com>
Cc: Giridhar Malavali <giridhar.malavali@qlogic.com>
---
  drivers/scsi/qla2xxx/qla_nx.c | 3 ++-
  1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/scsi/qla2xxx/qla_nx.c b/drivers/scsi/qla2xxx/qla_nx.c
index eb0cc54..b6b4cfd 100644
--- a/drivers/scsi/qla2xxx/qla_nx.c
+++ b/drivers/scsi/qla2xxx/qla_nx.c
@@ -433,7 +433,7 @@ qla82xx_pci_get_crb_addr_2M(struct qla_hw_data *ha, 
ulong off_in,
  	if (off_in < QLA82XX_PCI_CRBSPACE)
  		return -1;

-	*off_out = (void __iomem *)(off_in - QLA82XX_PCI_CRBSPACE);
+	off_in -= QLA82XX_PCI_CRBSPACE;

  	/* Try direct map */
  	m = &crb_128M_2M_map[CRB_BLK(off_in)].sub_block[CRB_SUBBLK(off_in)];
@@ -443,6 +443,7 @@ qla82xx_pci_get_crb_addr_2M(struct qla_hw_data *ha, 
ulong off_in,
  		return 0;
  	}
  	/* Not in direct map, use crb window */
+	*off_out = (void __iomem *)off_in;
  	return 1;
  }

-- 
2.1.4

Re: [PATCH 2/2] qla2xxx: fix rwlock recursion

[Himanshu Madhani]

Hi Bart, 

> On Oct 14, 2015, at 10:33 AM, Bart Van Assche <bart.vanassche@sandisk.com> wrote:
> 
> On 10/14/2015 08:57 AM, Himanshu Madhani wrote:
>> This patch fixes rwlock recursion introduced for ISP82XX by
>> commit 8dfa4b5a ("qla2xxx: Fix sparse annotations")
> 
> Hello Himanshu,
> 
> Would it be possible to test whether the (untested) patch below also fixes the observed rwlock recursion ?
> 
> Thanks,
> 
> Bart.
> 
> 
> 
> [PATCH] qla2xxx: Fix rwlock recursion
> 
> Fixes: 8dfa4b5a9b44 ("qla2xxx: Fix sparse annotation")
> Signed-off-by: Bart Van Assche <bart.vanassche@sandisk.com>
> Reported-by: Himanshu Madhani <himanshu.madhani@qlogic.com>
> Cc: Giridhar Malavali <giridhar.malavali@qlogic.com>
> ---
> drivers/scsi/qla2xxx/qla_nx.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/scsi/qla2xxx/qla_nx.c b/drivers/scsi/qla2xxx/qla_nx.c
> index eb0cc54..b6b4cfd 100644
> --- a/drivers/scsi/qla2xxx/qla_nx.c
> +++ b/drivers/scsi/qla2xxx/qla_nx.c
> @@ -433,7 +433,7 @@ qla82xx_pci_get_crb_addr_2M(struct qla_hw_data *ha, ulong off_in,
> 	if (off_in < QLA82XX_PCI_CRBSPACE)
> 		return -1;
> 
> -	*off_out = (void __iomem *)(off_in - QLA82XX_PCI_CRBSPACE);
> +	off_in -= QLA82XX_PCI_CRBSPACE;
> 
> 	/* Try direct map */
> 	m = &crb_128M_2M_map[CRB_BLK(off_in)].sub_block[CRB_SUBBLK(off_in)];
> @@ -443,6 +443,7 @@ qla82xx_pci_get_crb_addr_2M(struct qla_hw_data *ha, ulong off_in,
> 		return 0;
> 	}
> 	/* Not in direct map, use crb window */
> +	*off_out = (void __iomem *)off_in;
> 	return 1;
> }
> 

This change  also resolved rwlock recursion on the driver load. Will perform some more basic sanity and report if there are issues seen. 

> -- 
> 2.1.4
> 
> 

Re: [PATCH 1/2] qla2xxx: Remove unavailable firmware files

[Julian Calaby]

Hi Himanshu,

On Thu, Oct 15, 2015 at 2:57 AM, Himanshu Madhani
<himanshu.madhani@qlogic.com> wrote:
> Remove firmware binary names for the ISPs, which
> are not submitted to linux-firmware
>
> Signed-off-by: Himanshu Madhani <himanshu.madhani@qlogic.com>
> Signed-off-by: Giridhar Malavali <giridhar.malavali@qlogic.com>

For what it's worth, this looks good to me.

Thanks,

-- 
Julian Calaby

Email: julian.calaby@gmail.com
Profile: http://www.google.com/profiles/julian.calaby/

Re: [PATCH 1/2] qla2xxx: Remove unavailable firmware files

[Hannes Reinecke]

On 10/14/2015 05:57 PM, Himanshu Madhani wrote:
> Remove firmware binary names for the ISPs, which
> are not submitted to linux-firmware
> 
> Signed-off-by: Himanshu Madhani <himanshu.madhani@qlogic.com>
> Signed-off-by: Giridhar Malavali <giridhar.malavali@qlogic.com>
> ---
>  drivers/scsi/qla2xxx/qla_os.c |   29 -----------------------------
>  1 files changed, 0 insertions(+), 29 deletions(-)
> 
Thank you. This has cause quite some confusion with some customers.

Reviewed-by: Hannes Reinecke <hare@suse.de>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		               zSeries & Storage
hare@suse.de			               +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/2] qla2xxx: fix rwlock recursion

[Hannes Reinecke]

On 10/14/2015 05:57 PM, Himanshu Madhani wrote:
> This patch fixes rwlock recursion introduced for ISP82XX by
> commit 8dfa4b5a ("qla2xxx: Fix sparse annotations")
> 
> Original patch fixes other sparse warnings which did not cause
> any regression. So instead of reverting complete patch, this patch
> will revert only part which caused recursion.
> 
> stack track has following signature
> 
> kernel:BUG: rwlock recursion on CPU#2, insmod/39333, ffff8803e998cb28
> kernel: ffffffff8181d7de ffff88040f2af8a8 ffffffff810a8047 ffff8803e998cb28
> kernel: ffff8803e998cb40 ffff88040f2af8c8 ffffffff810a833a 0000000000000086
> kernel: Call Trace:
> kernel: [<ffffffff812bce44>] dump_stack+0x48/0x64
> kernel: [<ffffffff810a8047>] rwlock_bug+0x67/0x70
> kernel: [<ffffffff810a833a>] do_raw_write_lock+0x8a/0xa0
> kernel: [<ffffffff815f3033>] _raw_write_lock_irqsave+0x63/0x80
> kernel: [<ffffffffa08087c8>] ? qla82xx_rd_32+0xe8/0x140 [qla2xxx]
> kernel: [<ffffffffa08087c8>] qla82xx_rd_32+0xe8/0x140 [qla2xxx]
> kernel: [<ffffffffa0808845>] qla82xx_crb_win_lock+0x25/0x60 [qla2xxx]
> kernel: [<ffffffffa0808976>] qla82xx_wr_32+0xf6/0x150 [qla2xxx]
> kernel: [<ffffffffa0808aa4>] ? qla82xx_disable_intrs+0x34/0x80 [qla2xxx]
> kernel: [<ffffffffa0808ac0>] qla82xx_disable_intrs+0x50/0x80 [qla2xxx]
> kernel: [<ffffffffa080630a>] qla82xx_reset_chip+0x1a/0x20 [qla2xxx]
> kernel: [<ffffffffa07d6ef2>] qla2x00_initialize_adapter+0x132/0x420 [qla2xxx]
> kernel: [<ffffffffa08087c8>] ? qla82xx_rd_32+0xe8/0x140 [qla2xxx]
> kernel: [<ffffffffa08087c8>] qla82xx_rd_32+0xe8/0x140 [qla2xxx]
> kernel: [<ffffffffa0808845>] qla82xx_crb_win_lock+0x25/0x60 [qla2xxx]
> kernel: [<ffffffffa0808976>] qla82xx_wr_32+0xf6/0x150 [qla2xxx]
> kernel: [<ffffffffa0808aa4>] ? qla82xx_disable_intrs+0x34/0x80 [qla2xxx]
> kernel: [<ffffffffa0808ac0>] qla82xx_disable_intrs+0x50/0x80 [qla2xxx]
> kernel: [<ffffffffa080630a>] qla82xx_reset_chip+0x1a/0x20 [qla2xxx]
> kernel: [<ffffffffa07d6ef2>] qla2x00_initialize_adapter+0x132/0x420 [qla2xxx]
> kernel: [<ffffffffa07c964e>] qla2x00_probe_one+0xefe/0x2130 [qla2xxx]
> kernel: [<ffffffff813ee31c>] ? __pm_runtime_resume+0x6c/0x90
> kernel: [<ffffffff8130052c>] local_pci_probe+0x4c/0xa0
> kernel: [<ffffffff81300603>] pci_call_probe+0x83/0xa0
> kernel: [<ffffffff813008cf>] pci_device_probe+0x7f/0xb0
> kernel: [<ffffffff813e2e83>] really_probe+0x133/0x390
> kernel: [<ffffffff813e3139>] driver_probe_device+0x59/0xd0
> kernel: [<ffffffff813e3251>] __driver_attach+0xa1/0xb0
> kernel: [<ffffffff813e31b0>] ? driver_probe_device+0xd0/0xd0
> kernel: [<ffffffff813e0cdd>] bus_for_each_dev+0x8d/0xb0
> kernel: [<ffffffff813e28ee>] driver_attach+0x1e/0x20
> kernel: [<ffffffff813e2252>] bus_add_driver+0x1d2/0x290
> kernel: [<ffffffff813e3970>] driver_register+0x60/0xe0
> kernel: [<ffffffff813009e4>] __pci_register_driver+0x64/0x70
> kernel: [<ffffffffa04bc1cb>] qla2x00_module_init+0x1cb/0x21b [qla2xxx]
> kernel: [<ffffffffa04bc000>] ? 0xffffffffa04bc000
> kernel: [<ffffffff8100027d>] do_one_initcall+0xad/0x1c0
> kernel: [<ffffffff810e2859>] do_init_module+0x69/0x210
> kernel: [<ffffffff810e4e5c>] load_module+0x5cc/0x750
> kernel: [<ffffffff810e1980>] ? mod_sysfs_teardown+0x140/0x140
> kernel: [<ffffffff812c9565>] ? copy_user_enhanced_fast_string+0x5/0x10
> kernel: [<ffffffff810e12f0>] ? module_sect_show+0x30/0x30
> kernel: [<ffffffff810e1a8d>] ? copy_module_from_user+0x8d/0xf0
> kernel: [<ffffffff810e5162>] SyS_init_module+0x92/0xc0
> kernel: [<ffffffff815f37d7>] entry_SYSCALL_64_fastpath+0x12/0x6f
> 
> Signed-off-by: Himanshu Madhani <himanshu.madhani@qlogic.com>
> Signed-off-by: Giridhar Malavali <giridhar.malavali@qlogic.com>
> ---
>  drivers/scsi/qla2xxx/qla_nx.c |   58 ++++++++++++++++++++--------------------
>  1 files changed, 29 insertions(+), 29 deletions(-)
> 
> diff --git a/drivers/scsi/qla2xxx/qla_nx.c b/drivers/scsi/qla2xxx/qla_nx.c
> index eb0cc54..96f9b39 100644
> --- a/drivers/scsi/qla2xxx/qla_nx.c
> +++ b/drivers/scsi/qla2xxx/qla_nx.c
> @@ -347,31 +347,33 @@ char *qdev_state(uint32_t dev_state)
>  }
>  
>  /*
> - * In: 'off_in' is offset from CRB space in 128M pci map
> - * Out: 'off_out' is 2M pci map addr
> + * In: 'off' is offset from CRB space in 128M pci map
> + * Out: 'off' is 2M pci map addr
>   * side effect: lock crb window
>   */
>  static void
> -qla82xx_pci_set_crbwindow_2M(struct qla_hw_data *ha, ulong off_in,
> -			     void __iomem **off_out)
> +qla82xx_pci_set_crbwindow_2M(struct qla_hw_data *ha, ulong *off)
>  {
>  	u32 win_read;
>  	scsi_qla_host_t *vha = pci_get_drvdata(ha->pdev);
>  
> -	ha->crb_win = CRB_HI(off_in);
> -	writel(ha->crb_win, CRB_WINDOW_2M + ha->nx_pcibase);
> +	ha->crb_win = CRB_HI(*off);
> +	writel(ha->crb_win,
> +	    (void __iomem *)(CRB_WINDOW_2M + ha->nx_pcibase));
>  
>  	/* Read back value to make sure write has gone through before trying
>  	 * to use it.
>  	 */
> -	win_read = RD_REG_DWORD(CRB_WINDOW_2M + ha->nx_pcibase);
> +	win_read = RD_REG_DWORD((void __iomem *)
> +	    (CRB_WINDOW_2M + ha->nx_pcibase));
>  	if (win_read != ha->crb_win) {
>  		ql_dbg(ql_dbg_p3p, vha, 0xb000,
>  		    "%s: Written crbwin (0x%x) "
>  		    "!= Read crbwin (0x%x), off=0x%lx.\n",
> -		    __func__, ha->crb_win, win_read, off_in);
> +		    __func__, ha->crb_win, win_read, *off);
>  	}
> -	*off_out = (off_in & MASK(16)) + CRB_INDIRECT_2M + ha->nx_pcibase;
> +	*off = (*off & MASK(16)) + CRB_INDIRECT_2M +
> +	    (unsigned long)ha->nx_pcibase;
>  }
>  
Weelll ... I'm not a hardware designer, but this looks dodgy as
hell. You are increasing the offset every time you call this
function. Let's better hope no-one would be stupid enough to call
this function twice ...

Can't you make them idempotent, ie that multiple calls to this
function return the same 'off' number?
That would certainly decrease the likelyhood of an error ...

At the very least add some warning in the function description.

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		               zSeries & Storage
hare@suse.de			               +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/2] qla2xxx: fix rwlock recursion

[Himanshu Madhani]

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

Hi Bart, 


On 10/14/15, 4:07 PM, "linux-scsi-owner@vger.kernel.org on behalf of
Himanshu Madhani" <linux-scsi-owner@vger.kernel.org on behalf of
himanshu.madhani@qlogic.com> wrote:

>Hi Bart, 
>
>> On Oct 14, 2015, at 10:33 AM, Bart Van Assche
>><bart.vanassche@sandisk.com> wrote:
>> 
>> On 10/14/2015 08:57 AM, Himanshu Madhani wrote:
>>> This patch fixes rwlock recursion introduced for ISP82XX by
>>> commit 8dfa4b5a ("qla2xxx: Fix sparse annotations")
>> 
>> Hello Himanshu,
>> 
>> Would it be possible to test whether the (untested) patch below also
>>fixes the observed rwlock recursion ?
>> 
>> Thanks,
>> 
>> Bart.
>> 
>> 
>> 
>> [PATCH] qla2xxx: Fix rwlock recursion
>> 
>> Fixes: 8dfa4b5a9b44 ("qla2xxx: Fix sparse annotation")
>> Signed-off-by: Bart Van Assche <bart.vanassche@sandisk.com>
>> Reported-by: Himanshu Madhani <himanshu.madhani@qlogic.com>
>> Cc: Giridhar Malavali <giridhar.malavali@qlogic.com>
>> ---
>> drivers/scsi/qla2xxx/qla_nx.c | 3 ++-
>> 1 file changed, 2 insertions(+), 1 deletion(-)
>> 
>> diff --git a/drivers/scsi/qla2xxx/qla_nx.c
>>b/drivers/scsi/qla2xxx/qla_nx.c
>> index eb0cc54..b6b4cfd 100644
>> --- a/drivers/scsi/qla2xxx/qla_nx.c
>> +++ b/drivers/scsi/qla2xxx/qla_nx.c
>> @@ -433,7 +433,7 @@ qla82xx_pci_get_crb_addr_2M(struct qla_hw_data *ha,
>>ulong off_in,
>> 	if (off_in < QLA82XX_PCI_CRBSPACE)
>> 		return -1;
>> 
>> -	*off_out = (void __iomem *)(off_in - QLA82XX_PCI_CRBSPACE);
>> +	off_in -= QLA82XX_PCI_CRBSPACE;
>> 
>> 	/* Try direct map */
>> 	m = &crb_128M_2M_map[CRB_BLK(off_in)].sub_block[CRB_SUBBLK(off_in)];
>> @@ -443,6 +443,7 @@ qla82xx_pci_get_crb_addr_2M(struct qla_hw_data *ha,
>>ulong off_in,
>> 		return 0;
>> 	}
>> 	/* Not in direct map, use crb window */
>> +	*off_out = (void __iomem *)off_in;
>> 	return 1;
>> }
>> 
>
>This change  also resolved rwlock recursion on the driver load. Will
>perform some more basic sanity and report if there are issues seen.

Testing looks good so far. Please send official patch with this fix.



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


Thanks,
Himanshu


>


[-- Attachment #2: winmail.dat --]
[-- Type: application/ms-tnef, Size: 5302 bytes --]

Re: [PATCH 1/2] qla2xxx: Remove unavailable firmware files

[Xose Vazquez Perez]

On 10/14/2015 05:57 PM, Himanshu Madhani wrote:

> Remove firmware binary names for the ISPs, which
> are not submitted to linux-firmware
> 
> Signed-off-by: Himanshu Madhani <himanshu.madhani@qlogic.com>
> Signed-off-by: Giridhar Malavali <giridhar.malavali@qlogic.com>
> ---
>  drivers/scsi/qla2xxx/qla_os.c |   29 -----------------------------
>  1 files changed, 0 insertions(+), 29 deletions(-)
> 
> diff --git a/drivers/scsi/qla2xxx/qla_os.c b/drivers/scsi/qla2xxx/qla_os.c
> index c2dd17b..41aca16 100644
> --- a/drivers/scsi/qla2xxx/qla_os.c
> +++ b/drivers/scsi/qla2xxx/qla_os.c
> @@ -5293,11 +5293,6 @@ qla2x00_timer(scsi_qla_host_t *vha)
>  #define FW_ISP2322	3
>  #define FW_ISP24XX	4
>  #define FW_ISP25XX	5
> -#define FW_ISP81XX	6
> -#define FW_ISP82XX	7
> -#define FW_ISP2031	8
> -#define FW_ISP8031	9
> -#define FW_ISP27XX	10
>  
>  #define FW_FILE_ISP21XX	"ql2100_fw.bin"
>  #define FW_FILE_ISP22XX	"ql2200_fw.bin"
> @@ -5305,12 +5300,6 @@ qla2x00_timer(scsi_qla_host_t *vha)
>  #define FW_FILE_ISP2322	"ql2322_fw.bin"
>  #define FW_FILE_ISP24XX	"ql2400_fw.bin"
>  #define FW_FILE_ISP25XX	"ql2500_fw.bin"
> -#define FW_FILE_ISP81XX	"ql8100_fw.bin"
> -#define FW_FILE_ISP82XX	"ql8200_fw.bin"
> -#define FW_FILE_ISP2031	"ql2600_fw.bin"
> -#define FW_FILE_ISP8031	"ql8300_fw.bin"
> -#define FW_FILE_ISP27XX	"ql2700_fw.bin"
> -
>  
>  static DEFINE_MUTEX(qla_fw_lock);
>  
> @@ -5321,11 +5310,6 @@ static struct fw_blob qla_fw_blobs[FW_BLOBS] = {
>  	{ .name = FW_FILE_ISP2322, .segs = { 0x800, 0x1c000, 0x1e000, 0 }, },
>  	{ .name = FW_FILE_ISP24XX, },
>  	{ .name = FW_FILE_ISP25XX, },
> -	{ .name = FW_FILE_ISP81XX, },
> -	{ .name = FW_FILE_ISP82XX, },
> -	{ .name = FW_FILE_ISP2031, },
> -	{ .name = FW_FILE_ISP8031, },
> -	{ .name = FW_FILE_ISP27XX, },
>  };
>  
>  struct fw_blob *
> @@ -5346,16 +5330,6 @@ qla2x00_request_firmware(scsi_qla_host_t *vha)
>  		blob = &qla_fw_blobs[FW_ISP24XX];
>  	} else if (IS_QLA25XX(ha)) {
>  		blob = &qla_fw_blobs[FW_ISP25XX];
> -	} else if (IS_QLA81XX(ha)) {
> -		blob = &qla_fw_blobs[FW_ISP81XX];
> -	} else if (IS_QLA82XX(ha)) {
> -		blob = &qla_fw_blobs[FW_ISP82XX];
> -	} else if (IS_QLA2031(ha)) {
> -		blob = &qla_fw_blobs[FW_ISP2031];
> -	} else if (IS_QLA8031(ha)) {
> -		blob = &qla_fw_blobs[FW_ISP8031];
> -	} else if (IS_QLA27XX(ha)) {
> -		blob = &qla_fw_blobs[FW_ISP27XX];
>  	} else {
>  		return NULL;
>  	}
> @@ -5844,6 +5818,3 @@ MODULE_FIRMWARE(FW_FILE_ISP2300);
>  MODULE_FIRMWARE(FW_FILE_ISP2322);
>  MODULE_FIRMWARE(FW_FILE_ISP24XX);
>  MODULE_FIRMWARE(FW_FILE_ISP25XX);
> -MODULE_FIRMWARE(FW_FILE_ISP2031);
> -MODULE_FIRMWARE(FW_FILE_ISP8031);
> -MODULE_FIRMWARE(FW_FILE_ISP27XX);
> -- 
> 1.7.7

You should remove this info also from Kconfig:
      2031              ql2600_fw.bin
      8031              ql8300_fw.bin
      27xx              ql2700_fw.bin

Anyway,

Reviewed-by: Xose Vazquez Perez <xose.vazquez@gmail.com>


BTW, linux-firmware still contains release 7.03.00 of ql2400_fw.bin and ql2500_fw.bin .
SLES-11_SP4 already brings 8.00.00. And I did test 8.01.00 time ago and it worked flawlessly.

So, please *regularly* send latest FW releases to linux-firmware.git

-thanks-