RE: Problem with twa in HEAD

> -----Original Message-----
> From: Scott Long [mailto:scottl_at_samsco.org]
> Sent: Thursday, April 28, 2005 11:57 PM
> To: Vinod Kashyap
> Cc: Bjoern A. Zeeb; freebsd-current_at_FreeBSD.org
> Subject: Re: Problem with twa in HEAD
> 
> 
> Vinod Kashyap wrote:
> >
> >>-----Original Message-----
> >>From: Bjoern A. Zeeb [mailto:bz_at_FreeBSD.org]
> >>Sent: Tuesday, April 26, 2005 3:26 AM
> >>To: Vinod Kashyap
> >>Subject: RE: Problem with twa in HEAD
> >>
> >>
> >>On Mon, 25 Apr 2005, Vinod Kashyap wrote:
> >>
> >>Hi,
> >>
> >>
> >>>>-----Original Message-----
> >>>>From: Bjoern A. Zeeb [mailto:bz_at_FreeBSD.org]
> >>>>Sent: Monday, April 25, 2005 6:45 AM
> >>>>To: Vinod Kashyap
> >>>>Subject: Re: Problem with twa in HEAD
> >>>>
> >>>>
> >>>>On Fri, 22 Apr 2005, Bjoern A. Zeeb wrote:
> >>>>
> >>>>Hi,
> >>>>
> >>>>
> >>>>>scottl redirected me to you.
> >>>>>
> >>>>>I am currently debugging "hangs" on reboot and shutdown on a
> >>>>>SMP machine with 12 discs at a
> >>>>>
> >>>>>3ware device driver for 9000 series storage controllers,
> >>>>
> >>>>version: 3.60.00.016
> >>>>
> >>>>>twa0: <3ware 9000 series Storage Controller> port
> >>>>
> >>>>0x9800-0x98ff mem 0xfe8ffc00-0xfe8ffcff,0xfb800000-0xfbffffff
> >>>>irq 28 at device 6.0 on pci3
> >>>>
> >>>>>twa0: [FAST]
> >>>>>twa0: INFO: (0x15: 0x1300): Controller details:: 12 ports,
> >>>>
> >>>>Firmware FE9X 2.06.00.009, BIOS BE9X 2.03.01.051
> >>>>
> >>>>>
> >>>>>What I know so far is that Giant is held by sync.
> >>>>>
> >>>>>Things a "spinning" in cam/cam_xpt.c around:
> >>>>>
> >>>>>--- cam_xpt.c   31 Mar 2005 21:42:49 -0000      1.152
> >>>>>+++ cam_xpt.c   22 Apr 2005 18:42:43 -0000
> >>>>>_at__at_ -3643,6 +3643,7 _at__at_ xpt_polled_action(union ccb *start_ccb)
> >>>>>                            != CAM_REQ_INPROG)
> >>>>>                                break;
> >>>>>                        DELAY(1000);
> >>>>>                        printf("XXX status=%02x\n",
> >>>>
> >>>>start_ccb->ccb_h.status);
> >>>>
> >>>>>                }
> >>>>>                if (timeout == 0) {
> >>>>>                        /*
> >>>>>
> >>>>>
> >>>>>with status being 0x200.
> >>>>>
> >>>>>Seems the twa has a command stuck in it.
> >>>>>
> >>>>>I have seen the comment in dev/twa/tw_osl_cam.c ~ line 253 about
> >>>>>queuing and CAM_SIM_QUEUED but I don't know enough about cam.
> >>>>>I seems no all patchs out of this functions seem to
> >>
> >>clear that from
> >>
> >>>>>status?
> >>>>>
> >>>>>Any help apreaciated ;) I can try patches; as long as I
> >>
> >>can break
> >>
> >>>>>to db> to reboot.
> >>>>
> >>>>further debugging shows that is seems to be spinning in twa_poll.
> >>>>see debug output from TWA_DEBUG 3. The problem is that at
> >>
> >>this point
> >>
> >>>>I am no longer able to break to debugger.
> >>>>
> >>>>twa0: tw_osli_execute_scsi: XPT_SCSI_IO: Single virtual address!
> >>>>twa0: tw_osli_execute_scsi: XPT_SCSI_IO: Single virtual address!
> >>>>unmount of /dev failed (BUSY)
> >>>>twa0: tw_osli_execute_scsi: XPT_SCSI_IO: Single virtual address!
> >>>>twa0: tw_osli_execute_scsi: XPT_SCSI_IO: Single virtual address!
> >>>>Uptime: 2m57s
> >>>>twa0: tw_osli_execute_scsi: XPT_SCSI_IO: Single virtual address!
> >>>>twa0: twa_poll: entering; sc = 0xc57bb200
> >>>>twa0: twa_poll: exiting; sc = 0xc57bb200
> >>>>twa0: twa_poll: entering; sc = 0xc57bb200
> >>>>twa0: twa_poll: exiting; sc = 0xc57bb200
> >>>>twa0: twa_poll: entering; sc = 0xc57bb200
> >>>>twa0: twa_poll: exiting; sc = 0xc57bb200
> >>>>twa0: twa_poll: entering; sc = 0xc57bb200
> >>>>twa0: twa_poll: exiting; sc = 0xc57bb200
> >>>>twa0: twa_poll: entering; sc = 0xc57bb200
> >>>>twa0: twa_poll: exiting; sc = 0xc57bb200
> >>>>twa0: twa_poll: entering; sc = 0xc57bb200
> >>>>twa0: twa_poll: exiting; sc = 0xc57bb200
> >>>>twa0: twa_poll: entering; sc = 0xc57bb200
> >>>>twa0: twa_poll: exiting; sc = 0xc57bb200
> >>>>...
> >>>>
> >>>
> >>>I am in the middle of an office move right now.
> >>>I will get back to you once I have some time to look into this.
> >>
> >>
> >>thanks for the information; I'll be able to test at least 
> until end of
> >>this week and hopefully next week too.
> >>
> >
> >
> > I looked into this, and this is what is happening:
> > On reboot/halt, the following function calling sequence happens:
> > ... --> dashutdown --> xpt_polled_action --> twa_poll.
> > But, the interrupt handler in twa is still active at this time,
> > since twa_detach/twa_shutdown hasn't been called yet.  Before
> > twa_poll can fetch the response for the posted command, the ISR
> > gets called when the firmware posts the response.  The ISR clears
> > the interrupt bit on the controller, registers a taskqueue 
> handler like
> > it always does, and exits.  Meanwhile, xpt_polled_action continues
> > to call twa_poll, which cannot determine that the command 
> has completed,
> > since the interrupt bit on the controller is already cleared.  So,
> > we get into a (near) never-ending loop (the timeout for 
> scsi_synchronize_cache,
> > which is what is being tried here, is, for whatever reason, 
> 60 minutes,
> > and so, the system is as good as hung).
> >
> > Now, does anyone know why xpt_polled_action is being called from
> > dashutdown, even before the ISR has been unregistered (via 
> twa_detach)?
> >
> > Bjoern, this patch should work-around your problem, 
> although it's not
> > the fix.  Also, it still leaves a window for the race 
> condition described
> > above.
> >
> 
> xpt_polled_action() expects that it can simulate interrupts by calling
> the driver poll vector, and that by calling it enough times the driver
> will eventually complete all the outstanding I/O it has.  As you note,
> it'll repeat this for a very long time.  So the question is 
> then why the
> twa driver isn't completing the outstanding I/O.  If I were you I'd
> remove the call to tw_cl_interrupt() in twa_poll() and just
> unconditionally call tw_cl_deferred_interrupt() and have it check
> everything.

The call to tw_cl_interrupt cannot be removed as that's where the interrupt
is cleared.  However, the call to tw_cl_deferred_interrupt can be made
not conditional to the return value from tw_cl_interrupt.
Whatever that is, my question is why polling is being resorted to, when
interrupts are available?

>  The locking here (and in twa_pci_intr()) is 
> flawed anyways,
> you have a race between when tw_cl_interrupt() drops its lock right
> before return and when you check it's return value.  I'd like say that
> it's harmless, except that you expect to pass state from one 
> function to
> the next, so the race is a real one.  It's likely why this case is

If the state passed to the second function is invalid when the second
function executes, it just does nothing.  So, this is pretty harmless.

> failing.  An ideal FAST handler should only clear the 
> hardware interrupt
> register and launch the appropriate handlers, it shouldn't try to pass
> state to the handlers.  Look at aac for an example here, but 
> also please
> recall that I've already discouraged you from using a a fast handler
> plus taskqueue for this driver.  If your taskqueue handlers need state
> from when the interrupt was cleared, then they simply aren't a good
> candidate for this model.
> 

In twa, state will need to be passed to the taskqueue handlers since the
ISR clears the state on the controller.