Fix early_ioremap on x86-64

Fix early_ioremap on x86-64

[Venki, sorry for blaming PAT for this earlier. It was innocent.] [Note the patch is on top of git-x86 + gbpages applied. I think there will be an trivial reject without gbpages-direct applied first]

I had ACPI failures on several machines since a few days. Symptom was NUMA nodes not getting detected or worse cores not getting detected. They all came from ACPI not being able to read various of its tables. I finally bisected it down to Jeremy's "put _PAGE_GLOBAL into PAGE_KERNEL" change. With that the fix was fairly obvious. The problem was that early_ioremap() didn't use a "_all" flush that would affect the global PTEs too. So with global bits getting used everywhere now an early_ioremap would not actually flush a mapping if something else was mapped previously on that slot (which can happen with early_iounmap inbetween)

This patch changes all flushes in init_64.c to be __flush_tlb_all() and fixes the problem here.

Index: linux/arch/x86/mm/init_64.c
===================================================================
@@ -242,7 +242,7 @@ void *early_ioremap(unsigned long addr, 
 		addr &= PMD_MASK;
 		for (i = 0; i < pmds; i++, addr += PMD_SIZE)
 			set_pmd(pmd+i, __pmd(addr | __PAGE_KERNEL_LARGE_EXEC));
-		__flush_tlb();
+		__flush_tlb_all();
 		printk(KERN_CONT "%016ld\n", vaddr);
 		return (void *)vaddr;
 	next:

@@ -265,7 +265,7 @@ void early_iounmap(void *addr, unsigned 
 	pmd = level2_kernel_pgt + pmd_index(vaddr);
 	for (i = 0; i < pmds; i++)
 		pmd_clear(pmd + i);
-	__flush_tlb();
+	__flush_tlb_all();
 }
 
 static unsigned long direct_entry(unsigned long paddr)

@@ -339,7 +339,7 @@ static void __meminit phys_pud_init(pud_
 		spin_unlock(&init_mm.page_table_lock);
 		unmap_low_page(pmd);
 	}
-	__flush_tlb();
+	__flush_tlb_all();
 } 
 
 static void __init find_early_table_space(unsigned long end)

ah, very nice! This is the bad commit:

------------------------->
Subject: x86: fold _PAGE_GLOBAL into __PAGE_KERNEL
From: Jeremy Fitzhardinge <[email protected]>

With the iounmap problem resolved, it should be OK to always set _PAGE_GLOBAL in __PAGE_KERNEL*.

[ Did this patch cause problems before? ]
<------------------------

Jeremy did suspect something about this change, as indicated in the changelog. But because the change was so finegrained, the bisection almost directly led to the fix. _That_ i think clearly demonstrates the power of bisection and finegrained changes.

but note what the fundamental problem is - we've turned a previously safe flushing API into an unsafe one - __flush_tlb() will only be safe in the rarest of circumstances. There are some other matches:

./mm/init_64.c: __flush_tlb();
./kernel/head64.c: __flush_tlb();

The boot identity mappings zapped do not have PGE set at the moment, but they could in the future (once we do native pagetable setup straight from flat mode) - and this is not a performance critical path anyway.

./kernel/cpu/mtrr/generic.c: __flush_tlb();
./kernel/cpu/mtrr/generic.c: __flush_tlb();

these include an open-coded version of __flush_tlb_all() so they are safe.

and we might as well make the non-PGE flush the 'special API'. I.e. rename __flush_tlb() to __flush_tlb_partial() and rename __flush_tlb_all() to __flush_tlb(). This makes it very apparent which should be used by default and which does what.

Ingo

Actually it is pretty hard to do bisects in current git-x86 currently because you don't ammend original buggy changes but just stuff the fixes at the end. This means there are large windows of breakage inbetween that make it hard to pinpoint other bugs during bisection.

This is why I cannot easily bisect this new breakage.
Ok I suspect I can cook up some script later that always create a new branch and reapplies the bug fixes. Or ammend the patch locally. But frankly it is very unpleasant.

It works for user space which is by far the most common case.

You're right I missed some. I tried to just redefine it in tlbflush.h (which covers user space fork() too so it's too much as a real fix, but reasonable for debugging) but it unfortunately still results in the same crash during NUMA node discovery:

There is some garbage in the stack trace; i don't think it's actually in bootmem. [My kingdom for Jan's unwinder!]

RAT: Node 0 PXM 0 0-a0000
SRAT: Node 0 PXM 0 0-d0000000
SRAT: Node 0 PXM 0 0-130000000
SRAT: Node 1 PXM 1 130000000-330000000 early_iounmap(ffffffff828e08d9, 00000150)
Bootmem setup node 0 0000000000000000-00000000d0000000
PANIC: early exception 06 rip 10:ffffffff8082df35 error 0 cr2 0
Pid: 0, comm: swapper Not tainted 2.6.24-rc8-dirty #47

Call Trace:
 [<ffffffff8082df35>] free_bootmem_core+0x1a/0x6b
 [<ffffffff8082f14b>] free_bootmem_with_active_regions+0x51/0x67
 [<ffffffff8082a0fb>] setup_node_bootmem+0x16b/0x1b3
 [<ffffffff8082b3ac>] acpi_scan_nodes+0x136/0x230
 [<ffffffff8082a6c3>] numa_initmem_init+0x331/0x459
 [<ffffffff80234ef6>] release_console_sem+0x1b4/0x1cd
 [<ffffffff80234ef6>] release_console_sem+0x1b4/0x1cd
 [<ffffffff80355726>] number+0x10e/0x1f9
 [<ffffffff802354d8>] printk+0x4e/0x56
 [<ffffffff80355726>] number+0x10e/0x1f9
 [<ffffffff803563f8>] vsnprintf+0x53f/0x583
 [<ffffffff80355726>] number+0x10e/0x1f9
 [<ffffffff80234ef6>] release_console_sem+0x1b4/0x1cd
 [<ffffffff803563f8>] vsnprintf+0x53f/0x583
 [<ffffffff8022468a>] early_serial_write+0x22/0x32
 [<ffffffff80234b32>] __call_console_drivers+0x5e/0x6c
 [<ffffffff80234ef6>] release_console_sem+0x1b4/0x1cd
 [<ffffffff802354d8>] printk+0x4e/0x56
 [<ffffffff802354d8>] printk+0x4e/0x56
 [<ffffffff802354d8>] printk+0x4e/0x56
 [<ffffffff8082b164>] acpi_numa_memory_affinity_init+0x228/0x33a
 [<ffffffff8022635c>] early_iounmap+0x20/0x70
 [<ffffffff80836045>] acpi_parse_memory_affinity+0x0/0x1f
 [<ffffffff8083470c>] acpi_table_parse_entries+0x115/0x126
 [<ffffffff80836011>] acpi_parse_slit+0x0/0x17
 [<ffffffff808345d0>] acpi_table_parse+0x4c/0x73
 [<ffffffff808215af>] setup_arch+0x266/0x414
 [<ffffffff8081b8b7>] start_kernel+0x6f/0x2bd
 [<ffffffff8081b1ce>] _sinittext+0x1ce/0x1d5

RIP 0x10

-Andi (seemingly chief QA officer of git-x86 currently)

i backmerge fixes all the time. x86.git consists of more than 800 patches and there are less than 10 tail-fixes in it - they are all at the tail for a reason: because they have not all been fully qa-ed yet so they might make things worse than what they try to fix. Once they are proven i backmerge them to their proper spot.

and i frequently bisect x86.git#mm myself (almost daily) - and others do it frequently too. There have been dozens of successful bisections done on x86.git. (I believe it is so successful partly because we run overnight automated bisectability tests: to make sure x86.git builds and boots at every bisection point.)

Yours is the first claim of x86.git#mm being hard to bisect - so far all other feedback was to the contrary. I claim that it is one of the most bisectable subsystems in the kernel.

But we all err: if something slipped through despite all the testing, let us know and we'll fix it up - it's just that your vague complaints about non-bisectability wont help much in resolving the problem. You are ignoring months of track record of excellent bisectability of x86.git and your claims are quite unjust.

Reordering of patches is usually easy and trickling back the tail-fixes to the actual buggy patches is easy and routine as well, except when someone submits conceptually non-bisectable patches, like:

|   Subject: Undo pat cpa patch
|   From: Andi Kleen <[email protected]>
|
|   Going to implement this differently

and then 4 patches later you do:

|   Subject: CPA: Implement change_page_attr_addr entry point for i386
|   From: Andi Kleen <[email protected]>

this breaks PAT completely in that window and makes it non-bisectable.

So i find it a bit dishonest from you that you are now complaining about the supposed non-bisectability of x86.git ...

Andi, could you please stop these snide remarks? x86.git is the development tip of the new arch/x86 tree and you should know that. I boot it more than a 1000 times a day on my QA-grid, with different kernel configs, so it's far from being unusable.

I update x86.git multiple times a day so that people can see the changes immediately. But you are now abusing this transparency to create a false impression of flakiness. If you cannot stop being negative then please just dont deal with x86.git#mm - it's not obligatory to contribute.

Furthermore, your complaint is doubly unjust because we've fixed a good deal of bugs in x86.git introduced by _you_. You are by far not the only person who fixes bugs in x86.git and you are by far not the most active one either - but you are certainly the loudest one :-(

Ingo

Hmm, I see new early crashes with that patch here in some cases unfortunately:

PANIC: early exception 06 rip 10:ffffffff8082df2d error 0 cr2 0
Pid: 0, comm: swapper Not tainted 2.6.24-rc8-g7ada4254 #39

Call Trace:
 [<ffffffff8082df2d>] free_bootmem_core+0x1a/0x6b
 [<ffffffff8082f143>] free_bootmem_with_active_regions+0x51/0x67
 [<ffffffff8082a0f3>] setup_node_bootmem+0x16b/0x1b3
 [<ffffffff8082b3a4>] acpi_scan_nodes+0x136/0x230
 [<ffffffff8082a6bb>] numa_initmem_init+0x331/0x459
 [<ffffffff80234ef6>] release_console_sem+0x1b4/0x1cd
 [<ffffffff80234ef6>] release_console_sem+0x1b4/0x1cd
 [<ffffffff80355726>] number+0x10e/0x1f9
 [<ffffffff802354d8>] printk+0x4e/0x56
 [<ffffffff80355726>] number+0x10e/0x1f9
 [<ffffffff803563f8>] vsnprintf+0x53f/0x583
 [<ffffffff80355726>] number+0x10e/0x1f9
 [<ffffffff80234ef6>] release_console_sem+0x1b4/0x1cd
 [<ffffffff803563f8>] vsnprintf+0x53f/0x583
 [<ffffffff8022468a>] early_serial_write+0x22/0x32

Not sure what's going wrong. I think the patch is correct, but there
are probably more bugs around triggered by some change. Perhaps it is better to just revert Jeremy's patch (789bad4ca2b604f2f21ae7e3d64b67325ec8f9bb) for now.

-Andi

It's a purely "make things prettier" patch, so I have no problem with that. If the non-global flags used in early 64-bit pagetables is very deeply ingrained, I wonder if its worth making a separate set of explicitly non-global _PAGE_KERNEL flags for its use, so that at least _PAGE_KERNEL* is consistent between 32 and 64 bits. Otherwise its going to be the sort of difference which gets increasingly awkward as things get unified (I guess its mitigated by the fact that 32-bit code can never assume that global mappings are available and present).

J