Re: working hotplug for busy devices on mpii(4)

Previous thread: Merry XxXMass by Mallena on Thursday, December 23, 2010 - 6:12 pm. (1 message)

Next thread: timeout io on mpii(4) by David Gwynne on Thursday, December 23, 2010 - 11:09 pm. (4 messages)
From: David Gwynne
Date: Thursday, December 23, 2010 - 11:01 pm

hi guys,

this makes mpii properly detach devices, which helps a lot if they
have commands in flight. to relevant changes are:

- call the activate(DVACT_DEACTIVATE) function against all the luns
on the target that is going away.
- issue the target reset BEFORE detaching the children devices.
this is needed now tha the midlayer will sleep until all outstanding
commands on a device come back from the adapter before calling the
child devices attach routine.

i have tested this on straight disks, but not on raid volumes. i
need someone to test disk removal behind a raid set before i can
commit it.

assuming testing goes well, can i get oks too?

dlg

Index: mpii.c
===================================================================
RCS file: /cvs/src/sys/dev/pci/mpii.c,v
retrieving revision 1.35
diff -u -p -r1.35 mpii.c
--- mpii.c	23 Aug 2010 00:53:36 -0000	1.35
+++ mpii.c	24 Dec 2010 05:46:34 -0000
@@ -3417,6 +3417,8 @@ mpii_event_sas(struct mpii_softc *sc, st
 			mpii_remove_dev(sc, dev);
 			if (sc->sc_scsibus) {
 				SET(dev->flags, MPII_DF_DETACH);
+				scsi_activate(sc->sc_scsibus, dev->slot, -1,
+				    DVACT_DEACTIVATE);
 				if (scsi_task(mpii_event_defer, sc,
 				    dev, 0) != 0)
 					printf("%s: unable to run device "
@@ -3529,27 +3531,19 @@ mpii_event_defer(void *xsc, void *arg)
 	struct mpii_softc	*sc = xsc;
 	struct mpii_device	*dev = arg;
 
-	/*
-	 * SAS and IR events are delivered separately, so it won't hurt
-	 * to wait for a second.
-	 */
-	tsleep(sc, PRIBIO, "mpiipause", hz);
-
-	if (!ISSET(dev->flags, MPII_DF_HIDDEN)) {
-		if (ISSET(dev->flags, MPII_DF_ATTACH))
-			scsi_probe_target(sc->sc_scsibus, dev->slot);
-		else if (ISSET(dev->flags, MPII_DF_DETACH))
-			scsi_detach_target(sc->sc_scsibus, dev->slot,
-			    DETACH_FORCE);
-	}
-
 	if (ISSET(dev->flags, MPII_DF_DETACH)) {
 		mpii_sas_remove_device(sc, dev->dev_handle);
+		if (!ISSET(dev->flags, MPII_DF_HIDDEN)) {
+			scsi_detach_target(sc->sc_scsibus, dev->slot,
+			    ...
From: Kenneth R Westerback
Date: Friday, December 24, 2010 - 9:44 am

If testing goes well, ok krw@. Alas I don't have any to test.


From: David Gwynne
Date: Tuesday, December 28, 2010 - 6:01 pm

ive had no takers on testing.

i cant see how raid and sas events will race in the current code,
so i think the 1second sleep to avoid confusion is unecessary. i
will put it in and deal with fallout if it comes up.


From: Kenneth R Westerback
Date: Tuesday, December 28, 2010 - 6:19 pm

Works for me. Full moral support from krw@.


From: Mike Belopuhov
Date: Thursday, December 30, 2010 - 11:03 am

i didn't look thru the diff yet, sorry for that.
the race you're talking about is that for IR enabled setups
you first get an event for SAS and then for IR, and the purpose
of the IR event handler is to set the MPII_DF_HIDDEN flag.

as you can see handling for SAS drives and volume drives is 
different:

	if (!ISSET(dev->flags, MPII_DF_HIDDEN)) {
		if (ISSET(dev->flags, MPII_DF_ATTACH))
			scsi_probe_target(sc->sc_scsibus, dev->slot);
		else if (ISSET(dev->flags, MPII_DF_DETACH))
			scsi_detach_target(sc->sc_scsibus, dev->slot,
			    DETACH_FORCE);
	}

	if (ISSET(dev->flags, MPII_DF_DETACH)) {
		mpii_sas_remove_device(sc, dev->dev_handle);
		free(dev, M_DEVBUF);
		return;
	}

we're not supposed to call scsi_detach_target on volume disks.
so your change looks wrong to me, iirc how it works.

From: David Gwynne
Date: Thursday, December 30, 2010 - 2:48 pm

i'll wire up an IR volume and muck around then. does this mean that the
physical and logical disks have colliding or overlapping entries in the
sc_devs array?

dlg

From: Mike Belopuhov
Date: Saturday, January 1, 2011 - 6:28 am

no, the border value is controlled by sc_pd_id_start and sc_vd_id_low.
a quick test would be pulling out a drive which is part of the mirror volume
while doing io to it.

Previous thread: Merry XxXMass by Mallena on Thursday, December 23, 2010 - 6:12 pm. (1 message)

Next thread: timeout io on mpii(4) by David Gwynne on Thursday, December 23, 2010 - 11:09 pm. (4 messages)