On Wednesday 17 May 2017 10:27 AM, Benjamin Herrenschmidt wrote:

On Wed, 2017-05-17 at 08:57 +0530, Aneesh Kumar K.V wrote:

quoted

Benjamin Herrenschmidt [off-list ref] writes:

quoted

On Tue, 2017-05-16 at 14:56 +0530, Aneesh Kumar K.V wrote:

quoted

+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 ?

quoted

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 ?

Yes, CONFIG_TRACE_IRQFLAGS are enabled. So in my local_t patchset,
I have added a patch to do the same with a flag "CONFIG_IRQ_DEBUG_SUPPORT"

mpe reported boot hang with the current version of the
local_t patchset in Booke system, and have a fix for the
same and it is being tested. Will post a newer version
once the patch verified.

Maddy

Also we could add additional checks, such as MSR_EE matching paca-

quoted

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.