Changeset 4db49344 in mainline for uspace/lib/usbhost/src/endpoint.c

Timestamp:

2018-01-23T21:52:28Z (7 years ago)

Author:

Ondřej Hlavatý <aearsis@…>

Branches:

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

Children:

3dd80f8

Parents:

a6afb4c

git-author:

Ondřej Hlavatý <aearsis@…> (2018-01-23 20:49:35)

git-committer:

Ondřej Hlavatý <aearsis@…> (2018-01-23 21:52:28)

Message:

usb: fix wrong design of transfer aborting

Apparently, we didn't do a good job in thinking through the problem.
In older HCs, it was done just wrong - the UHCI implementation commited
a batch that could have been already aborted, and EHCI+OHCI might miss
an interrupt because they commited the batch sooner than they added it
to their checked list.

This commit takes everything from the other end, which is probably the
only right one. Instead of an endpoint having an extra mutex, it
inherits a mutex from the outside. It never locks it though, it just
checks if the mutex is locked and uses it for waiting on condition
variables.

This mutex is supposed to be the one which the HC driver uses for
locking its structures in scheduling. This way, we avoid the ABBA
deadlock completely, while preserving the synchronization on an
endpoint.

The good thing is that this implementation is much easier to extend with
multiple active batches per endpoint.

File:

• uspace/lib/usbhost/src/endpoint.c (modified) (3 diffs)

uspace/lib/usbhost/src/endpoint.c

@@ -61 +61 @@
 
         atomic_set(&ep->refcnt, 0);
-        fibril_mutex_initialize(&ep->guard);
         fibril_condvar_initialize(&ep->avail);
 
@@ -122 +121 @@
 
 /**
- * Wait until the endpoint have no transfer scheduled.
+ * Mark the endpoint as online. Supply a guard to be used for this endpoint
+ * synchronization.
+ */
+void endpoint_set_online(endpoint_t *ep, fibril_mutex_t *guard)
+{
+        ep->guard = guard;
+        ep->online = true;
+}
+
+/**
+ * Mark the endpoint as offline. All other fibrils waiting to activate this
+ * endpoint will be interrupted.
+ */
+void endpoint_set_offline_locked(endpoint_t *ep)
+{
+        assert(ep);
+        assert(fibril_mutex_is_locked(ep->guard));
+
+        ep->online = false;
+        fibril_condvar_broadcast(&ep->avail);
+}
+
+/**
+ * Wait until a transfer finishes. Can be used even when the endpoint is
+ * offline (and is interrupted by the endpoint going offline).
  */
 void endpoint_wait_timeout_locked(endpoint_t *ep, suseconds_t timeout)
 {
-        assert(fibril_mutex_is_locked(&ep->guard));
-
-        if (ep->active_batch != NULL)
-                fibril_condvar_wait_timeout(&ep->avail, &ep->guard, timeout);
-
-        while (timeout == 0 && ep->active_batch != NULL)
-                fibril_condvar_wait_timeout(&ep->avail, &ep->guard, timeout);
+        assert(ep);
+        assert(fibril_mutex_is_locked(ep->guard));
+
+        if (ep->active_batch == NULL)
+                return;
+
+        fibril_condvar_wait_timeout(&ep->avail, ep->guard, timeout);
 }
 
@@ -140 +163 @@
  *
  * Call only under endpoint guard. After you activate the endpoint and release
- * the guard, you must assume that particular transfer is already finished/aborted.
- *
- * @param ep endpoint_t structure.
- * @param batch Transfer batch this endpoint is bocked by.
- */
-void endpoint_activate_locked(endpoint_t *ep, usb_transfer_batch_t *batch)
+ * the guard, you must assume that particular transfer is already
+ * finished/aborted.
+ *
+ * Activation and deactivation is not done by the library to maximize
+ * performance. The HC might want to prepare some memory buffers prior to
+ * interfering with other world.
+ *
+ * @param batch Transfer batch this endpoint is blocked by.
+ */
+int endpoint_activate_locked(endpoint_t *ep, usb_transfer_batch_t *batch)
 {
         assert(ep);
         assert(batch);
         assert(batch->ep == ep);
-
-        endpoint_wait_timeout_locked(ep, 0);
+        assert(ep->guard);
+        assert(fibril_mutex_is_locked(ep->guard));
+
+        while (ep->online && ep->active_batch != NULL)
+                fibril_condvar_wait(&ep->avail, ep->guard);
+
+        if (!ep->online)
+                return EINTR;
+
+        assert(ep->active_batch == NULL);
         ep->active_batch = batch;
+        return EOK;
 }
 
 /**
  * Mark the endpoint as inactive and allow access for further fibrils.
- *
- * @param ep endpoint_t structure.
  */
 void endpoint_deactivate_locked(endpoint_t *ep)
 {
         assert(ep);
-        assert(fibril_mutex_is_locked(&ep->guard));
+        assert(fibril_mutex_is_locked(ep->guard));
+
         ep->active_batch = NULL;
         fibril_condvar_signal(&ep->avail);