| Message ID | 1489637143.5578.1.camel@sandisk.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
> -----Original Message----- > From: Bart Van Assche [mailto:Bart.VanAssche@sandisk.com] > Sent: Wednesday, March 15, 2017 9:06 PM > To: Madhani, Himanshu <Himanshu.Madhani@cavium.com>; target- > devel@vger.kernel.org; nab@linux-iscsi.org > Cc: Malavali, Giridhar <Giridhar.Malavali@cavium.com> > Subject: Re: [PATCH v4 00/14] qla2xxx: Bug Fixes and updates for target. > > On Wed, 2017-03-15 at 09:48 -0700, Himanshu Madhani wrote: > > o Fixed regression repored by Bart in following patch. > > "qla2xxx: Fix delayed response to command for loop mode/direct > connect." > > Thanks for having fixed this! BTW, while retesting this patch series I ran into > another (longstanding) bug. Please consider to include the (untested) patch > below in a future patch series. > > Bart. > > > [PATCH] qla2xxx: Avoid using smp_processor_id() where inappropriate > > Use queue_work(...) instead of queue_work_on(raw_smp_processor_id(), > ...). These two calls have the same effect for workqueues for which > WQ_UNBOUND has not been set. This patch avoids that the following is > reported: > > BUG: using smp_processor_id() in preemptible [00000000] code: > kworker/u16:0/5 caller is debug_smp_processor_id+0x17/0x20 > CPU: 4 PID: 5 Comm: kworker/u16:0 Not tainted 4.11.0-rc2-dbg+ #1 > Workqueue: tmr-iblock target_tmr_work [target_core_mod] Call Trace: > dump_stack+0x86/0xc3 > check_preemption_disabled+0xe2/0xf0 > debug_smp_processor_id+0x17/0x20 > tcm_qla2xxx_free_cmd+0x92/0xd0 [tcm_qla2xxx] > tcm_qla2xxx_aborted_task+0x74/0x80 [tcm_qla2xxx] > transport_cmd_finish_abort+0x54/0x80 [target_core_mod] > core_tmr_lun_reset+0x38f/0x800 [target_core_mod] > target_tmr_work+0xdb/0x130 [target_core_mod] > process_one_work+0x20d/0x6c0 > worker_thread+0x4e/0x4a0 > kthread+0x10c/0x140 > ret_from_fork+0x31/0x40 > > Signed-off-by: Bart Van Assche <bart.vanassche@sandisk.com> > --- > drivers/scsi/qla2xxx/qla_target.c | 14 ++++---------- > drivers/scsi/qla2xxx/tcm_qla2xxx.c | 4 ++-- > 2 files changed, 6 insertions(+), 12 deletions(-) > > diff --git a/drivers/scsi/qla2xxx/qla_target.c > b/drivers/scsi/qla2xxx/qla_target.c > index 0e03ca2ab3e5..8c9906ac9bbc 100644 > --- a/drivers/scsi/qla2xxx/qla_target.c > +++ b/drivers/scsi/qla2xxx/qla_target.c > @@ -4197,6 +4197,7 @@ static int qlt_handle_cmd_for_atio(struct > scsi_qla_host *vha, > struct fc_port *sess; > struct qla_tgt_cmd *cmd; > unsigned long flags; > + int cpu; > > if (unlikely(tgt->tgt_stop)) { > ql_dbg(ql_dbg_io, vha, 0x3061, > @@ -4263,16 +4264,9 @@ static int qlt_handle_cmd_for_atio(struct > scsi_qla_host *vha, > spin_unlock_irqrestore(&vha->cmd_list_lock, flags); > > INIT_WORK(&cmd->work, qlt_do_work); > - if (ha->msix_count) { > - if (cmd->atio.u.isp24.fcp_cmnd.rddata) > - queue_work_on(smp_processor_id(), qla_tgt_wq, > - &cmd->work); > - else > - queue_work_on(cmd->se_cmd.cpuid, qla_tgt_wq, > - &cmd->work); > - } else { > - queue_work(qla_tgt_wq, &cmd->work); > - } > + cpu = ha->msix_count && !cmd->atio.u.isp24.fcp_cmnd.rddata ? > + cmd->se_cmd.cpuid : raw_smp_processor_id(); > + queue_work_on(cpu, qla_tgt_wq, &cmd->work); > return 0; > > } > diff --git a/drivers/scsi/qla2xxx/tcm_qla2xxx.c > b/drivers/scsi/qla2xxx/tcm_qla2xxx.c > index 7443e4efa3ae..0f1ee1680eb8 100644 > --- a/drivers/scsi/qla2xxx/tcm_qla2xxx.c > +++ b/drivers/scsi/qla2xxx/tcm_qla2xxx.c > @@ -303,7 +303,7 @@ static void tcm_qla2xxx_free_cmd(struct qla_tgt_cmd > *cmd) > cmd->trc_flags |= TRC_CMD_DONE; > > INIT_WORK(&cmd->work, tcm_qla2xxx_complete_free); > - queue_work_on(smp_processor_id(), tcm_qla2xxx_free_wq, > &cmd->work); > + queue_work(tcm_qla2xxx_free_wq, &cmd->work); > } > > /* > @@ -570,7 +570,7 @@ static void tcm_qla2xxx_handle_data(struct > qla_tgt_cmd *cmd) > cmd->trc_flags |= TRC_DATA_IN; > cmd->cmd_in_wq = 1; > INIT_WORK(&cmd->work, tcm_qla2xxx_handle_data_work); > - queue_work_on(smp_processor_id(), tcm_qla2xxx_free_wq, > &cmd->work); > + queue_work(tcm_qla2xxx_free_wq, &cmd->work); > } > > static int tcm_qla2xxx_chk_dif_tags(uint32_t tag) > -- > 2.12.0 Sure. I will include in my next series once I validate changes on my setup
diff --git a/drivers/scsi/qla2xxx/qla_target.c b/drivers/scsi/qla2xxx/qla_target.c index 0e03ca2ab3e5..8c9906ac9bbc 100644 --- a/drivers/scsi/qla2xxx/qla_target.c +++ b/drivers/scsi/qla2xxx/qla_target.c @@ -4197,6 +4197,7 @@ static int qlt_handle_cmd_for_atio(struct scsi_qla_host *vha, struct fc_port *sess; struct qla_tgt_cmd *cmd; unsigned long flags; + int cpu; if (unlikely(tgt->tgt_stop)) { ql_dbg(ql_dbg_io, vha, 0x3061, @@ -4263,16 +4264,9 @@ static int qlt_handle_cmd_for_atio(struct scsi_qla_host *vha, spin_unlock_irqrestore(&vha->cmd_list_lock, flags); INIT_WORK(&cmd->work, qlt_do_work); - if (ha->msix_count) { - if (cmd->atio.u.isp24.fcp_cmnd.rddata) - queue_work_on(smp_processor_id(), qla_tgt_wq, - &cmd->work); - else - queue_work_on(cmd->se_cmd.cpuid, qla_tgt_wq, - &cmd->work); - } else { - queue_work(qla_tgt_wq, &cmd->work); - } + cpu = ha->msix_count && !cmd->atio.u.isp24.fcp_cmnd.rddata ? + cmd->se_cmd.cpuid : raw_smp_processor_id(); + queue_work_on(cpu, qla_tgt_wq, &cmd->work); return 0; } diff --git a/drivers/scsi/qla2xxx/tcm_qla2xxx.c b/drivers/scsi/qla2xxx/tcm_qla2xxx.c index 7443e4efa3ae..0f1ee1680eb8 100644 --- a/drivers/scsi/qla2xxx/tcm_qla2xxx.c +++ b/drivers/scsi/qla2xxx/tcm_qla2xxx.c @@ -303,7 +303,7 @@ static void tcm_qla2xxx_free_cmd(struct qla_tgt_cmd *cmd) cmd->trc_flags |= TRC_CMD_DONE; INIT_WORK(&cmd->work, tcm_qla2xxx_complete_free); - queue_work_on(smp_processor_id(), tcm_qla2xxx_free_wq, &cmd->work); + queue_work(tcm_qla2xxx_free_wq, &cmd->work); } /* @@ -570,7 +570,7 @@ static void tcm_qla2xxx_handle_data(struct qla_tgt_cmd *cmd) cmd->trc_flags |= TRC_DATA_IN; cmd->cmd_in_wq = 1; INIT_WORK(&cmd->work, tcm_qla2xxx_handle_data_work); - queue_work_on(smp_processor_id(), tcm_qla2xxx_free_wq, &cmd->work); + queue_work(tcm_qla2xxx_free_wq, &cmd->work); } static int tcm_qla2xxx_chk_dif_tags(uint32_t tag)