lkml.org 
[lkml]   [2018]   [Nov]   [9]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH] irq/irq_sim: add locking
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-koenig@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/ |

\
 
 \ /
  Last update: 2018-11-08 22:26    [W:0.050 / U:0.236 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site