On 07/30/2015 06:12 PM, Thomas Gleixner wrote: > On Thu, 30 Jul 2015, Peter Hurley wrote: >> Honestly, I'm not too sure this is the way to go. >> >> Messing around with irqsoff tracer for 30 mins turned up: >> 3.664ms in intel_unmap_page >> - iotlb flush, spinlock contention on iova_rbtree_lock >> 1.726ms in intel_map_page() >> - iommu core @ __alloc_and_insert_iova_range() >> 1.859ms in syslog_print_all() >> - which is holding the logbuf_lock so that's pretty bad anyway >> 387us in nouveau @ nv50_vm_flush() >> - gpu tlb flush >> >> I have irqsoff trace reports for all of these if anyone is interested. >> >> Looks like lockdep would need to be off as well because I saw but >> failed to capture a save_trace() in the 300us range. >> >> I think this is just the tip of the iceberg for irqsoff. > > I agree. > >> I'm not saying these don't need fixing as well, but there's no way >> irq probe will ever be reliable with this approach. > > irq probe is a known to be unreliable heuristic endavour anyway and it > cannot ever become truly reliable, except you put a gazillion of > restrictions to the system state on it. Yep, totally agree. As you note below, this functionality is completely disabled on every known distro kernel. >> Going back to the RFC idea of pinning the irq affinity to the cpu >> actually doing the probing (which is in a known context), what about >> that is broken on UP? Just the implementation or is it the fundamental >> concept? > > First of all, there is no guarantee that you can affine these > interrupts at all. We have interrupt controllers which cannot set the > affinity and they deliver to cpus in a round robin scheme or whatever > hardware designers thought would be clever. > > Second, what prevents the following scenario on UP or the affine core: > > probe_start() > interrupt > looong running handler (usb is an obvious candidate) > printk() > > That will swallow your uart state and ruin detection as well. Yeah, ok, so fundamentally broken concept to pin the irqs for probing. Thanks for clarifying that. > So for the problem at hand, we really need to prevent that something > is fiddling with the uart in the first place and the most obvious way > is to serialize against printk. I'm ok with just the original patch 1 (which I reviewed some time ago). > We can debate whether the autoprobe code is the right place or not, we > can actually stick it into the 8250 driver and be done with it > because: > > If you look at the actual autoprobe users aside of 8250. That's really > all ancient ISA hardware and hardly interesting. So all we really care > about are the 8250 serial ports. > > Now lets look at the 8250 serial ports. I just checked the random > collection of machines I have access to: > > In 100% of all cases ttyS0 is on irq4 and ttyS1 is on irq3 > > All of the machines have even a correct PNP entry of the irq resource > for the serial ports. And there is pretty old crap in that lot. > > So the real question is: Why would we autoprobe in the first place? > > Debian, Fedora, OpenSuse, SLES have: > > # CONFIG_SERIAL_8250_DETECT_IRQ is not set > > and so do my kernels. > > I just built one with that option enabled for the fun of it and it > still uses the PNP information. No autoprobing. > > So why are you interested in that serial irq autoprobe crap at all? Agree, but I guess the hardware in question is non-PNP serial-over-LAN. It's Taichi's hardware so he can be more specific. Also, this problem would not apply to 8250 ports @ the ISA addresses (3f8,irq 4 & 2f8,irq 3) because those are predefined on the platform. Taichi's original proposal was to add module parameters for the serial driver, which I am dead set against, having just struggled to deal with ancient module parameters while splitting the 8250 driver. I also noted in reviewing that proposal that user-space tools (setserial) can reset the irq to a known value after driver load. Ubuntu, for one, runs setserial as part of boot (to restore serial hardware to known-working configuration). Regards, Peter Hurley -- To unsubscribe from this list: send the line "unsubscribe linux-serial" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
