Date: Mon, 07 Jun 2010 12:27:06 -0500 From: Alan Cox <alc@cs.rice.edu> To: "C. Jayachandran" <c.jayachandran@gmail.com> Cc: "Jayachandran C." <jchandra@freebsd.org>, Konstantin Belousov <kostikbel@gmail.com>, Alan Cox <alc@cs.rice.edu>, mips@freebsd.org Subject: Re: svn commit: r208589 - head/sys/mips/mips Message-ID: <4C0D2BEA.6060103@cs.rice.edu> In-Reply-To: <AANLkTinmFOZY3OlaoKStxlNIRBt2G2I4ILkQ1P0CjozG@mail.gmail.com> 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> <AANLkTinmFOZY3OlaoKStxlNIRBt2G2I4ILkQ1P0CjozG@mail.gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
C. Jayachandran wrote: > 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. > > 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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?4C0D2BEA.6060103>