On Tue, Dec 10, 2024 at 3:19 PM Samuel Thibault <samuel.thiba...@gnu.org> wrote: > Sergey Bugaev, le mar. 10 déc. 2024 14:57:05 +0300, a ecrit: > > @@ -103,11 +105,15 @@ queue_intr (struct irqdev *dev, int id, user_intr_t > > *e) > > * disabled. Level-triggered interrupts would keep raising otherwise. */ > > __disable_irq (dev->irq[id]); > > > > - spl_t s = splhigh (); > > +#ifdef LINUX_DEV > > + spl_t s = simple_lock_irq(&intr_lock); > > +#endif > > Why only ifdef LINUX_DEV? AIUI We do want to always lock.
As said, I'm not sure if that's the correct thing. But the reasoning here has been that deliver_user_intr (which is what calls queue_intr) is called from two different contexts: without LINUX_DEV, from user_irq_handler in this same file, which does its own locking (and so would likely deadlock if we were to take the lock in queue_intr), and from linux_intr with LINUX_DEV, which doesn't seem to do its own locking (and so is probably quite broken on SMP, if we care about that). Sergey
