Hello,
On Thu, Nov 08, 2018 at 09:55:02PM +0100, Bartosz Golaszewski wrote:
> czw., 8 lis 2018 o 20:41 Uwe Kleine-König
> <u.kleine-koe...@pengutronix.de> napisał(a):
> > > @@ -142,10 +143,14 @@ EXPORT_SYMBOL_GPL(devm_irq_sim_init);
> > > */
> > > void irq_sim_fire(struct irq_sim *sim, unsigned int offset)
> > > {
> > > + mutex_lock(&sim->lock);
> > > +
> > > if (sim->irqs[offset].enabled) {
> > > sim->work_ctx.irq = irq_sim_irqnum(sim, offset);
> > > irq_work_queue(&sim->work_ctx.work);
> > > }
> > > +
> > > + mutex_unlock(&sim->lock);
> >
> > This doesn't fix the issue I think. irq_work_queue() only schedules the
> > work function. If after irq_sim_fire() returned but before the worker
> > runs another irq_sim_fire() is issued the value is still overwritten.
>
> Looking at irq_work_queue(): while there may be some arch-specific
> details deeper down the stack, it seems that unless the work is
> IRQ_WORK_LAZY, the handler should be executed immediately. I'll verify
> tomorrow though.
not considering the IRQ_WORK_LAZY case irq_work_queue adds the work
struct to a list and then calls arch_irq_work_raise(). The default
implementation for this function is empty. alpha, arm, arm64, powerpc,
sparc and x86 have alternative implementations. Quickly looking at the
arm one: It is only used on SMP. Also given that all relevant code of
irq_work_queue is protected by preempt_disable/preempt_enable this
cannot atomically call the work function, otherwise it would run with
preemption disabled which isn't the case AFAIK.
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | http://www.pengutronix.de/ |
