Date: Thu, 3 Jun 2010 10:20:43 +0530 From: "C. Jayachandran" <c.jayachandran@gmail.com> To: Alan Cox <alc@cs.rice.edu> Cc: "Jayachandran C." <jchandra@freebsd.org>, svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org, Randall Stewart <rrs@lakerest.net> Subject: Re: svn commit: r208589 - head/sys/mips/mips Message-ID: <AANLkTimIa3jmBPMhWIOcY6DenGpZ2ZYmqwDTWspVx0-u@mail.gmail.com> In-Reply-To: <4C0731A5.7090301@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>
next in thread | previous in thread | raw e-mail | index | archive | help
[-- Attachment #1 --] On Thu, Jun 3, 2010 at 10:07 AM, Alan Cox <alc@cs.rice.edu> wrote: > C. Jayachandran wrote: >> >> On Wed, Jun 2, 2010 at 11:57 AM, Alan Cox <alc@cs.rice.edu> wrote: >> >>> >>> C. Jayachandran wrote: >>> >>>> >>>> On Wed, Jun 2, 2010 at 3:23 AM, Alan Cox <alc@cs.rice.edu> wrote: >>>> >>>> >>>>> >>>>> On 5/27/2010 5:05 AM, Jayachandran C. wrote: >>>>> >>>>> >>>>>> >>>>>> Author: jchandra >>>>>> Date: Thu May 27 10:05:40 2010 >>>>>> New Revision: 208589 >>>>>> URL: http://svn.freebsd.org/changeset/base/208589 >>>>>> >>>>>> Log: >>>>>> Call VM_WAIT in pmap_ptpgzone_allocf() if M_WAITOK is set. >>>>>> Removed unused variable. >>>>>> >>>>>> Approved by: rrs (mentor) >>>>>> >>>>>> >>>>>> >>>>>> >>>>> >>>>> I'm afraid that this will work some of the time, but not all of the >>>>> time. >>>>> Specifically, there is no guarantee that any of the available free (or >>>>> cached) pages after the VM_WAIT will fall within the range of suitable >>>>> physical addresses. Moreover, and perhaps most worrisome, is that this >>>>> function could do a lot of spinning. Every time this function sleeps >>>>> and >>>>> a >>>>> single page is freed (or cached) by someone else, this function will be >>>>> reawakened. With a little bad luck, you could spin indefinitely. >>>>> >>>>> For essentially this reason, contigmalloc(), kmem_alloc_contig(), and >>>>> kmem_alloc_attr() don't use VM_WAIT, but instead a function called >>>>> vm_contig_grow_cache(). >>>>> >>>>> >>>> >>>> I had seen the vm_contig_grow_cache() usage, but could not use it as >>>> it was defined as a static function. >>>> >>>> I did not want to use contigmalloc()/kmem_alloc_contig() either, >>>> because the pages in that memory region are already direct mapped and >>>> setting up another mapping is not necessary. The overall idea is to >>>> allocate pages in the direct mapped region for the page table entries >>>> so that we don't take a TLB exception while accessing page tables >>>> (which is costly as MIPS has a software TLB exception handler). >>>> >>>> Can you suggest the right way to do this? I will make the changes and >>>> send a patch for approval. >>>> >>>> >>> >>> I would encourage you to make vm_contig_grow_cache() an extern function >>> and >>> use it. (vm/vm_pageout.h is probably the best place for the function >>> prototype.) >>> >> >> I'll create a patch for this and related changes in mips/mips/pmap.c >> >> > > Great. Thanks! > >>> If you're interested in taking this a small step further, it would make >>> sense to add two parameters to vm_contig_grow_cache() and >>> vm_contig_launder(): a "low" and a "high" physical address. >>> vm_contig_launder() could then skip pages that are outside the desired >>> range. >>> >> >> I am looking at this now, part of the logic which >> vm_phys_alloc_contig() uses to check pages address should work here. >> Will send out changes for this, if it works out. >> >> > > I suspect that you'll just need to add two or three lines to > vm_contig_launder(). If something doesn't make sense, just e-mail me. The current changes I have is attached - still testing it. JC. [-- Attachment #2 --] Index: sys/mips/mips/pmap.c =================================================================== --- sys/mips/mips/pmap.c (revision 208753) +++ sys/mips/mips/pmap.c (working copy) @@ -967,19 +967,23 @@ { vm_page_t m; vm_paddr_t paddr; + int tries; KASSERT(bytes == PAGE_SIZE, ("pmap_ptpgzone_allocf: invalid allocation size %d", bytes)); *flags = UMA_SLAB_PRIV; - for (;;) { - m = vm_phys_alloc_contig(1, 0, MIPS_KSEG0_LARGEST_PHYS, - PAGE_SIZE, PAGE_SIZE); - if (m != NULL) - break; - if ((wait & M_WAITOK) == 0) + tries = 0; +retry: + m = vm_phys_alloc_contig(1, 0, MIPS_KSEG0_LARGEST_PHYS, + PAGE_SIZE, PAGE_SIZE); + 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 return (NULL); - VM_WAIT; } paddr = VM_PAGE_TO_PHYS(m); Index: sys/vm/vm_pageout.h =================================================================== --- sys/vm/vm_pageout.h (revision 208753) +++ sys/vm/vm_pageout.h (working copy) @@ -105,5 +105,6 @@ int vm_pageout_flush(vm_page_t *, int, int); void vm_pageout_oom(int shortage); boolean_t vm_pageout_page_lock(vm_page_t, vm_page_t *); +void vm_contig_grow_cache(int, vm_paddr_t, vm_paddr_t); #endif #endif /* _VM_VM_PAGEOUT_H_ */ Index: sys/vm/vm_contig.c =================================================================== --- sys/vm/vm_contig.c (revision 208753) +++ sys/vm/vm_contig.c (working copy) @@ -87,8 +87,6 @@ #include <vm/vm_phys.h> #include <vm/vm_extern.h> -static void vm_contig_grow_cache(int tries); - static int vm_contig_launder_page(vm_page_t m, vm_page_t *next) { @@ -157,9 +155,10 @@ } static int -vm_contig_launder(int queue) +vm_contig_launder(int queue, vm_paddr_t low, vm_paddr_t high) { vm_page_t m, next; + vm_paddr_t pa; int error; TAILQ_FOREACH_SAFE(m, &vm_page_queues[queue].pl, pageq, next) { @@ -168,8 +167,13 @@ if ((m->flags & PG_MARKER) != 0) continue; + pa = VM_PAGE_TO_PHYS(m); + if (pa < low && pa + PAGE_SIZE > high) + continue; + if (!vm_pageout_page_lock(m, &next)) continue; + KASSERT(VM_PAGE_INQUEUE2(m, queue), ("vm_contig_launder: page %p's queue is not %d", m, queue)); error = vm_contig_launder_page(m, &next); @@ -201,8 +205,8 @@ /* * Increase the number of cached pages. */ -static void -vm_contig_grow_cache(int tries) +void +vm_contig_grow_cache(int tries, vm_paddr_t low, vm_paddr_t high) { int actl, actmax, inactl, inactmax; @@ -212,11 +216,11 @@ actl = 0; actmax = tries < 2 ? 0 : cnt.v_active_count; again: - if (inactl < inactmax && vm_contig_launder(PQ_INACTIVE)) { + if (inactl < inactmax && vm_contig_launder(PQ_INACTIVE, low, high)) { inactl++; goto again; } - if (actl < actmax && vm_contig_launder(PQ_ACTIVE)) { + if (actl < actmax && vm_contig_launder(PQ_ACTIVE, low, high)) { actl++; goto again; } @@ -259,7 +263,7 @@ if (tries < ((flags & M_NOWAIT) != 0 ? 1 : 3)) { VM_OBJECT_UNLOCK(object); vm_map_unlock(map); - vm_contig_grow_cache(tries); + vm_contig_grow_cache(tries, low, high); vm_map_lock(map); VM_OBJECT_LOCK(object); goto retry; @@ -366,7 +370,7 @@ pages = vm_phys_alloc_contig(npgs, low, high, alignment, boundary); if (pages == NULL) { if (tries < ((flags & M_NOWAIT) != 0 ? 1 : 3)) { - vm_contig_grow_cache(tries); + vm_contig_grow_cache(tries, low, high); tries++; goto retry; }
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?AANLkTimIa3jmBPMhWIOcY6DenGpZ2ZYmqwDTWspVx0-u>