svn commit: r333268 - head/sys/kern

[Andriy Gapon]

Author: avg
Date: Sat May  5 05:19:32 2018
New Revision: 333268
URL: https://svnweb.freebsd.org/changeset/base/333268

Log:
  for bus suspend, detach and shutdown iterate children in reverse order
  
  For most buses all children are equal, so the order does not matter.
  Other buses, such as acpi, carefully order their child devices to
  express implicit dependencies between them.  For such buses it is safer
  to bring down devices in the reverse order.
  
  I believe that this is the reason why hpet_suspend had to be disabled.
  Some drivers depend on a working event timer until they are suspended.
  But previously we would suspend hpet very early.
  
  I tested this change by makinbg hpet_suspend actually stop HPET timers
  and tested that too.
  
  Note that this change is not a complete solution as it does not take
  into account bus passes.
  A better approach would be to track the actual attach order of the
  devices and to use the reverse of that.
  
  Reviewed by:	imp, mav
  MFC after:	3 weeks
  Differential Revision: https://reviews.freebsd.org/D15291

Modified:
  head/sys/kern/subr_bus.c

Modified: head/sys/kern/subr_bus.c
==============================================================================
--- head/sys/kern/subr_bus.c	Fri May  4 22:48:10 2018	(r333267)
+++ head/sys/kern/subr_bus.c	Sat May  5 05:19:32 2018	(r333268)
@@ -3708,7 +3708,11 @@ bus_generic_detach(device_t dev)
 	if (dev->state != DS_ATTACHED)
 		return (EBUSY);
 
-	TAILQ_FOREACH(child, &dev->children, link) {
+	/*
+	 * Detach children in the reverse order.
+	 * See bus_generic_suspend for details.
+	 */
+	TAILQ_FOREACH_REVERSE(child, &dev->children, device_list, link) {
 		if ((error = device_detach(child)) != 0)
 			return (error);
 	}
@@ -3728,7 +3732,11 @@ bus_generic_shutdown(device_t dev)
 {
 	device_t child;
 
-	TAILQ_FOREACH(child, &dev->children, link) {
+	/*
+	 * Shut down children in the reverse order.
+	 * See bus_generic_suspend for details.
+	 */
+	TAILQ_FOREACH_REVERSE(child, &dev->children, device_list, link) {
 		device_shutdown(child);
 	}
 
@@ -3781,15 +3789,23 @@ int
 bus_generic_suspend(device_t dev)
 {
 	int		error;
-	device_t	child, child2;
+	device_t	child;
 
-	TAILQ_FOREACH(child, &dev->children, link) {
+	/*
+	 * Suspend children in the reverse order.
+	 * For most buses all children are equal, so the order does not matter.
+	 * Other buses, such as acpi, carefully order their child devices to
+	 * express implicit dependencies between them.  For such buses it is
+	 * safer to bring down devices in the reverse order.
+	 */
+	TAILQ_FOREACH_REVERSE(child, &dev->children, device_list, link) {
 		error = BUS_SUSPEND_CHILD(dev, child);
-		if (error) {
-			for (child2 = TAILQ_FIRST(&dev->children);
-			     child2 && child2 != child;
-			     child2 = TAILQ_NEXT(child2, link))
-				BUS_RESUME_CHILD(dev, child2);
+		if (error != 0) {
+			child = TAILQ_NEXT(child, link);
+			if (child != NULL) {
+				TAILQ_FOREACH_FROM(child, &dev->children, link)
+					BUS_RESUME_CHILD(dev, child);
+			}
 			return (error);
 		}
 	}