On Mar 28, 2007, at 7:47 PM, Greg Oster wrote:
> In an effort to reduce the amount of time spent in
"system" when
> doing a pkgsrc build, I've been mucking around with
pmap.c on amd64.
> One of the changes I've made is to have one page of
> pmap_tlb_shootdown_job's per CPU. This eliminates the
global variable
> used to hold the "free pool" of such jobs,
and also eliminates all
> the locking contention associated with that free pool.
From the
> testing I've done, performance for building a set of
166 packages
> ('time make -k -j 8') goes from this (baseline):
...
Cool.
This reminds me -- I really ought to change pool_cache to
support per-
CPU caches. For another day...
Anyway, I think this is probably OK for now, but we should
certainly
revisit TLB shootdown in general.
>
> 18925.7u 25439.2s 3:05:03.98 399.5% 0+0k 229+2870194io
3371pf+57w
>
> to this (when just doubling the number of shootdown job
slots
> available
> per CPU):
>
> 18776.5u 23900.5s 2:56:29.96 402.9% 0+0k 292+2864111io
3374pf+0w
>
> to this (double the number of job slots, and eliminate
the global
> "free queue"):
>
> 17941.4u 20939.2s 2:43:56.35 395.2% 0+0k
6048+2639046io 6331pf+0w
>
> with my most recent tweaks to pmap.c (included here).
>
> Comments are more than welcome (yes, the diff would
need to be
> cleaned up before it could be checked in  ) A
review by someone
> more pmap/amd64-savvy would certainly be appreciated.
I
suspect
> similar changes could be made to the i386 pmap.c. (I
just havn't made
> time to make the changes there too and do testing)
>
> Later...
>
> Greg Oster
>
>
> --- pmap.c.orig 2007-03-27 09:17:49.000000000 -0600
> +++ pmap.c 2007-03-28 15:49:22.000000000 -0600
>  -359,19 +359,24 struct
pmap_tlb_shootdown_q {
> __cpu_simple_lock_t pq_slock; /* spin lock on queue
*/
> int pq_flushg; /* pending flush global */
> int pq_flushu; /* pending flush user */
> + struct pmap_tlb_shootdown_job *pj_free;
> + union pmap_tlb_shootdown_job_al *pj_page;
> } pmap_tlb_shootdown_q[X86_MAXPROCS];
>
> +#if 0
> #define PMAP_TLB_MAXJOBS 16
> +#endif
> +#define PMAP_TLB_MAXJOBS 32
>
> void pmap_tlb_shootdown_q_drain __P((struct
pmap_tlb_shootdown_q *));
> struct pmap_tlb_shootdown_job
*pmap_tlb_shootdown_job_get
> __P((struct pmap_tlb_shootdown_q *));
> void pmap_tlb_shootdown_job_put __P((struct
pmap_tlb_shootdown_q *,
> struct pmap_tlb_shootdown_job *));
> -
> +#if 0
> __cpu_simple_lock_t pmap_tlb_shootdown_job_lock;
> union pmap_tlb_shootdown_job_al *pj_page, *pj_free;
> -
> +#endif
> /*
> * global data structures
> */
> -1089,7 +1094,9 pmap_bootstrap(kva_start)
> * Initialize the TLB shootdown queues.
> */
>
> +#if 0
>
__cpu_simple_lock_init(&pmap_tlb_shootdown_job_lock);
> +#endif
>
> for (i = 0; i < X86_MAXPROCS; i++) {
> TAILQ_INIT(&pmap_tlb_shootdown_q[i].pq_head);
> -1148,6 +1155,9
pmap_prealloc_lowmem_ptps(void)
> void
> pmap_init()
> {
> + struct pmap_tlb_shootdown_q *pq;
> + struct cpu_info *ci;
> + CPU_INFO_ITERATOR cii;
> int npages, lcv, i;
> vaddr_t addr;
> vsize_t s;
> -1212,17 +1222,20 pmap_init()
>
> pv_nfpvents = 0;
>
> - pj_page = (void *)uvm_km_alloc(kernel_map, PAGE_SIZE,
0,
> UVM_KMF_WIRED);
> - if (pj_page == NULL)
> - panic("pmap_init: pj_page");
> -
> - for (i = 0;
> - i < (PAGE_SIZE / sizeof (union
pmap_tlb_shootdown_job_al) - 1);
> - i++)
> - pj_page[i].pja_job.pj_nextfree = &pj_page[i +
1].pja_job;
> - pj_page[i].pja_job.pj_nextfree = NULL;
> - pj_free = &pj_page[0];
> + for (CPU_INFO_FOREACH(cii, ci)) {
> + pq = &pmap_tlb_shootdown_q[ci->ci_cpuid];
> + pq->pj_page = (void *)uvm_km_alloc(kernel_map,
PAGE_SIZE,
> + 0, UVM_KMF_WIRED);
> + if (pq->pj_page == NULL)
> + panic("pmap_init: pj_page");
>
> + for (i = 0;
> + i < (PAGE_SIZE / sizeof (union
pmap_tlb_shootdown_job_al) -
> 1);
> + i++)
> + pq->pj_page[i].pja_job.pj_nextfree =
&pq->pj_page[i + 1].pja_job;
> + pq->pj_page[i].pja_job.pj_nextfree = NULL;
> + pq->pj_free = (struct pmap_tlb_shootdown_job
*)&(pq->pj_page[0]);
> + }
> /*
> * done: pmap module is up (and ready for business)
> */
> -3660,7 +3673,6
pmap_do_tlb_shootdown(struct cpu_info *s
> if ((!pq->pq_flushu &&
pmap_is_curpmap(pj->pj_pmap)) ||
> (pj->pj_pte & pmap_pg_g))
> pmap_update_pg(pj->pj_va);
> -
> pmap_tlb_shootdown_job_put(pq, pj);
> }
>
> -3717,22 +3729,12
pmap_tlb_shootdown_job_get(pq)
> if (pq->pq_count >= PMAP_TLB_MAXJOBS)
> return (NULL);
>
> -#ifdef MULTIPROCESSOR
> - __cpu_simple_lock(&pmap_tlb_shootdown_job_lock);
> -#endif
> - if (pj_free == NULL) {
> -#ifdef MULTIPROCESSOR
>
- __cpu_simple_unlock(&pmap_tlb_shootdown_job_lock);
> -#endif
> + if (pq->pj_free == NULL) {
> return NULL;
> + } else {
> + pj = pq->pj_free;
> + pq->pj_free = pj->pj_nextfree;
> }
> - pj = &pj_free->pja_job;
> - pj_free =
> - (union pmap_tlb_shootdown_job_al
*)pj_free->pja_job.pj_nextfree;
> -#ifdef MULTIPROCESSOR
>
- __cpu_simple_unlock(&pmap_tlb_shootdown_job_lock);
> -#endif
> -
> pq->pq_count++;
> return (pj);
> }
> -3754,14 +3756,7
pmap_tlb_shootdown_job_put(pq, pj)
> if (pq->pq_count == 0)
> panic("pmap_tlb_shootdown_job_put: queue length
inconsistency");
> #endif
> -#ifdef MULTIPROCESSOR
> - __cpu_simple_lock(&pmap_tlb_shootdown_job_lock);
> -#endif
> - pj->pj_nextfree = &pj_free->pja_job;
> - pj_free = (union pmap_tlb_shootdown_job_al *)pj;
> -#ifdef MULTIPROCESSOR
>
- __cpu_simple_unlock(&pmap_tlb_shootdown_job_lock);
> -#endif
> -
> + pj->pj_nextfree = pq->pj_free;
> + pq->pj_free = pj;
> pq->pq_count--;
> }
-- thorpej
|
