Date: Sun, 6 Jun 2010 04:10:16 +0530 From: "C. Jayachandran" <c.jayachandran@gmail.com> To: Alan Cox <alc@cs.rice.edu> Cc: "Jayachandran C." <jchandra@freebsd.org>, Konstantin Belousov <kostikbel@gmail.com>, mips@freebsd.org Subject: Re: svn commit: r208589 - head/sys/mips/mips Message-ID: <AANLkTinmFOZY3OlaoKStxlNIRBt2G2I4ILkQ1P0CjozG@mail.gmail.com> In-Reply-To: <4C09345F.9040300@cs.rice.edu> References: <201005271005.o4RA5eVu032269@svn.freebsd.org> <4C058145.3000707@cs.rice.edu> <AANLkTil955Ek-a3tek4Ony9NqrK5l2j7lNA4baRVPBzb@mail.gmail.com> <4C05F9BC.40409@cs.rice.edu> <AANLkTikdnvXsTwm8onl3MZPQmVfV-2GovB9--KMNnMgC@mail.gmail.com> <4C0731A5.7090301@cs.rice.edu> <AANLkTimIa3jmBPMhWIOcY6DenGpZ2ZYmqwDTWspVx0-u@mail.gmail.com> <AANLkTil2gE1niUWCHnsTlQvibhxBh7QYwD0TTWo0rj5c@mail.gmail.com> <AANLkTinA2D5iTDGPbflHVzLyAZW-ZewjJkUWWL8FVskr@mail.gmail.com> <4C07E07B.9060802@cs.rice.edu> <AANLkTimjyPc_AXKP1yaJaF1BN7CAGBeNikVzcp9OCb4P@mail.gmail.com> <4C09345F.9040300@cs.rice.edu>
next in thread | previous in thread | raw e-mail | index | archive | help
[-- Attachment #1 --]
On Fri, Jun 4, 2010 at 10:44 PM, Alan Cox <alc@cs.rice.edu> wrote:
> C. Jayachandran wrote:
>>
>> On Thu, Jun 3, 2010 at 10:33 PM, Alan Cox <alc@cs.rice.edu> wrote:
>>
>
> [snip]
>>
>>> Add a static counter to pmap_ptpgzone_allocf(). When the counter reaches
>>> some not-too-small prime number, don't call vm_phys_alloc_contig(),
>>> instead
>>> set "m" to NULL and reset the counter to zero. Setting "m" to NULL
>>> should
>>> then trigger the vm_contig_grow_cache() call. Make sense?
>>>
>>
>> Is this to move the vm_contig_grow_cache() out of the
>> pmap_ptpgzone_allocf() and into the function calling uma_zalloc()? I
>> am wondering why the prime number is needed.
>>
>> Another implementation I had thought about was to have a kernel
>> process maintain a pool of direct mapped pages for MIPS. This will be
>> woken up if the pool goes below a low water mark - any comments on
>> this?
>>
>>
>
> Here is how the page table page allocation should be done. It's not much
> harder than the vm_contig_grow_cache() change.
>
> 1. Look at vm/vm_phys.c, in particular, vm_phys_init(). Create a new
> freelist, like VM_FREELIST_ISADMA, for the direct-mapped memory on
> MIPS. Perhaps, call it VM_FREELIST_LOWMEM. The controlling macros should
> be defined in mips/include/vmparam.h.
>
> 2. Trivially refactor vm_phys_alloc_pages(). Specifically, take the body of
> the outer for loop and make it a new function, vm_phys_alloc_freelist(),
> that takes the variable "flind" as a parameter.
>
> 3. Eliminate the UMA zone for page table pages. In place of
> vm_phys_alloc_contig() call your new function with VM_FREELIST_LOWMEM. (The
> vm_contig_grow_cache() remains unchanged.) Go back to calling
> vm_page_free() to release page table pages.
>
> For the record, the changes that you made to start using a zone for page
> table page allocation introduced at least one possible race condition
> between pmap_remove_pages() and pmap_remove_all(). Specifically, by
> dropping and reacquiring the page queues and pmap lock when you free a page
> table page, you allow a pmap_remove_all() call while pmap_remove_pages() is
> running to leave the variable "npv" in pmap_remove_pages() pointing at a
> freed pv entry.
My first attempt is attached, it comes up multiuser but crashes if I
stress it a bit (panic: Bad link elm 0xc0078f00 prev->next != elm).
Will look at this tomorrow, and see if I can find the cause.
In the meantime, it may be worth looking at the patch to see if this
is in line with the approach you had suggested. I have tried to use
VM_FREELIST_HIGHMEM which is already there, instead of adding
VM_FREELIST_LOWMEM.
JC.
[-- Attachment #2 --]
Index: sys/mips/include/vmparam.h
===================================================================
--- sys/mips/include/vmparam.h (revision 208848)
+++ sys/mips/include/vmparam.h (working copy)
@@ -103,8 +103,9 @@
#define VM_MAXUSER_ADDRESS ((vm_offset_t)0x80000000)
#define VM_MAX_MMAP_ADDR VM_MAXUSER_ADDRESS
-#define VM_MIN_KERNEL_ADDRESS ((vm_offset_t)0xC0000000)
-#define VM_MAX_KERNEL_ADDRESS ((vm_offset_t)0xFFFFC000)
+#define VM_MIN_KERNEL_ADDRESS ((vm_offset_t)0xC0000000)
+#define VM_MAX_KERNEL_ADDRESS ((vm_offset_t)0xFFFFC000)
+#define VM_HIGHMEM_ADDRESS ((vm_paddr_t)0x20000000)
#if 0
#define KERNBASE (VM_MIN_KERNEL_ADDRESS)
#else
@@ -168,13 +169,15 @@
#define VM_FREEPOOL_DIRECT 1
/*
- * we support 1 free list:
+ * we support 2 free lists:
*
- * - DEFAULT for all systems
+ * - LOWMEM for Direct mapped (KSEG0) pages
+ * - DEFAULT for the rest
*/
-#define VM_NFREELIST 1
-#define VM_FREELIST_DEFAULT 0
+#define VM_NFREELIST 2
+#define VM_FREELIST_DEFAULT 1
+#define VM_FREELIST_HIGHMEM 0
/*
* The largest allocation size is 1MB.
Index: sys/mips/mips/pmap.c
===================================================================
--- sys/mips/mips/pmap.c (revision 208848)
+++ sys/mips/mips/pmap.c (working copy)
@@ -184,8 +184,6 @@
static int init_pte_prot(vm_offset_t va, vm_page_t m, vm_prot_t prot);
static void pmap_TLB_invalidate_kernel(vm_offset_t);
static void pmap_TLB_update_kernel(vm_offset_t, pt_entry_t);
-static vm_page_t pmap_alloc_pte_page(pmap_t, unsigned int, int, vm_offset_t *);
-static void pmap_release_pte_page(vm_page_t);
#ifdef SMP
static void pmap_invalidate_page_action(void *arg);
@@ -193,10 +191,6 @@
static void pmap_update_page_action(void *arg);
#endif
-static void pmap_ptpgzone_dtor(void *mem, int size, void *arg);
-static void *pmap_ptpgzone_allocf(uma_zone_t, int, u_int8_t *, int);
-static uma_zone_t ptpgzone;
-
struct local_sysmaps {
struct mtx lock;
vm_offset_t base;
@@ -535,10 +529,6 @@
pv_entry_max = PMAP_SHPGPERPROC * maxproc + cnt.v_page_count;
pv_entry_high_water = 9 * (pv_entry_max / 10);
uma_zone_set_obj(pvzone, &pvzone_obj, pv_entry_max);
-
- ptpgzone = uma_zcreate("PT ENTRY", PAGE_SIZE, NULL, pmap_ptpgzone_dtor,
- NULL, NULL, PAGE_SIZE - 1, UMA_ZONE_NOFREE | UMA_ZONE_ZINIT);
- uma_zone_set_allocf(ptpgzone, pmap_ptpgzone_allocf);
}
/***************************************************
@@ -886,11 +876,7 @@
* If the page is finally unwired, simply free it.
*/
atomic_subtract_int(&cnt.v_wire_count, 1);
- PMAP_UNLOCK(pmap);
- vm_page_unlock_queues();
- pmap_release_pte_page(m);
- vm_page_lock_queues();
- PMAP_LOCK(pmap);
+ vm_page_free_zero(m);
return (1);
}
@@ -949,96 +935,50 @@
bzero(&pmap->pm_stats, sizeof pmap->pm_stats);
}
-static void
-pmap_ptpgzone_dtor(void *mem, int size, void *arg)
+static vm_page_t
+pmap_alloc_pte_page(pmap_t pmap, unsigned int index, int wait, vm_offset_t *vap)
{
-#ifdef INVARIANTS
- static char zeropage[PAGE_SIZE];
-
- KASSERT(size == PAGE_SIZE,
- ("pmap_ptpgzone_dtor: invalid size %d", size));
- KASSERT(bcmp(mem, zeropage, PAGE_SIZE) == 0,
- ("pmap_ptpgzone_dtor: freeing a non-zeroed page"));
-#endif
-}
-
-static void *
-pmap_ptpgzone_allocf(uma_zone_t zone, int bytes, u_int8_t *flags, int wait)
-{
- vm_page_t m;
vm_paddr_t paddr;
- int tries;
-
- KASSERT(bytes == PAGE_SIZE,
- ("pmap_ptpgzone_allocf: invalid allocation size %d", bytes));
+ vm_page_t m;
+ int tries, flags;
- *flags = UMA_SLAB_PRIV;
tries = 0;
retry:
- m = vm_phys_alloc_contig(1, 0, MIPS_KSEG0_LARGEST_PHYS,
- PAGE_SIZE, PAGE_SIZE);
+ m = vm_phys_alloc_freelist_pages(VM_FREELIST_DEFAULT,
+ VM_FREEPOOL_DEFAULT, 0);
+ if (tries > 0)
+ printf("[%d] %s wait %x, got %p\n", tries, __func__,
+ wait, m);
if (m == NULL) {
if (tries < ((wait & M_NOWAIT) != 0 ? 1 : 3)) {
vm_contig_grow_cache(tries, 0, MIPS_KSEG0_LARGEST_PHYS);
tries++;
goto retry;
- } else
+ } else {
+ printf("[%d] %s wait %x, fail!\n", tries, __func__,
+ wait);
return (NULL);
+ }
}
paddr = VM_PAGE_TO_PHYS(m);
- return ((void *)MIPS_PHYS_TO_KSEG0(paddr));
-}
-
-static vm_page_t
-pmap_alloc_pte_page(pmap_t pmap, unsigned int index, int wait, vm_offset_t *vap)
-{
- vm_paddr_t paddr;
- void *va;
- vm_page_t m;
- int locked;
-
- locked = mtx_owned(&pmap->pm_mtx);
- if (locked) {
- mtx_assert(&vm_page_queue_mtx, MA_OWNED);
- PMAP_UNLOCK(pmap);
- vm_page_unlock_queues();
- }
- va = uma_zalloc(ptpgzone, wait);
- if (locked) {
- vm_page_lock_queues();
- PMAP_LOCK(pmap);
- }
- if (va == NULL)
- return (NULL);
-
- paddr = MIPS_KSEG0_TO_PHYS(va);
- m = PHYS_TO_VM_PAGE(paddr);
-
- if (!locked)
- vm_page_lock_queues();
m->pindex = index;
m->valid = VM_PAGE_BITS_ALL;
m->wire_count = 1;
- if (!locked)
- vm_page_unlock_queues();
+ if (m->flags & PG_ZERO)
+ vm_page_zero_count--;
+ else
+ pmap_zero_page(m);
+ flags = PG_ZERO | PG_UNMANAGED;
+ m->flags = flags;
+ m->oflags = 0;
+ m->act_count = 0;
atomic_add_int(&cnt.v_wire_count, 1);
- *vap = (vm_offset_t)va;
+ *vap = MIPS_PHYS_TO_KSEG0(paddr);
return (m);
}
-static void
-pmap_release_pte_page(vm_page_t m)
-{
- void *va;
- vm_paddr_t paddr;
-
- paddr = VM_PAGE_TO_PHYS(m);
- va = (void *)MIPS_PHYS_TO_KSEG0(paddr);
- uma_zfree(ptpgzone, va);
-}
-
/*
* Initialize a preallocated and zeroed pmap structure,
* such as one in a vmspace structure.
@@ -1193,7 +1133,7 @@
ptdpg->wire_count--;
atomic_subtract_int(&cnt.v_wire_count, 1);
- pmap_release_pte_page(ptdpg);
+ vm_page_free_zero(ptdpg);
PMAP_LOCK_DESTROY(pmap);
}
Index: sys/vm/vm_phys.c
===================================================================
--- sys/vm/vm_phys.c (revision 208848)
+++ sys/vm/vm_phys.c (working copy)
@@ -301,10 +301,8 @@
vm_page_t
vm_phys_alloc_pages(int pool, int order)
{
- struct vm_freelist *fl;
- struct vm_freelist *alt;
- int flind, oind, pind;
vm_page_t m;
+ int flind;
KASSERT(pool < VM_NFREEPOOL,
("vm_phys_alloc_pages: pool %d is out of range", pool));
@@ -312,38 +310,55 @@
("vm_phys_alloc_pages: order %d is out of range", order));
mtx_assert(&vm_page_queue_free_mtx, MA_OWNED);
for (flind = 0; flind < vm_nfreelists; flind++) {
- fl = vm_phys_free_queues[flind][pool];
- for (oind = order; oind < VM_NFREEORDER; oind++) {
- m = TAILQ_FIRST(&fl[oind].pl);
+ m = vm_phys_alloc_freelist_pages(flind, pool, order);
+ if (m != NULL)
+ return m;
+ }
+ return (NULL);
+}
+
+vm_page_t
+vm_phys_alloc_freelist_pages(int flind, int pool, int order)
+{
+ struct vm_freelist *fl;
+ struct vm_freelist *alt;
+ int oind, pind;
+ vm_page_t m;
+
+ KASSERT(flind < VM_NFREELIST,
+ ("vm_phys_alloc_pages: freelist %d is out of range", flind));
+
+ fl = vm_phys_free_queues[flind][pool];
+ for (oind = order; oind < VM_NFREEORDER; oind++) {
+ m = TAILQ_FIRST(&fl[oind].pl);
+ if (m != NULL) {
+ TAILQ_REMOVE(&fl[oind].pl, m, pageq);
+ fl[oind].lcnt--;
+ m->order = VM_NFREEORDER;
+ vm_phys_split_pages(m, oind, fl, order);
+ return (m);
+ }
+ }
+
+ /*
+ * The given pool was empty. Find the largest
+ * contiguous, power-of-two-sized set of pages in any
+ * pool. Transfer these pages to the given pool, and
+ * use them to satisfy the allocation.
+ */
+ for (oind = VM_NFREEORDER - 1; oind >= order; oind--) {
+ for (pind = 0; pind < VM_NFREEPOOL; pind++) {
+ alt = vm_phys_free_queues[flind][pind];
+ m = TAILQ_FIRST(&alt[oind].pl);
if (m != NULL) {
- TAILQ_REMOVE(&fl[oind].pl, m, pageq);
- fl[oind].lcnt--;
+ TAILQ_REMOVE(&alt[oind].pl, m, pageq);
+ alt[oind].lcnt--;
m->order = VM_NFREEORDER;
+ vm_phys_set_pool(pool, m, oind);
vm_phys_split_pages(m, oind, fl, order);
return (m);
}
}
-
- /*
- * The given pool was empty. Find the largest
- * contiguous, power-of-two-sized set of pages in any
- * pool. Transfer these pages to the given pool, and
- * use them to satisfy the allocation.
- */
- for (oind = VM_NFREEORDER - 1; oind >= order; oind--) {
- for (pind = 0; pind < VM_NFREEPOOL; pind++) {
- alt = vm_phys_free_queues[flind][pind];
- m = TAILQ_FIRST(&alt[oind].pl);
- if (m != NULL) {
- TAILQ_REMOVE(&alt[oind].pl, m, pageq);
- alt[oind].lcnt--;
- m->order = VM_NFREEORDER;
- vm_phys_set_pool(pool, m, oind);
- vm_phys_split_pages(m, oind, fl, order);
- return (m);
- }
- }
- }
}
return (NULL);
}
Index: sys/vm/vm_phys.h
===================================================================
--- sys/vm/vm_phys.h (revision 208848)
+++ sys/vm/vm_phys.h (working copy)
@@ -44,6 +44,7 @@
vm_page_t vm_phys_alloc_contig(unsigned long npages,
vm_paddr_t low, vm_paddr_t high,
unsigned long alignment, unsigned long boundary);
+vm_page_t vm_phys_alloc_freelist_pages(int flind, int pool, int order);
vm_page_t vm_phys_alloc_pages(int pool, int order);
vm_paddr_t vm_phys_bootstrap_alloc(vm_size_t size, unsigned long alignment);
void vm_phys_free_pages(vm_page_t m, int order);
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?AANLkTinmFOZY3OlaoKStxlNIRBt2G2I4ILkQ1P0CjozG>