svn commit: r208589 - head/sys/mips/mips
Alan Cox
alc at cs.rice.edu
Mon Jun 7 17:27:16 UTC 2010
C. Jayachandran wrote:
> On Fri, Jun 4, 2010 at 10:44 PM, Alan Cox <alc at cs.rice.edu> wrote:
>
>> C. Jayachandran wrote:
>>
>>> On Thu, Jun 3, 2010 at 10:33 PM, Alan Cox <alc at 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.
>
>
Where exactly is this occurring?
> 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.
>
>
Basically, yes. At first, I was taken aback by defining
VM_FREELIST_DEFAULT as other than zero, but it shouldn't break anything.
The one issue that I see with the patch is that you'll need to release
and reacquire the pmap and page queues locks around the call to
vm_contig_grow_cache(). However, if you release these locks, you need
to restart the pmap_allocpte(), i.e., perform the "goto retry;", because
while you slept another processor/thread might have already allocated
and installed the page table page that you are trying to allocate into
the page table.
For what it's worth, this same race condition exists in the current code
in subversion using an UMA zone, but not in the code that predates use
of an UMA zone.
Alan
More information about the freebsd-mips
mailing list