lkml.org 
[lkml]   [2005]   [Aug]   [6]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: [PATCH 1/1] i386 Encapsulate copying of pgd entries
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");
}
\
 
 \ /
  Last update: 2005-08-06 08:05    [W:0.037 / U:0.252 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site