> 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.