svn commit: r208589 - head/sys/mips/mips
Alan Cox
alc at cs.rice.edu
Thu Jun 3 16:33:07 UTC 2010
C. Jayachandran wrote:
> 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.
>
The patch looks good. Please go ahead and commit it.
Thanks,
Alan
More information about the svn-src-all
mailing list