Re: [PATCH 1/1] i386 Encapsulate copying of pgd entries
From: Zachary Amsden
Date: Sat Aug 06 2005 - 01:03:09 EST
Zachary Amsden wrote:
Chris Wright wrote:
Why memset was never done on PAE?
That's a good point. The memset() is redundant on PAE, since it
allocates all 4 PMDs immediately after that (in pgd_alloc). There are
two reasons for moving the memset() - one is that it can potentially
perform useful work ahead of the lock and effectively act as a
prefetch. The second is that at least on a hypervisor,
clone_pgd_range() is likely to be taken as a page allocation hint, and
thus moving the memset() before this operation allows only the
actually present page directory entry updates to be passed to the
hypervisor.
Actually, the memset() could be redundant on non-PAE as well, since we
should have gone through free_pgtables, which would have done a
pmd_clear() on each user level pmd, and the kernel level pmds are
copied in again inside the lock.
I'll try it out to see if this is possible.
Zach
So that turned out to be a really bad idea. But, I did notice that the
pmds in PAE mode could be cached with the pgds instead of destroying and
re-allocating them. Unfortunately, this spends three pages per cached
PAE pgd, and doesn't look like a big win. I ran microbenchmarks, stolen
mostly from lmbench (thank you Larry!), and this patch shows almost no
improvement. Judging by the fact the the kmem slab cache seems to work
very efficiently, I don't think the extra overhead from memset in the
constructor is of much significance.
Here's the benchmark results on native hardware (P4, 2.4 GHz, PAE kernel):
Before:
(getpid and segv truncated beyond my scrollback, but of no significance)
forkwait: 0.596u 3.932s 0:04.54 99.5% 0+0k 0+0io 0pf+0w
forkwait: 0.632u 3.876s 0:04.50 100.0% 0+0k 0+0io 0pf+0w
forkwait: 0.468u 4.048s 0:04.51 99.7% 0+0k 0+0io 0pf+0w
forkwait: 0.516u 3.988s 0:04.50 99.7% 0+0k 0+0io 0pf+0w
forkwait: 0.644u 3.908s 0:04.55 99.7% 0+0k 0+0io 0pf+0w
divzero: 1.356u 6.712s 0:08.07 99.8% 0+0k 0+0io 0pf+0w
divzero: 1.332u 6.620s 0:07.94 100.1% 0+0k 0+0io 0pf+0w
divzero: 1.300u 6.652s 0:07.95 100.0% 0+0k 0+0io 0pf+0w
divzero: 1.672u 6.312s 0:07.98 100.0% 0+0k 0+0io 0pf+0w
divzero: 1.128u 6.824s 0:07.95 99.8% 0+0k 0+0io 0pf+0w
lat_pipe: 0.228u 8.196s 0:16.98 49.5% 0+0k 0+0io 0pf+0w
lat_pipe: 0.220u 8.420s 0:17.15 50.3% 0+0k 0+0io 0pf+0w
lat_pipe: 0.236u 8.376s 0:17.00 50.5% 0+0k 0+0io 0pf+0w
lat_pipe: 0.220u 8.140s 0:16.97 49.2% 0+0k 0+0io 0pf+0w
lat_pipe: 0.232u 8.488s 0:16.86 51.6% 0+0k 0+0io 0pf+0w
Switch: 5.896u 7.172s 0:11.97 109.1% 0+0k 0+0io 53pf+0w
Switch: 6.168u 6.792s 0:11.23 115.3% 0+0k 0+0io 1pf+0w
Switch: 6.084u 7.044s 0:11.22 116.9% 0+0k 0+0io 1pf+0w
Switch: 6.044u 7.088s 0:11.34 115.6% 0+0k 0+0io 1pf+0w
Switch: 6.252u 7.212s 0:11.45 117.5% 0+0k 0+0io 1pf+0w
After:
zach-dev2:Micro-bench $ cat out.post-patch
getpid: 0.076u 0.000s 0:00.08 87.5% 0+0k 0+0io 0pf+0w
getpid: 0.076u 0.004s 0:00.07 100.0% 0+0k 0+0io 0pf+0w
getpid: 0.080u 0.000s 0:00.08 100.0% 0+0k 0+0io 0pf+0w
getpid: 0.076u 0.000s 0:00.07 100.0% 0+0k 0+0io 0pf+0w
getpid: 0.072u 0.004s 0:00.07 100.0% 0+0k 0+0io 0pf+0w
segv: 1.168u 8.552s 0:09.72 99.8% 0+0k 0+0io 0pf+0w
segv: 1.160u 8.544s 0:09.70 100.0% 0+0k 0+0io 0pf+0w
segv: 1.248u 8.364s 0:09.61 99.8% 0+0k 0+0io 0pf+0w
segv: 1.296u 8.368s 0:09.66 99.8% 0+0k 0+0io 0pf+0w
segv: 1.312u 8.288s 0:09.59 100.0% 0+0k 0+0io 0pf+0w
forkwait: 0.600u 3.932s 0:04.53 100.0% 0+0k 0+0io 0pf+0w
forkwait: 0.580u 3.940s 0:04.51 100.2% 0+0k 0+0io 0pf+0w
forkwait: 0.576u 3.948s 0:04.52 99.7% 0+0k 0+0io 0pf+0w
forkwait: 0.492u 3.996s 0:04.48 100.0% 0+0k 0+0io 0pf+0w
forkwait: 0.604u 3.908s 0:04.51 99.7% 0+0k 0+0io 0pf+0w
divzero: 1.304u 6.740s 0:08.04 100.0% 0+0k 0+0io 0pf+0w
divzero: 1.360u 6.704s 0:08.06 100.0% 0+0k 0+0io 0pf+0w
divzero: 1.344u 6.696s 0:08.03 100.0% 0+0k 0+0io 0pf+0w
divzero: 1.428u 6.600s 0:08.02 100.0% 0+0k 0+0io 0pf+0w
divzero: 1.308u 6.720s 0:08.02 100.0% 0+0k 0+0io 0pf+0w
lat_pipe: 0.212u 7.648s 0:16.40 47.8% 0+0k 0+0io 0pf+0w
lat_pipe: 0.268u 8.208s 0:16.78 50.4% 0+0k 0+0io 0pf+0w
lat_pipe: 0.188u 8.296s 0:16.42 51.5% 0+0k 0+0io 0pf+0w
lat_pipe: 0.180u 8.084s 0:16.91 48.8% 0+0k 0+0io 0pf+0w
lat_pipe: 0.160u 7.668s 0:16.85 46.4% 0+0k 0+0io 0pf+0w
Switch: 6.168u 6.740s 0:11.91 108.3% 0+0k 0+0io 53pf+0w
Switch: 5.860u 7.332s 0:11.45 115.1% 0+0k 0+0io 1pf+0w
Switch: 5.804u 7.140s 0:11.34 114.1% 0+0k 0+0io 1pf+0w
Switch: 6.168u 6.644s 0:11.12 115.1% 0+0k 0+0io 1pf+0w
Switch: 6.076u 6.896s 0:11.34 114.2% 0+0k 0+0io 1pf+0w
So lat_pipe seems to have improved slightly.. but it could be noise.
Yeah, not worth it. Plus, this patch is obviously broken - the panic()
could be avoided by reworking the code, but this seems like a large
amount of work for very little gain. Nevertheless, I have attached the
patch for posterity's sake.
Zach
Index: linux-2.6.13-rc4-mm1/arch/i386/mm/pgtable.c
===================================================================
--- linux-2.6.13-rc4-mm1.orig/arch/i386/mm/pgtable.c 2005-08-04 05:42:32.000000000 -0700
+++ linux-2.6.13-rc4-mm1/arch/i386/mm/pgtable.c 2005-08-05 13:04:14.000000000 -0700
@@ -214,8 +214,16 @@
clone_pgd_range((pgd_t *)pgd + USER_PTRS_PER_PGD,
swapper_pg_dir + USER_PTRS_PER_PGD,
PTRS_PER_PGD - USER_PTRS_PER_PGD);
- if (PTRS_PER_PMD > 1)
+ if (PTRS_PER_PMD > 1) {
+ int i;
+ for (i = 0; i < USER_PTRS_PER_PGD; ++i) {
+ pmd_t *pmd = kmem_cache_alloc(pmd_cache, GFP_KERNEL);
+ if (!pmd)
+ panic("oom");
+ set_pgd(&((pgd_t *)pgd)[i], __pgd(1 + __pa(pmd)));
+ }
return;
+ }
pgd_list_add(pgd);
spin_unlock_irqrestore(&pgd_lock, flags);
@@ -226,6 +234,14 @@
{
unsigned long flags; /* can be called from interrupt context */
+ /* in the PAE case user pgd entries are overwritten before usage */
+ if (PTRS_PER_PMD > 1) {
+ int i;
+ for (i = 0; i < USER_PTRS_PER_PGD; ++i)
+ kmem_cache_free(pmd_cache, (void *)__va(pgd_val(((pgd_t *)pgd)[i])-1));
+ return;
+ }
+
spin_lock_irqsave(&pgd_lock, flags);
pgd_list_del(pgd);
spin_unlock_irqrestore(&pgd_lock, flags);
@@ -233,35 +249,12 @@
pgd_t *pgd_alloc(struct mm_struct *mm)
{
- int i;
pgd_t *pgd = kmem_cache_alloc(pgd_cache, GFP_KERNEL);
-
- if (PTRS_PER_PMD == 1 || !pgd)
- return pgd;
-
- for (i = 0; i < USER_PTRS_PER_PGD; ++i) {
- pmd_t *pmd = kmem_cache_alloc(pmd_cache, GFP_KERNEL);
- if (!pmd)
- goto out_oom;
- set_pgd(&pgd[i], __pgd(1 + __pa(pmd)));
- }
return pgd;
-out_oom:
- for (i--; i >= 0; i--)
- kmem_cache_free(pmd_cache, (void *)__va(pgd_val(pgd[i])-1));
- kmem_cache_free(pgd_cache, pgd);
- return NULL;
}
void pgd_free(pgd_t *pgd)
{
- int i;
-
- /* in the PAE case user pgd entries are overwritten before usage */
- if (PTRS_PER_PMD > 1)
- for (i = 0; i < USER_PTRS_PER_PGD; ++i)
- kmem_cache_free(pmd_cache, (void *)__va(pgd_val(pgd[i])-1));
- /* in the non-PAE case, free_pgtables() clears user pgd entries */
kmem_cache_free(pgd_cache, pgd);
}
Index: linux-2.6.13-rc4-mm1/arch/i386/mm/init.c
===================================================================
--- linux-2.6.13-rc4-mm1.orig/arch/i386/mm/init.c 2005-08-04 04:01:27.000000000 -0700
+++ linux-2.6.13-rc4-mm1/arch/i386/mm/init.c 2005-08-05 13:02:34.000000000 -0700
@@ -635,7 +635,7 @@
PTRS_PER_PGD*sizeof(pgd_t),
0,
pgd_ctor,
- PTRS_PER_PMD == 1 ? pgd_dtor : NULL);
+ pgd_dtor);
if (!pgd_cache)
panic("pgtable_cache_init(): Cannot create pgd cache");
}