Re: [PATCH v4 1/4] usb: dbc: early driver for xhci debug capability

[Peter Zijlstra]

On Fri, Nov 11, 2016 at 12:33:29PM +0800, Lu Baolu wrote:

> Things become complicated when it comes to USB debug port.
> But it's still addressable.
> 
> At this time, we can do it like this.
> 
> write()
> {
> 	if (in_nmi() && raw_spin_is_locked(&lock))
> 		return;
> 
> 	raw_spinlock_irqsave(&lock, flags);
> 	....
> 

Please use raw_spin_trlock_irqsave() instead, spin_is_locked() is fairly
icky.

Also, there's a bunch of exception contexts that do not set in_nmi().
That is in_nmi() is really only set for #NM. #MC and #DB and
others do not set this.

> This will filter some messages from NMI handler in case that
> another thread is holding the spinlock. I have no idea about
> how much chance could a debug user faces this. But it might
> further be fixed with below enhancement.
> 
> write()
> {
> 	if (in_nmi() && raw_spin_is_locked(&lock)) {
> 		produce_a_pending_item_in_ring();
> 		return;
> 	}
> 
> 	raw_spinlock_irqsave(&lock, flags);
> 
> 	while (!pending_item_ring_is_empty)
> 		consume_a_pending_item_in_ring();
> 
> 	....
> 
> 
> We can design the pending item ring in a producer-consumer
> model. It's easy to avoid race between the producer and
> consumer.

Problem is that the consumer might never happen, those are the fun most
bugs.

Not being able to deal with random nested exception context really
reduces the utility of this thing.

Again, a UART rules. Make a virtual UART in hardware, that'd be totally
awesome. This thing, I'm not convinced its worth having.