Re: Memory barrier needed with wake_up_process()?

2017-01-16 Thread Felipe Balbi
Hi, Felipe Balbi writes: > Alan Stern writes: >> On Mon, 16 Jan 2017, Felipe Balbi wrote: >> >>> Sorry for the long delay, I finally have more information on this. All >>> this time I was doing something that I never considered to matter: I've >>> been running host and peripheral on the same ma

Re: Memory barrier needed with wake_up_process()?

2017-01-16 Thread Felipe Balbi
Hi, Alan Stern writes: > On Mon, 16 Jan 2017, Felipe Balbi wrote: > >> Sorry for the long delay, I finally have more information on this. All >> this time I was doing something that I never considered to matter: I've >> been running host and peripheral on the same machine. Now that I have >> tra

Re: Memory barrier needed with wake_up_process()?

2017-01-16 Thread Alan Stern
On Mon, 16 Jan 2017, Felipe Balbi wrote: > Sorry for the long delay, I finally have more information on this. All > this time I was doing something that I never considered to matter: I've > been running host and peripheral on the same machine. Now that I have > tracepoints on xHCI as well, I could

Re: Memory barrier needed with wake_up_process()?

2017-01-16 Thread Felipe Balbi
Hi, Alan Stern writes: > On Tue, 20 Sep 2016, Felipe Balbi wrote: > >> And here's trace output (complete, scroll to bottom). It seems to me >> like the thread didn't wake up at all. It didn't even try to >> execute. I'll add some more traces and try to get better information >> about what's goin

Re: Memory barrier needed with wake_up_process()?

2016-09-20 Thread Alan Stern
On Tue, 20 Sep 2016, Felipe Balbi wrote: > And here's trace output (complete, scroll to bottom). It seems to me > like the thread didn't wake up at all. It didn't even try to > execute. I'll add some more traces and try to get better information > about what's going on. > > > > # tracer: nop >

Re: Memory barrier needed with wake_up_process()?

2016-09-20 Thread Felipe Balbi
Hi, Felipe Balbi writes: > [ Unknown signature status ] > > Hi, > > Alan Stern writes: > > [...] > >>> $ grep -RnH "raise_exception\|fsg_main_thread" /tmp/trace.txt >>> /tmp/trace.txt:111743: irq/17-dwc3-3527 [003] d..164.833078: >>> raise_exception: raise_exception 4 >>> /tmp/trace.

Re: Memory barrier needed with wake_up_process()?

2016-09-19 Thread Alan Stern
On Mon, 19 Sep 2016, Felipe Balbi wrote: > >> file-storage-3578 [002] 21167.727072: fsg_main_thread: next: bh > >> 880111e69a80 state 1 > >> file-storage-3578 [002] 21167.729458: fsg_main_thread: next: bh > >> 880111e6aac0 state 2 > >> irq/17-dwc3-3579 [003] d..

Re: Memory barrier needed with wake_up_process()?

2016-09-19 Thread Felipe Balbi
Hi Alan, Alan Stern writes: > On Fri, 9 Sep 2016, Felipe Balbi wrote: > >> Finally :-) Here's the diff I used: >> >> diff --git a/drivers/usb/gadget/function/f_mass_storage.c >> b/drivers/usb/gadget/function/f_mass_storage.c >> index 8f3659b65f53..0716024f6b65 100644 >> --- a/drivers/usb/gadge

Re: Memory barrier needed with wake_up_process()?

2016-09-09 Thread Alan Stern
On Fri, 9 Sep 2016, Felipe Balbi wrote: > Finally :-) Here's the diff I used: > > diff --git a/drivers/usb/gadget/function/f_mass_storage.c > b/drivers/usb/gadget/function/f_mass_storage.c > index 8f3659b65f53..0716024f6b65 100644 > --- a/drivers/usb/gadget/function/f_mass_storage.c > +++ b/driv

Re: Memory barrier needed with wake_up_process()?

2016-09-09 Thread Felipe Balbi
Hi, Felipe Balbi writes: > Alan Stern writes: >> On Tue, 6 Sep 2016, Peter Zijlstra wrote: >> >>> On Tue, Sep 06, 2016 at 01:49:37PM +0200, Peter Zijlstra wrote: >>> > On Tue, Sep 06, 2016 at 02:43:39PM +0300, Felipe Balbi wrote: >>> >>> > > My fear now, however, is that changing smp_[rw]mb()

Re: Memory barrier needed with wake_up_process()?

2016-09-07 Thread Felipe Balbi
Hi, Alan Stern writes: > On Tue, 6 Sep 2016, Peter Zijlstra wrote: > >> On Tue, Sep 06, 2016 at 01:49:37PM +0200, Peter Zijlstra wrote: >> > On Tue, Sep 06, 2016 at 02:43:39PM +0300, Felipe Balbi wrote: >> >> > > My fear now, however, is that changing smp_[rw]mb() to smp_mb() just >> > > adds e

Re: Memory barrier needed with wake_up_process()?

2016-09-06 Thread Peter Zijlstra
On Tue, Sep 06, 2016 at 10:46:55AM -0400, Alan Stern wrote: Not knowing where INFO() goes, you should use trace_printk() not printk(), as the former is strictly per cpu, while the latter is globally serialized and can hide all these problems. > Index: usb-4.x/drivers/usb/gadget/function/f_mass_st

Re: Memory barrier needed with wake_up_process()?

2016-09-06 Thread Alan Stern
On Tue, 6 Sep 2016, Peter Zijlstra wrote: > On Tue, Sep 06, 2016 at 01:49:37PM +0200, Peter Zijlstra wrote: > > On Tue, Sep 06, 2016 at 02:43:39PM +0300, Felipe Balbi wrote: > > > > My fear now, however, is that changing smp_[rw]mb() to smp_mb() just > > > adds extra overhead which makes the prob

Re: Memory barrier needed with wake_up_process()?

2016-09-06 Thread Peter Zijlstra
On Tue, Sep 06, 2016 at 01:49:37PM +0200, Peter Zijlstra wrote: > On Tue, Sep 06, 2016 at 02:43:39PM +0300, Felipe Balbi wrote: > > My fear now, however, is that changing smp_[rw]mb() to smp_mb() just > > adds extra overhead which makes the problem much, much less likely to > > happen. Does that s

Re: Memory barrier needed with wake_up_process()?

2016-09-06 Thread Peter Zijlstra
On Tue, Sep 06, 2016 at 02:43:39PM +0300, Felipe Balbi wrote: > > Could you confirm that bulk_{in,out}_complete() work on different > > usb_request structures, and they can not, at any time, get called on the > > _same_ request? > > usb_requests are allocated for a specific endpoint and USB Devic

Re: Memory barrier needed with wake_up_process()?

2016-09-06 Thread Felipe Balbi
Hi, Peter Zijlstra writes: > On Mon, Sep 05, 2016 at 11:29:26AM -0400, Alan Stern wrote: >> On Mon, 5 Sep 2016, Peter Zijlstra wrote: >> >> > > Actually, seeing it written out like this, one realizes that it really >> > > ought to be: >> > > >> > > CPU 0 CPU 1 >> >

Re: Memory barrier needed with wake_up_process()?

2016-09-06 Thread Peter Zijlstra
On Mon, Sep 05, 2016 at 11:29:26AM -0400, Alan Stern wrote: > On Mon, 5 Sep 2016, Peter Zijlstra wrote: > > > > Actually, seeing it written out like this, one realizes that it really > > > ought to be: > > > > > > CPU 0 CPU 1 > > > -

Re: Memory barrier needed with wake_up_process()?

2016-09-06 Thread Peter Zijlstra
On Mon, Sep 05, 2016 at 10:43:11AM +0100, Will Deacon wrote: > On Sat, Sep 03, 2016 at 12:16:29AM +0200, Peter Zijlstra wrote: > > Forgot to Cc Will. Will, does ARM64 need to make smp_mb__before_spinlock > > smp_mb() too? > > Yes, probably. Just to confirm, the test is something like: > > > CPU

Re: Memory barrier needed with wake_up_process()?

2016-09-05 Thread Alan Stern
On Mon, 5 Sep 2016, Peter Zijlstra wrote: > > Actually, seeing it written out like this, one realizes that it really > > ought to be: > > > > CPU 0 CPU 1 > > - - > > Start DMA Handle DMA-complet

Re: Memory barrier needed with wake_up_process()?

2016-09-05 Thread Alan Stern
On Mon, 5 Sep 2016, Peter Zijlstra wrote: > > You know, I never went through and verified that _all_ the invocations > > of sleep_thread() are like that. > > Well, thing is, they're all inside a loop which checks other conditions > for forward progress. Therefore the loop inside sleep_thread()

Re: Memory barrier needed with wake_up_process()?

2016-09-05 Thread Will Deacon
On Sat, Sep 03, 2016 at 12:16:29AM +0200, Peter Zijlstra wrote: > On Sat, Sep 03, 2016 at 12:14:13AM +0200, Peter Zijlstra wrote: > > On Fri, Sep 02, 2016 at 04:16:54PM -0400, Alan Stern wrote: > > > > > > Actually, that's not entirely true (although presumably it works okay > > > for most archite

Re: Memory barrier needed with wake_up_process()?

2016-09-05 Thread Peter Zijlstra
On Sat, Sep 03, 2016 at 10:49:39AM -0400, Alan Stern wrote: > On Sat, 3 Sep 2016, Alan Stern wrote: > > > In other words, we have: > > > > CPU 0 CPU 1 > > - - > > Start DMA Handle DMA-complete irq > >

Re: Memory barrier needed with wake_up_process()?

2016-09-05 Thread Peter Zijlstra
On Sat, Sep 03, 2016 at 04:51:07PM +0300, Felipe Balbi wrote: > > That said, I cannot spot an obvious fail, > > okay, but a fail does exist. Any hints on what extra information I could > capture to help figuring this one out? More information on which sleep is not waking woudl help I suppose. Tha

Re: Memory barrier needed with wake_up_process()?

2016-09-05 Thread Peter Zijlstra
On Sat, Sep 03, 2016 at 10:16:31AM -0400, Alan Stern wrote: > > Sorry, but that is horrible code. A barrier cannot ensure writes are > > 'complete', at best they can ensure order between writes (or reads > > etc..). > > The code is better than the comment. What I really meant was that the > wri

Re: Memory barrier needed with wake_up_process()?

2016-09-03 Thread Alan Stern
On Sat, 3 Sep 2016, Alan Stern wrote: > In other words, we have: > > CPU 0 CPU 1 > - - > Start DMA Handle DMA-complete irq > Sleep until bh->state Set bh->state >

Re: Memory barrier needed with wake_up_process()?

2016-09-03 Thread Alan Stern
On Sat, 3 Sep 2016, Peter Zijlstra wrote: > On Fri, Sep 02, 2016 at 04:29:19PM -0400, Alan Stern wrote: > > I'm afraid so. The code doesn't use wait_event(), in part because > > there's no wait_queue (since only one task is involved). > > You can use wait_queue fine with just one task, and it wo

Re: Memory barrier needed with wake_up_process()?

2016-09-03 Thread Alan Stern
On Sat, 3 Sep 2016, Peter Zijlstra wrote: > 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 (e

Re: Memory barrier needed with wake_up_process()?

2016-09-03 Thread Felipe Balbi
Hi, Peter Zijlstra writes: > 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

Re: Memory barrier needed with wake_up_process()?

2016-09-03 Thread Peter Zijlstra
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

Re: Memory barrier needed with wake_up_process()?

2016-09-03 Thread Peter Zijlstra
On Fri, Sep 02, 2016 at 04:29:19PM -0400, Alan Stern wrote: > I'm afraid so. The code doesn't use wait_event(), in part because > there's no wait_queue (since only one task is involved). You can use wait_queue fine with just one task, and it would clean up the code tremendously. You can replace

Re: Memory barrier needed with wake_up_process()?

2016-09-03 Thread Paul E. McKenney
On Fri, Sep 02, 2016 at 04:29:19PM -0400, Alan Stern wrote: > On Fri, 2 Sep 2016, Paul E. McKenney wrote: > > > On Fri, Sep 02, 2016 at 02:10:13PM -0400, Alan Stern wrote: > > > Paul, Peter, and Ingo: > > > > > > This must have come up before, but I don't know what was decided. > > > > > > Isn't

Re: Memory barrier needed with wake_up_process()?

2016-09-02 Thread Felipe Balbi
hi, Peter Zijlstra writes: > On Fri, Sep 02, 2016 at 04:16:54PM -0400, Alan Stern wrote: >> Felipe, your tests will show whether my guess was totally off-base. >> For the new people, Felipe is tracking down a problem that involves >> exactly the code sequence listed at the top of the email, wh

Re: Memory barrier needed with wake_up_process()?

2016-09-02 Thread Peter Zijlstra
On Fri, Sep 02, 2016 at 04:16:54PM -0400, Alan Stern wrote: > Felipe, your tests will show whether my guess was totally off-base. > For the new people, Felipe is tracking down a problem that involves > exactly the code sequence listed at the top of the email, where we know > that the wakeup routi

Re: Memory barrier needed with wake_up_process()?

2016-09-02 Thread Peter Zijlstra
On Sat, Sep 03, 2016 at 12:14:13AM +0200, Peter Zijlstra wrote: > On Fri, Sep 02, 2016 at 04:16:54PM -0400, Alan Stern wrote: > > > > Actually, that's not entirely true (although presumably it works okay > > for most architectures). > > Yeah, all load-store archs (with exception of PowerPC and AR

Re: Memory barrier needed with wake_up_process()?

2016-09-02 Thread Peter Zijlstra
On Fri, Sep 02, 2016 at 04:16:54PM -0400, Alan Stern wrote: > > Actually, that's not entirely true (although presumably it works okay > for most architectures). Yeah, all load-store archs (with exception of PowerPC and ARM64 and possibly MIPS) implement ACQUIRE with a general fence (after the ll/

Re: Memory barrier needed with wake_up_process()?

2016-09-02 Thread Alan Stern
On Fri, 2 Sep 2016, Paul E. McKenney wrote: > On Fri, Sep 02, 2016 at 02:10:13PM -0400, Alan Stern wrote: > > Paul, Peter, and Ingo: > > > > This must have come up before, but I don't know what was decided. > > > > Isn't it often true that a memory barrier is needed before a call to > > wake_up

Re: Memory barrier needed with wake_up_process()?

2016-09-02 Thread Alan Stern
On Fri, 2 Sep 2016, Peter Zijlstra wrote: > On Fri, Sep 02, 2016 at 02:10:13PM -0400, Alan Stern wrote: > > Paul, Peter, and Ingo: > > > > This must have come up before, but I don't know what was decided. > > > > Isn't it often true that a memory barrier is needed before a call to > > wake_up_p

Re: Memory barrier needed with wake_up_process()?

2016-09-02 Thread Peter Zijlstra
On Fri, Sep 02, 2016 at 02:10:13PM -0400, Alan Stern wrote: > Paul, Peter, and Ingo: > > This must have come up before, but I don't know what was decided. > > Isn't it often true that a memory barrier is needed before a call to > wake_up_process()? A typical scenario might look like this: > >

Re: Memory barrier needed with wake_up_process()?

2016-09-02 Thread Paul E. McKenney
On Fri, Sep 02, 2016 at 02:10:13PM -0400, Alan Stern wrote: > Paul, Peter, and Ingo: > > This must have come up before, but I don't know what was decided. > > Isn't it often true that a memory barrier is needed before a call to > wake_up_process()? A typical scenario might look like this: > >

Memory barrier needed with wake_up_process()?

2016-09-02 Thread Alan Stern
Paul, Peter, and Ingo: This must have come up before, but I don't know what was decided. Isn't it often true that a memory barrier is needed before a call to wake_up_process()? A typical scenario might look like this: CPU 0 - for (;;) { set_current_s