Date: Wed, 10 Jul 2013 10:48:39 +0300 From: Konstantin Belousov <kostikbel@gmail.com> To: Neel Natu <neelnatu@gmail.com> Cc: svn-src-projects@freebsd.org, src-committers@freebsd.org Subject: Re: svn commit: r253135 - in projects/bhyve_npt_pmap/sys/amd64: include vmm Message-ID: <20130710074839.GZ91021@kib.kiev.ua> In-Reply-To: <CAFgRE9GLeE_8jVT4DBzZ-yAAaw6yPLVL43Lv1s=9ytZ-1cNWwA@mail.gmail.com> References: <201307100712.r6A7CtsB031581@svn.freebsd.org> <20130710072036.GY91021@kib.kiev.ua> <CAFgRE9GLeE_8jVT4DBzZ-yAAaw6yPLVL43Lv1s=9ytZ-1cNWwA@mail.gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
--jZi48drba+QTPlSF Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Wed, Jul 10, 2013 at 12:36:38AM -0700, Neel Natu wrote: > Hi Konstantin, >=20 > On Wed, Jul 10, 2013 at 12:20 AM, Konstantin Belousov > <kostikbel@gmail.com> wrote: > > On Wed, Jul 10, 2013 at 07:12:55AM +0000, Neel Natu wrote: > >> Author: neel > >> Date: Wed Jul 10 07:12:55 2013 > >> New Revision: 253135 > >> URL: http://svnweb.freebsd.org/changeset/base/253135 > >> > >> Log: > >> Replace vm_gpa2hpa() with a pair of functions vm_gpa_hold()/vm_gpa_r= elease(). > >> > >> We guarantee that the vm_page backing the 'gpa' is not reclaimed by > >> the page daemon until the caller indicates that they are done using = it > >> by calling 'vm_gpa_release()'. > >> > >> Modified: > >> projects/bhyve_npt_pmap/sys/amd64/include/vmm.h > >> projects/bhyve_npt_pmap/sys/amd64/vmm/vmm.c > >> projects/bhyve_npt_pmap/sys/amd64/vmm/vmm_dev.c > >> projects/bhyve_npt_pmap/sys/amd64/vmm/vmm_instruction_emul.c > >> > >> Modified: projects/bhyve_npt_pmap/sys/amd64/include/vmm.h > >> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D > >> --- projects/bhyve_npt_pmap/sys/amd64/include/vmm.h Wed Jul 10 06:46= :46 2013 (r253134) > >> +++ projects/bhyve_npt_pmap/sys/amd64/include/vmm.h Wed Jul 10 07:12= :55 2013 (r253135) > >> @@ -93,6 +93,9 @@ const char *vm_name(struct vm *vm); > >> int vm_malloc(struct vm *vm, vm_paddr_t gpa, size_t len); > >> int vm_map_mmio(struct vm *vm, vm_paddr_t gpa, size_t len, vm_paddr_t= hpa); > >> int vm_unmap_mmio(struct vm *vm, vm_paddr_t gpa, size_t len); > >> +void *vm_gpa_hold(struct vm *, vm_paddr_t gpa, size_t len, int prot, > >> + void **cookie); > >> +void vm_gpa_release(void *cookie); > >> vm_paddr_t vm_gpa2hpa(struct vm *vm, vm_paddr_t gpa, size_t size); > >> int vm_gpabase2memseg(struct vm *vm, vm_paddr_t gpabase, > >> struct vm_memory_segment *seg); > >> > >> Modified: projects/bhyve_npt_pmap/sys/amd64/vmm/vmm.c > >> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D > >> --- projects/bhyve_npt_pmap/sys/amd64/vmm/vmm.c Wed Jul 10 06:46= :46 2013 (r253134) > >> +++ projects/bhyve_npt_pmap/sys/amd64/vmm/vmm.c Wed Jul 10 07:12= :55 2013 (r253135) > >> @@ -439,16 +439,48 @@ vm_malloc(struct vm *vm, vm_paddr_t gpa, > >> return (0); > >> } > >> > >> -vm_paddr_t > >> -vm_gpa2hpa(struct vm *vm, vm_paddr_t gpa, size_t len) > >> +void * > >> +vm_gpa_hold(struct vm *vm, vm_paddr_t gpa, size_t len, int reqprot, > >> + void **cookie) > >> { > >> - vm_paddr_t nextpage; > >> + int rv, pageoff; > >> + vm_page_t m; > >> + struct proc *p; > >> + > >> + pageoff =3D gpa & PAGE_MASK; > >> + if (len > PAGE_SIZE - pageoff) > >> + panic("vm_gpa_hold: invalid gpa/len: 0x%016lx/%lu", gpa,= len); > >> + > >> + p =3D curthread->td_proc; > >> + > >> + PROC_LOCK(p); > >> + p->p_lock++; > >> + PROC_UNLOCK(p); > > Why do you need to hold the process there ? >=20 > I was following the idiom in trap_pfault() - the comment in trap.c > about swapout was enough to convince me :-) >=20 > > I do not think that hold in the trap handler is really useful, probably > > the reverse. >=20 > I am happy to remove the lock if isn't needed. >=20 > Could you explain a bit more on why it may be detrimental or point me > in the right direction so I can look for myself. The hold prevents the swap out of the process. This means that kernel stack is assured to be resident for all process threads, and the vm_pageout_map_deactivate_pages() is not called for the process. In other words, in the low memory condition, the VM choice of the pages to reuse is limited, which is probably esp. bad for the large address spaces like whole virtual machine. The swapped-out state of the process interacts correctly with the vm_fault_hold(), the synchronization of the access to map and objects would do the right thing. IMO the PHOLD() makes the page fault potentially faster to proceed by the cost of making the deadlock due to low memory condition more probable. --jZi48drba+QTPlSF Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.20 (FreeBSD) iQIcBAEBAgAGBQJR3RHWAAoJEJDCuSvBvK1B4hgP/0nrOL0sdSB/M7Eg4J/aS1qq 7l19H4O64C/9SM7G06HRiS5IC9B5hdYJR4gfNOBN1Ro63m4McM2Alk7IQgrQrwnf aLtFPNNCCnLXqh2Dxgfa2xaMo74XTdFN4/iN0WtF7gRKgrMFF6P/6Y1r1pXsAkiA KoUnrFiEt5nji5XE5v15OLuZnBTD0+XYBMSejGbHQF8rv6mFnDcjkKYGeWg0F31M Tg6NrauM/BubVMEWjL+c4Q9KjZnR5FOvEnTuhcihlLsRmTu10xhZDtr2G1NoYZBj uf9/6NTVODhuujxXHDnTK91USiWhxzEOaMTWpNkSCVM2V2mVJ7RX1N3K2d/aqPgU MdBLjcPhzFgvwFs19rfDJfno2N89KrD+VVC1kfVN/2nGNNCUPepc0tHM8FXUX+zI QIGW/iNJi/nlPcjmmE0Oe6XcYtGI7xG1S+nvVb1povfBwkRrjlo9bnI2CKarRkPK vg2JNSPJw/Zk7mCZD1fBGHqJwDCMiz2Aoc5ltmhRBNMkYaPFGU6XNdTJ3h0llGcj n+geWTi5IFRpMgnHPhadKfHdAub24jxt5H06TeCOezNGvyuTo9DaWw6T1Ic1rIHH l9UxQ55AZrQUP0GAJuJyh9ZUdLo0MwuocVfAIkkswbLKk0mh8R/FJbXcVoA10CGp Syh0prFXXy2dG/3ptPaD =LY6l -----END PGP SIGNATURE----- --jZi48drba+QTPlSF--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20130710074839.GZ91021>