On 4 March 2015 at 10:21, Gao, Liming <liming....@intel.com> wrote: > Ard: > Got your point. I have no other comments. Your patch is fine to me. If you > require me to commit this change, please let me know. > > Reviewed-by: Liming Gao <liming....@intel.com> >
Thank you. Could you please go ahead and commit the patches? Regards, Ard. > -----Original Message----- > From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org] > Sent: Wednesday, March 04, 2015 12:03 AM > To: edk2-devel@lists.sourceforge.net > Subject: Re: [edk2] [PATCH 0/6] update pool allocator to better support 64 KB > runtime regions > > On 3 March 2015 at 15:56, Gao, Liming <liming....@intel.com> wrote: >> Ard: >> The data is good. >> >> For my another comment, please give your feedback. >>>>> Your patch defines mPoolSizeTable as below. Its max value is 24128. >>>>> When Granularity is 64K, the request pool size that larger than 24128 >>>>> will require allocate Granularity size. Is it the expected result? Or >>>>> expand to the value nearly to 64K? >>>>> STATIC CONST UINT16 mPoolSizeTable[] = { >>>>> 64, 128, 192, 320, 512, 832, 1344, 2176, 3520, 5696, 9216, 14912, 24128 >>>>> // append 39040, 63168? >>>>> }; >> > > OK, to clarify a bit more: > the largest allocation size we intend to support is 64k, so I stopped at the > largest bin size < 64k / 2 == 24128 > > The reasoning behind it is as follows: > when deciding to allocate directly rather than from a bin, there is a > tradeoff between wasted memory in the allocation footprint (e,g, allocate 32k > and reserve a 64k chunk, wasting 32k) versus wasted memory if the 64k chunk > can never be freed due to the fact that there is a single small allocation > still using it, which could happen if you perform several small and large > allocations and then free the large ones. > > For instance, serving a 60k allocation from the pool using a 63168 bin will > have little benefit over allocating a 64k chunk directly in terms of relative > memory waste, but there is a high likelihood that the remaining 2k is > allocated to another pool allocation which doesn't get freed, wasting almost > 64k if the 60k allocation is freed later on. > > I have no measurements to back up this reasoning, so if you prefer to add the > higher bin sizes I will not object, but I thought half the maximum allocation > / 2 was a reasonable upper bound. > > Regards, > Ard. > > >> -----Original Message----- >> From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org] >> Sent: Tuesday, March 3, 2015 9:01 PM >> To: edk2-devel@lists.sourceforge.net >> Subject: Re: [edk2] [PATCH 0/6] update pool allocator to better >> support 64 KB runtime regions >> >> On 3 March 2015 at 13:54, Ard Biesheuvel <ard.biesheu...@linaro.org> wrote: >>> On 3 March 2015 at 09:37, Gao, Liming <liming....@intel.com> wrote: >>>> Ard: >>>> Could you give another data with all patches applied except for #6? I >>>> want to compare it with the Baseline. >>>> >>>>> LoaderData used 3136 allocated 8192 (4K) >>>>> BootSvData used 195344 allocated 270336 (4K) >>>>> RuntSvData used ? allocated ? (4K) >>>> >>> >>> I couldn't find the original patch, so I reimplemented the >>> instrumentation >>> >>> Baseline: >>> >> >> To clarify, this is *not* your baseline, but the result with patches >> #1 to #5 applied. >> The original baseline is in the original commit log >> >>> Type: 0x02, Used: 3616, Allocated: 8192 >>> Type: 0x04, Used: 232968, Allocated: 327680 >>> Type: 0x06, Used: 17720, Allocated: 28672 >>> >>> With patch #6 applied >>> >>> Type: 0x02, Used: 3280, Allocated: 8192 >>> Type: 0x04, Used: 232968, Allocated: 327680 >>> Type: 0x06, Used: 23264, Allocated: 65536 >>> >> >> >>>> -----Original Message----- >>>> From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org] >>>> Sent: Tuesday, March 03, 2015 4:06 PM >>>> To: edk2-devel@lists.sourceforge.net >>>> Subject: Re: [edk2] [PATCH 0/6] update pool allocator to better >>>> support 64 KB runtime regions >>>> >>>> On 3 March 2015 at 06:55, Gao, Liming <liming....@intel.com> wrote: >>>>> Ard: >>>>> I think current region is 128 (1 << 7) bytes. But, I agree use >>>>> exponential bin sizes is better than linearly increasing with 128 bytes. >>>>> >>>>> Your patch defines mPoolSizeTable as below. Its max value is 24128. >>>>> When Granularity is 64K, the request pool size that larger than 24128 >>>>> will require allocate Granularity size. Is it the expected result? Or >>>>> expand to the value nearly to 64K? >>>>> STATIC CONST UINT16 mPoolSizeTable[] = { >>>>> 64, 128, 192, 320, 512, 832, 1344, 2176, 3520, 5696, 9216, 14912, 24128 >>>>> // append 39040, 63168? >>>>> }; >>>>> >>>>> Besides, could you verify this patch on the normal case that >>>>> Granularity is always 4K and collect the memory usage info? >>>>> >>>> >>>> Actually, this wais already in the original cover letter: >>>> >>>> Baseline: >>>> >>>>> LoaderData used 3232 allocated 8192 (4K) >>>>> BootSvData used 198872 allocated 319488 (4K) >>>>> RuntSvData used 17592 allocated 32768 (4K) >>>>> >>>> >>>> With only patches #1 and #6 applied: >>>> >>>>> LoaderData used 3424 allocated 4096 (4K) >>>>> BootSvData used 198872 allocated 319488 (4K) >>>>> RuntSvData used 17592 allocated 524288 (64K) >>>>> >>>> >>>> With all patches applied >>>> >>>>> LoaderData used 3136 allocated 8192 (4K) >>>>> BootSvData used 195344 allocated 270336 (4K) >>>>> RuntSvData used 23136 allocated 65536 (64K) >>>> >>>> >>>> -- >>>> Ard. >>>> >>>> >>>>> -----Original Message----- >>>>> From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org] >>>>> Sent: Monday, February 09, 2015 12:51 AM >>>>> To: edk2-devel@lists.sourceforge.net; ler...@redhat.com; >>>>> olivier.mar...@arm.com; roy.fr...@linaro.org; >>>>> leif.lindh...@linaro.org; Justen, Jordan L; Tian, Feng >>>>> Subject: [edk2] [PATCH 0/6] update pool allocator to better support >>>>> 64 KB runtime regions >>>>> >>>>> This series implements an updated pool allocator that performs better in >>>>> a situation where the boot time and runtime allocation alignment is >>>>> different, and where either may be substantially larger than a single >>>>> page. >>>>> >>>>> Without these patches applied, the pool allocations (in bytes) that are >>>>> served from the bins are as follows (on AArch64-QEMU with Intel BDS using >>>>> -kernel): >>>>> >>>>> LoaderData used 3232 allocated 8192 >>>>> BootSvData used 198872 allocated 319488 >>>>> RuntSvData used 17592 allocated 32768 >>>>> >>>>> However, when increasing the granularity of the allocations for >>>>> runtime regions (patches #1 and #6 applied), it becomes apparent >>>>> that the current code does not cope well in the runtime case >>>>> >>>>> LoaderData used 3424 allocated 4096 >>>>> BootSvData used 198872 allocated 319488 >>>>> RuntSvData used 17592 allocated 524288 >>>>> >>>>> This series attempts to address this by changing a couple of things >>>>> in the pool >>>>> allocator: >>>>> - use exponential bin sizes rather than linearly increasing with 32 >>>>> bytes >>>>> - if a bin is depleted, get a block from the next bin and split it >>>>> up >>>>> >>>>> With all patches applied, the numbers become >>>>> >>>>> LoaderData used 3136 allocated 8192 >>>>> BootSvData used 195344 allocated 270336 >>>>> RuntSvData used 23136 allocated 65536 >>>>> >>>>> Note the final 'used' number has increased: this is most likely due to a >>>>> pool allocation of > 4 KB that was formerly served by a passthrough >>>>> allocation (i.e., a pool allocation that is backed directly by a call to >>>>> AllocatePages ()), but is now served from one of the bins. >>>>> >>>>> Ard Biesheuvel (6): >>>>> MdeModulePkg: use correct granularity when allocating pool pages >>>>> MdeModulePkg: improve scalability of memory pools >>>>> MdeModulePkg: use index to traverse free pool pages >>>>> MdeModulePkg: carve pool pages into the largest chunks possible >>>>> MdeModulePkg: serve allocations from higher-up bins if current bin is >>>>> empty >>>>> MdeModulePkg: use 64 KB granularity for runtime allocations on >>>>> AArch64 >>>>> >>>>> MdeModulePkg/Core/Dxe/Mem/Imem.h | 9 +++ >>>>> MdeModulePkg/Core/Dxe/Mem/Pool.c | 145 >>>>> ++++++++++++++++++++++++++------------- >>>>> 2 files changed, 107 insertions(+), 47 deletions(-) >>>>> >>>>> -- >>>>> 1.8.3.2 >>>>> >>>>> >>>>> ------------------------------------------------------------------- >>>>> - >>>>> ---------- Dive into the World of Parallel Programming. The Go >>>>> Parallel Website, sponsored by Intel and developed in partnership >>>>> with Slashdot Media, is your hub for all things parallel software >>>>> development, from weekly thought leadership blogs to news, videos, >>>>> case studies, tutorials and more. Take a look and join the >>>>> conversation now. http://goparallel.sourceforge.net/ >>>>> _______________________________________________ >>>>> edk2-devel mailing list >>>>> edk2-devel@lists.sourceforge.net >>>>> https://lists.sourceforge.net/lists/listinfo/edk2-devel >>>>> >>>>> ------------------------------------------------------------------- >>>>> - >>>>> ---------- Dive into the World of Parallel Programming The Go >>>>> Parallel Website, sponsored by Intel and developed in partnership >>>>> with Slashdot Media, is your hub for all things parallel software >>>>> development, from weekly thought leadership blogs to news, videos, >>>>> case studies, tutorials and more. Take a look and join the >>>>> conversation now. http://goparallel.sourceforge.net/ >>>>> _______________________________________________ >>>>> edk2-devel mailing list >>>>> edk2-devel@lists.sourceforge.net >>>>> https://lists.sourceforge.net/lists/listinfo/edk2-devel >>>> >>>> -------------------------------------------------------------------- >>>> - >>>> --------- Dive into the World of Parallel Programming The Go >>>> Parallel Website, sponsored by Intel and developed in partnership >>>> with Slashdot Media, is your hub for all things parallel software >>>> development, from weekly thought leadership blogs to news, videos, >>>> case studies, tutorials and more. Take a look and join the >>>> conversation now. http://goparallel.sourceforge.net/ >>>> _______________________________________________ >>>> edk2-devel mailing list >>>> edk2-devel@lists.sourceforge.net >>>> https://lists.sourceforge.net/lists/listinfo/edk2-devel >>>> >>>> -------------------------------------------------------------------- >>>> - >>>> --------- Dive into the World of Parallel Programming The Go >>>> Parallel Website, sponsored by Intel and developed in partnership >>>> with Slashdot Media, is your hub for all things parallel software >>>> development, from weekly thought leadership blogs to news, videos, >>>> case studies, tutorials and more. Take a look and join the >>>> conversation now. http://goparallel.sourceforge.net/ >>>> _______________________________________________ >>>> edk2-devel mailing list >>>> edk2-devel@lists.sourceforge.net >>>> https://lists.sourceforge.net/lists/listinfo/edk2-devel >> >> ---------------------------------------------------------------------- >> -------- Dive into the World of Parallel Programming The Go Parallel >> Website, sponsored by Intel and developed in partnership with Slashdot >> Media, is your hub for all things parallel software development, from >> weekly thought leadership blogs to news, videos, case studies, >> tutorials and more. Take a look and join the conversation now. >> http://goparallel.sourceforge.net/ >> _______________________________________________ >> edk2-devel mailing list >> edk2-devel@lists.sourceforge.net >> https://lists.sourceforge.net/lists/listinfo/edk2-devel >> >> ---------------------------------------------------------------------- >> -------- Dive into the World of Parallel Programming The Go Parallel >> Website, sponsored by Intel and developed in partnership with Slashdot >> Media, is your hub for all things parallel software development, from >> weekly thought leadership blogs to news, videos, case studies, >> tutorials and more. Take a look and join the conversation now. >> http://goparallel.sourceforge.net/ >> _______________________________________________ >> edk2-devel mailing list >> edk2-devel@lists.sourceforge.net >> https://lists.sourceforge.net/lists/listinfo/edk2-devel > > ------------------------------------------------------------------------------ > Dive into the World of Parallel Programming The Go Parallel Website, > sponsored by Intel and developed in partnership with Slashdot Media, is your > hub for all things parallel software development, from weekly thought > leadership blogs to news, videos, case studies, tutorials and more. Take a > look and join the conversation now. http://goparallel.sourceforge.net/ > _______________________________________________ > edk2-devel mailing list > edk2-devel@lists.sourceforge.net > https://lists.sourceforge.net/lists/listinfo/edk2-devel > > ------------------------------------------------------------------------------ > Dive into the World of Parallel Programming The Go Parallel Website, sponsored > by Intel and developed in partnership with Slashdot Media, is your hub for all > things parallel software development, from weekly thought leadership blogs to > news, videos, case studies, tutorials and more. Take a look and join the > conversation now. http://goparallel.sourceforge.net/ > _______________________________________________ > edk2-devel mailing list > edk2-devel@lists.sourceforge.net > https://lists.sourceforge.net/lists/listinfo/edk2-devel ------------------------------------------------------------------------------ Dive into the World of Parallel Programming The Go Parallel Website, sponsored by Intel and developed in partnership with Slashdot Media, is your hub for all things parallel software development, from weekly thought leadership blogs to news, videos, case studies, tutorials and more. Take a look and join the conversation now. http://goparallel.sourceforge.net/ _______________________________________________ edk2-devel mailing list edk2-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/edk2-devel