On Mon, Sep 21, 2015 at 03:48:53PM -0400, Alan Stern wrote:
> On Mon, 21 Sep 2015, Felipe Balbi wrote:
>
> > On Mon, Sep 21, 2015 at 03:01:42PM -0400, Steven Rostedt wrote:
> > > On Mon, 21 Sep 2015 12:04:16 -0500
> > > Felipe Balbi <balbi@xxxxxx> wrote:
> > >
> > >
> > > > no, no. you're missing the point here. The problem is that when RT
> > > > is applied, spinlocks get reimplemented as RT-aware mutexes which
> > > > works pretty well as long as you don't install your own top and bottom
> > > > halves. If you do, RT patch can't force your handler to run as a thread
> > > > and you're left with, essentially, a mutex for synchronization in
> > > > hardirq context.
> > > >
> > > > So the question is really for Steve, what should we do here ? Use a
> > > > raw_spinlock ? I'd like to avoid that if possible. If we have another
> > > > option, I'm all ears.
> > > >
> > >
> > > I may be missing something here. Yes, in RT spinlocks become rtmutexes
> > > and can sleep. But in RT all interrupts become threads. With the slight
> > > exception that if you declare your interrupt handle as a thread, then
> > > you do have a top half handler. This top half should not be grabbing
> > > any spin locks, and should simply disable any more interrupts from
> > > happening on the device until the thread handler can run.
> >
> > and that's basically we're doing. $SUBJECT is removing the spin lock for
> > that reason. Register accesses should be satomic anyway, right??
>
> I guess the patch is the right thing to do. But the description sure
> could be improved. Roger wrote: "Using spin_lock() in hard irq handler
> is pointless and causes a BUG() in RT (real-time) configuration",
> which is ungrammatical as well as misleading.
>
> It should say something more along the lines of:
>
> Top-half handlers for threaded interrupts are not supposed to
> acquire any locks. None are needed because the top half is
> guaranteed to be mutually exclusive with the bottom-half
> thread handler. Furthermore, spin_lock() in a top-half handler
> causes a BUG() in RT-enabled kernels.
>
> Remove the unnecessary spin_lock() in dwc3_interrupt().
I kept Roger's commit log and just added one extra paragraph:
Author: Roger Quadros <rogerq@xxxxxx>
Date: Mon Sep 21 11:08:36 2015 +0300
usb: dwc3: gadget: Fix BUG in RT config
Using spin_lock() in hard irq handler is pointless
and causes a BUG() in RT (real-time) configuration
so get rid of it.
The reason it's pointless is because the driver is
basically accessing register which is, anyways,
atomic.
Signed-off-by: Roger Quadros <rogerq@xxxxxx>
Signed-off-by: Felipe Balbi <balbi@xxxxxx>
Probably should have added the detail that top and bottom
halves are guaranteed to be mutually exclusive, sorry.
--
balbi
Attachment:
signature.asc
Description: PGP signature
