Re: [PATCH 0/4] scsi: fixup dma_set_mask_and_coherent() calls
On 13/02/2019 18:51, Ewan D. Milne wrote: On Wed, 2019-02-13 at 12:42 +0100, Hannes Reinecke wrote: The recent patchset to use dma_set_mask_and_coherent() introduced a regression where a call to set a 64-bit DMA mask was followed by a call to set a 32-bit DMA mask, leading to I/O errors and data corruption. Patchset is based on a suggestions from Ewan Milne. Hannes Reinecke (4): lpfc: fix calls to dma_set_mask_and_coherent() hptiop: fix calls to dma_set_mask_and_coherent() bfa: fix calls to dma_set_mask_and_coherent() hisi_sas: fix calls to dma_set_mask_and_coherent() drivers/scsi/bfa/bfad.c | 10 +++--- drivers/scsi/hisi_sas/hisi_sas_main.c | 8 ++-- drivers/scsi/hptiop.c | 10 +++--- drivers/scsi/lpfc/lpfc_init.c | 9 ++--- 4 files changed, 26 insertions(+), 11 deletions(-) Isn't there a few more to fix up, like: drivers/scsi/3w-9xxx.c drivers/scsi/3w-sas.c drivers/scsi/csiostor/csio_init.c Thanks, John Thanks for posting this set, Hannes. I've got some individual comments on each... -Ewan
Re: [PATCH 3/6] mpt3sas: Irq poll to avoid CPU hard lockups.
Hi Suganath, I love your patch! Yet something to improve: [auto build test ERROR on scsi/for-next] [also build test ERROR on v5.0-rc4 next-20190214] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system] url: https://github.com/0day-ci/linux/commits/Suganath-Prabu/Irq-poll-to-address-cpu-lockup/20190214-172626 base: https://git.kernel.org/pub/scm/linux/kernel/git/jejb/scsi.git for-next config: x86_64-lkp (attached as .config) compiler: gcc-7 (Debian 7.3.0-1) 7.3.0 reproduce: # save the attached .config to linux build tree make ARCH=x86_64 All errors (new ones prefixed by >>): >> ERROR: "irq_poll_init" [drivers/scsi/mpt3sas/mpt3sas.ko] undefined! >> ERROR: "irq_poll_enable" [drivers/scsi/mpt3sas/mpt3sas.ko] undefined! >> ERROR: "irq_poll_sched" [drivers/scsi/mpt3sas/mpt3sas.ko] undefined! >> ERROR: "irq_poll_disable" [drivers/scsi/mpt3sas/mpt3sas.ko] undefined! >> ERROR: "irq_poll_complete" [drivers/scsi/mpt3sas/mpt3sas.ko] undefined! --- 0-DAY kernel test infrastructureOpen Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation .config.gz Description: application/gzip
Re: [Lsf-pc] LSF/MM 2019: Call for Proposals (UPDATED!)
On Thu 07-02-19 08:35:06, Jens Axboe wrote: [...] > 2) Requests to attend the summit for those that are not proposing a > topic should be sent to: > > lsf...@lists.linux-foundation.org > > Please summarize what expertise you will bring to the meeting, and > what you would like to discuss. Please also tag your email with > [LSF/MM ATTEND] and send it as a new thread so there is less chance of > it getting lost. Just a reminder. Please do not forget to send the ATTEND request and make it clear which track you would like to participate in the most. It is quite common that people only send topic proposals without and expclicit ATTEND request or they do not mention the track. Thanks! -- Michal Hocko SUSE Labs
[PATCH] scsi: target: move spin_lock_bh to spin_lock in tasklet
as you are already in a tasklet, it is unnecessary to call spin_lock_bh, because softirq already disable BH. Signed-off-by: Zhiwei Jiang --- drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c b/drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c index cc9cae469c4b..29e7c51fcc4b 100644 --- a/drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c +++ b/drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c @@ -3346,7 +3346,7 @@ static void ibmvscsis_handle_crq(unsigned long data) bool ack = true; volatile u8 valid; - spin_lock_bh(&vscsi->intr_lock); + spin_lock(&vscsi->intr_lock); dev_dbg(&vscsi->dev, "got interrupt\n"); @@ -3360,7 +3360,7 @@ static void ibmvscsis_handle_crq(unsigned long data) dev_dbg(&vscsi->dev, "handle_crq, don't process: flags 0x%x, state 0x%hx\n", vscsi->flags, vscsi->state); - spin_unlock_bh(&vscsi->intr_lock); + spin_unlock(&vscsi->intr_lock); return; } @@ -3437,7 +3437,7 @@ static void ibmvscsis_handle_crq(unsigned long data) (int)list_empty(&vscsi->schedule_q), vscsi->flags, vscsi->state); - spin_unlock_bh(&vscsi->intr_lock); + spin_unlock(&vscsi->intr_lock); } static int ibmvscsis_probe(struct vio_dev *vdev, -- 2.19.0
[PATCH] Move debug messages before sending srb preventing panic
When sending an srb with qla2x00_start_sp, the sp can complete and be
freed by the time we log the debug message saying we sent it. This can
cause a panic if sp gets reused quickly or when running a kernel that
poisons freed memory.
This was partially fixed by (not every case was addressed):
commit 9fe278f44b4b ("scsi: qla2xxx: Move log messages before issuing command
to firmware")
Signed-off-by: Bill Kuzeja
---
drivers/scsi/qla2xxx/qla_gs.c | 66 ++-
drivers/scsi/qla2xxx/qla_init.c | 26 ---
drivers/scsi/qla2xxx/qla_target.c | 8 ++---
3 files changed, 55 insertions(+), 45 deletions(-)
diff --git a/drivers/scsi/qla2xxx/qla_gs.c b/drivers/scsi/qla2xxx/qla_gs.c
index cbc3bc4..b19185a 100644
--- a/drivers/scsi/qla2xxx/qla_gs.c
+++ b/drivers/scsi/qla2xxx/qla_gs.c
@@ -657,15 +657,16 @@ static int qla_async_rftid(scsi_qla_host_t *vha,
port_id_t *d_id)
sp->u.iocb_cmd.timeout = qla2x00_async_iocb_timeout;
sp->done = qla2x00_async_sns_sp_done;
+ ql_dbg(ql_dbg_disc, vha, 0x,
+ "Async-%s - hdl=%x portid %06x.\n",
+ sp->name, sp->handle, d_id->b24);
+
rval = qla2x00_start_sp(sp);
if (rval != QLA_SUCCESS) {
ql_dbg(ql_dbg_disc, vha, 0x2043,
"RFT_ID issue IOCB failed (%d).\n", rval);
goto done_free_sp;
}
- ql_dbg(ql_dbg_disc, vha, 0x,
- "Async-%s - hdl=%x portid %06x.\n",
- sp->name, sp->handle, d_id->b24);
return rval;
done_free_sp:
sp->free(sp);
@@ -752,6 +753,10 @@ static int qla_async_rffid(scsi_qla_host_t *vha, port_id_t
*d_id,
sp->u.iocb_cmd.timeout = qla2x00_async_iocb_timeout;
sp->done = qla2x00_async_sns_sp_done;
+ ql_dbg(ql_dbg_disc, vha, 0x,
+ "Async-%s - hdl=%x portid %06x feature %x type %x.\n",
+ sp->name, sp->handle, d_id->b24, fc4feature, fc4type);
+
rval = qla2x00_start_sp(sp);
if (rval != QLA_SUCCESS) {
ql_dbg(ql_dbg_disc, vha, 0x2047,
@@ -759,9 +764,6 @@ static int qla_async_rffid(scsi_qla_host_t *vha, port_id_t
*d_id,
goto done_free_sp;
}
- ql_dbg(ql_dbg_disc, vha, 0x,
- "Async-%s - hdl=%x portid %06x feature %x type %x.\n",
- sp->name, sp->handle, d_id->b24, fc4feature, fc4type);
return rval;
done_free_sp:
@@ -844,15 +846,16 @@ static int qla_async_rnnid(scsi_qla_host_t *vha,
port_id_t *d_id,
sp->u.iocb_cmd.timeout = qla2x00_async_iocb_timeout;
sp->done = qla2x00_async_sns_sp_done;
+ ql_dbg(ql_dbg_disc, vha, 0x,
+ "Async-%s - hdl=%x portid %06x\n",
+ sp->name, sp->handle, d_id->b24);
+
rval = qla2x00_start_sp(sp);
if (rval != QLA_SUCCESS) {
ql_dbg(ql_dbg_disc, vha, 0x204d,
"RNN_ID issue IOCB failed (%d).\n", rval);
goto done_free_sp;
}
- ql_dbg(ql_dbg_disc, vha, 0x,
- "Async-%s - hdl=%x portid %06x\n",
- sp->name, sp->handle, d_id->b24);
return rval;
@@ -957,15 +960,16 @@ static int qla_async_rsnn_nn(scsi_qla_host_t *vha)
sp->u.iocb_cmd.timeout = qla2x00_async_iocb_timeout;
sp->done = qla2x00_async_sns_sp_done;
+ ql_dbg(ql_dbg_disc, vha, 0x,
+ "Async-%s - hdl=%x.\n",
+ sp->name, sp->handle);
+
rval = qla2x00_start_sp(sp);
if (rval != QLA_SUCCESS) {
ql_dbg(ql_dbg_disc, vha, 0x2043,
"RFT_ID issue IOCB failed (%d).\n", rval);
goto done_free_sp;
}
- ql_dbg(ql_dbg_disc, vha, 0x,
- "Async-%s - hdl=%x.\n",
- sp->name, sp->handle);
return rval;
@@ -3578,14 +3582,14 @@ int qla24xx_async_gffid(scsi_qla_host_t *vha, fc_port_t
*fcport)
sp->done = qla24xx_async_gffid_sp_done;
- rval = qla2x00_start_sp(sp);
- if (rval != QLA_SUCCESS)
- goto done_free_sp;
-
ql_dbg(ql_dbg_disc, vha, 0x2132,
"Async-%s hdl=%x %8phC.\n", sp->name,
sp->handle, fcport->port_name);
+ rval = qla2x00_start_sp(sp);
+ if (rval != QLA_SUCCESS)
+ goto done_free_sp;
+
return rval;
done_free_sp:
sp->free(sp);
@@ -4067,6 +4071,10 @@ static int qla24xx_async_gnnft(scsi_qla_host_t *vha,
struct srb *sp,
sp->done = qla2x00_async_gpnft_gnnft_sp_done;
+ ql_dbg(ql_dbg_disc, vha, 0x,
+ "Async-%s hdl=%x FC4Type %x.\n", sp->name,
+ sp->handle, ct_req->req.gpn_ft.port_type);
+
rval = qla2x00_start_sp(sp);
if (rval != QLA_SUCCESS) {
spin_lock_irqsave(&vha->work_lock, flags);
@@ -4075,9 +4083,6 @@ static int qla24xx_async_gnnft(scsi_qla_host_t *vha,
struct srb *sp,
goto done_free_sp;
}
- ql_dbg(ql_dbg_disc, vha, 0x,
- "A
[PATCH] qla2xxx - Move debug messages before sending srb preventing panic
(Sorry for the resend, forgot to put qla2xxx in the subject line)
When sending an srb with qla2x00_start_sp, the sp can complete and be
freed by the time we log the debug message saying we sent it. This can
cause a panic if sp gets reused quickly or when running a kernel that
poisons freed memory.
This was partially fixed by (not every case was addressed):
commit 9fe278f44b4b ("scsi: qla2xxx: Move log messages before issuing command
to firmware")
Signed-off-by: Bill Kuzeja
---
drivers/scsi/qla2xxx/qla_gs.c | 66 ++-
drivers/scsi/qla2xxx/qla_init.c | 26 ---
drivers/scsi/qla2xxx/qla_target.c | 8 ++---
3 files changed, 55 insertions(+), 45 deletions(-)
diff --git a/drivers/scsi/qla2xxx/qla_gs.c b/drivers/scsi/qla2xxx/qla_gs.c
index cbc3bc4..b19185a 100644
--- a/drivers/scsi/qla2xxx/qla_gs.c
+++ b/drivers/scsi/qla2xxx/qla_gs.c
@@ -657,15 +657,16 @@ static int qla_async_rftid(scsi_qla_host_t *vha,
port_id_t *d_id)
sp->u.iocb_cmd.timeout = qla2x00_async_iocb_timeout;
sp->done = qla2x00_async_sns_sp_done;
+ ql_dbg(ql_dbg_disc, vha, 0x,
+ "Async-%s - hdl=%x portid %06x.\n",
+ sp->name, sp->handle, d_id->b24);
+
rval = qla2x00_start_sp(sp);
if (rval != QLA_SUCCESS) {
ql_dbg(ql_dbg_disc, vha, 0x2043,
"RFT_ID issue IOCB failed (%d).\n", rval);
goto done_free_sp;
}
- ql_dbg(ql_dbg_disc, vha, 0x,
- "Async-%s - hdl=%x portid %06x.\n",
- sp->name, sp->handle, d_id->b24);
return rval;
done_free_sp:
sp->free(sp);
@@ -752,6 +753,10 @@ static int qla_async_rffid(scsi_qla_host_t *vha, port_id_t
*d_id,
sp->u.iocb_cmd.timeout = qla2x00_async_iocb_timeout;
sp->done = qla2x00_async_sns_sp_done;
+ ql_dbg(ql_dbg_disc, vha, 0x,
+ "Async-%s - hdl=%x portid %06x feature %x type %x.\n",
+ sp->name, sp->handle, d_id->b24, fc4feature, fc4type);
+
rval = qla2x00_start_sp(sp);
if (rval != QLA_SUCCESS) {
ql_dbg(ql_dbg_disc, vha, 0x2047,
@@ -759,9 +764,6 @@ static int qla_async_rffid(scsi_qla_host_t *vha, port_id_t
*d_id,
goto done_free_sp;
}
- ql_dbg(ql_dbg_disc, vha, 0x,
- "Async-%s - hdl=%x portid %06x feature %x type %x.\n",
- sp->name, sp->handle, d_id->b24, fc4feature, fc4type);
return rval;
done_free_sp:
@@ -844,15 +846,16 @@ static int qla_async_rnnid(scsi_qla_host_t *vha,
port_id_t *d_id,
sp->u.iocb_cmd.timeout = qla2x00_async_iocb_timeout;
sp->done = qla2x00_async_sns_sp_done;
+ ql_dbg(ql_dbg_disc, vha, 0x,
+ "Async-%s - hdl=%x portid %06x\n",
+ sp->name, sp->handle, d_id->b24);
+
rval = qla2x00_start_sp(sp);
if (rval != QLA_SUCCESS) {
ql_dbg(ql_dbg_disc, vha, 0x204d,
"RNN_ID issue IOCB failed (%d).\n", rval);
goto done_free_sp;
}
- ql_dbg(ql_dbg_disc, vha, 0x,
- "Async-%s - hdl=%x portid %06x\n",
- sp->name, sp->handle, d_id->b24);
return rval;
@@ -957,15 +960,16 @@ static int qla_async_rsnn_nn(scsi_qla_host_t *vha)
sp->u.iocb_cmd.timeout = qla2x00_async_iocb_timeout;
sp->done = qla2x00_async_sns_sp_done;
+ ql_dbg(ql_dbg_disc, vha, 0x,
+ "Async-%s - hdl=%x.\n",
+ sp->name, sp->handle);
+
rval = qla2x00_start_sp(sp);
if (rval != QLA_SUCCESS) {
ql_dbg(ql_dbg_disc, vha, 0x2043,
"RFT_ID issue IOCB failed (%d).\n", rval);
goto done_free_sp;
}
- ql_dbg(ql_dbg_disc, vha, 0x,
- "Async-%s - hdl=%x.\n",
- sp->name, sp->handle);
return rval;
@@ -3578,14 +3582,14 @@ int qla24xx_async_gffid(scsi_qla_host_t *vha, fc_port_t
*fcport)
sp->done = qla24xx_async_gffid_sp_done;
- rval = qla2x00_start_sp(sp);
- if (rval != QLA_SUCCESS)
- goto done_free_sp;
-
ql_dbg(ql_dbg_disc, vha, 0x2132,
"Async-%s hdl=%x %8phC.\n", sp->name,
sp->handle, fcport->port_name);
+ rval = qla2x00_start_sp(sp);
+ if (rval != QLA_SUCCESS)
+ goto done_free_sp;
+
return rval;
done_free_sp:
sp->free(sp);
@@ -4067,6 +4071,10 @@ static int qla24xx_async_gnnft(scsi_qla_host_t *vha,
struct srb *sp,
sp->done = qla2x00_async_gpnft_gnnft_sp_done;
+ ql_dbg(ql_dbg_disc, vha, 0x,
+ "Async-%s hdl=%x FC4Type %x.\n", sp->name,
+ sp->handle, ct_req->req.gpn_ft.port_type);
+
rval = qla2x00_start_sp(sp);
if (rval != QLA_SUCCESS) {
spin_lock_irqsave(&vha->work_lock, flags);
@@ -4075,9 +4083,6 @@ static int qla24xx_async_gnnft(scsi_qla_host_t *vha,
struct srb *sp,
goto done_free_sp;
[PATCH] scsi: libsas: Fix rphy phy_identifier for PHYs with end devices attached
The sysfs phy_identifier attribute for a sas_end_device comes
from the rphy phy_identifier value.
Currently this is not being set for rphys with an end device attached,
so we see incorrect symlinks from systemd disk/by-path:
root@localhost:~# ls -l /dev/disk/by-path/
total 0
lrwxrwxrwx 1 root root 9 Feb 13 12:26
platform-HISI0162:01-sas-exp0x500e004aaa1f-phy0-lun-0 -> ../../sdb
lrwxrwxrwx 1 root root 10 Feb 13 12:26
platform-HISI0162:01-sas-exp0x500e004aaa1f-phy0-lun-0-part1 -> ../../sdb1
lrwxrwxrwx 1 root root 10 Feb 13 12:26
platform-HISI0162:01-sas-exp0x500e004aaa1f-phy0-lun-0-part2 -> ../../sdb2
lrwxrwxrwx 1 root root 10 Feb 13 12:26
platform-HISI0162:01-sas-exp0x500e004aaa1f-phy0-lun-0-part3 -> ../../sdc3
Indeed, each sas_end_device phy_identifier value is 0:
root@localhost:/# more sys/class/sas_device/end_device-0\:0\:2/phy_identifier
0
root@localhost:/# more sys/class/sas_device/end_device-0\:0\:10/phy_identifier
0
This patch fixes the discovery code to set the phy_identifier.
With this, we now get proper symlinks:
root@localhost:~# ls -l /dev/disk/by-path/
total 0
lrwxrwxrwx 1 root root 9 Feb 13 11:53
platform-HISI0162:01-sas-exp0x500e004aaa1f-phy10-lun-0 -> ../../sdg
lrwxrwxrwx 1 root root 9 Feb 13 11:53
platform-HISI0162:01-sas-exp0x500e004aaa1f-phy11-lun-0 -> ../../sdh
lrwxrwxrwx 1 root root 9 Feb 13 11:53
platform-HISI0162:01-sas-exp0x500e004aaa1f-phy2-lun-0 -> ../../sda
lrwxrwxrwx 1 root root 10 Feb 13 11:53
platform-HISI0162:01-sas-exp0x500e004aaa1f-phy2-lun-0-part1 -> ../../sda1
lrwxrwxrwx 1 root root 9 Feb 13 11:53
platform-HISI0162:01-sas-exp0x500e004aaa1f-phy3-lun-0 -> ../../sdb
lrwxrwxrwx 1 root root 10 Feb 13 11:53
platform-HISI0162:01-sas-exp0x500e004aaa1f-phy3-lun-0-part1 -> ../../sdb1
lrwxrwxrwx 1 root root 10 Feb 13 11:53
platform-HISI0162:01-sas-exp0x500e004aaa1f-phy3-lun-0-part2 -> ../../sdb2
lrwxrwxrwx 1 root root 9 Feb 13 11:53
platform-HISI0162:01-sas-exp0x500e004aaa1f-phy4-lun-0 -> ../../sdc
lrwxrwxrwx 1 root root 10 Feb 13 11:53
platform-HISI0162:01-sas-exp0x500e004aaa1f-phy4-lun-0-part1 -> ../../sdc1
lrwxrwxrwx 1 root root 10 Feb 13 11:53
platform-HISI0162:01-sas-exp0x500e004aaa1f-phy4-lun-0-part2 -> ../../sdc2
lrwxrwxrwx 1 root root 10 Feb 13 11:53
platform-HISI0162:01-sas-exp0x500e004aaa1f-phy4-lun-0-part3 -> ../../sdc3
lrwxrwxrwx 1 root root 9 Feb 13 11:53
platform-HISI0162:01-sas-exp0x500e004aaa1f-phy5-lun-0 -> ../../sdd
lrwxrwxrwx 1 root root 9 Feb 13 11:53
platform-HISI0162:01-sas-exp0x500e004aaa1f-phy7-lun-0 -> ../../sde
lrwxrwxrwx 1 root root 10 Feb 13 11:53
platform-HISI0162:01-sas-exp0x500e004aaa1f-phy7-lun-0-part1 -> ../../sde1
lrwxrwxrwx 1 root root 10 Feb 13 11:53
platform-HISI0162:01-sas-exp0x500e004aaa1f-phy7-lun-0-part2 -> ../../sde2
lrwxrwxrwx 1 root root 10 Feb 13 11:53
platform-HISI0162:01-sas-exp0x500e004aaa1f-phy7-lun-0-part3 -> ../../sde3
lrwxrwxrwx 1 root root 9 Feb 13 11:53
platform-HISI0162:01-sas-exp0x500e004aaa1f-phy8-lun-0 -> ../../sdf
lrwxrwxrwx 1 root root 10 Feb 13 11:53
platform-HISI0162:01-sas-exp0x500e004aaa1f-phy8-lun-0-part1 -> ../../sdf1
lrwxrwxrwx 1 root root 10 Feb 13 11:53
platform-HISI0162:01-sas-exp0x500e004aaa1f-phy8-lun-0-part2 -> ../../sdf2
lrwxrwxrwx 1 root root 10 Feb 13 11:53
platform-HISI0162:01-sas-exp0x500e004aaa1f-phy8-lun-0-part3 -> ../../sdf3
Fixes: 2908d778ab3e ("[SCSI] aic94xx: new driver")
Reported-by: dann frazier
Signed-off-by: John Garry
diff --git a/drivers/scsi/libsas/sas_expander.c
b/drivers/scsi/libsas/sas_expander.c
index 17eb4185f29d..f21c93bbb35c 100644
--- a/drivers/scsi/libsas/sas_expander.c
+++ b/drivers/scsi/libsas/sas_expander.c
@@ -828,6 +828,7 @@ static struct domain_device *sas_ex_discover_end_dev(
rphy = sas_end_device_alloc(phy->port);
if (!rphy)
goto out_free;
+ rphy->identify.phy_identifier = phy_id;
child->rphy = rphy;
get_device(&rphy->dev);
@@ -854,6 +855,7 @@ static struct domain_device *sas_ex_discover_end_dev(
child->rphy = rphy;
get_device(&rphy->dev);
+ rphy->identify.phy_identifier = phy_id;
sas_fill_in_rphy(child, rphy);
list_add_tail(&child->disco_list_node,
&parent->port->disco_list);
--
2.17.1
Re: [PATCH] Move debug messages before sending srb preventing panic
Hi Bill,
On 2/14/19, 7:52 AM, "Bill Kuzeja" wrote:
External Email
When sending an srb with qla2x00_start_sp, the sp can complete and be
freed by the time we log the debug message saying we sent it. This can
cause a panic if sp gets reused quickly or when running a kernel that
poisons freed memory.
This was partially fixed by (not every case was addressed):
commit 9fe278f44b4b ("scsi: qla2xxx: Move log messages before issuing
command to firmware")
Signed-off-by: Bill Kuzeja
---
drivers/scsi/qla2xxx/qla_gs.c | 66
++-
drivers/scsi/qla2xxx/qla_init.c | 26 ---
drivers/scsi/qla2xxx/qla_target.c | 8 ++---
3 files changed, 55 insertions(+), 45 deletions(-)
diff --git a/drivers/scsi/qla2xxx/qla_gs.c b/drivers/scsi/qla2xxx/qla_gs.c
index cbc3bc4..b19185a 100644
--- a/drivers/scsi/qla2xxx/qla_gs.c
+++ b/drivers/scsi/qla2xxx/qla_gs.c
@@ -657,15 +657,16 @@ static int qla_async_rftid(scsi_qla_host_t *vha,
port_id_t *d_id)
sp->u.iocb_cmd.timeout = qla2x00_async_iocb_timeout;
sp->done = qla2x00_async_sns_sp_done;
+ ql_dbg(ql_dbg_disc, vha, 0x,
+ "Async-%s - hdl=%x portid %06x.\n",
+ sp->name, sp->handle, d_id->b24);
+
rval = qla2x00_start_sp(sp);
if (rval != QLA_SUCCESS) {
ql_dbg(ql_dbg_disc, vha, 0x2043,
"RFT_ID issue IOCB failed (%d).\n", rval);
goto done_free_sp;
}
- ql_dbg(ql_dbg_disc, vha, 0x,
- "Async-%s - hdl=%x portid %06x.\n",
- sp->name, sp->handle, d_id->b24);
return rval;
done_free_sp:
sp->free(sp);
@@ -752,6 +753,10 @@ static int qla_async_rffid(scsi_qla_host_t *vha,
port_id_t *d_id,
sp->u.iocb_cmd.timeout = qla2x00_async_iocb_timeout;
sp->done = qla2x00_async_sns_sp_done;
+ ql_dbg(ql_dbg_disc, vha, 0x,
+ "Async-%s - hdl=%x portid %06x feature %x type %x.\n",
+ sp->name, sp->handle, d_id->b24, fc4feature, fc4type);
+
rval = qla2x00_start_sp(sp);
if (rval != QLA_SUCCESS) {
ql_dbg(ql_dbg_disc, vha, 0x2047,
@@ -759,9 +764,6 @@ static int qla_async_rffid(scsi_qla_host_t *vha,
port_id_t *d_id,
goto done_free_sp;
}
- ql_dbg(ql_dbg_disc, vha, 0x,
- "Async-%s - hdl=%x portid %06x feature %x type %x.\n",
- sp->name, sp->handle, d_id->b24, fc4feature, fc4type);
return rval;
done_free_sp:
@@ -844,15 +846,16 @@ static int qla_async_rnnid(scsi_qla_host_t *vha,
port_id_t *d_id,
sp->u.iocb_cmd.timeout = qla2x00_async_iocb_timeout;
sp->done = qla2x00_async_sns_sp_done;
+ ql_dbg(ql_dbg_disc, vha, 0x,
+ "Async-%s - hdl=%x portid %06x\n",
+ sp->name, sp->handle, d_id->b24);
+
rval = qla2x00_start_sp(sp);
if (rval != QLA_SUCCESS) {
ql_dbg(ql_dbg_disc, vha, 0x204d,
"RNN_ID issue IOCB failed (%d).\n", rval);
goto done_free_sp;
}
- ql_dbg(ql_dbg_disc, vha, 0x,
- "Async-%s - hdl=%x portid %06x\n",
- sp->name, sp->handle, d_id->b24);
return rval;
@@ -957,15 +960,16 @@ static int qla_async_rsnn_nn(scsi_qla_host_t *vha)
sp->u.iocb_cmd.timeout = qla2x00_async_iocb_timeout;
sp->done = qla2x00_async_sns_sp_done;
+ ql_dbg(ql_dbg_disc, vha, 0x,
+ "Async-%s - hdl=%x.\n",
+ sp->name, sp->handle);
+
rval = qla2x00_start_sp(sp);
if (rval != QLA_SUCCESS) {
ql_dbg(ql_dbg_disc, vha, 0x2043,
"RFT_ID issue IOCB failed (%d).\n", rval);
goto done_free_sp;
}
- ql_dbg(ql_dbg_disc, vha, 0x,
- "Async-%s - hdl=%x.\n",
- sp->name, sp->handle);
return rval;
@@ -3578,14 +3582,14 @@ int qla24xx_async_gffid(scsi_qla_host_t *vha,
fc_port_t *fcport)
sp->done = qla24xx_async_gffid_sp_done;
- rval = qla2x00_start_sp(sp);
- if (rval != QLA_SUCCESS)
- goto done_free_sp;
-
ql_dbg(ql_dbg_disc, vha, 0x2132,
"Async-%s hdl=%x %8phC.\n", sp->name,
sp->handle, fcport->port_name);
+ rval = qla2x00_start_sp(sp);
+ if (rval != QLA_SUCCESS)
+ goto done_free_sp;
+
return rval;
done_free_sp:
sp->free(sp);
@@ -4067,6 +4071,10 @@ static int qla24xx_async_gnnft(scsi_qla_host_t *vha,
str
Re: [PATCH v2 06/12] qla2xxx: Add support for setting port speed
Hi Bart,
On 2/13/19, 4:55 PM, "Bart Van Assche" wrote:
On Wed, 2019-02-13 at 10:53 -0800, Himanshu Madhani wrote:
> static ssize_t
> +qla2x00_sysfs_set_port_speed(struct file *filp, struct kobject *kobj,
> +struct bin_attribute *bin_attr, char *buf, loff_t off, size_t count)
> +{
> + struct scsi_qla_host *vha = shost_priv(dev_to_shost(container_of(kobj,
> + struct device, kobj)));
> + int type;
> + int mode = QLA_SET_DATA_RATE_LR;
> + int rval;
> + struct qla_hw_data *ha = vha->hw;
> + int speed, oldspeed;
> +
> + if (!IS_QLA27XX(vha->hw)) {
> + ql_log(ql_log_warn, vha, 0x70d8,
> + "Speed setting not supported \n");
> + return -EINVAL;
> + }
> +
> + speed = type = simple_strtol(buf, NULL, 10);
> + if (type == 40 || type == 80 || type == 160 ||
> + type == 320) {
> + ql_log(ql_log_warn, vha, 0x70d9,
> + "Setting will be affected after a loss of sync\n");
> + type = type/10;
> + mode = QLA_SET_DATA_RATE_NOLR;
> + }
Anil, you are supposed to use checkpatch before submitting a patch.
Checkpatch
complains about the above code:
WARNING: simple_strtol is obsolete, use kstrtol instead
#197: FILE: drivers/scsi/qla2xxx/qla_attr.c:651:
+ speed = type = simple_strtol(buf, NULL, 10);
This Warning got missed in my checkpatch testing as well. Will fix up and
repost this patch.
> + oldspeed = ha->set_data_rate;
> +
> + switch (type) {
> + case 0:
> + ha->set_data_rate = PORT_SPEED_AUTO;
> + break;
> + case 4:
> + ha->set_data_rate = PORT_SPEED_4GB;
> + break;
> + case 8:
> + ha->set_data_rate = PORT_SPEED_8GB;
> + break;
> + case 16:
> + ha->set_data_rate = PORT_SPEED_16GB;
> + break;
> + case 32:
> + ha->set_data_rate = PORT_SPEED_32GB;
> + break;
> + default:
> + ql_log(ql_log_warn, vha, 0x1199,
> + "Unrecognized speed setting:%d. Setting Autoneg\n",
> + speed);
> + ha->set_data_rate = PORT_SPEED_AUTO;
> + }
The default case is weird. Most sysfs store methods do not change any
settings
if an invalid input parameter has been supplied.
We do want to set data rate to Auto in case we get as default, if we don’t
recognize user input.
> +
> + if (qla2x00_chip_is_down(vha) || (oldspeed == ha->set_data_rate))
> + return count;
Wouldn't -EAGAIN be a better return value in this case?
Agree. Will fix this up
> +static ssize_t
> +qla2x00_sysfs_get_port_speed(struct file *filp, struct kobject *kobj,
> + struct bin_attribute *bin_attr,
> + char *buf, loff_t off, size_t count)
> +{
> + struct scsi_qla_host *vha = shost_priv(dev_to_shost(container_of(kobj,
> + struct device, kobj)));
> + struct qla_hw_data *ha = vha->hw;
> + ssize_t rval;
> + char *spd[7] = {"0", "0", "0", "4", "8", "16", "32"};
This should be a "static const char *" array.
Yes. Will update patch
> + ql_log(ql_log_info, vha, 0x70d6,
> + "port speed:%d\n", ha->link_data_rate);
This looks like a debug statement. Log level "debug" is probably more
appropriate.
Yes. Will update
> +static struct bin_attribute sysfs_port_speed_attr = {
> + .attr = {
> + .name = "port_speed",
> + .mode = 0600,
> + },
> + .size = 16,
> + .write = qla2x00_sysfs_set_port_speed,
> + .read = qla2x00_sysfs_get_port_speed,
> +};
This needs an explanation of why you choose to make this a binary
attribute. And if
you don't have a very good reason to make this attribute a binary
attribute, it should
be changed into a regular attribute.
It does look like there is no specific reason for this attribute to be binary.
Will update it to use regular attribute and post revised patch
> +/* Set the specified data rate */
> +int
> +qla2x00_set_data_rate(scsi_qla_host_t *vha, uint16_t mode)
> +{
> + int rval;
> + mbx_cmd_t mc;
> + mbx_cmd_t *mcp = &mc;
> + struct qla_hw_data *ha = vha->hw;
> + uint16_t val;
> +
> + ql_dbg(ql_dbg_mbx + ql_dbg_verbose, vha, 0x1106,
> + "Entered %s speed:0x%x mode:0x%x.\n", __func__, ha->set_data_rate,
> + mode);
> +
> + if (!IS_FWI2_CAPABLE(ha))
> + return QLA_FUNCTION_FAILED;
> +
> + memset(mcp, 0, sizeof(mbx_cmd_t));
Please change sizeof(mbx_cmd_t) into sizeof(*mcp).
Will Do.
Thanks,
Bart.
Thanks,
-- Himanshu
Re: [PATCH 0/4] scsi: fixup dma_set_mask_and_coherent() calls
On Thu, Feb 14, 2019 at 09:29:05AM +, John Garry wrote: > On 13/02/2019 18:51, Ewan D. Milne wrote: >> On Wed, 2019-02-13 at 12:42 +0100, Hannes Reinecke wrote: >>> The recent patchset to use dma_set_mask_and_coherent() introduced >>> a regression where a call to set a 64-bit DMA mask was followed >>> by a call to set a 32-bit DMA mask, leading to I/O errors and >>> data corruption. >>> >>> Patchset is based on a suggestions from Ewan Milne. >>> >>> Hannes Reinecke (4): >>> lpfc: fix calls to dma_set_mask_and_coherent() >>> hptiop: fix calls to dma_set_mask_and_coherent() >>> bfa: fix calls to dma_set_mask_and_coherent() >>> hisi_sas: fix calls to dma_set_mask_and_coherent() >>> >>> drivers/scsi/bfa/bfad.c | 10 +++--- >>> drivers/scsi/hisi_sas/hisi_sas_main.c | 8 ++-- >>> drivers/scsi/hptiop.c | 10 +++--- >>> drivers/scsi/lpfc/lpfc_init.c | 9 ++--- >>> 4 files changed, 26 insertions(+), 11 deletions(-) > > Isn't there a few more to fix up, like: > drivers/scsi/3w-9xxx.c > drivers/scsi/3w-sas.c > drivers/scsi/csiostor/csio_init.c Yeah, there is a few more. And the sad part is as of a few kernel release ago we shouldn't even need the fallback 32-bit dma mask anymore - we've cleaned up all the mess that required it.
Re: [PATCH -next] scsi: lpfc: Remove set but not used variable 'phys_id'
On 2/13/2019 5:51 PM, YueHaibing wrote:
Fixes gcc '-Wunused-but-set-variable' warning:
drivers/scsi/lpfc/lpfc_init.c: In function 'lpfc_cpu_affinity_check':
drivers/scsi/lpfc/lpfc_init.c:10599:19: warning:
variable 'phys_id' set but not used [-Wunused-but-set-variable]
It never used since introduction in commit 6a828b0f6192 ("scsi: lpfc: Support
non-uniform allocation of MSIX vectors to hardware queues")
Signed-off-by: YueHaibing
---
drivers/scsi/lpfc/lpfc_init.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
Looks fine. Thanks
Signed-off-by: James Smart
-- james
Re: [PATCH -next] scsi: lpfc: Remove set but not used variable 'phys_id'
On Thu, 2019-02-14 at 10:52 -0800, James Smart wrote:
>
> On 2/13/2019 5:51 PM, YueHaibing wrote:
> > Fixes gcc '-Wunused-but-set-variable' warning:
> >
> > drivers/scsi/lpfc/lpfc_init.c: In function
> > 'lpfc_cpu_affinity_check':
> > drivers/scsi/lpfc/lpfc_init.c:10599:19: warning:
> > variable 'phys_id' set but not used [-Wunused-but-set-variable]
> >
> > It never used since introduction in commit 6a828b0f6192 ("scsi:
> > lpfc: Support
> > non-uniform allocation of MSIX vectors to hardware queues")
> >
> > Signed-off-by: YueHaibing
> > ---
> > drivers/scsi/lpfc/lpfc_init.c | 3 +--
> > 1 file changed, 1 insertion(+), 2 deletions(-)
> >
> >
>
> Looks fine. Thanks
>
> Signed-off-by: James Smart
Under the DCO this can't be a Signed-off-by tag: signoffs track the
patch transmission path under the DCO, so unless you send it you can't
add your signoff.
If you just want Martin to apply it now, and you don't want to gather
and resend it with your other lpfc patches, I think the tag you want is
Acked-by.
James
Re: [EXT] Re: [PATCH v2 08/12] qla2xxx: Add workqueue to delete fcport from bsg sp->free().
Hi Bart,
On 2/13/19, 4:57 PM, "Bart Van Assche" wrote:
External Email
--
On Wed, 2019-02-13 at 10:53 -0800, Himanshu Madhani wrote:
> + ha->free_fcport = create_workqueue("free_fcport");
> + if (!ha->free_fcport) {
> + ql_log(ql_log_info, base_vha, 0xee00,
> + "Failed to allocate workqueue ha->free_fcport\n");
> + }
Above this code either an explanation should be added why the system
work queues are not appropriate or this code should be modified such
that it uses one of the system workqueues.
Will look into using system work queue and repost later.
Bart.
Thanks,
Himanshu
Re: [PATCH -next] scsi: lpfc: Remove set but not used variable 'phys_id'
James, > If you just want Martin to apply it now, and you don't want to gather > and resend it with your other lpfc patches, I think the tag you want > is Acked-by. I usually fix these up when I commit so no need to resend. But please make sure to use the right tag. -- Martin K. Petersen Oracle Linux Engineering
[PATCH] sd: disable logical block provisioning if 'lbpme' is not set
From: Hannes Reinecke
When evaluating the 'block limits' VPD page we need to check if
the 'lbpme' (logical block provisioning management enable) bit
is set in the READ CAPACITY (16) output.
If it isn't we can safely assume that we cannot use DISCARD on
this device.
[JD: forward-ported to kernel v4.20]
Signed-off-by: Hannes Reinecke
Signed-off-by: Jean Delvare
---
Hannes, please double-check that my forward-port is correct.
drivers/scsi/sd.c | 11 ++-
1 file changed, 10 insertions(+), 1 deletion(-)
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -411,6 +411,13 @@ provisioning_mode_store(struct device *d
if (mode < 0)
return -EINVAL;
+ /*
+* If logical block provisioning isn't enabled we can only
+* select 'disable' here.
+*/
+ if (!sdkp->lbpme && mode != SD_LBP_DISABLE)
+ return -EINVAL;
+
sd_config_discard(sdkp, mode);
return count;
@@ -2942,8 +2949,10 @@ static void sd_read_block_limits(struct
sdkp->max_ws_blocks = (u32)get_unaligned_be64(&buffer[36]);
- if (!sdkp->lbpme)
+ if (!sdkp->lbpme) {
+ sd_config_discard(sdkp, SD_LBP_DISABLE);
goto out;
+ }
lba_count = get_unaligned_be32(&buffer[20]);
desc_count = get_unaligned_be32(&buffer[24]);
--
Jean Delvare
SUSE L3 Support
Re: [PATCH -next] scsi: lpfc: Remove set but not used variable 'phys_id'
On 2/14/2019 11:39 AM, James Bottomley wrote:
On Thu, 2019-02-14 at 10:52 -0800, James Smart wrote:
On 2/13/2019 5:51 PM, YueHaibing wrote:
Fixes gcc '-Wunused-but-set-variable' warning:
drivers/scsi/lpfc/lpfc_init.c: In function
'lpfc_cpu_affinity_check':
drivers/scsi/lpfc/lpfc_init.c:10599:19: warning:
variable 'phys_id' set but not used [-Wunused-but-set-variable]
It never used since introduction in commit 6a828b0f6192 ("scsi:
lpfc: Support
non-uniform allocation of MSIX vectors to hardware queues")
Signed-off-by: YueHaibing
---
drivers/scsi/lpfc/lpfc_init.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
Looks fine. Thanks
Signed-off-by: James Smart
Under the DCO this can't be a Signed-off-by tag: signoffs track the
patch transmission path under the DCO, so unless you send it you can't
add your signoff.
If you just want Martin to apply it now, and you don't want to gather
and resend it with your other lpfc patches, I think the tag you want is
Acked-by.
James
I've been told multiple answers on which way to reply over the years. My
initial position followed your statement, but a later rebuke (would have
to look hard to find it) told me as maintainer that I should be doing
something different. I'll go back to the DCO definitions and follow those.
Thanks for cleaning it up Martin.
-- james
Re: [PATCH -next] scsi: lpfc: Remove set but not used variable 'phys_id'
On Thu, 2019-02-14 at 13:19 -0800, James Smart wrote:
>
> On 2/14/2019 11:39 AM, James Bottomley wrote:
> > On Thu, 2019-02-14 at 10:52 -0800, James Smart wrote:
> > > On 2/13/2019 5:51 PM, YueHaibing wrote:
> > > > Fixes gcc '-Wunused-but-set-variable' warning:
> > > >
> > > > drivers/scsi/lpfc/lpfc_init.c: In function
> > > > 'lpfc_cpu_affinity_check':
> > > > drivers/scsi/lpfc/lpfc_init.c:10599:19: warning:
> > > >variable 'phys_id' set but not used [-Wunused-but-set-
> > > > variable]
> > > >
> > > > It never used since introduction in commit 6a828b0f6192 ("scsi:
> > > > lpfc: Support
> > > > non-uniform allocation of MSIX vectors to hardware queues")
> > > >
> > > > Signed-off-by: YueHaibing
> > > > ---
> > > >drivers/scsi/lpfc/lpfc_init.c | 3 +--
> > > >1 file changed, 1 insertion(+), 2 deletions(-)
> > > >
> > > >
> > >
> > > Looks fine. Thanks
> > >
> > > Signed-off-by: James Smart
> >
> > Under the DCO this can't be a Signed-off-by tag: signoffs track the
> > patch transmission path under the DCO, so unless you send it you
> > can't
> > add your signoff.
> >
> > If you just want Martin to apply it now, and you don't want to
> > gather and resend it with your other lpfc patches, I think the tag
> > you want is Acked-by.
> >
> > James
>
> I've been told multiple answers on which way to reply over the years.
> My initial position followed your statement, but a later rebuke
> (would have to look hard to find it)
If you could, that would help me find and correct the source ...
> told me as maintainer that I should be doing something different.
> I'll go back to the DCO definitions and follow
> those.
Great, thanks. Our only tag with formal requirements from the DCO is
Signed-off-by: everything else is undefined by the DCO (and thus more
prone to being argued over and having more per-subsystem meanings that
lead to irreconcilable differences between subsystem use).
James
> Thanks for cleaning it up Martin.
>
> -- james
>
[PATCH] scsi: reset host byte in DID_NEXUS_FAILURE case
Up to 4.12, __scsi_error_from_host_byte() would reset the host
byte to DID_OK for various cases including DID_NEXUS_FAILURE.
Commit 2a842acab109 ("block: introduce new block status code type")
replaced this function with scsi_result_to_blk_status() and removed
the host-byte resetting code for the DID_NEXUS_FAILURE case.
As the line set_host_byte(cmd, DID_OK) was preserved for the other
cases, I suppose this was an editing mistake.
The fact that the host byte remains set after 4.13 is causing
problems with the sg_persist tool, which now returns success
rather then exit status 24 when a RESERVATION CONFLICT error is
encountered.
Fixes: 2a842acab109 "block: introduce new block status code type"
Signed-off-by: Martin Wilck
---
drivers/scsi/scsi_lib.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index 6d65ac5..f8d51c3 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -655,6 +655,7 @@ static blk_status_t scsi_result_to_blk_status(struct
scsi_cmnd *cmd, int result)
set_host_byte(cmd, DID_OK);
return BLK_STS_TARGET;
case DID_NEXUS_FAILURE:
+ set_host_byte(cmd, DID_OK);
return BLK_STS_NEXUS;
case DID_ALLOC_FAILURE:
set_host_byte(cmd, DID_OK);
--
2.20.1
[PATCH AUTOSEL 3.18 10/16] scsi: csiostor: fix NULL pointer dereference in csio_vport_set_state()
From: Varun Prakash [ Upstream commit fe35a40e675473eb65f2f5462b82770f324b5689 ] Assign fc_vport to ln->fc_vport before calling csio_fcoe_alloc_vnp() to avoid a NULL pointer dereference in csio_vport_set_state(). ln->fc_vport is dereferenced in csio_vport_set_state(). Signed-off-by: Varun Prakash Signed-off-by: Martin K. Petersen Signed-off-by: Sasha Levin --- drivers/scsi/csiostor/csio_attr.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/scsi/csiostor/csio_attr.c b/drivers/scsi/csiostor/csio_attr.c index 065a87ace623..22b800b5ac7f 100644 --- a/drivers/scsi/csiostor/csio_attr.c +++ b/drivers/scsi/csiostor/csio_attr.c @@ -582,12 +582,12 @@ csio_vport_create(struct fc_vport *fc_vport, bool disable) } fc_vport_set_state(fc_vport, FC_VPORT_INITIALIZING); + ln->fc_vport = fc_vport; if (csio_fcoe_alloc_vnp(hw, ln)) goto error; *(struct csio_lnode **)fc_vport->dd_data = ln; - ln->fc_vport = fc_vport; if (!fc_vport->node_name) fc_vport->node_name = wwn_to_u64(csio_ln_wwnn(ln)); if (!fc_vport->port_name) -- 2.19.1
[PATCH AUTOSEL 4.4 13/20] scsi: csiostor: fix NULL pointer dereference in csio_vport_set_state()
From: Varun Prakash [ Upstream commit fe35a40e675473eb65f2f5462b82770f324b5689 ] Assign fc_vport to ln->fc_vport before calling csio_fcoe_alloc_vnp() to avoid a NULL pointer dereference in csio_vport_set_state(). ln->fc_vport is dereferenced in csio_vport_set_state(). Signed-off-by: Varun Prakash Signed-off-by: Martin K. Petersen Signed-off-by: Sasha Levin --- drivers/scsi/csiostor/csio_attr.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/scsi/csiostor/csio_attr.c b/drivers/scsi/csiostor/csio_attr.c index 2d1c4ebd40f9..6587f20cff1a 100644 --- a/drivers/scsi/csiostor/csio_attr.c +++ b/drivers/scsi/csiostor/csio_attr.c @@ -582,12 +582,12 @@ csio_vport_create(struct fc_vport *fc_vport, bool disable) } fc_vport_set_state(fc_vport, FC_VPORT_INITIALIZING); + ln->fc_vport = fc_vport; if (csio_fcoe_alloc_vnp(hw, ln)) goto error; *(struct csio_lnode **)fc_vport->dd_data = ln; - ln->fc_vport = fc_vport; if (!fc_vport->node_name) fc_vport->node_name = wwn_to_u64(csio_ln_wwnn(ln)); if (!fc_vport->port_name) -- 2.19.1
[PATCH AUTOSEL 4.9 17/27] scsi: csiostor: fix NULL pointer dereference in csio_vport_set_state()
From: Varun Prakash [ Upstream commit fe35a40e675473eb65f2f5462b82770f324b5689 ] Assign fc_vport to ln->fc_vport before calling csio_fcoe_alloc_vnp() to avoid a NULL pointer dereference in csio_vport_set_state(). ln->fc_vport is dereferenced in csio_vport_set_state(). Signed-off-by: Varun Prakash Signed-off-by: Martin K. Petersen Signed-off-by: Sasha Levin --- drivers/scsi/csiostor/csio_attr.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/scsi/csiostor/csio_attr.c b/drivers/scsi/csiostor/csio_attr.c index 2d1c4ebd40f9..6587f20cff1a 100644 --- a/drivers/scsi/csiostor/csio_attr.c +++ b/drivers/scsi/csiostor/csio_attr.c @@ -582,12 +582,12 @@ csio_vport_create(struct fc_vport *fc_vport, bool disable) } fc_vport_set_state(fc_vport, FC_VPORT_INITIALIZING); + ln->fc_vport = fc_vport; if (csio_fcoe_alloc_vnp(hw, ln)) goto error; *(struct csio_lnode **)fc_vport->dd_data = ln; - ln->fc_vport = fc_vport; if (!fc_vport->node_name) fc_vport->node_name = wwn_to_u64(csio_ln_wwnn(ln)); if (!fc_vport->port_name) -- 2.19.1
[PATCH AUTOSEL 4.14 28/40] scsi: csiostor: fix NULL pointer dereference in csio_vport_set_state()
From: Varun Prakash [ Upstream commit fe35a40e675473eb65f2f5462b82770f324b5689 ] Assign fc_vport to ln->fc_vport before calling csio_fcoe_alloc_vnp() to avoid a NULL pointer dereference in csio_vport_set_state(). ln->fc_vport is dereferenced in csio_vport_set_state(). Signed-off-by: Varun Prakash Signed-off-by: Martin K. Petersen Signed-off-by: Sasha Levin --- drivers/scsi/csiostor/csio_attr.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/scsi/csiostor/csio_attr.c b/drivers/scsi/csiostor/csio_attr.c index 2d1c4ebd40f9..6587f20cff1a 100644 --- a/drivers/scsi/csiostor/csio_attr.c +++ b/drivers/scsi/csiostor/csio_attr.c @@ -582,12 +582,12 @@ csio_vport_create(struct fc_vport *fc_vport, bool disable) } fc_vport_set_state(fc_vport, FC_VPORT_INITIALIZING); + ln->fc_vport = fc_vport; if (csio_fcoe_alloc_vnp(hw, ln)) goto error; *(struct csio_lnode **)fc_vport->dd_data = ln; - ln->fc_vport = fc_vport; if (!fc_vport->node_name) fc_vport->node_name = wwn_to_u64(csio_ln_wwnn(ln)); if (!fc_vport->port_name) -- 2.19.1
[PATCH AUTOSEL 4.19 45/65] scsi: csiostor: fix NULL pointer dereference in csio_vport_set_state()
From: Varun Prakash [ Upstream commit fe35a40e675473eb65f2f5462b82770f324b5689 ] Assign fc_vport to ln->fc_vport before calling csio_fcoe_alloc_vnp() to avoid a NULL pointer dereference in csio_vport_set_state(). ln->fc_vport is dereferenced in csio_vport_set_state(). Signed-off-by: Varun Prakash Signed-off-by: Martin K. Petersen Signed-off-by: Sasha Levin --- drivers/scsi/csiostor/csio_attr.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/scsi/csiostor/csio_attr.c b/drivers/scsi/csiostor/csio_attr.c index 8a004036e3d7..9bd2bd8dc2be 100644 --- a/drivers/scsi/csiostor/csio_attr.c +++ b/drivers/scsi/csiostor/csio_attr.c @@ -594,12 +594,12 @@ csio_vport_create(struct fc_vport *fc_vport, bool disable) } fc_vport_set_state(fc_vport, FC_VPORT_INITIALIZING); + ln->fc_vport = fc_vport; if (csio_fcoe_alloc_vnp(hw, ln)) goto error; *(struct csio_lnode **)fc_vport->dd_data = ln; - ln->fc_vport = fc_vport; if (!fc_vport->node_name) fc_vport->node_name = wwn_to_u64(csio_ln_wwnn(ln)); if (!fc_vport->port_name) -- 2.19.1
[PATCH AUTOSEL 4.19 43/65] scsi: lpfc: nvme: avoid hang / use-after-free when destroying localport
From: "Ewan D. Milne"
[ Upstream commit 7961cba6f7d8215fa632df3d220e5154bb825249 ]
We cannot wait on a completion object in the lpfc_nvme_lport structure in
the _destroy_localport() code path because the NVMe/fc transport will free
that structure immediately after the .localport_delete() callback. This
results in a use-after-free, and a hang if slub_debug=FZPU is enabled.
Fix this by putting the completion on the stack.
Signed-off-by: Ewan D. Milne
Acked-by: James Smart
Signed-off-by: Martin K. Petersen
Signed-off-by: Sasha Levin
---
drivers/scsi/lpfc/lpfc_nvme.c | 16 +---
drivers/scsi/lpfc/lpfc_nvme.h | 2 +-
2 files changed, 10 insertions(+), 8 deletions(-)
diff --git a/drivers/scsi/lpfc/lpfc_nvme.c b/drivers/scsi/lpfc/lpfc_nvme.c
index 918ae18ef8a8..ca62117a2d13 100644
--- a/drivers/scsi/lpfc/lpfc_nvme.c
+++ b/drivers/scsi/lpfc/lpfc_nvme.c
@@ -297,7 +297,8 @@ lpfc_nvme_localport_delete(struct nvme_fc_local_port
*localport)
lport);
/* release any threads waiting for the unreg to complete */
- complete(&lport->lport_unreg_done);
+ if (lport->vport->localport)
+ complete(lport->lport_unreg_cmp);
}
/* lpfc_nvme_remoteport_delete
@@ -2556,7 +2557,8 @@ lpfc_nvme_create_localport(struct lpfc_vport *vport)
*/
void
lpfc_nvme_lport_unreg_wait(struct lpfc_vport *vport,
- struct lpfc_nvme_lport *lport)
+ struct lpfc_nvme_lport *lport,
+ struct completion *lport_unreg_cmp)
{
#if (IS_ENABLED(CONFIG_NVME_FC))
u32 wait_tmo;
@@ -2568,8 +2570,7 @@ lpfc_nvme_lport_unreg_wait(struct lpfc_vport *vport,
*/
wait_tmo = msecs_to_jiffies(LPFC_NVME_WAIT_TMO * 1000);
while (true) {
- ret = wait_for_completion_timeout(&lport->lport_unreg_done,
- wait_tmo);
+ ret = wait_for_completion_timeout(lport_unreg_cmp, wait_tmo);
if (unlikely(!ret)) {
lpfc_printf_vlog(vport, KERN_ERR, LOG_NVME_IOERR,
"6176 Lport %p Localport %p wait "
@@ -2603,12 +2604,12 @@ lpfc_nvme_destroy_localport(struct lpfc_vport *vport)
struct lpfc_nvme_lport *lport;
struct lpfc_nvme_ctrl_stat *cstat;
int ret;
+ DECLARE_COMPLETION_ONSTACK(lport_unreg_cmp);
if (vport->nvmei_support == 0)
return;
localport = vport->localport;
- vport->localport = NULL;
lport = (struct lpfc_nvme_lport *)localport->private;
cstat = lport->cstat;
@@ -2619,13 +2620,14 @@ lpfc_nvme_destroy_localport(struct lpfc_vport *vport)
/* lport's rport list is clear. Unregister
* lport and release resources.
*/
- init_completion(&lport->lport_unreg_done);
+ lport->lport_unreg_cmp = &lport_unreg_cmp;
ret = nvme_fc_unregister_localport(localport);
/* Wait for completion. This either blocks
* indefinitely or succeeds
*/
- lpfc_nvme_lport_unreg_wait(vport, lport);
+ lpfc_nvme_lport_unreg_wait(vport, lport, &lport_unreg_cmp);
+ vport->localport = NULL;
kfree(cstat);
/* Regardless of the unregister upcall response, clear
diff --git a/drivers/scsi/lpfc/lpfc_nvme.h b/drivers/scsi/lpfc/lpfc_nvme.h
index cfd4719be25c..b234d0298994 100644
--- a/drivers/scsi/lpfc/lpfc_nvme.h
+++ b/drivers/scsi/lpfc/lpfc_nvme.h
@@ -50,7 +50,7 @@ struct lpfc_nvme_ctrl_stat {
/* Declare nvme-based local and remote port definitions. */
struct lpfc_nvme_lport {
struct lpfc_vport *vport;
- struct completion lport_unreg_done;
+ struct completion *lport_unreg_cmp;
/* Add stats counters here */
struct lpfc_nvme_ctrl_stat *cstat;
atomic_t fc4NvmeLsRequests;
--
2.19.1
[PATCH AUTOSEL 4.19 44/65] scsi: lpfc: nvmet: avoid hang / use-after-free when destroying targetport
From: "Ewan D. Milne"
[ Upstream commit c41f59884be5cca293ed61f3d64637dbba3a6381 ]
We cannot wait on a completion object in the lpfc_nvme_targetport structure
in the _destroy_targetport() code path because the NVMe/fc transport will
free that structure immediately after the .targetport_delete() callback.
This results in a use-after-free, and a hang if slub_debug=FZPU is enabled.
Fix this by putting the completion on the stack.
Signed-off-by: Ewan D. Milne
Acked-by: James Smart
Signed-off-by: Martin K. Petersen
Signed-off-by: Sasha Levin
---
drivers/scsi/lpfc/lpfc_nvmet.c | 8 +---
drivers/scsi/lpfc/lpfc_nvmet.h | 2 +-
2 files changed, 6 insertions(+), 4 deletions(-)
diff --git a/drivers/scsi/lpfc/lpfc_nvmet.c b/drivers/scsi/lpfc/lpfc_nvmet.c
index b766afe10d3d..e2575c8ec93e 100644
--- a/drivers/scsi/lpfc/lpfc_nvmet.c
+++ b/drivers/scsi/lpfc/lpfc_nvmet.c
@@ -1003,7 +1003,8 @@ lpfc_nvmet_targetport_delete(struct nvmet_fc_target_port
*targetport)
struct lpfc_nvmet_tgtport *tport = targetport->private;
/* release any threads waiting for the unreg to complete */
- complete(&tport->tport_unreg_done);
+ if (tport->phba->targetport)
+ complete(tport->tport_unreg_cmp);
}
static void
@@ -1700,6 +1701,7 @@ lpfc_nvmet_destroy_targetport(struct lpfc_hba *phba)
struct lpfc_nvmet_tgtport *tgtp;
struct lpfc_queue *wq;
uint32_t qidx;
+ DECLARE_COMPLETION_ONSTACK(tport_unreg_cmp);
if (phba->nvmet_support == 0)
return;
@@ -1709,9 +1711,9 @@ lpfc_nvmet_destroy_targetport(struct lpfc_hba *phba)
wq = phba->sli4_hba.nvme_wq[qidx];
lpfc_nvmet_wqfull_flush(phba, wq, NULL);
}
- init_completion(&tgtp->tport_unreg_done);
+ tgtp->tport_unreg_cmp = &tport_unreg_cmp;
nvmet_fc_unregister_targetport(phba->targetport);
- wait_for_completion_timeout(&tgtp->tport_unreg_done, 5);
+ wait_for_completion_timeout(&tport_unreg_cmp, 5);
lpfc_nvmet_cleanup_io_context(phba);
}
phba->targetport = NULL;
diff --git a/drivers/scsi/lpfc/lpfc_nvmet.h b/drivers/scsi/lpfc/lpfc_nvmet.h
index 1aaff63f1f41..0ec1082ce7ef 100644
--- a/drivers/scsi/lpfc/lpfc_nvmet.h
+++ b/drivers/scsi/lpfc/lpfc_nvmet.h
@@ -34,7 +34,7 @@
/* Used for NVME Target */
struct lpfc_nvmet_tgtport {
struct lpfc_hba *phba;
- struct completion tport_unreg_done;
+ struct completion *tport_unreg_cmp;
/* Stats counters - lpfc_nvmet_unsol_ls_buffer */
atomic_t rcv_ls_req_in;
--
2.19.1
[PATCH AUTOSEL 4.20 53/77] scsi: csiostor: fix NULL pointer dereference in csio_vport_set_state()
From: Varun Prakash [ Upstream commit fe35a40e675473eb65f2f5462b82770f324b5689 ] Assign fc_vport to ln->fc_vport before calling csio_fcoe_alloc_vnp() to avoid a NULL pointer dereference in csio_vport_set_state(). ln->fc_vport is dereferenced in csio_vport_set_state(). Signed-off-by: Varun Prakash Signed-off-by: Martin K. Petersen Signed-off-by: Sasha Levin --- drivers/scsi/csiostor/csio_attr.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/scsi/csiostor/csio_attr.c b/drivers/scsi/csiostor/csio_attr.c index 8a004036e3d7..9bd2bd8dc2be 100644 --- a/drivers/scsi/csiostor/csio_attr.c +++ b/drivers/scsi/csiostor/csio_attr.c @@ -594,12 +594,12 @@ csio_vport_create(struct fc_vport *fc_vport, bool disable) } fc_vport_set_state(fc_vport, FC_VPORT_INITIALIZING); + ln->fc_vport = fc_vport; if (csio_fcoe_alloc_vnp(hw, ln)) goto error; *(struct csio_lnode **)fc_vport->dd_data = ln; - ln->fc_vport = fc_vport; if (!fc_vport->node_name) fc_vport->node_name = wwn_to_u64(csio_ln_wwnn(ln)); if (!fc_vport->port_name) -- 2.19.1
[PATCH AUTOSEL 4.20 52/77] scsi: lpfc: nvmet: avoid hang / use-after-free when destroying targetport
From: "Ewan D. Milne"
[ Upstream commit c41f59884be5cca293ed61f3d64637dbba3a6381 ]
We cannot wait on a completion object in the lpfc_nvme_targetport structure
in the _destroy_targetport() code path because the NVMe/fc transport will
free that structure immediately after the .targetport_delete() callback.
This results in a use-after-free, and a hang if slub_debug=FZPU is enabled.
Fix this by putting the completion on the stack.
Signed-off-by: Ewan D. Milne
Acked-by: James Smart
Signed-off-by: Martin K. Petersen
Signed-off-by: Sasha Levin
---
drivers/scsi/lpfc/lpfc_nvmet.c | 8 +---
drivers/scsi/lpfc/lpfc_nvmet.h | 2 +-
2 files changed, 6 insertions(+), 4 deletions(-)
diff --git a/drivers/scsi/lpfc/lpfc_nvmet.c b/drivers/scsi/lpfc/lpfc_nvmet.c
index 6245f442d784..95fee83090eb 100644
--- a/drivers/scsi/lpfc/lpfc_nvmet.c
+++ b/drivers/scsi/lpfc/lpfc_nvmet.c
@@ -1003,7 +1003,8 @@ lpfc_nvmet_targetport_delete(struct nvmet_fc_target_port
*targetport)
struct lpfc_nvmet_tgtport *tport = targetport->private;
/* release any threads waiting for the unreg to complete */
- complete(&tport->tport_unreg_done);
+ if (tport->phba->targetport)
+ complete(tport->tport_unreg_cmp);
}
static void
@@ -1692,6 +1693,7 @@ lpfc_nvmet_destroy_targetport(struct lpfc_hba *phba)
struct lpfc_nvmet_tgtport *tgtp;
struct lpfc_queue *wq;
uint32_t qidx;
+ DECLARE_COMPLETION_ONSTACK(tport_unreg_cmp);
if (phba->nvmet_support == 0)
return;
@@ -1701,9 +1703,9 @@ lpfc_nvmet_destroy_targetport(struct lpfc_hba *phba)
wq = phba->sli4_hba.nvme_wq[qidx];
lpfc_nvmet_wqfull_flush(phba, wq, NULL);
}
- init_completion(&tgtp->tport_unreg_done);
+ tgtp->tport_unreg_cmp = &tport_unreg_cmp;
nvmet_fc_unregister_targetport(phba->targetport);
- wait_for_completion_timeout(&tgtp->tport_unreg_done, 5);
+ wait_for_completion_timeout(&tport_unreg_cmp, 5);
lpfc_nvmet_cleanup_io_context(phba);
}
phba->targetport = NULL;
diff --git a/drivers/scsi/lpfc/lpfc_nvmet.h b/drivers/scsi/lpfc/lpfc_nvmet.h
index 1aaff63f1f41..0ec1082ce7ef 100644
--- a/drivers/scsi/lpfc/lpfc_nvmet.h
+++ b/drivers/scsi/lpfc/lpfc_nvmet.h
@@ -34,7 +34,7 @@
/* Used for NVME Target */
struct lpfc_nvmet_tgtport {
struct lpfc_hba *phba;
- struct completion tport_unreg_done;
+ struct completion *tport_unreg_cmp;
/* Stats counters - lpfc_nvmet_unsol_ls_buffer */
atomic_t rcv_ls_req_in;
--
2.19.1
[PATCH AUTOSEL 4.20 51/77] scsi: lpfc: nvme: avoid hang / use-after-free when destroying localport
From: "Ewan D. Milne"
[ Upstream commit 7961cba6f7d8215fa632df3d220e5154bb825249 ]
We cannot wait on a completion object in the lpfc_nvme_lport structure in
the _destroy_localport() code path because the NVMe/fc transport will free
that structure immediately after the .localport_delete() callback. This
results in a use-after-free, and a hang if slub_debug=FZPU is enabled.
Fix this by putting the completion on the stack.
Signed-off-by: Ewan D. Milne
Acked-by: James Smart
Signed-off-by: Martin K. Petersen
Signed-off-by: Sasha Levin
---
drivers/scsi/lpfc/lpfc_nvme.c | 16 +---
drivers/scsi/lpfc/lpfc_nvme.h | 2 +-
2 files changed, 10 insertions(+), 8 deletions(-)
diff --git a/drivers/scsi/lpfc/lpfc_nvme.c b/drivers/scsi/lpfc/lpfc_nvme.c
index ba831def9301..b6fe88de372a 100644
--- a/drivers/scsi/lpfc/lpfc_nvme.c
+++ b/drivers/scsi/lpfc/lpfc_nvme.c
@@ -297,7 +297,8 @@ lpfc_nvme_localport_delete(struct nvme_fc_local_port
*localport)
lport);
/* release any threads waiting for the unreg to complete */
- complete(&lport->lport_unreg_done);
+ if (lport->vport->localport)
+ complete(lport->lport_unreg_cmp);
}
/* lpfc_nvme_remoteport_delete
@@ -2547,7 +2548,8 @@ lpfc_nvme_create_localport(struct lpfc_vport *vport)
*/
void
lpfc_nvme_lport_unreg_wait(struct lpfc_vport *vport,
- struct lpfc_nvme_lport *lport)
+ struct lpfc_nvme_lport *lport,
+ struct completion *lport_unreg_cmp)
{
#if (IS_ENABLED(CONFIG_NVME_FC))
u32 wait_tmo;
@@ -2559,8 +2561,7 @@ lpfc_nvme_lport_unreg_wait(struct lpfc_vport *vport,
*/
wait_tmo = msecs_to_jiffies(LPFC_NVME_WAIT_TMO * 1000);
while (true) {
- ret = wait_for_completion_timeout(&lport->lport_unreg_done,
- wait_tmo);
+ ret = wait_for_completion_timeout(lport_unreg_cmp, wait_tmo);
if (unlikely(!ret)) {
lpfc_printf_vlog(vport, KERN_ERR, LOG_NVME_IOERR,
"6176 Lport %p Localport %p wait "
@@ -2594,12 +2595,12 @@ lpfc_nvme_destroy_localport(struct lpfc_vport *vport)
struct lpfc_nvme_lport *lport;
struct lpfc_nvme_ctrl_stat *cstat;
int ret;
+ DECLARE_COMPLETION_ONSTACK(lport_unreg_cmp);
if (vport->nvmei_support == 0)
return;
localport = vport->localport;
- vport->localport = NULL;
lport = (struct lpfc_nvme_lport *)localport->private;
cstat = lport->cstat;
@@ -2610,13 +2611,14 @@ lpfc_nvme_destroy_localport(struct lpfc_vport *vport)
/* lport's rport list is clear. Unregister
* lport and release resources.
*/
- init_completion(&lport->lport_unreg_done);
+ lport->lport_unreg_cmp = &lport_unreg_cmp;
ret = nvme_fc_unregister_localport(localport);
/* Wait for completion. This either blocks
* indefinitely or succeeds
*/
- lpfc_nvme_lport_unreg_wait(vport, lport);
+ lpfc_nvme_lport_unreg_wait(vport, lport, &lport_unreg_cmp);
+ vport->localport = NULL;
kfree(cstat);
/* Regardless of the unregister upcall response, clear
diff --git a/drivers/scsi/lpfc/lpfc_nvme.h b/drivers/scsi/lpfc/lpfc_nvme.h
index cfd4719be25c..b234d0298994 100644
--- a/drivers/scsi/lpfc/lpfc_nvme.h
+++ b/drivers/scsi/lpfc/lpfc_nvme.h
@@ -50,7 +50,7 @@ struct lpfc_nvme_ctrl_stat {
/* Declare nvme-based local and remote port definitions. */
struct lpfc_nvme_lport {
struct lpfc_vport *vport;
- struct completion lport_unreg_done;
+ struct completion *lport_unreg_cmp;
/* Add stats counters here */
struct lpfc_nvme_ctrl_stat *cstat;
atomic_t fc4NvmeLsRequests;
--
2.19.1
Re: [PATCH] scsi: libsas: Fix rphy phy_identifier for PHYs with end devices attached
Good, I had a quick test and the issue is fixed, thanks. Reviewed-by: Jason Yan
Re: [PATCH 0/4] scsi: fixup dma_set_mask_and_coherent() calls
On 2/14/19 6:31 PM, Christoph Hellwig wrote: On Thu, Feb 14, 2019 at 09:29:05AM +, John Garry wrote: On 13/02/2019 18:51, Ewan D. Milne wrote: On Wed, 2019-02-13 at 12:42 +0100, Hannes Reinecke wrote: The recent patchset to use dma_set_mask_and_coherent() introduced a regression where a call to set a 64-bit DMA mask was followed by a call to set a 32-bit DMA mask, leading to I/O errors and data corruption. Patchset is based on a suggestions from Ewan Milne. Hannes Reinecke (4): lpfc: fix calls to dma_set_mask_and_coherent() hptiop: fix calls to dma_set_mask_and_coherent() bfa: fix calls to dma_set_mask_and_coherent() hisi_sas: fix calls to dma_set_mask_and_coherent() drivers/scsi/bfa/bfad.c | 10 +++--- drivers/scsi/hisi_sas/hisi_sas_main.c | 8 ++-- drivers/scsi/hptiop.c | 10 +++--- drivers/scsi/lpfc/lpfc_init.c | 9 ++--- 4 files changed, 26 insertions(+), 11 deletions(-) Isn't there a few more to fix up, like: drivers/scsi/3w-9xxx.c drivers/scsi/3w-sas.c drivers/scsi/csiostor/csio_init.c Yeah, there is a few more. And the sad part is as of a few kernel release ago we shouldn't even need the fallback 32-bit dma mask anymore - we've cleaned up all the mess that required it. Care to elaborate? Can you point me to the respective commits facilitating that? I'd gladly update the drivers... Cheers, Hannes
Re: [PATCH] scsi: reset host byte in DID_NEXUS_FAILURE case
On 2/14/19 10:57 PM, Martin Wilck wrote:
Up to 4.12, __scsi_error_from_host_byte() would reset the host
byte to DID_OK for various cases including DID_NEXUS_FAILURE.
Commit 2a842acab109 ("block: introduce new block status code type")
replaced this function with scsi_result_to_blk_status() and removed
the host-byte resetting code for the DID_NEXUS_FAILURE case.
As the line set_host_byte(cmd, DID_OK) was preserved for the other
cases, I suppose this was an editing mistake.
The fact that the host byte remains set after 4.13 is causing
problems with the sg_persist tool, which now returns success
rather then exit status 24 when a RESERVATION CONFLICT error is
encountered.
Fixes: 2a842acab109 "block: introduce new block status code type"
Signed-off-by: Martin Wilck
---
drivers/scsi/scsi_lib.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index 6d65ac5..f8d51c3 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -655,6 +655,7 @@ static blk_status_t scsi_result_to_blk_status(struct
scsi_cmnd *cmd, int result)
set_host_byte(cmd, DID_OK);
return BLK_STS_TARGET;
case DID_NEXUS_FAILURE:
+ set_host_byte(cmd, DID_OK);
return BLK_STS_NEXUS;
case DID_ALLOC_FAILURE:
set_host_byte(cmd, DID_OK);
Ah _that_ was the issue. Thanks for figuring it out.
Reviewed-by: Hannes Reinecke
Cheers,
Hannes
Re: [PATCH] scsi: aic94xx: fix module loading
On 1/31/19 1:42 AM, James Bottomley wrote:
The aic94xx driver is currently failing to load with errors like
sysfs: cannot create duplicate filename
'/devices/pci:00/:00:03.0/:02:00.3/:07:02.0/revision'
Because the PCI code had recently added a file named 'revision' to
every PCI device. Fix this by renaming the aic94xx revision file to
aic_revision. This is safe to do for us because as far as I can tell,
there's nothing in userspace relying on the current aic94xx revision
file so it can be renamed without breaking anything.
Fixes: 702ed3be1b1b (PCI: Create revision file in sysfs)
Cc: sta...@vger.kernel.org
Signed-off-by: James Bottomley
---
Patch is currently compile tested only ... I'm just excavating my
aic94xx based sas system from the unused equipment pile in the garage.
diff --git a/drivers/scsi/aic94xx/aic94xx_init.c
b/drivers/scsi/aic94xx/aic94xx_init.c
index 41c4d8abdd4a..38e768057129 100644
--- a/drivers/scsi/aic94xx/aic94xx_init.c
+++ b/drivers/scsi/aic94xx/aic94xx_init.c
@@ -281,7 +281,7 @@ static ssize_t asd_show_dev_rev(struct device *dev,
return snprintf(buf, PAGE_SIZE, "%s\n",
asd_dev_rev[asd_ha->revision_id]);
}
-static DEVICE_ATTR(revision, S_IRUGO, asd_show_dev_rev, NULL);
+static DEVICE_ATTR(aic_revision, S_IRUGO, asd_show_dev_rev, NULL);
static ssize_t asd_show_dev_bios_build(struct device *dev,
struct device_attribute *attr,char *buf)
@@ -478,7 +478,7 @@ static int asd_create_dev_attrs(struct asd_ha_struct
*asd_ha)
{
int err;
- err = device_create_file(&asd_ha->pcidev->dev, &dev_attr_revision);
+ err = device_create_file(&asd_ha->pcidev->dev, &dev_attr_aic_revision);
if (err)
return err;
@@ -500,13 +500,13 @@ static int asd_create_dev_attrs(struct asd_ha_struct *asd_ha)
err_biosb:
device_remove_file(&asd_ha->pcidev->dev, &dev_attr_bios_build);
err_rev:
- device_remove_file(&asd_ha->pcidev->dev, &dev_attr_revision);
+ device_remove_file(&asd_ha->pcidev->dev, &dev_attr_aic_revision);
return err;
}
static void asd_remove_dev_attrs(struct asd_ha_struct *asd_ha)
{
- device_remove_file(&asd_ha->pcidev->dev, &dev_attr_revision);
+ device_remove_file(&asd_ha->pcidev->dev, &dev_attr_aic_revision);
device_remove_file(&asd_ha->pcidev->dev, &dev_attr_bios_build);
device_remove_file(&asd_ha->pcidev->dev, &dev_attr_pcba_sn);
device_remove_file(&asd_ha->pcidev->dev, &dev_attr_update_bios);
Raising the question why you create attributes under the PCI device at
all. Typically these are created under the scsi_host sysfs device.
Why not here?
Cheers,
Hannes
Re: [PATCH RFC v2] fcoe: make use of fip_mode enum complete
On 2/12/19 11:42 PM, Nathan Chancellor wrote:
On Tue, Feb 12, 2019 at 08:50:04AM +0100, Sedat Dilek wrote:
On Mon, Feb 11, 2019 at 6:53 PM Nathan Chancellor
wrote:
On Mon, Feb 11, 2019 at 06:07:51PM +0100, Sedat Dilek wrote:
From: Sedat Dilek
commit 1917d42d14b7 ("fcoe: use enum for fip_mode") introduces a separate
enum for the fip_mode that shall be used during initialisation handling
until it is passed to fcoe_ctrl_link_up to set the initial fip_state.
That change was incomplete and gcc quietly converted in various places
between the fip_mode and the fip_state enum values with implicit enum
conversions, which fortunately cannot cause any issues in the actual
code's execution.
clang however warns about these implicit enum conversions in the scsi
drivers. This commit consolidates the use of the two enums, guided by
clang's enum-conversion warnings.
This commit now completes the use of the fip_mode:
It expects and uses fip_mode in {bnx2fc,fcoe}_interface_create and
fcoe_ctlr_init, and it explicitly converts from fip_mode to fip_state
only at the single point in fcoe_ctlr_link_up.
To eliminate that adding or removing values from fip_mode or fip_state enum
break the right mapping of values, all fip_mode values are assigned to
their fip_state counterparts.
Link: https://github.com/ClangBuiltLinux/linux/issues/151
Fixes: 1917d42d14b7 ("fcoe: use enum for fip_mode")
Reported-by: Dmitry Golovin "Twisted Pair in my Hair"
CC: Lukas Bulwahn
CC: Nick Desaulniers
CC: Nathan Chancellor
CC: Hannes Reinecke
Suggested-by: Johannes Thumshirn
---
[ v2:
- Based on the original patch by Lukas Bulwahn
- Suggestion by Johannes T. [1] required some changes:
+ s/case FIP_ST_VMMP_START/case FIP_ST_V*N*MP_START
+ s/return FIP_ST_AUTO/return FIP_MODE_AUTO
- Add Link to ClangBuiltLinux issue #151
- S-o-b line later when there is an OK from the FCoE maintainers
NOTE: Compile-tested against Linux-v5.0-rc6 with LLVM/Clang from Git with
experimental asm-goto support via executing:
$ make V=1 CC=clang-9 HOSTCC=clang-9 drivers/scsi/fcoe/
[1] https://marc.info/?l=linux-scsi&m=154745527612152&w=2
-dileks ]
drivers/scsi/bnx2fc/bnx2fc_fcoe.c | 2 +-
drivers/scsi/fcoe/fcoe.c | 2 +-
drivers/scsi/fcoe/fcoe_ctlr.c | 4 ++--
drivers/scsi/fcoe/fcoe_transport.c | 2 +-
drivers/scsi/qedf/qedf_main.c | 2 +-
include/scsi/libfcoe.h | 22 --
6 files changed, 26 insertions(+), 8 deletions(-)
diff --git a/drivers/scsi/bnx2fc/bnx2fc_fcoe.c
b/drivers/scsi/bnx2fc/bnx2fc_fcoe.c
index 2e4e7159ebf9..a75e74ad1698 100644
--- a/drivers/scsi/bnx2fc/bnx2fc_fcoe.c
+++ b/drivers/scsi/bnx2fc/bnx2fc_fcoe.c
@@ -1438,7 +1438,7 @@ static struct bnx2fc_hba *bnx2fc_hba_create(struct
cnic_dev *cnic)
static struct bnx2fc_interface *
bnx2fc_interface_create(struct bnx2fc_hba *hba,
struct net_device *netdev,
- enum fip_state fip_mode)
+ enum fip_mode fip_mode)
{
struct fcoe_ctlr_device *ctlr_dev;
struct bnx2fc_interface *interface;
diff --git a/drivers/scsi/fcoe/fcoe.c b/drivers/scsi/fcoe/fcoe.c
index cd19be3f3405..8ba8862d3292 100644
--- a/drivers/scsi/fcoe/fcoe.c
+++ b/drivers/scsi/fcoe/fcoe.c
@@ -389,7 +389,7 @@ static int fcoe_interface_setup(struct fcoe_interface *fcoe,
* Returns: pointer to a struct fcoe_interface or NULL on error
*/
static struct fcoe_interface *fcoe_interface_create(struct net_device *netdev,
- enum fip_state fip_mode)
+ enum fip_mode fip_mode)
{
struct fcoe_ctlr_device *ctlr_dev;
struct fcoe_ctlr *ctlr;
diff --git a/drivers/scsi/fcoe/fcoe_ctlr.c b/drivers/scsi/fcoe/fcoe_ctlr.c
index 54da3166da8d..a52d3ad94876 100644
--- a/drivers/scsi/fcoe/fcoe_ctlr.c
+++ b/drivers/scsi/fcoe/fcoe_ctlr.c
@@ -147,7 +147,7 @@ static void fcoe_ctlr_map_dest(struct fcoe_ctlr *fip)
* fcoe_ctlr_init() - Initialize the FCoE Controller instance
* @fip: The FCoE controller to initialize
*/
-void fcoe_ctlr_init(struct fcoe_ctlr *fip, enum fip_state mode)
+void fcoe_ctlr_init(struct fcoe_ctlr *fip, enum fip_mode mode)
{
fcoe_ctlr_set_state(fip, FIP_ST_LINK_WAIT);
fip->mode = mode;
@@ -454,7 +454,7 @@ void fcoe_ctlr_link_up(struct fcoe_ctlr *fip)
mutex_unlock(&fip->ctlr_mutex);
fc_linkup(fip->lp);
} else if (fip->state == FIP_ST_LINK_WAIT) {
- fcoe_ctlr_set_state(fip, fip->mode);
+ fcoe_ctlr_set_state(fip, (enum fip_state) fip->mode);
switch (fip->mode) {
default:
LIBFCOE_FIP_DBG(fip, "invalid mode %d\n", fip->mode);
Watch out for this one.
fip_mode != fip_state.
This arguably is an error even now; it might _just_ work if we have
fip->mode == FIP_MODE_FABRIC, but we probably will be getting
interesting resu
[v1 4/7] mpt3sas: Irq poll to avoid CPU hard lockups.
Issue Discription: We have seen cpu lock up issue from fields if system has greater (more than 96) logical cpu count. SAS3.0 controller (Invader series) supports at max 96 msix vector and SAS3.5 product (Ventura) supports at max 128 msix vectors. This may be a generic issue (if PCI device support completion on multiple reply queues). Let me explain it w.r.t to mpt3sas supported h/w just to simplify the problem and possible changes to handle such issues. IT HBA (mpt3sas) supports multiple reply queues in completion path. Driver creates MSI-x vectors for controller as "min of (FW supported Reply queue, Logical CPUs)". If submitter is not interrupted via completion on same CPU, there is a loop in the IO path. This behavior can cause hard/soft CPU lockups, IO timeout, system sluggish etc. Example - one CPU (e.g. CPU A) is busy submitting the IOs and another CPU (e.g. CPU B) is busy with processing the corresponding IO's reply descriptors from reply descriptor queue upon receiving the interrupts from HBA. If the CPU A is continuously pumping the IOs then always CPU B (which is executing the ISR) will see the valid reply descriptors in the reply descriptor queue and it will be continuously processing those reply descriptor in a loop without quitting the ISR handler. Mpt3sas driver will exit ISR handler if it finds unused reply descriptor in the reply descriptor queue. Since CPU A will be continuously sending the IOs, CPU B may always see a valid reply descriptor (posted by HBA Firmware after processing the IO) in the reply descriptor queue. In worst case, driver will not quit from this loop in the ISR handler. Eventually, CPU lockup will be detected by watchdog. Above mentioned behavior is not common if "rq_affinity" set to 2 or affinity_hint is honored by irqbalance as "exact". If rq_affinity is set to 2, submitter will be always interrupted via completion on same CPU. If irqbalance is using "exact" policy, interrupt will be delivered to submitter CPU. Problem statement - If CPU counts to MSI-X vectors (reply descriptor Queues) count ratio is not 1:1, we still have exposure of issue explained above and for that we don't have any solution. Exposure of soft/hard lockup if CPU count is more than MSI-x supported by device. If CPUs count to MSI-x vectors count ratio is not 1:1, (Other way, if CPU counts to MSI-x vector count ratio is something like X:1, where X > 1) then 'exact' irqbalance policy OR rq_affinity = 2 won't help to avoid CPU hard/soft lockups. There won't be any one to one mapping between CPU to MSI-x vector instead one MSI-x interrupt (or reply descriptor queue) is shared with group/set of CPUs and there is a possibility of having a loop in the IO path within that CPU group and may observe lockups. For example: Consider a system having two NUMA nodes and each node having four logical CPUs and also consider that number of MSI-x vectors enabled on the HBA is two, then CPUs count to MSI-x vector count ratio as 4:1. e.g. MSIx vector 0 is affinity to CPU 0, CPU 1, CPU 2 & CPU 3 of NUMA node 0 and MSI-x vector 1 is affinity to CPU 4, CPU 5, CPU 6 & CPU 7 of NUMA node 1. numactl --hardware available: 2 nodes (0-1) node 0 cpus: 0 1 2 3 --> MSI-x 0 node 0 size: 65536 MB node 0 free: 63176 MB node 1 cpus: 4 5 6 7 -->MSI-x 1 node 1 size: 65536 MB node 1 free: 63176 MB Assume that user started an application which uses all the CPUs of NUMA node 0 for issuing the IOs. Only one CPU from affinity list (it can be any cpu since this behavior depends upon irqbalance) CPU0 will receive the interrupts from MSIx vector 0 for all the IOs. Eventually, CPU 0 IO submission percentage will be decreasing and ISR processing percentage will be increasing as it is more busy with processing the interrupts. Gradually IO submission percentage on CPU 0 will be zero and it's ISR processing percentage will be 100 percentage as IO loop has already formed within the NUMA node 0, i.e. CPU 1, CPU 2 & CPU 3 will be continuously busy with submitting the heavy IOs and only CPU 0 is busy in the ISR path as it always find the valid reply descriptor in the reply descriptor queue. Eventually, we will observe the hard lockup here. Chances of occurring of hard/soft lockups are directly proportional to value of X. If value of X is high, then chances of observing CPU lockups is high. Solution - Fix - Use IRQ poll interface defined in " irq_poll.c". mpt3sas driver will execute ISR routine in Softirq context and it will always quit the loop based on budget provided in IRQ poll interface. In these scenarios( i.e. where CPUs count to MSI-X vectors count ratio is X:1 (where X > 1)), IRQ poll interface will avoid CPU hard lockups due to voluntary exit from the reply queue processing based on budget. Note - Only one MSI-x vector is busy doing processing. Irqstat output - IRQs / 1 second(s) IRQ# TOTAL NODE0 NODE1 NODE2 NODE3 NAME 44122871 122871 0 0 0 IR-PCI-MSI-edge mpt3sas0-msi
[v1 2/7] mpt3sas: simplify interrupt handler.
Separate out processing of reply descriptor post queue
from _base_interrupt to _base_process_reply_queue.
Signed-off-by: Suganath Prabu
---
drivers/scsi/mpt3sas/mpt3sas_base.c | 49 +
1 file changed, 33 insertions(+), 16 deletions(-)
diff --git a/drivers/scsi/mpt3sas/mpt3sas_base.c
b/drivers/scsi/mpt3sas/mpt3sas_base.c
index 4184d96..076428d 100644
--- a/drivers/scsi/mpt3sas/mpt3sas_base.c
+++ b/drivers/scsi/mpt3sas/mpt3sas_base.c
@@ -1383,16 +1383,16 @@ union reply_descriptor {
};
/**
- * _base_interrupt - MPT adapter (IOC) specific interrupt handler.
- * @irq: irq number (not used)
- * @bus_id: bus identifier cookie == pointer to MPT_ADAPTER structure
+ * _base_process_reply_queue - Process reply descriptors from reply
+ * descriptor post queue.
+ * @reply_q: per IRQ's reply queue object.
*
- * Return: IRQ_HANDLED if processed, else IRQ_NONE.
+ * Return: number of reply descriptors processed from reply
+ * descriptor queue.
*/
-static irqreturn_t
-_base_interrupt(int irq, void *bus_id)
+static int
+_base_process_reply_queue(struct adapter_reply_queue *reply_q)
{
- struct adapter_reply_queue *reply_q = bus_id;
union reply_descriptor rd;
u32 completed_cmds;
u8 request_descript_type;
@@ -1404,21 +1404,18 @@ _base_interrupt(int irq, void *bus_id)
Mpi2ReplyDescriptorsUnion_t *rpf;
u8 rc;
- if (ioc->mask_interrupts)
- return IRQ_NONE;
-
+ completed_cmds = 0;
if (!atomic_add_unless(&reply_q->busy, 1, 1))
- return IRQ_NONE;
+ return completed_cmds;
rpf = &reply_q->reply_post_free[reply_q->reply_post_host_index];
request_descript_type = rpf->Default.ReplyFlags
& MPI2_RPY_DESCRIPT_FLAGS_TYPE_MASK;
if (request_descript_type == MPI2_RPY_DESCRIPT_FLAGS_UNUSED) {
atomic_dec(&reply_q->busy);
- return IRQ_NONE;
+ return completed_cmds;
}
- completed_cmds = 0;
cb_idx = 0xFF;
do {
rd.word = le64_to_cpu(rpf->Words);
@@ -1521,14 +1518,14 @@ _base_interrupt(int irq, void *bus_id)
if (!completed_cmds) {
atomic_dec(&reply_q->busy);
- return IRQ_NONE;
+ return completed_cmds;
}
if (ioc->is_warpdrive) {
writel(reply_q->reply_post_host_index,
ioc->reply_post_host_index[msix_index]);
atomic_dec(&reply_q->busy);
- return IRQ_HANDLED;
+ return completed_cmds;
}
/* Update Reply Post Host Index.
@@ -1555,7 +1552,27 @@ _base_interrupt(int irq, void *bus_id)
MPI2_RPHI_MSIX_INDEX_SHIFT),
&ioc->chip->ReplyPostHostIndex);
atomic_dec(&reply_q->busy);
- return IRQ_HANDLED;
+ return completed_cmds;
+}
+
+/**
+ * _base_interrupt - MPT adapter (IOC) specific interrupt handler.
+ * @irq: irq number (not used)
+ * @bus_id: bus identifier cookie == pointer to MPT_ADAPTER structure
+ *
+ * Return: IRQ_HANDLED if processed, else IRQ_NONE.
+ */
+static irqreturn_t
+_base_interrupt(int irq, void *bus_id)
+{
+ struct adapter_reply_queue *reply_q = bus_id;
+ struct MPT3SAS_ADAPTER *ioc = reply_q->ioc;
+
+ if (ioc->mask_interrupts)
+ return IRQ_NONE;
+
+ return ((_base_process_reply_queue(reply_q) > 0) ?
+ IRQ_HANDLED : IRQ_NONE);
}
/**
--
1.8.3.1
[v1 7/7] mpt3sas: Update mpt3sas driver version to 28.100.00.00
Updated driver version to 28.100.00.00, which is equivalent to OOB Ph9 Signed-off-by: Suganath Prabu --- drivers/scsi/mpt3sas/mpt3sas_base.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/scsi/mpt3sas/mpt3sas_base.h b/drivers/scsi/mpt3sas/mpt3sas_base.h index dab64f3..480219f 100644 --- a/drivers/scsi/mpt3sas/mpt3sas_base.h +++ b/drivers/scsi/mpt3sas/mpt3sas_base.h @@ -76,9 +76,9 @@ #define MPT3SAS_DRIVER_NAME"mpt3sas" #define MPT3SAS_AUTHOR "Avago Technologies " #define MPT3SAS_DESCRIPTION"LSI MPT Fusion SAS 3.0 Device Driver" -#define MPT3SAS_DRIVER_VERSION "27.102.00.00" -#define MPT3SAS_MAJOR_VERSION 27 -#define MPT3SAS_MINOR_VERSION 102 +#define MPT3SAS_DRIVER_VERSION "28.100.00.00" +#define MPT3SAS_MAJOR_VERSION 28 +#define MPT3SAS_MINOR_VERSION 100 #define MPT3SAS_BUILD_VERSION 0 #define MPT3SAS_RELEASE_VERSION00 -- 1.8.3.1
[v1 1/7] mpt3sas: Fix typo in request_desript_type.
Fixed typo in request_desript_type.
request_desript_type --> request_descript_type.
Signed-off-by: Suganath Prabu
---
drivers/scsi/mpt3sas/mpt3sas_base.c | 18 +-
1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/drivers/scsi/mpt3sas/mpt3sas_base.c
b/drivers/scsi/mpt3sas/mpt3sas_base.c
index 0a6cb8f..4184d96 100644
--- a/drivers/scsi/mpt3sas/mpt3sas_base.c
+++ b/drivers/scsi/mpt3sas/mpt3sas_base.c
@@ -1395,7 +1395,7 @@ _base_interrupt(int irq, void *bus_id)
struct adapter_reply_queue *reply_q = bus_id;
union reply_descriptor rd;
u32 completed_cmds;
- u8 request_desript_type;
+ u8 request_descript_type;
u16 smid;
u8 cb_idx;
u32 reply;
@@ -1411,9 +1411,9 @@ _base_interrupt(int irq, void *bus_id)
return IRQ_NONE;
rpf = &reply_q->reply_post_free[reply_q->reply_post_host_index];
- request_desript_type = rpf->Default.ReplyFlags
+ request_descript_type = rpf->Default.ReplyFlags
& MPI2_RPY_DESCRIPT_FLAGS_TYPE_MASK;
- if (request_desript_type == MPI2_RPY_DESCRIPT_FLAGS_UNUSED) {
+ if (request_descript_type == MPI2_RPY_DESCRIPT_FLAGS_UNUSED) {
atomic_dec(&reply_q->busy);
return IRQ_NONE;
}
@@ -1426,11 +1426,11 @@ _base_interrupt(int irq, void *bus_id)
goto out;
reply = 0;
smid = le16_to_cpu(rpf->Default.DescriptorTypeDependent1);
- if (request_desript_type ==
+ if (request_descript_type ==
MPI25_RPY_DESCRIPT_FLAGS_FAST_PATH_SCSI_IO_SUCCESS ||
- request_desript_type ==
+ request_descript_type ==
MPI2_RPY_DESCRIPT_FLAGS_SCSI_IO_SUCCESS ||
- request_desript_type ==
+ request_descript_type ==
MPI26_RPY_DESCRIPT_FLAGS_PCIE_ENCAPSULATED_SUCCESS) {
cb_idx = _base_get_cb_idx(ioc, smid);
if ((likely(cb_idx < MPT_MAX_CALLBACKS)) &&
@@ -1440,7 +1440,7 @@ _base_interrupt(int irq, void *bus_id)
if (rc)
mpt3sas_base_free_smid(ioc, smid);
}
- } else if (request_desript_type ==
+ } else if (request_descript_type ==
MPI2_RPY_DESCRIPT_FLAGS_ADDRESS_REPLY) {
reply = le32_to_cpu(
rpf->AddressReply.ReplyFrameAddress);
@@ -1486,7 +1486,7 @@ _base_interrupt(int irq, void *bus_id)
(reply_q->reply_post_host_index ==
(ioc->reply_post_queue_depth - 1)) ? 0 :
reply_q->reply_post_host_index + 1;
- request_desript_type =
+ request_descript_type =
reply_q->reply_post_free[reply_q->reply_post_host_index].
Default.ReplyFlags & MPI2_RPY_DESCRIPT_FLAGS_TYPE_MASK;
completed_cmds++;
@@ -1509,7 +1509,7 @@ _base_interrupt(int irq, void *bus_id)
}
completed_cmds = 1;
}
- if (request_desript_type == MPI2_RPY_DESCRIPT_FLAGS_UNUSED)
+ if (request_descript_type == MPI2_RPY_DESCRIPT_FLAGS_UNUSED)
goto out;
if (!reply_q->reply_post_host_index)
rpf = reply_q->reply_post_free;
--
1.8.3.1
[v1 6/7] mpt3sas: Improve the threshold value and introduce module param.
* Reduce the threshold value to 1/4 of the queue depth.
* With this FW can find enough entries to post the Reply
Descriptors in the reply descriptor post queue.
* With module param, user can play with threshold value,
the same irqpoll_weight is used as the budget in processing
of reply descriptor post queues in
_base_process_reply_queue.
Signed-off-by: Suganath Prabu
---
drivers/scsi/mpt3sas/mpt3sas_base.c | 14 --
drivers/scsi/mpt3sas/mpt3sas_base.h | 3 +++
2 files changed, 15 insertions(+), 2 deletions(-)
diff --git a/drivers/scsi/mpt3sas/mpt3sas_base.c
b/drivers/scsi/mpt3sas/mpt3sas_base.c
index aadd9e2..880bd22 100644
--- a/drivers/scsi/mpt3sas/mpt3sas_base.c
+++ b/drivers/scsi/mpt3sas/mpt3sas_base.c
@@ -94,6 +94,11 @@ module_param(max_msix_vectors, int, 0);
MODULE_PARM_DESC(max_msix_vectors,
" max msix vectors");
+static int irqpoll_weight = -1;
+module_param(irqpoll_weight, int, 0);
+MODULE_PARM_DESC(irqpoll_weight,
+ "irq poll weight (default= one fourth of HBA queue depth)");
+
static int mpt3sas_fwfault_debug;
MODULE_PARM_DESC(mpt3sas_fwfault_debug,
" enable detection of firmware fault and halt firmware - (default=0)");
@@ -1404,7 +1409,7 @@ static int
_base_process_reply_queue(struct adapter_reply_queue *reply_q)
{
union reply_descriptor rd;
- u32 completed_cmds;
+ u64 completed_cmds;
u8 request_descript_type;
u16 smid;
u8 cb_idx;
@@ -1502,7 +1507,7 @@ _base_process_reply_queue(struct adapter_reply_queue
*reply_q)
* So that FW can find enough entries to post the Reply
* Descriptors in the reply descriptor post queue.
*/
- if (completed_cmds > ioc->hba_queue_depth/3) {
+ if (!base_mod64(completed_cmds, ioc->thresh_hold)) {
if (ioc->combined_reply_queue) {
writel(reply_q->reply_post_host_index |
((msix_index & 7) <<
@@ -6608,6 +6613,11 @@ mpt3sas_base_attach(struct MPT3SAS_ADAPTER *ioc)
if (r)
goto out_free_resources;
+ if (irqpoll_weight > 0)
+ ioc->thresh_hold = irqpoll_weight;
+ else
+ ioc->thresh_hold = ioc->hba_queue_depth/4;
+
_base_init_irqpolls(ioc);
init_waitqueue_head(&ioc->reset_wq);
diff --git a/drivers/scsi/mpt3sas/mpt3sas_base.h
b/drivers/scsi/mpt3sas/mpt3sas_base.h
index 3895407..dab64f3 100644
--- a/drivers/scsi/mpt3sas/mpt3sas_base.h
+++ b/drivers/scsi/mpt3sas/mpt3sas_base.h
@@ -1028,6 +1028,8 @@ typedef void (*MPT3SAS_FLUSH_RUNNING_CMDS)(struct
MPT3SAS_ADAPTER *ioc);
* @msix_load_balance: Enables load balancing of interrupts across
* the multiple MSIXs
* @schedule_dead_ioc_flush_running_cmds: callback to flush pending commands
+ * @thresh_hold: Max number of reply descriptors processed
+ * before updating Host Index
* @scsi_io_cb_idx: shost generated commands
* @tm_cb_idx: task management commands
* @scsih_cb_idx: scsih internal commands
@@ -1205,6 +1207,7 @@ struct MPT3SAS_ADAPTER {
u32 non_operational_loop;
atomic64_t total_io_cnt;
boolmsix_load_balance;
+ u16 thresh_hold;
/* internal commands, callback index */
u8 scsi_io_cb_idx;
--
1.8.3.1
[v1 0/7] Irq poll to address cpu lockup.
We have seen cpu lock up issue from fields if system has greater (more than 96) logical cpu count. SAS3.0 controller (Invader series) supports at max 96 msix vector and SAS3.5 product (Ventura) supports at max 128 msix vectors. This may be a generic issue (if PCI device support completion on multiple reply queues). Let me explain it w.r.t to mpt3sas supported h/w just to simplify the problem and possible changes to handle such issues. IT HBA (mpt3sas) supports multiple reply queues in completion path. Driver creates MSI-x vectors for controller as "min of (FW supported Reply queue, Logical CPUs)". If submitter is not interrupted via completion on same CPU, there is a loop in the IO path. This behavior can cause hard/soft CPU lockups, IO timeout, system sluggish etc. Example - one CPU (e.g. CPU A) is busy submitting the IOs and another CPU (e.g. CPU B) is busy with processing the corresponding IO's reply descriptors from reply descriptor queue upon receiving the interrupts from HBA. If the CPU A is continuously pumping the IOs then always CPU B (which is executing the ISR) will see the valid reply descriptors in the reply descriptor queue and it will be continuously processing those reply descriptor in a loop without quitting the ISR handler. Mpt3sas driver will exit ISR handler if it finds unused reply descriptor in the reply descriptor queue. Since CPU A will be continuously sending the IOs, CPU B may always see a valid reply descriptor (posted by HBA Firmware after processing the IO) in the reply descriptor queue. In worst case, driver will not quit from this loop in the ISR handler. Eventually, CPU lockup will be detected by watchdog. Above mentioned behavior is not common if "rq_affinity" set to 2 or affinity_hint is honored by irqbalance as "exact". If rq_affinity is set to 2, submitter will be always interrupted via completion on same CPU. If irqbalance is using "exact" policy, interrupt will be delivered to submitter CPU. Problem statement - If CPU counts to MSI-X vectors (reply descriptor Queues) count ratio is not 1:1, we still have exposure of issue explained above and for that we don't have any solution. Exposure of soft/hard lockup if CPU count is more than MSI-x supported by device. If CPUs count to MSI-x vectors count ratio is not 1:1, (Other way, if CPU counts to MSI-x vector count ratio is something like X:1, where X > 1) then 'exact' irqbalance policy OR rq_affinity = 2 won't help to avoid CPU hard/soft lockups. There won't be any one to one mapping between CPU to MSI-x vector instead one MSI-x interrupt (or reply descriptor queue) is shared with group/set of CPUs and there is a possibility of having a loop in the IO path within that CPU group and may observe lockups. For example: Consider a system having two NUMA nodes and each node having four logical CPUs and also consider that number of MSI-x vectors enabled on the HBA is two, then CPUs count to MSI-x vector count ratio as 4:1. e.g. MSIx vector 0 is affinity to CPU 0, CPU 1, CPU 2 & CPU 3 of NUMA node 0 and MSI-x vector 1 is affinity to CPU 4, CPU 5, CPU 6 & CPU 7 of NUMA node 1. numactl --hardware available: 2 nodes (0-1) node 0 cpus: 0 1 2 3 --> MSI-x 0 node 0 size: 65536 MB node 0 free: 63176 MB node 1 cpus: 4 5 6 7 -->MSI-x 1 node 1 size: 65536 MB node 1 free: 63176 MB Assume that user started an application which uses all the CPUs of NUMA node 0 for issuing the IOs. Only one CPU from affinity list (it can be any cpu since this behavior depends upon irqbalance) CPU0 will receive the interrupts from MSIx vector 0 for all the IOs. Eventually, CPU 0 IO submission percentage will be decreasing and ISR processing percentage will be increasing as it is more busy with processing the interrupts. Gradually IO submission percentage on CPU 0 will be zero and it's ISR processing percentage will be 100 percentage as IO loop has already formed within the NUMA node 0, i.e. CPU 1, CPU 2 & CPU 3 will be continuously busy with submitting the heavy IOs and only CPU 0 is busy in the ISR path as it always find the valid reply descriptor in the reply descriptor queue. Eventually, we will observe the hard lockup here. Chances of occurring of hard/soft lockups are directly proportional to value of X. If value of X is high, then chances of observing CPU lockups is high. Solution - Fix-1 = Use IRQ poll interface defined in " irq_poll.c". mpt3sas driver will execute ISR routine in Softirq context and it will always quit the loop based on budget provided in IRQ poll interface. In these scenarios( i.e. where CPUs count to MSI-X vectors count ratio is X:1 (where X > 1)), IRQ poll interface will avoid CPU hard lockups due to voluntary exit from the reply queue processing based on budget. Note - Only one MSI-x vector is busy doing processing. Irqstat output - IRQs / 1 second(s) IRQ# TOTAL NODE0 NODE1 NODE2 NODE3 NAME 44122871 122871 0 0 0 IR-PCI-MSI-edge mpt3sas0-msix0 45
[v1 5/7] mpt3sas: Load balance to improve performance and avoid soft lockups.
Driver uses "reply descriptor post queues" in round robin
fashion so that IO's are distributed to all the available
reply descriptor post queues equally.
With this each reply descriptor post queue load is balanced.
This is enabled only if CPUs count to MSI-X vector
count ratio is X:1 (where X > 1)
This improves performance and also fixes soft lockups.
Signed-off-by: Suganath Prabu
---
drivers/scsi/mpt3sas/mpt3sas_base.c | 21 +
drivers/scsi/mpt3sas/mpt3sas_base.h | 5 +
2 files changed, 26 insertions(+)
diff --git a/drivers/scsi/mpt3sas/mpt3sas_base.c
b/drivers/scsi/mpt3sas/mpt3sas_base.c
index 1f358bc..aadd9e2 100644
--- a/drivers/scsi/mpt3sas/mpt3sas_base.c
+++ b/drivers/scsi/mpt3sas/mpt3sas_base.c
@@ -1382,6 +1382,16 @@ union reply_descriptor {
} u;
};
+static u32 base_mod64(u64 dividend, u32 divisor)
+{
+ u32 remainder;
+
+ if (!divisor)
+ pr_err("mpt3sas: DIVISOR is zero, in div fn\n");
+ remainder = do_div(dividend, divisor);
+ return remainder;
+}
+
/**
* _base_process_reply_queue - Process reply descriptors from reply
* descriptor post queue.
@@ -2845,6 +2855,11 @@ _base_assign_reply_queues(struct MPT3SAS_ADAPTER *ioc)
if (!_base_is_controller_msix_enabled(ioc))
return;
+ ioc->msix_load_balance = false;
+ if (ioc->reply_queue_count < num_online_cpus()) {
+ ioc->msix_load_balance = true;
+ return;
+ }
memset(ioc->cpu_msix_table, 0, ioc->cpu_msix_table_sz);
@@ -3248,6 +3263,12 @@ mpt3sas_base_get_reply_virt_addr(struct MPT3SAS_ADAPTER
*ioc, u32 phys_addr)
static inline u8
_base_get_msix_index(struct MPT3SAS_ADAPTER *ioc)
{
+ /* Enables reply_queue load balancing */
+ if (ioc->msix_load_balance)
+ return ioc->reply_queue_count ?
+ base_mod64(atomic64_add_return(1,
+ &ioc->total_io_cnt), ioc->reply_queue_count) : 0;
+
return ioc->cpu_msix_table[raw_smp_processor_id()];
}
diff --git a/drivers/scsi/mpt3sas/mpt3sas_base.h
b/drivers/scsi/mpt3sas/mpt3sas_base.h
index fb572cd..3895407 100644
--- a/drivers/scsi/mpt3sas/mpt3sas_base.h
+++ b/drivers/scsi/mpt3sas/mpt3sas_base.h
@@ -1024,6 +1024,9 @@ typedef void (*MPT3SAS_FLUSH_RUNNING_CMDS)(struct
MPT3SAS_ADAPTER *ioc);
* @msix_vector_count: number msix vectors
* @cpu_msix_table: table for mapping cpus to msix index
* @cpu_msix_table_sz: table size
+ * @total_io_cnt: Gives total IO count, used to load balance the interrupts
+ * @msix_load_balance: Enables load balancing of interrupts across
+ * the multiple MSIXs
* @schedule_dead_ioc_flush_running_cmds: callback to flush pending commands
* @scsi_io_cb_idx: shost generated commands
* @tm_cb_idx: task management commands
@@ -1200,6 +1203,8 @@ struct MPT3SAS_ADAPTER {
u32 ioc_reset_count;
MPT3SAS_FLUSH_RUNNING_CMDS schedule_dead_ioc_flush_running_cmds;
u32 non_operational_loop;
+ atomic64_t total_io_cnt;
+ boolmsix_load_balance;
/* internal commands, callback index */
u8 scsi_io_cb_idx;
--
1.8.3.1
[v1 3/7] mpt3sas: Select IRQ_POLL to avoid build error.
In patch 4, driver uses irq poll, select it to avoid below build error. ERROR: "irq_poll_init" [drivers/scsi/mpt3sas/mpt3sas.ko] undefined! ERROR: "irq_poll_enable" [drivers/scsi/mpt3sas/mpt3sas.ko] undefined! ERROR: "irq_poll_sched" [drivers/scsi/mpt3sas/mpt3sas.ko] undefined! ERROR: "irq_poll_disable" [drivers/scsi/mpt3sas/mpt3sas.ko] undefined! ERROR: "irq_poll_complete" [drivers/scsi/mpt3sas/mpt3sas.ko] undefined! Signed-off-by: Suganath Prabu --- drivers/scsi/mpt3sas/Kconfig | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/scsi/mpt3sas/Kconfig b/drivers/scsi/mpt3sas/Kconfig index b736dbc..a072187 100644 --- a/drivers/scsi/mpt3sas/Kconfig +++ b/drivers/scsi/mpt3sas/Kconfig @@ -45,6 +45,7 @@ config SCSI_MPT3SAS depends on PCI && SCSI select SCSI_SAS_ATTRS select RAID_ATTRS + select IRQ_POLL ---help--- This driver supports PCI-Express SAS 12Gb/s Host Adapters. -- 1.8.3.1
Re: [PATCH] scsi: reset host byte in DID_NEXUS_FAILURE case
Looks good, Reviewed-by: Christoph Hellwig
Re: [PATCH -next] scsi: lpfc: Remove set but not used variable 'phys_id'
Signed-off-by is like a legal document to say that you haven't added any proprietary SCO UNIXWARE code to the patch. You have sign every patch if you email it. Acked-by and Reviewed-by are less clear. Acked-by means you want it to be merged, I guess? I never use Acked by because I'm not a maintainer so it's not my place to approve things. regards, dan carpenter
Re: [PATCH 0/4] scsi: fixup dma_set_mask_and_coherent() calls
On Fri, Feb 15, 2019 at 07:55:39AM +0100, Hannes Reinecke wrote: >> Yeah, there is a few more. And the sad part is as of a few kernel >> release ago we shouldn't even need the fallback 32-bit dma mask >> anymore - we've cleaned up all the mess that required it. >> > Care to elaborate? > Can you point me to the respective commits facilitating that? It mostly has been that way for a long time, but we still had a few oddball architectures checking for an exact 32-bit match in their arch specific direct mapping routines. With the move to the generic dma-direct implementation all those got fixed.
Re: [PATCH RFC v2] fcoe: make use of fip_mode enum complete
On Fri, Feb 15, 2019 at 8:35 AM Hannes Reinecke wrote:
>
> On 2/12/19 11:42 PM, Nathan Chancellor wrote:
> > On Tue, Feb 12, 2019 at 08:50:04AM +0100, Sedat Dilek wrote:
> >> On Mon, Feb 11, 2019 at 6:53 PM Nathan Chancellor
> >> wrote:
> >>>
> >>> On Mon, Feb 11, 2019 at 06:07:51PM +0100, Sedat Dilek wrote:
> From: Sedat Dilek
>
> commit 1917d42d14b7 ("fcoe: use enum for fip_mode") introduces a separate
> enum for the fip_mode that shall be used during initialisation handling
> until it is passed to fcoe_ctrl_link_up to set the initial fip_state.
> That change was incomplete and gcc quietly converted in various places
> between the fip_mode and the fip_state enum values with implicit enum
> conversions, which fortunately cannot cause any issues in the actual
> code's execution.
>
> clang however warns about these implicit enum conversions in the scsi
> drivers. This commit consolidates the use of the two enums, guided by
> clang's enum-conversion warnings.
>
> This commit now completes the use of the fip_mode:
> It expects and uses fip_mode in {bnx2fc,fcoe}_interface_create and
> fcoe_ctlr_init, and it explicitly converts from fip_mode to fip_state
> only at the single point in fcoe_ctlr_link_up.
>
> To eliminate that adding or removing values from fip_mode or fip_state
> enum
> break the right mapping of values, all fip_mode values are assigned to
> their fip_state counterparts.
>
> Link: https://github.com/ClangBuiltLinux/linux/issues/151
> Fixes: 1917d42d14b7 ("fcoe: use enum for fip_mode")
> Reported-by: Dmitry Golovin "Twisted Pair in my Hair"
> CC: Lukas Bulwahn
> CC: Nick Desaulniers
> CC: Nathan Chancellor
> CC: Hannes Reinecke
> Suggested-by: Johannes Thumshirn
> ---
>
> [ v2:
> - Based on the original patch by Lukas Bulwahn
> - Suggestion by Johannes T. [1] required some changes:
> + s/case FIP_ST_VMMP_START/case FIP_ST_V*N*MP_START
> + s/return FIP_ST_AUTO/return FIP_MODE_AUTO
> - Add Link to ClangBuiltLinux issue #151
> - S-o-b line later when there is an OK from the FCoE maintainers
>
> NOTE: Compile-tested against Linux-v5.0-rc6 with LLVM/Clang from Git with
> experimental asm-goto support via executing:
> $ make V=1 CC=clang-9 HOSTCC=clang-9 drivers/scsi/fcoe/
>
> [1] https://marc.info/?l=linux-scsi&m=154745527612152&w=2
>
> -dileks ]
>
> drivers/scsi/bnx2fc/bnx2fc_fcoe.c | 2 +-
> drivers/scsi/fcoe/fcoe.c | 2 +-
> drivers/scsi/fcoe/fcoe_ctlr.c | 4 ++--
> drivers/scsi/fcoe/fcoe_transport.c | 2 +-
> drivers/scsi/qedf/qedf_main.c | 2 +-
> include/scsi/libfcoe.h | 22 --
> 6 files changed, 26 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/scsi/bnx2fc/bnx2fc_fcoe.c
> b/drivers/scsi/bnx2fc/bnx2fc_fcoe.c
> index 2e4e7159ebf9..a75e74ad1698 100644
> --- a/drivers/scsi/bnx2fc/bnx2fc_fcoe.c
> +++ b/drivers/scsi/bnx2fc/bnx2fc_fcoe.c
> @@ -1438,7 +1438,7 @@ static struct bnx2fc_hba *bnx2fc_hba_create(struct
> cnic_dev *cnic)
> static struct bnx2fc_interface *
> bnx2fc_interface_create(struct bnx2fc_hba *hba,
> struct net_device *netdev,
> - enum fip_state fip_mode)
> + enum fip_mode fip_mode)
> {
> struct fcoe_ctlr_device *ctlr_dev;
> struct bnx2fc_interface *interface;
> diff --git a/drivers/scsi/fcoe/fcoe.c b/drivers/scsi/fcoe/fcoe.c
> index cd19be3f3405..8ba8862d3292 100644
> --- a/drivers/scsi/fcoe/fcoe.c
> +++ b/drivers/scsi/fcoe/fcoe.c
> @@ -389,7 +389,7 @@ static int fcoe_interface_setup(struct
> fcoe_interface *fcoe,
> * Returns: pointer to a struct fcoe_interface or NULL on error
> */
> static struct fcoe_interface *fcoe_interface_create(struct net_device
> *netdev,
> - enum fip_state
> fip_mode)
> + enum fip_mode fip_mode)
> {
> struct fcoe_ctlr_device *ctlr_dev;
> struct fcoe_ctlr *ctlr;
> diff --git a/drivers/scsi/fcoe/fcoe_ctlr.c
> b/drivers/scsi/fcoe/fcoe_ctlr.c
> index 54da3166da8d..a52d3ad94876 100644
> --- a/drivers/scsi/fcoe/fcoe_ctlr.c
> +++ b/drivers/scsi/fcoe/fcoe_ctlr.c
> @@ -147,7 +147,7 @@ static void fcoe_ctlr_map_dest(struct fcoe_ctlr *fip)
> * fcoe_ctlr_init() - Initialize the FCoE Controller instance
> * @fip: The FCoE controller to initialize
> */
> -void fcoe_ctlr_init(struct fcoe_ctlr *fip, enum fip_state mode)
> +void fcoe_ctlr_init(str
