Ignore:
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:
1 edited

Legend:

Unmodified
Added
Removed
  • uspace/drv/bus/usb/uhci/transfer_list.c

    ra6afb4c r4db49344  
    102102}
    103103
    104 /** Add transfer batch to the list and queue.
    105  *
    106  * @param[in] instance List to use.
    107  * @param[in] batch Transfer batch to submit.
     104/**
     105 * Add transfer batch to the list and queue.
    108106 *
    109107 * The batch is added to the end of the list and queue.
    110  */
    111 void transfer_list_add_batch(
     108 *
     109 * @param[in] instance List to use.
     110 * @param[in] batch Transfer batch to submit. After return, the batch must
     111 *                  not be used further.
     112 */
     113int transfer_list_add_batch(
    112114    transfer_list_t *instance, uhci_transfer_batch_t *uhci_batch)
    113115{
     
    117119        endpoint_t *ep = uhci_batch->base.ep;
    118120
    119         /* First, wait until the endpoint is free to use */
    120         fibril_mutex_lock(&ep->guard);
    121         endpoint_activate_locked(ep, &uhci_batch->base);
    122         fibril_mutex_unlock(&ep->guard);
     121        fibril_mutex_lock(&instance->guard);
     122
     123        const int err = endpoint_activate_locked(ep, &uhci_batch->base);
     124        if (err) {
     125                fibril_mutex_unlock(&instance->guard);
     126                return err;
     127        }
    123128
    124129        usb_log_debug2("Batch %p adding to queue %s.",
    125130            uhci_batch, instance->name);
    126 
    127         fibril_mutex_lock(&instance->guard);
    128131
    129132        /* Assume there is nothing scheduled */
     
    155158            USB_TRANSFER_BATCH_ARGS(uhci_batch->base), instance->name);
    156159        fibril_mutex_unlock(&instance->guard);
     160        return EOK;
    157161}
    158162
     
    171175 * @param[in] done list to fill
    172176 */
    173 void transfer_list_remove_finished(transfer_list_t *instance, list_t *done)
    174 {
    175         assert(instance);
    176         assert(done);
     177void transfer_list_check_finished(transfer_list_t *instance)
     178{
     179        assert(instance);
    177180
    178181        fibril_mutex_lock(&instance->guard);
    179         link_t *current = list_first(&instance->batch_list);
    180         while (current && current != &instance->batch_list.head) {
    181                 link_t * const next = current->next;
    182                 uhci_transfer_batch_t *batch =
    183                     uhci_transfer_batch_from_link(current);
     182        list_foreach_safe(instance->batch_list, current, next) {
     183                uhci_transfer_batch_t *batch = uhci_transfer_batch_from_link(current);
    184184
    185185                if (uhci_transfer_batch_check_completed(batch)) {
    186                         /* Remove from schedule, save for processing */
    187                         fibril_mutex_lock(&batch->base.ep->guard);
    188186                        assert(batch->base.ep->active_batch == &batch->base);
     187                        endpoint_deactivate_locked(batch->base.ep);
    189188                        hc_reset_toggles(&batch->base, &uhci_reset_toggle);
    190                         endpoint_deactivate_locked(batch->base.ep);
    191189                        transfer_list_remove_batch(instance, batch);
    192                         fibril_mutex_unlock(&batch->base.ep->guard);
    193 
    194                         list_append(current, done);
     190                        usb_transfer_batch_finish(&batch->base);
    195191                }
    196                 current = next;
    197192        }
    198193        fibril_mutex_unlock(&instance->guard);
Note: See TracChangeset for help on using the changeset viewer.