Does this really have to be so hard?
First of all I'm not sure I understand the changes about interrupt
handling / IS_NOPOLLING_TYPE -- I don't see how we go back to legacy
interrupt mode, since MSI-X is still enabled as far as I can tell. In
any case what's wrong with getting interrupts during the restart?
Also why do we need to move the "online=1" out of restart_isp? I
haven't tried the stress test you did, but it seemed to work in my
testing just to add a call to process the atio queue right after the
existing "online=1" assignment. The patch that I came up with
(independent of yours and probably not as rigorously tested) is the
following, which is pretty small if you leave out the debugging check
junk I put in:
--- linux-2.6.git.orig/drivers/scsi/qla2xxx/qla_def.h 2011-09-25
22:08:59.026884599 -0700
+++ linux-2.6.git/drivers/scsi/qla2xxx/qla_def.h 2011-09-25
22:09:32.996971614 -0700
@@ -2886,6 +2886,7 @@ typedef struct scsi_qla_host {
uint32_t online :1;
uint32_t rscn_queue_overflow :1;
uint32_t reset_active :1;
+ uint32_t atio_ignored :1;
uint32_t management_server_logged_in :1;
uint32_t process_response_queue :1;
--- linux-2.6.git.orig/drivers/scsi/qla2xxx/qla_init.c 2011-09-25
22:10:17.357085241 -0700
+++ linux-2.6.git/drivers/scsi/qla2xxx/qla_init.c 2011-09-25
22:13:46.467620871 -0700
@@ -498,6 +498,7 @@ qla2x00_initialize_adapter(scsi_qla_host
/* Clear adapter flags. */
vha->flags.online = 0;
+ vha->flags.atio_ignored = 0;
ha->flags.chip_reset_done = 0;
vha->flags.reset_active = 0;
ha->flags.pci_channel_io_perm_failure = 0;
@@ -4248,6 +4249,7 @@ qla2x00_restart_isp(scsi_qla_host_t *vha
struct qla_hw_data *ha = vha->hw;
struct req_que *req = ha->req_q_map[0];
struct rsp_que *rsp = ha->rsp_q_map[0];
+ unsigned long flags;
/* If firmware needs to be loaded */
if (qla2x00_isp_firmware(vha)) {
@@ -4272,6 +4274,19 @@ qla2x00_restart_isp(scsi_qla_host_t *vha
qla2x00_marker(vha, req, rsp, 0, 0, MK_SYNC_ALL);
vha->flags.online = 1;
+
+ /*
+ * Process any ATIO queue entries that came in
+ * while we weren't online.
+ */
+ spin_lock_irqsave(&ha->hardware_lock, flags);
+ if (vha->flags.atio_ignored) {
+ qla_printk(KERN_INFO, ha, "Processing ignored ATIO entries\n");
+ vha->flags.atio_ignored = 0;
+ }
+ qla_tgt_24xx_process_atio_queue(vha);
+ spin_unlock_irqrestore(&ha->hardware_lock, flags);
+
/* Wait at most MAX_TARGET RSCNs for a stable link. */
wait_time = 256;
do {
--- linux-2.6.git.orig/drivers/scsi/qla2xxx/qla_target.c 2011-09-25
22:07:24.917072474 -0700
+++ linux-2.6.git/drivers/scsi/qla2xxx/qla_target.c 2011-09-25
22:10:03.537049841 -0700
@@ -5340,8 +5340,11 @@ qla_tgt_24xx_process_atio_queue(struct s
atio_t *pkt;
int cnt, i;
- if (!vha->flags.online)
+ if (!vha->flags.online) {
+ vha->flags.atio_ignored = 1;
+ qla_printk(KERN_INFO, ha, "ATIO entry ignored because online == 0\n");
return;
+ }
while (ha->atio_ring_ptr->signature != ATIO_PROCESSED) {
pkt = ha->atio_ring_ptr;
On Sun, Sep 25, 2011 at 9:26 PM, Nicholas A. Bellinger
<nab@xxxxxxxxxxxxxxx> wrote:
> From: Nicholas Bellinger <nab@xxxxxxxxxxxxxxx>
>
> This patch addresses an issue between the assignment of vha->flags.online = 1
> to signal operational status in process context via qla2x00_do_dpc() ->
> qla2x00_abort_isp() -> qla2x00_restart_isp(), and processing of individual
> ATIO packets in interrupt context within qla_tgt_24xx_process_atio_queue().
> This was causing qla_tgt_24xx_process_atio_queue() to explictly ignore
> new packets with non-operational status before completion of qla2x00_restart_isp()
> -> target mode operation transition, and more specifically ELS packets
> that are expected to drive the re-creation of active qla_tgt_sess nexuses in
> qla_target.c + tcm_qla2xxx code.
>
> This v2 patch takes a different approach than v1, which originally tried to
> allow ATIO ring processing in qla_tgt_24xx_process_atio_queue() during
> qla2x00_abort_isp() reset operation. This patch:
>
> *) Keep !vha->flags.online check in qla_tgt_24xx_process_atio_queue() to
> prevent all ATIO ring processing during qla2x00_abort_isp() reset.
> *) Prevent vha->flags.online = 1 from being set during qla2x00_abort_isp()
> and qla2x00_restart_isp() processing.
> *) Set vha->flags.online = 1 upon the completion of qla2x00_abort_isp(),
> and make a direct call into qla_tgt_24xx_process_atio_queue() from
> qla2x00_do_dpc() process context to handle ATIO packets that have
> accumulated during qla2x00_abort_isp().
> *) Add extra paranoid qla_tgt_mode_enabled_base_vha() checks around
> existing IS_NOPOLLING_TYPE() usage in qla2x00_mailbox_command(),
> qla24xx_disable_intrs() and qla24xx_reset_risc(). This allows
> target mode to run in legacy interrupt mode for mailbox commands
> during qla2x00_abort_isp() reset, and allows qla24xx_disable_intrs()
> to clear qla_hw_data->interrupts_on during reset.
>
> This patch also depends upon 'Disable initial LIP' being cleared within
> qla_tgt_24xx_config_nvram_stage1() for firmware_options_1:
>
> /* Enable initial LIP */
> nv->firmware_options_1 &= __constant_cpu_to_le32(~BIT_9);
>
> Enabling this bit to 'Disable initial LIP' ends up causing intermittent
> reset hangs with target mode operation, eventually causing the port to
> be taken offline.
>
> Reported-by: Roland Dreier <roland@xxxxxxxxxxxxxxx>
> Cc: Roland Dreier <roland@xxxxxxxxxxxxxxx>
> Reported-by: Patrick Lee <patrick@xxxxxxxxxxxxxxx>
> Cc: Patrick Lee <patrick@xxxxxxxxxxxxxxx>
> Cc: Andrew Vasquez <andrew.vasquez@xxxxxxxxxx>
> Cc: Madhuranath Iyengar <mni@xxxxxxxxxxxxxxxxxxxxx>
> Signed-off-by: Nicholas Bellinger <nab@xxxxxxxxxxxxxxxxxxxxx>
> ---
> drivers/scsi/qla2xxx/qla_init.c | 20 +++++++++++++++++---
> drivers/scsi/qla2xxx/qla_mbx.c | 7 ++++---
> drivers/scsi/qla2xxx/qla_os.c | 2 +-
> drivers/scsi/qla2xxx/qla_target.c | 8 +++++++-
> drivers/scsi/qla2xxx/qla_target.h | 8 ++++++++
> 5 files changed, 37 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/scsi/qla2xxx/qla_init.c b/drivers/scsi/qla2xxx/qla_init.c
> index fca50d5..de8810d 100644
> --- a/drivers/scsi/qla2xxx/qla_init.c
> +++ b/drivers/scsi/qla2xxx/qla_init.c
> @@ -1068,7 +1068,7 @@ qla24xx_reset_risc(scsi_qla_host_t *vha)
>
> spin_unlock_irqrestore(&ha->hardware_lock, flags);
>
> - if (IS_NOPOLLING_TYPE(ha))
> + if (IS_NOPOLLING_TYPE(ha) && !qla_tgt_mode_enabled_base_vha(ha))
> ha->isp_ops->enable_intrs(ha);
> }
>
> @@ -4133,7 +4133,8 @@ qla2x00_abort_isp(scsi_qla_host_t *vha)
> vha->marker_needed = 1;
> }
>
> - vha->flags.online = 1;
> + if (!qla_tgt_mode_enabled(vha))
> + vha->flags.online = 1;
>
> qla_tgt_abort_isp(vha);
>
> @@ -4229,6 +4230,17 @@ qla2x00_abort_isp(scsi_qla_host_t *vha)
> }
> }
> spin_unlock_irqrestore(&ha->vport_slock, flags);
> + /*
> + * For target mode, wait until the last moment to set the
> + * online flag and kick the ATIO ring to process new packets
> + * that have accumulated during qla2x00_abort_isp().
> + */
> + if (qla_tgt_mode_enabled(vha)) {
> + spin_lock_irqsave(&ha->hardware_lock, flags);
> + vha->flags.online = 1;
> + qla_tgt_24xx_process_atio_queue(vha);
> + spin_unlock_irqrestore(&ha->hardware_lock, flags);
> + }
>
> } else {
> qla_printk(KERN_INFO, ha,
> @@ -4279,7 +4291,9 @@ qla2x00_restart_isp(scsi_qla_host_t *vha)
> /* Issue a marker after FW becomes ready. */
> qla2x00_marker(vha, req, rsp, 0, 0, MK_SYNC_ALL);
>
> - vha->flags.online = 1;
> + if (!qla_tgt_mode_enabled(vha))
> + vha->flags.online = 1;
> +
> /* Wait at most MAX_TARGET RSCNs for a stable link. */
> wait_time = 256;
> do {
> diff --git a/drivers/scsi/qla2xxx/qla_mbx.c b/drivers/scsi/qla2xxx/qla_mbx.c
> index 087b2cc..3813306 100644
> --- a/drivers/scsi/qla2xxx/qla_mbx.c
> +++ b/drivers/scsi/qla2xxx/qla_mbx.c
> @@ -148,7 +148,8 @@ qla2x00_mailbox_command(scsi_qla_host_t *vha, mbx_cmd_t *mcp)
>
> /* Wait for mbx cmd completion until timeout */
>
> - if ((!abort_active && io_lock_on) || IS_NOPOLLING_TYPE(ha)) {
> + if ((!abort_active && io_lock_on) ||
> + (IS_NOPOLLING_TYPE(ha) && !qla_tgt_mode_enabled_base_vha(ha))) {
> set_bit(MBX_INTR_WAIT, &ha->mbx_cmd_flags);
>
> if (IS_QLA82XX(ha)) {
> @@ -278,8 +279,8 @@ qla2x00_mailbox_command(scsi_qla_host_t *vha, mbx_cmd_t *mcp)
>
> /* Clean up */
> ha->mcp = NULL;
> -
> - if ((abort_active || !io_lock_on) && !IS_NOPOLLING_TYPE(ha)) {
> + if ((abort_active || !io_lock_on) &&
> + (!IS_NOPOLLING_TYPE(ha) || qla_tgt_mode_enabled_base_vha(ha))) {
> DEBUG11(printk("%s(%ld): checking for additional resp "
> "interrupt.\n", __func__, base_vha->host_no));
>
> diff --git a/drivers/scsi/qla2xxx/qla_os.c b/drivers/scsi/qla2xxx/qla_os.c
> index 2ab9564..0b61ca7 100644
> --- a/drivers/scsi/qla2xxx/qla_os.c
> +++ b/drivers/scsi/qla2xxx/qla_os.c
> @@ -1486,7 +1486,7 @@ qla24xx_disable_intrs(struct qla_hw_data *ha)
> unsigned long flags = 0;
> struct device_reg_24xx __iomem *reg = &ha->iobase->isp24;
>
> - if (IS_NOPOLLING_TYPE(ha))
> + if (IS_NOPOLLING_TYPE(ha) && !qla_tgt_mode_enabled_base_vha(ha))
> return;
> spin_lock_irqsave(&ha->hardware_lock, flags);
> ha->interrupts_on = 0;
> diff --git a/drivers/scsi/qla2xxx/qla_target.c b/drivers/scsi/qla2xxx/qla_target.c
> index 757e971..f26b743 100644
> --- a/drivers/scsi/qla2xxx/qla_target.c
> +++ b/drivers/scsi/qla2xxx/qla_target.c
> @@ -5328,7 +5328,13 @@ qla_tgt_24xx_process_atio_queue(struct scsi_qla_host *vha)
> struct device_reg_24xx __iomem *reg = &ha->iobase->isp24;
> atio_t *pkt;
> int cnt, i;
> -
> + /*
> + * During qla2x00_abort_isp() we expect to hit this check in order
> + * to prevent processing during reset. Upon completion of reset,
> + * qla2x00_abort_isp() will set vha->flags.online=1 and make a
> + * direct call to qla_tgt_24xx_process_atio_queue() to kick off
> + * ATIO ring processing from qla2x00_do_dpc() process context.
> + */
> if (!vha->flags.online)
> return;
>
> diff --git a/drivers/scsi/qla2xxx/qla_target.h b/drivers/scsi/qla2xxx/qla_target.h
> index 42d5479..e89a7e3 100644
> --- a/drivers/scsi/qla2xxx/qla_target.h
> +++ b/drivers/scsi/qla2xxx/qla_target.h
> @@ -1023,6 +1023,14 @@ static inline bool qla_tgt_mode_enabled(struct scsi_qla_host *ha)
> return ha->host->active_mode & MODE_TARGET;
> }
>
> +static inline bool qla_tgt_mode_enabled_base_vha(struct qla_hw_data *hw)
> +{
> + if (!hw->qla_tgt || !hw->qla_tgt->vha)
> + return false;
> +
> + return qla_tgt_mode_enabled(hw->qla_tgt->vha);
> +}
> +
> static inline bool qla_ini_mode_enabled(struct scsi_qla_host *ha)
> {
> return ha->host->active_mode & MODE_INITIATOR;
> --
> 1.7.2.5
>
>
--
To unsubscribe from this list: send the line "unsubscribe target-devel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
