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
On Thu, 8 Nov 2018, Uwe Kleine-König wrote:
> On Thu, Nov 08, 2018 at 05:47:48PM +0100, Bartosz Golaszewski wrote:
> > @@ -74,6 +74,7 @@ int irq_sim_init(struct irq_sim *sim, unsigned int num_irqs)
> > }
> >
> > init_irq_work(&sim->work_ctx.work, irq_sim_handle_irq);
> > + mutex_init(&sim->lock);
> > sim->irq_count = num_irqs;
> >
> > return sim->irq_base;
> > @@ -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.

Right. So the obvious solution is to avoid the irq number store and use a
bitfield instead.

struct irq_sim_work_ctx {
...
unsigned long pending;
};

fire(sim, offset)
{
if (!sim->irqs[offset].enabled)
return;

set_bit(offset, &sim->work_ctx.pending);
....

and in the work handler do:

handle(work)
{
struct irq_sim_work_ctx *ctx = container_of(work,....);

while (ctx->pending) {
offs = ffs(ctx->pending);
clr_bit(offs, &ctx->pending);
handle_simple_irq(offs);
}
}

Or something like that.

Thanks,

tglx
\
 
 \ /
  Last update: 2018-11-09 11:19    [W:0.075 / U:0.304 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site