[v4,00/14] qla2xxx: Bug Fixes and updates for target.
diff mbox

Message ID 1489637143.5578.1.camel@sandisk.com
State New, archived
Headers show

Commit Message

Bart Van Assche March 16, 2017, 4:05 a.m. UTC
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(-)

-- 
2.12.0

Comments

Madhani, Himanshu March 16, 2017, 4:33 a.m. UTC | #1
> -----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

Patch
diff mbox

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)