On Wed, 2017-05-17 at 08:57 +0530, Aneesh Kumar K.V wrote:
> Benjamin Herrenschmidt <b...@kernel.crashing.org> writes:
>
> > On Tue, 2017-05-16 at 14:56 +0530, Aneesh Kumar K.V wrote:
> > > +static inline pte_t *find_linux_pte(pgd_t *pgdir, unsigned long ea,
> > > + bool *is_thp, unsigned *hshift)
> > > +{
> > > + VM_WARN((!arch_irqs_disabled() && !__hard_irqs_disabled()) ,
> > > + "%s called with irq enabled\n", __func__);
> > > + return __find_linux_pte(pgdir, ea, is_thp, hshift);
> > > +}
> > > +
> >
> > When is arch_irqs_disabled() not sufficient ?
>
> We can do lockless page table walk in interrupt handlers where we find
> MSR_EE = 0.
Such as ?
> I was not sure we mark softenabled 0 there. What I wanted to
> indicate in the patch is that we are safe with either softenable = 0 or
> MSR_EE = 0
Reading the MSR is expensive...
Can you find a case where we are hard disabled and not soft disable in
C code ? I can't think of one off-hand ... I know we have some asm that
can do that very temporarily but I wouldn't think we have anything at
runtime.
Talking of which, we have this in irq.c:
#ifdef CONFIG_TRACE_IRQFLAGS
else {
/*
* We should already be hard disabled here. We had bugs
* where that wasn't the case so let's dbl check it and
* warn if we are wrong. Only do that when IRQ tracing
* is enabled as mfmsr() can be costly.
*/
if (WARN_ON(mfmsr() & MSR_EE))
__hard_irq_disable();
}
#endif
I think we should move that to a new CONFIG_PPC_DEBUG_LAZY_IRQ because
distros are likely to have CONFIG_TRACE_IRQFLAGS these days no ?
Also we could add additional checks, such as MSR_EE matching paca-
>irq_happened or the above you mentioned, ie, WARN if we find case
where IRQs are hard disabled but soft enabled.
If we find these, I think we should fix them.
Cheers,
Ben.
