On Sat, Oct 24, 2009 at 09:54:51AM +0200, Oliver Neukum wrote:
> Am Samstag, 24. Oktober 2009 02:53:00 schrieb Sarah Sharp:
> > @@ -507,8 +507,11 @@ static void handle_stopped_endpoint(struct xhci_hcd
> > *xhci, ep = &xhci->devs[slot_id]->eps[ep_index];
> > ep_ring = ep->ring;
> >
> > - if (list_empty(&ep->cancelled_td_list))
> > + if (list_empty(&ep->cancelled_td_list)) {
> > + ep->ep_state &= ~EP_HALT_PENDING;
> > + ring_ep_doorbell(xhci, slot_id, ep_index);
> > return;
> > + }
>
> Presumably the interrupt handler cares about ep->ep_state.
> Yet it may run on another CPU. Nothing makes sure ep->ep_state
> is flushed into main memory. I think you need an smp_wmb()
> before ring_ep_doorbell(), as it doesn't contain a memory ordering
> primitive.
I'm not quite sure I understand your concern. This function is part of
the interrupt handler. I've set up the xHCI hardware such that there
will only be one interrupt handler across all CPUs. The hardware won't
interrupt again while I'm in the middle of the interrupt handler because
of the "event handler busy" bit.
Further, the interrupt handler takes the xHCI spinlock, which does
contain a memory ordering primitive. So if the interrupt handler
changes ep_state and drops the lock, the other CPUs should see that
memory write.
Sarah
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
