Date: Wed, 06 Feb 2013 16:05:05 +0200 From: Andriy Gapon <avg@FreeBSD.org> To: Neel Natu <neelnatu@gmail.com> Cc: alc@FreeBSD.org, davide@FreeBSD.org, hackers@FreeBSD.org Subject: Re: dynamically calculating NKPT [was: Re: huge ktr buffer] Message-ID: <51126311.4060907@FreeBSD.org> In-Reply-To: <CAFgRE9F4JMutV9jJ_m7_9va67xiX4YXMT%2BRm6rUoDPMPymsg4w@mail.gmail.com> References: <CAFgRE9F4JMutV9jJ_m7_9va67xiX4YXMT%2BRm6rUoDPMPymsg4w@mail.gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
on 05/02/2013 01:05 Neel Natu said the following: > Hi, > > I have a patch to dynamically calculate NKPT for amd64 kernels. This > should fix the various issues that people pointed out in the email > thread. > > Please review and let me know if there are any objections to committing this. > > Also, thanks to Alan (alc@) for reviewing and providing feedback on > the initial version of the patch. > > Patch (also available at http://people.freebsd.org/~neel/patches/nkpt_diff.txt): It seems I am a little bit late with a review, but not too late :-) Some comments below: > Index: sys/amd64/include/pmap.h > =================================================================== > --- sys/amd64/include/pmap.h (revision 246277) > +++ sys/amd64/include/pmap.h (working copy) > @@ -113,13 +113,7 @@ > ((unsigned long)(l2) << PDRSHIFT) | \ > ((unsigned long)(l1) << PAGE_SHIFT)) > > -/* Initial number of kernel page tables. */ > -#ifndef NKPT > -#define NKPT 32 > -#endif I think that we could still keep this, if the below code is done slightly different: [snip] > +/* number of kernel PDP slots */ > +#define NKPDPE(ptpgs) howmany((ptpgs), NPDEPG) > + > static void > +nkpt_init(vm_paddr_t addr) > +{ > + int pt_pages; > + > +#ifdef NKPT > + pt_pages = NKPT; > +#else > + pt_pages = howmany(addr, 1 << PDRSHIFT); A very minor cosmetic note: perhaps NBPDR would look more concise here. > + pt_pages += NKPDPE(pt_pages); > + > + /* > + * Add some slop beyond the bare minimum required for bootstrapping > + * the kernel. > + * > + * This is quite important when allocating KVA for kernel modules. > + * The modules are required to be linked in the negative 2GB of > + * the address space. If we run out of KVA in this region then > + * pmap_growkernel() will need to allocate page table pages to map > + * the entire 512GB of KVA space which is an unnecessary tax on > + * physical memory. > + */ > + pt_pages += 4; /* 8MB additional slop for kernel modules */ > +#endif > + nkpt = pt_pages; > +} I would slightly re-organize this code so that it uses NKPT, if defined, as a default value for nkpt. Then, only if the calculated value is greater then it would override the default. There are tradeoffs, of course. So I am just voicing my opinion/preference. The "slack" thing is a little bit imperfect, but I am not a perfectionist :-) Thank you very much for this great feature. > +static void > create_pagetables(vm_paddr_t *firstaddr) > { > - int i, j, ndm1g; > + int i, j, ndm1g, nkpdpe; > > - /* Allocate pages */ > - KPTphys = allocpages(firstaddr, NKPT); > - KPML4phys = allocpages(firstaddr, 1); > - KPDPphys = allocpages(firstaddr, NKPML4E); > - KPDphys = allocpages(firstaddr, NKPDPE); > - > + /* Allocate page table pages for the direct map */ > ndmpdp = (ptoa(Maxmem) + NBPDP - 1) >> PDPSHIFT; > if (ndmpdp < 4) /* Minimum 4GB of dirmap */ > ndmpdp = 4; > @@ -517,6 +546,22 @@ > DMPDphys = allocpages(firstaddr, ndmpdp - ndm1g); > dmaplimit = (vm_paddr_t)ndmpdp << PDPSHIFT; > > + /* Allocate pages */ > + KPML4phys = allocpages(firstaddr, 1); > + KPDPphys = allocpages(firstaddr, NKPML4E); > + > + /* > + * Allocate the initial number of kernel page table pages required to > + * bootstrap. We defer this until after all memory-size dependent > + * allocations are done (e.g. direct map), so that we don't have to > + * build in too much slop in our estimate. > + */ > + nkpt_init(*firstaddr); > + nkpdpe = NKPDPE(nkpt); > + > + KPTphys = allocpages(firstaddr, nkpt); > + KPDphys = allocpages(firstaddr, nkpdpe); > + > /* Fill in the underlying page table pages */ > /* Read-only from zero to physfree */ > /* XXX not fully used, underneath 2M pages */ -- Andriy Gapon
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?51126311.4060907>