[SeaBIOS] [PATCH 2/2] malloc: Fallback to low-zone when alloc from high-zone fails
liran.alon at oracle.com
Wed Nov 14 00:58:47 CET 2018
> On 13 Nov 2018, at 20:16, Kevin O'Connor <kevin at koconnor.net> wrote:
> On Tue, Nov 13, 2018 at 05:53:41PM +0200, Liran Alon wrote:
>> When 32-bit code needs to allocate memory which remains permanent
>> after completion of the initialization phase, it doesn't care if
>> it is allocated from high-zone or low-zone.
>> Currently all these cases just attempt to allocate from high-zone
>> as low-zone memory is scarce.
>> However, in cases we don't have enough space left in high-zone,
>> it makes sense to use low-zone as a fallback.
> Thanks, but I fear this change would have poor secondary effects.
> There is only a tiny amount of ram available in the permanent "low"
> area of ram (typically less that 64K). If the code has exhausted the
> permanent "high" ram (typically 256K) then it is likely to rapidly
> exhaust the low ram and cause a worse failure when code that requires
> a tiny amount of "low" space fails.
I disagree with “likely”.
As I see it, the caller which allocates from high-zone doesn’t care if it is
satisfied eventually from high-zone or low-zone and if we got to the rare scenario
where we exhausted all high-zone memory and low-zone memory can save us,
why not try? This could save the guest of being able to boot on some scenarios
(As the scenario I have encountered when attaching many PVSCSI controllers).
And that seems better to me than to fail on allocation on SeaBIOS which is a
hard thing to debug for the common user which is not a SeaBIOS developer.
Also, why will failures on allocation on low-zone should be worse than
failures on allocation on high-zone? From the code I see that all allocations
failures eventually call warn_noalloc() which should make it very easy to
debug for a SeaBIOS developer. This is true both for allocations from high-zone
and allocations from low-zone.
> Typical work arounds are to extend the amount of high ram available
> (see BUILD_MAX_HIGHTABLE) or to use malloc_tmphigh() and manually
> reserve the space in the e820 map.
I think this patch is good regardless if modifying BUILD_MAX_HIGHTABLE can be served as a workaround.
(Also if it is a typical workaround, why isn’t it configurable as a Makefile parameter?)
I find it odd to allocate from malloc_tmphigh() and manually reserve the space in the e820 map.
As it basically undermines the definition of the Temporary High-Zone which should not be reserved
from OS and therefore must not be accessed after SeaBIOS initialisation phase.
Also, it is kinda hard to put the finger on which code you would change to use malloc_tmphigh() like
this all the time. Will you do this change for example in pvscsi.c code?
It seems odd to me that pvscsi code should deal with such low-level memory handling instead of
all this being transparently handled in the malloc_*() functions.
More information about the SeaBIOS