On Mon, 15 Feb 2021, Andy Shevchenko wrote: > On Sun, Feb 14, 2021 at 7:12 AM Finn Thain <fth...@telegraphics.com.au> wrote: > > On Sat, 13 Feb 2021, Song Bao Hua (Barry Song) wrote: > > > So what is really confusing and a pain to me is that: > > > For years people like me have been writing device drivers > > > with the idea that irq handlers run with interrupts > > > disabled after those commits in genirq. So I don't need > > > to care about if some other IRQs on the same cpu will > > > jump out to access the data the current IRQ handler > > > is accessing. > > > > > > but it turns out the assumption is not true on some platform. > > > So should I start to program devices driver with the new idea > > > interrupts can actually come while irqhandler is running? > > > > > > That's the question which really bothers me. > > > > > > > That scenario seems a little contrived to me (drivers for two or more > > devices sharing state through their interrupt handlers). Is it real? > > I suppose every platform has its quirks. The irq lock in sonic_interrupt() > > is only there because of a platform quirk (the same device can trigger > > either of two IRQs). Anyway, no-one expects all drivers to work on all > > platforms; I don't know why it bothers you so much when platforms differ. > > Isn't it any IRQ chip driver which may get IRQ on one or more lines > simultaneously? > The question here is can the handler be re-entrant on the same CPU in > that case? >
An irq_chip handler used only for interrupts having the same priority level cannot be re-entered, thanks to the priority mask. Where the priority levels are different, as in auto_irq_chip in arch/m68k/kernel/ints.c, handle_simple_irq() can be re-entered but not with the same descriptor (i.e. no shared state). Documentation/core-api/genericirq.rst says, The locking of chip registers is up to the architecture that defines the chip primitives. The per-irq structure is protected via desc->lock, by the generic layer. Since the synchronization of chip registers is left up to the architecture, I think the related code should be portable. Looking in kernel/irq/*.c, I see that desc->lock is sometimes acquired with raw_spin_lock_irq(&desc->lock) and sometimes raw_spin_lock(&desc->lock). I don't know whether there are portability issues lurking here; this code is subtle and largely unfamiliar to me.
