svn commit: r208589 - head/sys/mips/mips

C. Jayachandran c.jayachandran at gmail.com
Thu Jun 3 12:42:40 UTC 2010


On Thu, Jun 3, 2010 at 10:20 AM, C. Jayachandran
<c.jayachandran at gmail.com> wrote:
> On Thu, Jun 3, 2010 at 10:07 AM, Alan Cox <alc at cs.rice.edu> wrote:
>> C. Jayachandran wrote:
>>>
>>> On Wed, Jun 2, 2010 at 11:57 AM, Alan Cox <alc at cs.rice.edu> wrote:
>>>
>>>>
>>>> C. Jayachandran wrote:
>>>>
>>>>>
>>>>> On Wed, Jun 2, 2010 at 3:23 AM, Alan Cox <alc at 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.

I've attached the patch for review, but I was not able trigger the
condition in which vm_contig_grow_cache() is called during testing so
far.

But if you are okay with the patch I can commit it.

Thanks,
JC.
-------------- next part --------------
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,6 +167,10 @@
 		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),
@@ -201,8 +204,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 +215,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 +262,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 +369,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;
 		}


More information about the svn-src-head mailing list