Date: Thu, 3 Jun 2010 18:12:37 +0530 From: "C. Jayachandran" <c.jayachandran@gmail.com> To: Alan Cox <alc@cs.rice.edu> Cc: src-committers@freebsd.org, "Jayachandran C." <jchandra@freebsd.org>, svn-src-all@freebsd.org, Randall Stewart <rrs@lakerest.net>, Konstantin Belousov <kostikbel@gmail.com>, svn-src-head@freebsd.org Subject: Re: svn commit: r208589 - head/sys/mips/mips Message-ID: <AANLkTil2gE1niUWCHnsTlQvibhxBh7QYwD0TTWo0rj5c@mail.gmail.com> In-Reply-To: <AANLkTimIa3jmBPMhWIOcY6DenGpZ2ZYmqwDTWspVx0-u@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>
next in thread | previous in thread | raw e-mail | index | archive | help
--000e0cd13bbaa2f17b04881f8a67 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable On Thu, Jun 3, 2010 at 10:20 AM, C. Jayachandran <c.jayachandran@gmail.com> wrote: > 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: >>>>>>> =A0Call VM_WAIT in pmap_ptpgzone_allocf() if =A0M_WAITOK is set. >>>>>>> =A0Removed unused variable. >>>>>>> >>>>>>> =A0Approved by: rrs (mentor) >>>>>>> >>>>>>> >>>>>>> >>>>>>> >>>>>> >>>>>> I'm afraid that this will work some of the time, but not all of the >>>>>> time. >>>>>> =A0Specifically, there is no guarantee that any of the available fre= e (or >>>>>> cached) pages after the VM_WAIT will fall within the range of suitab= le >>>>>> physical addresses. =A0Moreover, and perhaps most worrisome, is that= this >>>>>> function could do a lot of spinning. =A0Every time this function sle= eps >>>>>> and >>>>>> a >>>>>> single page is freed (or cached) by someone else, this function will= be >>>>>> reawakened. =A0With a little bad luck, you could spin indefinitely. >>>>>> >>>>>> For essentially this reason, contigmalloc(), kmem_alloc_contig(), an= d >>>>>> 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 functio= n >>>> and >>>> use it. =A0(vm/vm_pageout.h is probably the best place for the functio= n >>>> prototype.) >>>> >>> >>> I'll create a patch for this and related changes in mips/mips/pmap.c >>> >>> >> >> Great. =A0Thanks! >> >>>> If you're interested in taking this a small step further, it would mak= e >>>> sense to add two parameters to vm_contig_grow_cache() and >>>> vm_contig_launder(): a "low" and a "high" physical address. >>>> =A0vm_contig_launder() could then skip pages that are outside the desi= red >>>> range. >>>> >>> >>> I am looking at this now, =A0part 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(). =A0If 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. --000e0cd13bbaa2f17b04881f8a67 Content-Type: text/plain; charset=US-ASCII; name="vm_contig_grow_cache.patch" Content-Disposition: attachment; filename="vm_contig_grow_cache.patch" Content-Transfer-Encoding: base64 X-Attachment-Id: f_g9zkrsvk1 SW5kZXg6IHN5cy9taXBzL21pcHMvcG1hcC5jCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIHN5cy9taXBzL21pcHMv cG1hcC5jCShyZXZpc2lvbiAyMDg3NTMpCisrKyBzeXMvbWlwcy9taXBzL3BtYXAuYwkod29ya2lu ZyBjb3B5KQpAQCAtOTY3LDE5ICs5NjcsMjMgQEAKIHsKIAl2bV9wYWdlX3QgbTsKIAl2bV9wYWRk cl90IHBhZGRyOworCWludCB0cmllczsKIAkKIAlLQVNTRVJUKGJ5dGVzID09IFBBR0VfU0laRSwK IAkJKCJwbWFwX3B0cGd6b25lX2FsbG9jZjogaW52YWxpZCBhbGxvY2F0aW9uIHNpemUgJWQiLCBi eXRlcykpOwogCiAJKmZsYWdzID0gVU1BX1NMQUJfUFJJVjsKLQlmb3IgKDs7KSB7Ci0JCW0gPSB2 bV9waHlzX2FsbG9jX2NvbnRpZygxLCAwLCBNSVBTX0tTRUcwX0xBUkdFU1RfUEhZUywKLQkJICAg IFBBR0VfU0laRSwgUEFHRV9TSVpFKTsKLQkJaWYgKG0gIT0gTlVMTCkKLQkJCWJyZWFrOwotCQlp ZiAoKHdhaXQgJiBNX1dBSVRPSykgPT0gMCkKKwl0cmllcyA9IDA7CityZXRyeToKKwltID0gdm1f cGh5c19hbGxvY19jb250aWcoMSwgMCwgTUlQU19LU0VHMF9MQVJHRVNUX1BIWVMsCisJICAgIFBB R0VfU0laRSwgUEFHRV9TSVpFKTsKKwlpZiAobSA9PSBOVUxMKSB7CisgICAgICAgICAgICAgICAg aWYgKHRyaWVzIDwgKCh3YWl0ICYgTV9OT1dBSVQpICE9IDAgPyAxIDogMykpIHsKKwkJCXZtX2Nv bnRpZ19ncm93X2NhY2hlKHRyaWVzLCAwLCBNSVBTX0tTRUcwX0xBUkdFU1RfUEhZUyk7CisJCQl0 cmllcysrOworCQkJZ290byByZXRyeTsKKwkJfSBlbHNlCiAJCQlyZXR1cm4gKE5VTEwpOwotCQlW TV9XQUlUOwogCX0KIAogCXBhZGRyID0gVk1fUEFHRV9UT19QSFlTKG0pOwpJbmRleDogc3lzL3Zt L3ZtX3BhZ2VvdXQuaAo9PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBzeXMvdm0vdm1fcGFnZW91dC5oCShyZXZpc2lv biAyMDg3NTMpCisrKyBzeXMvdm0vdm1fcGFnZW91dC5oCSh3b3JraW5nIGNvcHkpCkBAIC0xMDUs NSArMTA1LDYgQEAKIGludCB2bV9wYWdlb3V0X2ZsdXNoKHZtX3BhZ2VfdCAqLCBpbnQsIGludCk7 CiB2b2lkIHZtX3BhZ2VvdXRfb29tKGludCBzaG9ydGFnZSk7CiBib29sZWFuX3Qgdm1fcGFnZW91 dF9wYWdlX2xvY2sodm1fcGFnZV90LCB2bV9wYWdlX3QgKik7Cit2b2lkIHZtX2NvbnRpZ19ncm93 X2NhY2hlKGludCwgdm1fcGFkZHJfdCwgdm1fcGFkZHJfdCk7CiAjZW5kaWYKICNlbmRpZgkvKiBf Vk1fVk1fUEFHRU9VVF9IXyAqLwpJbmRleDogc3lzL3ZtL3ZtX2NvbnRpZy5jCj09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0K LS0tIHN5cy92bS92bV9jb250aWcuYwkocmV2aXNpb24gMjA4NzUzKQorKysgc3lzL3ZtL3ZtX2Nv bnRpZy5jCSh3b3JraW5nIGNvcHkpCkBAIC04Nyw4ICs4Nyw2IEBACiAjaW5jbHVkZSA8dm0vdm1f cGh5cy5oPgogI2luY2x1ZGUgPHZtL3ZtX2V4dGVybi5oPgogCi1zdGF0aWMgdm9pZCB2bV9jb250 aWdfZ3Jvd19jYWNoZShpbnQgdHJpZXMpOwotCiBzdGF0aWMgaW50CiB2bV9jb250aWdfbGF1bmRl cl9wYWdlKHZtX3BhZ2VfdCBtLCB2bV9wYWdlX3QgKm5leHQpCiB7CkBAIC0xNTcsOSArMTU1LDEw IEBACiB9CiAKIHN0YXRpYyBpbnQKLXZtX2NvbnRpZ19sYXVuZGVyKGludCBxdWV1ZSkKK3ZtX2Nv bnRpZ19sYXVuZGVyKGludCBxdWV1ZSwgdm1fcGFkZHJfdCBsb3csIHZtX3BhZGRyX3QgaGlnaCkK IHsKIAl2bV9wYWdlX3QgbSwgbmV4dDsKKwl2bV9wYWRkcl90IHBhOwogCWludCBlcnJvcjsKIAog CVRBSUxRX0ZPUkVBQ0hfU0FGRShtLCAmdm1fcGFnZV9xdWV1ZXNbcXVldWVdLnBsLCBwYWdlcSwg bmV4dCkgewpAQCAtMTY4LDYgKzE2NywxMCBAQAogCQlpZiAoKG0tPmZsYWdzICYgUEdfTUFSS0VS KSAhPSAwKQogCQkJY29udGludWU7CiAKKwkJcGEgPSBWTV9QQUdFX1RPX1BIWVMobSk7CisJCWlm IChwYSA8IGxvdyB8fCBwYSArIFBBR0VfU0laRSA+IGhpZ2gpCisJCQljb250aW51ZTsKKwogCQlp ZiAoIXZtX3BhZ2VvdXRfcGFnZV9sb2NrKG0sICZuZXh0KSkKIAkJCWNvbnRpbnVlOwogCQlLQVNT RVJUKFZNX1BBR0VfSU5RVUVVRTIobSwgcXVldWUpLApAQCAtMjAxLDggKzIwNCw4IEBACiAvKgog ICogSW5jcmVhc2UgdGhlIG51bWJlciBvZiBjYWNoZWQgcGFnZXMuCiAgKi8KLXN0YXRpYyB2b2lk Ci12bV9jb250aWdfZ3Jvd19jYWNoZShpbnQgdHJpZXMpCit2b2lkCit2bV9jb250aWdfZ3Jvd19j YWNoZShpbnQgdHJpZXMsIHZtX3BhZGRyX3QgbG93LCB2bV9wYWRkcl90IGhpZ2gpCiB7CiAJaW50 IGFjdGwsIGFjdG1heCwgaW5hY3RsLCBpbmFjdG1heDsKIApAQCAtMjEyLDExICsyMTUsMTEgQEAK IAlhY3RsID0gMDsKIAlhY3RtYXggPSB0cmllcyA8IDIgPyAwIDogY250LnZfYWN0aXZlX2NvdW50 OwogYWdhaW46Ci0JaWYgKGluYWN0bCA8IGluYWN0bWF4ICYmIHZtX2NvbnRpZ19sYXVuZGVyKFBR X0lOQUNUSVZFKSkgeworCWlmIChpbmFjdGwgPCBpbmFjdG1heCAmJiB2bV9jb250aWdfbGF1bmRl cihQUV9JTkFDVElWRSwgbG93LCBoaWdoKSkgewogCQlpbmFjdGwrKzsKIAkJZ290byBhZ2FpbjsK IAl9Ci0JaWYgKGFjdGwgPCBhY3RtYXggJiYgdm1fY29udGlnX2xhdW5kZXIoUFFfQUNUSVZFKSkg eworCWlmIChhY3RsIDwgYWN0bWF4ICYmIHZtX2NvbnRpZ19sYXVuZGVyKFBRX0FDVElWRSwgbG93 LCBoaWdoKSkgewogCQlhY3RsKys7CiAJCWdvdG8gYWdhaW47CiAJfQpAQCAtMjU5LDcgKzI2Miw3 IEBACiAJCQlpZiAodHJpZXMgPCAoKGZsYWdzICYgTV9OT1dBSVQpICE9IDAgPyAxIDogMykpIHsK IAkJCQlWTV9PQkpFQ1RfVU5MT0NLKG9iamVjdCk7CiAJCQkJdm1fbWFwX3VubG9jayhtYXApOwot CQkJCXZtX2NvbnRpZ19ncm93X2NhY2hlKHRyaWVzKTsKKwkJCQl2bV9jb250aWdfZ3Jvd19jYWNo ZSh0cmllcywgbG93LCBoaWdoKTsKIAkJCQl2bV9tYXBfbG9jayhtYXApOwogCQkJCVZNX09CSkVD VF9MT0NLKG9iamVjdCk7CiAJCQkJZ290byByZXRyeTsKQEAgLTM2Niw3ICszNjksNyBAQAogCXBh Z2VzID0gdm1fcGh5c19hbGxvY19jb250aWcobnBncywgbG93LCBoaWdoLCBhbGlnbm1lbnQsIGJv dW5kYXJ5KTsKIAlpZiAocGFnZXMgPT0gTlVMTCkgewogCQlpZiAodHJpZXMgPCAoKGZsYWdzICYg TV9OT1dBSVQpICE9IDAgPyAxIDogMykpIHsKLQkJCXZtX2NvbnRpZ19ncm93X2NhY2hlKHRyaWVz KTsKKwkJCXZtX2NvbnRpZ19ncm93X2NhY2hlKHRyaWVzLCBsb3csIGhpZ2gpOwogCQkJdHJpZXMr KzsKIAkJCWdvdG8gcmV0cnk7CiAJCX0K --000e0cd13bbaa2f17b04881f8a67--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?AANLkTil2gE1niUWCHnsTlQvibhxBh7QYwD0TTWo0rj5c>