On Mon, Apr 25, 2011 at 06:26:09PM +0200, Juergen Hannken-Illjes wrote:
> On Mon, Apr 25, 2011 at 10:33:14AM -0500, David Young wrote:
> > On Mon, Apr 25, 2011 at 02:14:23PM +0000, Juergen Hannken-Illjes wrote:
> > > Module Name: src
> > > Committed By: hannken
> > > Date: Mon Apr 25 14:14:22 UTC 2011
> > >
> > > Modified Files:
> > > src/sys/dev/scsipi: scsiconf.c
> > >
> > > Log Message:
> > > Don't kill outstanding requests when detaching a scsibus on shutdown.
> > > Both the controller and tyhe targets are still running.
> >
> > I don't think you have fixed the actual bug. It sounds like the
> > detachment routine is broken in every case where the controller was not
> > abruptly detached, but the driver relinquishes control of the controller
> > in an orderly fashion because of an operator command.
>
> The actual bug was i386 shutdown, a loop of vfs_unmountall1, config_detach_all
> and vfs_unmount_forceone. Here config_detach_all() detaches devices from
> the leafs up.
> For me it sometimes happend that the (scsi) root disk had outstanding xfers
> when it came to config_detach_all(). The disk would not detach but the
> bus detach would kill all outstanding operations. I don't want these xfers to
> abort but the disk continue operations until all xfers are done.
>
> And yes, this detach routine looks bogus at least ...
The bug was in scsibusdetach(), which is not doing things in the proper
order: it has to detach its children and check for error. If no error,
then it can release the resources that the children were using. See
attached patch.
Dave
--
David Young OJC Technologies
dyo...@ojctech.com Urbana, IL * (217) 344-0444 x24
Index: scsiconf.c
===================================================================
RCS file: /cvsroot/src/sys/dev/scsipi/scsiconf.c,v
retrieving revision 1.261
diff -u -p -r1.261 scsiconf.c
--- scsiconf.c 25 Apr 2011 14:14:22 -0000 1.261
+++ scsiconf.c 25 Apr 2011 17:30:31 -0000
@@ -256,11 +256,20 @@ scsibusdetach(device_t self, int flags)
struct scsipi_xfer *xs;
int error;
+ /*
+ * Detach all of the periphs.
+ */
+ if ((error = scsipi_target_detach(chan, -1, -1, flags)) != 0)
+ return error;
+
pmf_device_deregister(self);
/*
* Process outstanding commands (which will never complete as the
* controller is gone).
+ *
+ * XXX Surely this is redundant? If we get this far, the
+ * XXX peripherals have all been detached.
*/
for (ctarget = 0; ctarget < chan->chan_ntargets; ctarget++) {
if (ctarget == chan->chan_id)
@@ -269,8 +278,6 @@ scsibusdetach(device_t self, int flags)
periph = scsipi_lookup_periph(chan, ctarget, clun);
if (periph == NULL)
continue;
- if ((flags & DETACH_SHUTDOWN) != 0)
- return EBUSY;
TAILQ_FOREACH(xs, &periph->periph_xferq, device_q) {
callout_stop(&xs->xs_callout);
xs->error = XS_DRIVER_STUFFUP;
@@ -280,16 +287,10 @@ scsibusdetach(device_t self, int flags)
}
/*
- * Detach all of the periphs.
- */
- error = scsipi_target_detach(chan, -1, -1, flags);
-
- /*
* Now shut down the channel.
- * XXX only if no errors ?
*/
scsipi_channel_shutdown(chan);
- return (error);
+ return 0;
}
/*
