From: Thomas Gleixner on 26 May 2010 10:10 Chris, On Tue, 25 May 2010, Chris Metcalf wrote: > Thomas, thanks for your feedback. If I don't comment on something you > said it's because you were obviously right and I applied a suitable fix. :-) > > On 5/25/2010 4:12 PM, Thomas Gleixner wrote: > > +/** > > + * tile_request_irq() - Allocate an interrupt handling instance. > > [...] > > > > Why are you implementing your private interrupt handling > > infrastructure ? What's wrong with the generic interrupt handling > > code ? Why is each device driver forced to call tile_request_irq() > > which makes it incompatible to the rest of the kernel and therefor > > unshareable ? > > > > Our interrupt management code has evolved as we have developed this > code, so I won't present arguments as to why it's perfect the way it is, > but just why it IS the way it is. :-) > > The tile irq.c does not replace the generic Linux IRQ management code, > but instead provides a very limited set of virtual interrupts that are > only used by our para-virtualized device drivers, and delivered to Linux > via a single hypervisor downcall that atomically sets "virtual > interrupt" bits in a bitmask. The PCI root complex driver reserves four > of these bits (i.e. irqs) to map real PCI interrupts; they are then fed > forward into the regular Linux IRQ system to manage all "standard" > devices. The other tile-specific para-virtualized drivers that use this > interface are the PCI endpoint code, xgbe network driver, ATA-over-GPIO > driver, and the IPI layer. None of these para-virtualized drivers are > actually shareable with other Linux architectures in any case. Ok, still ... > We have an outstanding enhancement request in our bug tracking system to > switch to using the Linux generic IRQs directly, and plan to implement > it prior to our next major release. But we haven't done it yet, and > this code, though somewhat crufty, is reasonably stable. I'm also not > the primary maintainer of this particular piece of code, so I'd rather > wait until that person frees up and have him do it, instead of trying to > hack it myself. It should be rather straight forward and can be efficiently done with the handle_simple_irq() handler AFAICS. So I'd prefer to see that fixed befor the code gets merged. > > You check desc->handler, but you happily call the handler while > > dev_id might be still NULL. See above. > > > > Assuming we spinlock the irq request/free routines, I think this is > safe, since the unlock memory fence will guarantee visibility of the > fields prior to any attempt to use them. We always allocate the > interrupt, then tell the hypervisor to start delivering them; on device > unload we tell the hypervisor to stop delivering interrupts, then free > it. The "tell the hypervisor" steps use on_each_cpu() and wait, so are > fully synchronous. Going to generic irq will solve all of that :) > > +int show_interrupts(struct seq_file *p, void *v) > > +[...] > > > > So that prints which interrupts ? Now you refer to the generic code, > > while above you require that tile specific one. -ENOSENSE. > > > > Yes, this is confusing. This routine is required by procfs, and it > shows just the PCI interrupts, not the tile irqs. I'll add a comment, > and try to segregate the file into "generic irqs" and "tile irqs" more > obviously, for now. The routine itself will be more sensible once we > integrate our tile_irqs into the generic system. Ditto. Thanks, tglx -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Chris Metcalf on 28 May 2010 13:50 On 5/26/2010 10:05 AM, Thomas Gleixner wrote: >> We have an outstanding enhancement request in our bug tracking system to >> switch to using the Linux generic IRQs directly, and plan to implement >> it prior to our next major release. But we haven't done it yet, and >> this code, though somewhat crufty, is reasonably stable. I'm also not >> the primary maintainer of this particular piece of code, so I'd rather >> wait until that person frees up and have him do it, instead of trying to >> hack it myself. >> > It should be rather straight forward and can be efficiently done > with the handle_simple_irq() handler AFAICS. So I'd prefer to see > that fixed befor the code gets merged. > OK, we have made changes to integrate to the Linux PERCPU IRQ mechanism, which most closely matches what our interrupt layer did. You should see it a bit later when I post the revised diffs. Thanks! -- Chris Metcalf, Tilera Corp. http://www.tilera.com -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
|
Pages: 1 Prev: transitional config CONFIG_GENERIC_CLOCKEVENTS Next: yahoo.com.tw |