remove unnecessary PAE pgd set

PATCH In review Dave Hansen 2 participants 4 messages Apr 30, 2003 – May 1, 2003 Memory management

Share & tools

Dave Hansen

author

2003-04-30 23:42 UTC #0 lore raw

PATCH 1/1 remove unnecessary PAE pgd set

With PAE on, there are only 4 PGD entries. The kernel ones never change, so there is no need to copy them when a vmalloc fault occurs. This was this was causing problems with the split pmd patches, but it is still correct for mainline.

Tested with and without PAE. I ran it in a loop turning on and off 10 swap partitions, which is what excited the original bug.
http://bugme.osdl.org/show_bug.cgi?id=640

Signature · 2 lines

Dave Hansen
[email protected]

arch/i386/mm/fault.c renamed from arch/i386/mm/fault.c.orig +8 -0

@@ -405,7 +405,15 @@
 
 		if (!pgd_present(*pgd_k))
 			goto no_context;
+		/*
+		 * kernel pmd pages are shared among all processes
+		 * with PAE on.  Since vmalloc pages are always
+		 * in the kernel area, this will always be a 
+		 * waste with PAE on.
+		 */
+#ifndef CONFIG_X86_PAE
 		set_pgd(pgd, *pgd_k);
+#endif
 		
 		pmd = pmd_offset(pgd, address);
 		pmd_k = pmd_offset(pgd_k, address);

William Lee Irwin III

2003-05-01 03:22 UTC #1 lore raw

Quoted 7 lines · Dave Hansen

With PAE on, there are only 4 PGD entries. The kernel ones never change, so there is no need to copy them when a vmalloc fault occurs. This was this was causing problems with the split pmd patches, but it is still correct for mainline.
Tested with and without PAE. I ran it in a loop turning on and off 10 swap partitions, which is what excited the original bug.
http://bugme.osdl.org/show_bug.cgi?id=640

I suspect this set_pgd() should go away for non-PAE also.

-- wli
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

Dave Hansen

author

2003-05-01 03:39 UTC #2 lore raw

Quoted 11 lines · William Lee Irwin III

Quoted 7 lines · Dave Hansen
With PAE on, there are only 4 PGD entries. The kernel ones never change, so there is no need to copy them when a vmalloc fault occurs. This was this was causing problems with the split pmd patches, but it is still correct for mainline.
Tested with and without PAE. I ran it in a loop turning on and off 10 swap partitions, which is what excited the original bug.
http://bugme.osdl.org/show_bug.cgi?id=640
I suspect this set_pgd() should go away for non-PAE also.

Wouldn't it be analogous to the PMD set with PAE on, and necessary? Since processes don't share PGDs in 4G mode, the PGD entry could be missing in a process's pagetables after the kernel ones have been filled in. That set_pgd() will be necessary to move any new entry over.

- a process does a vmalloc, which eventually calls pte_alloc_kernel()
- pte_alloc_kernel() is always called with init_mm as its mm argument
- pte_alloc_kernel() populates init_mm PGD entries with the new pte page
- the process goes to use its new vmalloc'd area, and faults, because its pgd doesn't have the same entries. do_page_fault():vmalloc_fault brings over the necessary entries from init, and the fault is handled

The other option is to hold mmlist_lock and populate the entries around when pte_alloc_kernel() is called. The lazy way is better because not every process will go looking into the vmalloc area, _and_ and new pgds should be copied from the init one anyway, and inherit the PTE mapping.
The lazy update only needs to be done for processes when their PGDs
were allocated in the past.

Signature · 8 lines

Dave Hansen
[email protected]

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected].  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

William Lee Irwin III

2003-05-01 03:45 UTC #3 lore raw

Quoted 1 line · William Lee Irwin III

Quoted 1 line
I suspect this set_pgd() should go away for non-PAE also.

Quoted 4 lines · Dave Hansen

Wouldn't it be analogous to the PMD set with PAE on, and necessary? Since processes don't share PGDs in 4G mode, the PGD entry could be missing in a process's pagetables after the kernel ones have been filled in. That set_pgd() will be necessary to move any new entry over.

Not really; it should just fall through to the pmd "level", which as emulated should do all the instantiation needed for non-PAE.

Quoted 6 lines · Dave Hansen

- a process does a vmalloc, which eventually calls pte_alloc_kernel()
- pte_alloc_kernel() is always called with init_mm as its mm argument
- pte_alloc_kernel() populates init_mm PGD entries with the new pte page
- the process goes to use its new vmalloc'd area, and faults, because its pgd doesn't have the same entries. do_page_fault():vmalloc_fault brings over the necessary entries from init, and the fault is handled

Sounds like a good plan to me.

Quoted 6 lines · Dave Hansen

The other option is to hold mmlist_lock and populate the entries around when pte_alloc_kernel() is called. The lazy way is better because not every process will go looking into the vmalloc area, _and_ and new pgds should be copied from the init one anyway, and inherit the PTE mapping.
The lazy update only needs to be done for processes when their PGDs
were allocated in the past.

Yeah, that could get ugly. Best to keep things lazy.