From owner-freebsd-hackers@FreeBSD.ORG Wed Feb 6 14:05:10 2013 Return-Path: Delivered-To: hackers@FreeBSD.org Received: from mx1.freebsd.org (mx1.FreeBSD.org [8.8.178.115]) by hub.freebsd.org (Postfix) with ESMTP id 70A9840D; Wed, 6 Feb 2013 14:05:10 +0000 (UTC) (envelope-from avg@FreeBSD.org) Received: from citadel.icyb.net.ua (citadel.icyb.net.ua [212.40.38.140]) by mx1.freebsd.org (Postfix) with ESMTP id 3C8D1BF3; Wed, 6 Feb 2013 14:05:08 +0000 (UTC) Received: from odyssey.starpoint.kiev.ua (alpha-e.starpoint.kiev.ua [212.40.38.101]) by citadel.icyb.net.ua (8.8.8p3/ICyb-2.3exp) with ESMTP id QAA16906; Wed, 06 Feb 2013 16:05:06 +0200 (EET) (envelope-from avg@FreeBSD.org) Message-ID: <51126311.4060907@FreeBSD.org> Date: Wed, 06 Feb 2013 16:05:05 +0200 From: Andriy Gapon User-Agent: Mozilla/5.0 (X11; FreeBSD amd64; rv:17.0) Gecko/20130206 Thunderbird/17.0.2 MIME-Version: 1.0 To: Neel Natu Subject: Re: dynamically calculating NKPT [was: Re: huge ktr buffer] References: In-Reply-To: X-Enigmail-Version: 1.4.6 Content-Type: text/plain; charset=us-ascii Content-Transfer-Encoding: 7bit Cc: alc@FreeBSD.org, davide@FreeBSD.org, hackers@FreeBSD.org X-BeenThere: freebsd-hackers@freebsd.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: Technical Discussions relating to FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 06 Feb 2013 14:05:10 -0000 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