On Mon, 2011-09-26 at 14:49 -0700, Nicholas A. Bellinger wrote:
> On Mon, 2011-09-26 at 09:41 -0700, Roland Dreier wrote:
> > 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?
> >
>
> So in the process to get a working setup, not having the extra code to
> at least attempt to disable interrupts did have an negative effect
> during restart testing that eventually cased port login to not occur.
>
> Granted, this was before removing the bogus 'Disable LIP login' bit that
> was causing the LIP hangs during reset, but at least before this change
> it was easy to reproduce an problem w/o the extra interrupt changes.
>
> > 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:
> >
>
> Looks like a reasonable simplification..
>
> Btw, the tests with the original patch ran overnight w/o issue and
> reached 4500+ restarts.. I'm happy to give this simplified patch a shot
> this afternoon..
>
<SNIP>
>
> > --- 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;
> >
> >
This is working as expected with the non active I/O quick restart tests
w/ the cleared 'Disable initial LIP' bit. Dropping the original patch
is favor of this simplified version.
Thanks,
--nab
--
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
