[PATCH] remove unnecessary PAE pgd set

[PATCH] remove unnecessary PAE pgd set

[Dave Hansen]

[-- Attachment #1: Type: text/plain, Size: 444 bytes --]

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
-- 
Dave Hansen
haveblue@us.ibm.com

[-- Attachment #2: vmal_fault-optimization-PAE-2.5.68-0.patch --]
[-- Type: text/plain, Size: 553 bytes --]

--- linux-2.5.68-vmal_fault/arch/i386/mm/fault.c.orig	Wed Apr 30 13:36:49 2003
+++ linux-2.5.68-vmal_fault/arch/i386/mm/fault.c	Wed Apr 30 13:36:18 2003
@@ -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);

Re: [PATCH] remove unnecessary PAE pgd set

[William Lee Irwin III]

On Wed, Apr 30, 2003 at 04:42:25PM -0700, Dave Hansen wrote:
> 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 majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"aart@kvack.org"> aart@kvack.org </a>

Re: [PATCH] remove unnecessary PAE pgd set

[Dave Hansen]

William Lee Irwin III wrote:
> On Wed, Apr 30, 2003 at 04:42:25PM -0700, Dave Hansen wrote:
> 
>>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.
-- 
Dave Hansen
haveblue@us.ibm.com

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

Re: [PATCH] remove unnecessary PAE pgd set

[William Lee Irwin III]

William Lee Irwin III wrote:
>> I suspect this set_pgd() should go away for non-PAE also.

On Wed, Apr 30, 2003 at 08:39:04PM -0700, Dave Hansen wrote:
> 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.


On Wed, Apr 30, 2003 at 08:39:04PM -0700, Dave Hansen wrote:
> - 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.


On Wed, Apr 30, 2003 at 08:39:04PM -0700, Dave Hansen wrote:
> 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.


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