On September 21, 2015 9:27:43 AM PDT, Alan Stern <stern@xxxxxxxxxxxxxxxxxxx> wrote: >On Mon, 21 Sep 2015, Felipe Balbi wrote: > >> On Mon, Sep 21, 2015 at 11:37:47AM -0400, Alan Stern wrote: >> > On Mon, 21 Sep 2015, Felipe Balbi wrote: >> > >> > > On Mon, Sep 21, 2015 at 10:50:10AM -0400, Alan Stern wrote: >> > > > On Mon, 21 Sep 2015, Felipe Balbi wrote: >> > > > >> > > > > On Mon, Sep 21, 2015 at 10:31:15AM -0400, Alan Stern wrote: >> > > > > > On Mon, 21 Sep 2015, Roger Quadros wrote: >> > > > > > >> > > > > > > Using spin_lock() in hard irq handler is pointless >> > > > > > > and causes a BUG() in RT (real-time) configuration >> > > > > > > so get rid of it. >> > > > > > >> > > > > > Wait a minute. Who says spin_lock is pointless in an IRQ >handler? >> > > > > >> > > > > in the top half IRQs are already disabled, how can this race >? >> > > > >> > > > It can race with code running on a different CPU. >> > > >> > > fair point. >> > >> > In fact, isn't this the main purpose of spinlocks? There's no >point >> > using a spinlock to protect against races occurring on a single >CPU, >> > because (in non-RT situations) the kernel can't schedule or preempt >> > while a spinlock is held, so no race can occur. The whole idea of >> > spinlocks is to protect against cross-CPU races. >> > >> > Now, maybe the spinlock usage that Roger is removing really is >> > unnecessary in this top-half handler. I don't know; I haven't >looked >> > at the code. But in general, spinlocks are highly necessary in IRQ >> > handlers. >> >> this does raise some questions of what to do then ? We can't use >> spin_locks when in RT because they're reimplemented as mutexes >> and can sleep. Converting every single driver to raw_spin_locks >> defeats the purpose of having everything and their dogs preemptable. >> >> Difficult to say. Let's defer this patch until we can come up >> with a good answer to this. > >Someone more familiar with the -RT patches should be able to answer >these questions. Spin lock (without irqsave) may have a bug if the irq handler spin locks the same one, and the irqs aren't already disabled. E.g. any context (atomic or not) spin locks, then the irq handler takes place on *same* cpu and tries to spin lock again. It will be a deadlock. I believe that's what Roger misinterpreted with his patch description. Br, David Cohen > >Steve, any advice? > >Alan Stern > >-- >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 Hi, -- Sent from my Android device with K-9 Mail. Please excuse my brevity. -- 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
