Changeset 0f0f8bc in mainline for uspace/lib/drv/generic/driver.c

Timestamp:

2011-08-29T18:46:37Z (13 years ago)

Author:

Jiri Svoboda <jiri@…>

Branches:

lfn, master, serial, ticket/834-toolchain-update, topic/msim-upgrade, topic/simplify-dev-export

Children:

4d94002d

Parents:

8cc4ddb

Message:

Use reference counting to synchronize freeing of ddf_fun_t and ddf_dev_t.
This ensures that a function structure (device structure, potentially)
passed as argument to a driver entry point is not deallocated until
control returns from that entry point. We still need similar guarantee
for driver data.

File:

• uspace/lib/drv/generic/driver.c (modified) (17 diffs)

uspace/lib/drv/generic/driver.c

@@ -86 +86 @@
 static ddf_dev_t *create_device(void);
 static void delete_device(ddf_dev_t *);
+static void dev_add_ref(ddf_dev_t *);
+static void dev_del_ref(ddf_dev_t *);
+static void fun_add_ref(ddf_fun_t *);
+static void fun_del_ref(ddf_fun_t *);
 static remote_handler_t *function_get_default_handler(ddf_fun_t *);
 static void *function_get_ops(ddf_fun_t *, dev_inferface_idx_t);
@@ -261 +265 @@
 }
 
-static void driver_add_device(ipc_callid_t iid, ipc_call_t *icall)
+static void driver_dev_add(ipc_callid_t iid, ipc_call_t *icall)
 {
         char *dev_name = NULL;
@@ -270 +274 @@
         
         ddf_dev_t *dev = create_device();
+
+        /* Add one reference that will be dropped by driver_dev_remove() */
+        dev_add_ref(dev);
         dev->handle = dev_handle;
 
@@ -282 +289 @@
         
         res = driver->driver_ops->add_device(dev);
-        if (res != EOK)
-                delete_device(dev);
+        
+        if (res != EOK) {
+                dev_del_ref(dev);
+                async_answer_0(iid, res);
+                return;
+        }
         
         fibril_mutex_lock(&devices_mutex);
@@ -303 +314 @@
         fibril_mutex_lock(&devices_mutex);
         dev = driver_get_device(devh);
+        dev_add_ref(dev);
         fibril_mutex_unlock(&devices_mutex);
-        /* XXX need lock on dev */
         
         if (dev == NULL) {
@@ -316 +327 @@
                 rc = ENOTSUP;
         
+        if (rc == EOK)
+                dev_del_ref(dev);
+        
         async_answer_0(iid, (sysarg_t) rc);
 }
@@ -326 +340 @@
         
         funh = IPC_GET_ARG1(*icall);
+        
+        /*
+         * Look up the function. Bump reference count so that
+         * the function continues to exist until we return
+         * from the driver.
+         */
         fibril_mutex_lock(&functions_mutex);
+        
         fun = driver_get_function(funh);
+        if (fun != NULL)
+                fun_add_ref(fun);
+        
         fibril_mutex_unlock(&functions_mutex);
-        /* XXX Need lock on fun */
         
         if (fun == NULL) {
@@ -336 +359 @@
         }
         
+        /* Call driver entry point */
         if (driver->driver_ops->fun_online != NULL)
                 rc = driver->driver_ops->fun_online(fun);
@@ -341 +365 @@
                 rc = ENOTSUP;
         
+        fun_del_ref(fun);
+        
         async_answer_0(iid, (sysarg_t) rc);
 }
@@ -351 +377 @@
         
         funh = IPC_GET_ARG1(*icall);
+        
+        /*
+         * Look up the function. Bump reference count so that
+         * the function continues to exist until we return
+         * from the driver.
+         */
         fibril_mutex_lock(&functions_mutex);
+        
         fun = driver_get_function(funh);
+        if (fun != NULL)
+                fun_add_ref(fun);
+        
         fibril_mutex_unlock(&functions_mutex);
-        /* XXX Need lock on fun */
         
         if (fun == NULL) {
@@ -361 +396 @@
         }
         
+        /* Call driver entry point */
         if (driver->driver_ops->fun_offline != NULL)
                 rc = driver->driver_ops->fun_offline(fun);
@@ -383 +419 @@
                 switch (IPC_GET_IMETHOD(call)) {
                 case DRIVER_DEV_ADD:
-                        driver_add_device(callid, &call);
+                        driver_dev_add(callid, &call);
                         break;
                 case DRIVER_DEV_REMOVE:
@@ -575 +611 @@
         ddf_dev_t *dev;
 
-        dev = malloc(sizeof(ddf_dev_t));
+        dev = calloc(1, sizeof(ddf_dev_t));
         if (dev == NULL)
                 return NULL;
 
-        memset(dev, 0, sizeof(ddf_dev_t));
         return dev;
 }
@@ -610 +645 @@
 }
 
-/** Delete device structure.
+/** Delete function structure.
  *
  * @param dev           The device structure.
@@ -622 +657 @@
 }
 
+/** Increase device reference count. */
+static void dev_add_ref(ddf_dev_t *dev)
+{
+        atomic_inc(&dev->refcnt);
+}
+
+/** Decrease device reference count.
+ *
+ * Free the device structure if the reference count drops to zero.
+ */
+static void dev_del_ref(ddf_dev_t *dev)
+{
+        if (atomic_predec(&dev->refcnt) == 0)
+                delete_device(dev);
+}
+
+/** Increase function reference count. */
+static void fun_add_ref(ddf_fun_t *fun)
+{
+        atomic_inc(&fun->refcnt);
+}
+
+/** Decrease function reference count.
+ *
+ * Free the function structure if the reference count drops to zero.
+ */
+static void fun_del_ref(ddf_fun_t *fun)
+{
+        if (atomic_predec(&fun->refcnt) == 0)
+                delete_function(fun);
+}
+
 /** Create a DDF function node.
  *
@@ -652 +719 @@
         if (fun == NULL)
                 return NULL;
+
+        /* Add one reference that will be dropped by ddf_fun_destroy() */
+        fun_add_ref(fun);
 
         fun->bound = false;
@@ -676 +746 @@
 {
         assert(fun->bound == false);
-        delete_function(fun);
+
+        /*
+         * Drop the reference added by ddf_fun_create(). This will deallocate
+         * the function as soon as all other references are dropped (i.e.
+         * as soon control leaves all driver entry points called in context
+         * of this function.
+         */
+        fun_del_ref(fun);
 }