Messages in this thread | | | Date | Wed, 04 Apr 2012 17:00:19 +0600 | From | Mike Sinkovsky <> | Subject | Re: [PATCH v6 1/2] Ethernet driver for the WIZnet W5300 chip |
| |
04.04.2012 16:01, Mark Brown написал: > On Wed, Apr 04, 2012 at 12:18:12PM +0600, Mike Sinkovsky wrote: >> 03.04.2012 19:36, Mark Brown написал: > >>> ...you use devm_request_threaded_irq() here and rely on it for cleanup. >>> Are you sure there's no possibility of the interrupt firing after you >>> start to tear down the device? > >>> By using a specifically threaded IRQ you're also adding a performance >>> overhead for no good reason if you can call netdev_carrier_*() from IRQ >>> context and the GPIO is capable of generating a hard IRQ. If you use >>> request_any_context_irq() instead then the driver will get a hard IRQ if >>> that's supported. > >> There isn't devm* variant of request_any_context_irq(), and using >> plain version looks inconsistent with other resources handling. >> Anyway, this is not performance critical procedure, and latency >> around 100 millisecond is acceptable. Some our boards even don't >> have this gpio at all, and nothing bad happens, just userspace >> doesn't know is carrier on or off. > > None of this addresses the primary concern which is that because you're > not (as far as I can tell) ensuring that the interrupt won't fire the > driver might crash if the interrupt fires in between the resources it > needs to handle the interrupt being deallocated and the interrupt being > unregistered. Managed interrupts are relatively tricky to use because > of this issue, with most things like memory it doesn't matter exactly > when it's deallocated but interrupts can potentially trigger actions > themselves. > > If you can use it and devm_request_any_context_irq() doesn't exist it'd > be better to add it.
Ok, now I understand - better don't use devm* functions for interrupts, use plain request* in probe() and free_irq() in remove().
Will post v8.
-- Mike
-- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |