On Sat, Sep 03, 2016 at 09:58:09AM +0300, Felipe Balbi wrote: > > What arch are you seeing this on? > > x86. Skylake to be exact.
So it _cannot_ be the thing Alan mentioned. By the simple fact that spin_lock() is a full barrier on x86 (every LOCK prefixed instruction is). > The following change survived through the night: > > diff --git a/drivers/usb/gadget/function/f_mass_storage.c > b/drivers/usb/gadget/function/f_mass_storage.c > index 8f3659b65f53..d31581dd5ce5 100644 > --- a/drivers/usb/gadget/function/f_mass_storage.c > +++ b/drivers/usb/gadget/function/f_mass_storage.c > @@ -395,7 +395,7 @@ static int fsg_set_halt(struct fsg_dev *fsg, struct > usb_ep *ep) > /* Caller must hold fsg->lock */ > static void wakeup_thread(struct fsg_common *common) > { > - smp_wmb(); /* ensure the write of bh->state is complete */ > + smp_mb(); /* ensure the write of bh->state is complete */ > /* Tell the main thread that something has happened */ > common->thread_wakeup_needed = 1; > if (common->thread_task) > @@ -626,7 +626,7 @@ static int sleep_thread(struct fsg_common *common, bool > can_freeze) > } > __set_current_state(TASK_RUNNING); > common->thread_wakeup_needed = 0; > - smp_rmb(); /* ensure the latest bh->state is visible */ > + smp_mb(); /* ensure the latest bh->state is visible */ > return rc; > } Sorry, but that is horrible code. A barrier cannot ensure writes are 'complete', at best they can ensure order between writes (or reads etc..). Also, looking at that thing, that common->thread_wakeup_needed variable is 100% redundant. All sleep_thread() invocations are inside a loop of sorts and basically wait for other conditions to become true. For example: while (bh->state != BUF_STATE_EMPTY) { rc = sleep_thread(common, false); if (rc) return rc; } All you care about there is bh->state, _not_ common->thread_wakeup_needed. That said, I cannot spot an obvious fail, but the code can certainly use help. -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html