Am Montag, 26. Oktober 2009 18:36:00 schrieb Sarah Sharp:
> 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.
Sorry, I didn't notice you are calling this with a spinlock held.
In this case it is safe.
Sorry
Oliver
--
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
