[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <5cb5eccf-8edd-bdcc-844a-56bd94355747@arm.com>
Date: Thu, 1 Feb 2018 13:24:06 +0000
From: Marc Zyngier <marc.zyngier@....com>
To: shankerd@...eaurora.org, Will Deacon <will.deacon@....com>
Cc: linux-kernel <linux-kernel@...r.kernel.org>,
linux-arm-kernel <linux-arm-kernel@...ts.infradead.org>,
kvmarm <kvmarm@...ts.cs.columbia.edu>,
Thomas Gleixner <tglx@...utronix.de>,
Vikram Sethi <vikrams@...eaurora.org>,
Sean Campbell <scampbel@...eaurora.org>,
Thomas Speier <tspeier@...eaurora.org>
Subject: Re: [PATCH] irqchip/gic-v3: Use wmb() instead of smb_wmb() in
gic_raise_softirq()
On 01/02/18 12:55, Shanker Donthineni wrote:
> Hi Will, Thanks for your quick reply.
>
> On 02/01/2018 04:33 AM, Will Deacon wrote:
>> Hi Shanker,
>>
>> On Wed, Jan 31, 2018 at 06:03:42PM -0600, Shanker Donthineni wrote:
>>> A DMB instruction can be used to ensure the relative order of only
>>> memory accesses before and after the barrier. Since writes to system
>>> registers are not memory operations, barrier DMB is not sufficient
>>> for observability of memory accesses that occur before ICC_SGI1R_EL1
>>> writes.
>>>
>>> A DSB instruction ensures that no instructions that appear in program
>>> order after the DSB instruction, can execute until the DSB instruction
>>> has completed.
>>>
>>> Signed-off-by: Shanker Donthineni <shankerd@...eaurora.org>
>>> ---
>>> drivers/irqchip/irq-gic-v3.c | 2 +-
>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/irqchip/irq-gic-v3.c b/drivers/irqchip/irq-gic-v3.c
>>> index b56c3e2..980ae8e 100644
>>> --- a/drivers/irqchip/irq-gic-v3.c
>>> +++ b/drivers/irqchip/irq-gic-v3.c
>>> @@ -688,7 +688,7 @@ static void gic_raise_softirq(const struct cpumask *mask, unsigned int irq)
>>> * Ensure that stores to Normal memory are visible to the
>>> * other CPUs before issuing the IPI.
>>> */
>>> - smp_wmb();
>>> + wmb();
>>
>> I think this is the right thing to do and the smp_wmb() was accidentally
>> pulled in here as a copy-paste from the GICv2 driver where it is sufficient
>> in practice.
>>
>> Did you spot this by code inspection, or did the DMB actually cause
>> observable failures? (trying to figure out whether or not this need to go
>> to -stable).
>>
>
> We've inspected the code because kernel was causing failures in scheduler/IPI_RESCHDULE.
> After some time of debugging, we landed in GIC driver and found that the issue was due
> to the DMB barrier.
OK. I've applied this with a cc: stable and Will's Ack.
> Side note, we're also missing synchronization barriers in GIC driver after writing some
> of the ICC_XXX system registers. I'm planning to post those changes for comments.
>
> e.g: gic_write_sgi1r(val) and gic_write_eoir(irqnr);
Thanks,
M.
--
Jazz is not dead. It just smells funny...
Powered by blists - more mailing lists