> Subject: Re: [PATCH 1/2] arm: irqchip: add lock in irqchip_inject_pending
>
> On 03.12.19 09:27, Peng Fan wrote:
> > Thinking about core0 is inject SGI to core1, core1 is handling SGI
> > interrupt.
> >
> > That means core0 might be in path to enqueue SGI into the pending_irqs
> > array, core1 might be in path handling SGI and pick one from
> > pending_irqs array. So need to use lock to protect unqueue, not only
> > enqueue.
> >
> > Signed-off-by: Peng Fan <peng....@nxp.com>
> > ---
> >
> > V1:
> > The best case is only lock one entry, so no good solution, because
> > there is possibility that inject fail.
> >
> > hypervisor/arch/arm-common/irqchip.c | 5 +++++
> > 1 file changed, 5 insertions(+)
> >
> > diff --git a/hypervisor/arch/arm-common/irqchip.c
> > b/hypervisor/arch/arm-common/irqchip.c
> > index 1c881b64..fbaa3099 100644
> > --- a/hypervisor/arch/arm-common/irqchip.c
> > +++ b/hypervisor/arch/arm-common/irqchip.c
> > @@ -279,11 +279,14 @@ void irqchip_inject_pending(void)
> > struct pending_irqs *pending = &this_cpu_public()->pending_irqs;
> > u16 irq_id, sender;
> >
> > + spin_lock(&pending->lock);
> > +
> > while (pending->head != pending->tail) {
> > irq_id = pending->irqs[pending->head];
> > sender = pending->sender[pending->head];
> >
> > if (irqchip.inject_irq(irq_id, sender) == -EBUSY) {
> > + spin_unlock(&pending->lock);
> > /*
> > * The list registers are full, trigger maintenance
> > * interrupt and leave.
> > @@ -295,6 +298,8 @@ void irqchip_inject_pending(void)
> > pending->head = (pending->head + 1) % MAX_PENDING_IRQS;
> > }
> >
> > + spin_unlock(&pending->lock);
> > +
> > /*
> > * The software interrupt queue is empty - turn off the maintenance
> > * interrupt.
> >
>
> Did you see real corruptions?
No. just code inspection currently. We met some SGI inject return -EBUSY,
so go through the code, and think this piece code needs a lock.
>
> The idea behind this was that the injection to the lock-less queue happens in
> a way that it won't make changes visible to the consumer that are
> inconsistent, hence the barrier in irqchip_set_pending. Looking that again,
> though, we possibly need another barrier, right before updating
> pending->tail.
Barrier could not prohibit enqueue/unqueue contention.
enqueue will check head, unqueue will update head.
Thanks,
Peng.
>
> Jan
>
> --
> Siemens AG, Corporate Technology, CT RDA IOT SES-DE Corporate
> Competence Center Embedded Linux
--
You received this message because you are subscribed to the Google Groups
"Jailhouse" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to jailhouse-dev+unsubscr...@googlegroups.com.
To view this discussion on the web visit
https://groups.google.com/d/msgid/jailhouse-dev/AM0PR04MB4481FCD642715C2D03DA78EB88420%40AM0PR04MB4481.eurprd04.prod.outlook.com.
