Changes in uspace/srv/devman/devman.c [0d6915f:463e734] in mainline

File:

• uspace/srv/devman/devman.c (modified) (13 diffs)

uspace/srv/devman/devman.c

@@ -508 +508 @@
 /** Notify driver about the devices to which it was assigned.
  *
- * The driver's mutex must be locked.
- *
  * @param driver        The driver to which the devices are passed.
  */
@@ -518 +516 @@
         int phone;
 
-        printf(NAME ": pass_devices_to_driver\n");
-
-        phone = ipc_connect_me_to(driver->phone, DRIVER_DEVMAN, 0, 0);
-        if (phone > 0) {
-                
+        printf(NAME ": pass_devices_to_driver(`%s')\n", driver->name);
+
+        fibril_mutex_lock(&driver->driver_mutex);
+
+        phone = async_connect_me_to(driver->phone, DRIVER_DEVMAN, 0, 0);
+
+        if (phone < 0) {
+                fibril_mutex_unlock(&driver->driver_mutex);
+                return;
+        }
+
+        /*
+         * Go through devices list as long as there is some device
+         * that has not been passed to the driver.
+         */
+        link = driver->devices.next;
+        while (link != &driver->devices) {
+                dev = list_get_instance(link, node_t, driver_devices);
+                if (dev->passed_to_driver) {
+                        link = link->next;
+                        continue;
+                }
+
+                /*
+                 * We remove the device from the list to allow safe adding
+                 * of new devices (no one will touch our item this way).
+                 */
+                list_remove(link);
+
+                /*
+                 * Unlock to avoid deadlock when adding device
+                 * handled by itself.
+                 */
+                fibril_mutex_unlock(&driver->driver_mutex);
+
+                add_device(phone, driver, dev, tree);
+
+                /*
+                 * Lock again as we will work with driver's
+                 * structure.
+                 */
+                fibril_mutex_lock(&driver->driver_mutex);
+
+                /*
+                 * Insert the device back.
+                 * The order is not relevant here so no harm is done
+                 * (actually, the order would be preserved in most cases).
+                 */
+                list_append(link, &driver->devices);
+
+                /*
+                 * Restart the cycle to go through all devices again.
+                 */
                 link = driver->devices.next;
-                while (link != &driver->devices) {
-                        dev = list_get_instance(link, node_t, driver_devices);
-                        add_device(phone, driver, dev, tree);
-                        link = link->next;
-                }
-                
-                ipc_hangup(phone);
-        }
+        }
+
+        ipc_hangup(phone);
+
+        /*
+         * Once we passed all devices to the driver, we need to mark the
+         * driver as running.
+         * It is vital to do it here and inside critical section.
+         *
+         * If we would change the state earlier, other devices added to
+         * the driver would be added to the device list and started
+         * immediately and possibly started here as well.
+         */
+        printf(NAME ": driver %s goes into running state.\n", driver->name);
+        driver->state = DRIVER_RUNNING;
+
+        fibril_mutex_unlock(&driver->driver_mutex);
 }
 
@@ -545 +600 @@
 void initialize_running_driver(driver_t *driver, dev_tree_t *tree)
 {
-        printf(NAME ": initialize_running_driver\n");
-        fibril_mutex_lock(&driver->driver_mutex);
+        printf(NAME ": initialize_running_driver (`%s')\n", driver->name);
         
         /*
@@ -553 +607 @@
          */
         pass_devices_to_driver(driver, tree);
-        
-        /* Change driver's state to running. */
-        driver->state = DRIVER_RUNNING;
-        
-        fibril_mutex_unlock(&driver->driver_mutex);
 }
 
@@ -629 +678 @@
 }
 
+static FIBRIL_MUTEX_INITIALIZE(add_device_guard);
 
 /** Pass a device to running driver.
@@ -637 +687 @@
 void add_device(int phone, driver_t *drv, node_t *node, dev_tree_t *tree)
 {
-        printf(NAME ": add_device\n");
+        fibril_mutex_lock(&add_device_guard);
+
+        /*
+         * We do not expect to have driver's mutex locked as we do not
+         * access any structures that would affect driver_t.
+         */
+        printf(NAME ": add_device (driver `%s', device `%s')\n", drv->name,
+            node->name);
         
         ipcarg_t rc;
@@ -649 +706 @@
                 parent_handle = 0;
         }
+
         aid_t req = async_send_2(phone, DRIVER_ADD_DEVICE, node->handle,
             parent_handle, &answer);
@@ -658 +716 @@
                 /* TODO handle error */
         }
-        
+
         /* Wait for answer from the driver. */
         async_wait_for(req, &rc);
+
+        fibril_mutex_unlock(&add_device_guard);
+
         switch(rc) {
         case EOK:
@@ -673 +734 @@
         }
         
+        node->passed_to_driver = true;
+
         return;
 }
@@ -698 +761 @@
         attach_driver(node, drv);
         
+        fibril_mutex_lock(&drv->driver_mutex);
         if (drv->state == DRIVER_NOT_STARTED) {
                 /* Start the driver. */
                 start_driver(drv);
         }
-        
-        if (drv->state == DRIVER_RUNNING) {
+        fibril_mutex_unlock(&drv->driver_mutex);
+        
+        fibril_mutex_lock(&drv->driver_mutex);
+        bool is_running = drv->state == DRIVER_RUNNING;
+        fibril_mutex_unlock(&drv->driver_mutex);
+
+        if (is_running) {
                 /* Notify the driver about the new device. */
-                int phone = ipc_connect_me_to(drv->phone, DRIVER_DEVMAN, 0, 0);
+                int phone = async_connect_me_to(drv->phone, DRIVER_DEVMAN, 0, 0);
                 if (phone > 0) {
                         add_device(phone, drv, node, tree);
@@ -867 +936 @@
         node->name = dev_name;
         if (!set_dev_path(node, parent)) {
-                fibril_rwlock_write_unlock(&tree->rwlock);
                 return false;
         }
@@ -1020 +1088 @@
         
         size_t idx = get_new_class_dev_idx(cl);
-        asprintf(&dev_name, "%s%d", base_name, idx);
+        asprintf(&dev_name, "%s%zu", base_name, idx);
         
         return dev_name;
@@ -1089 +1157 @@
         while (link != &class_list->classes) {
                 cl = list_get_instance(link, dev_class_t, link);
-                if (str_cmp(cl->name, class_name) == 0)
+                if (str_cmp(cl->name, class_name) == 0) {
                         return cl;
+                }
+                link = link->next;
         }